Jeff King <peff <at> peff.net> writes:

quoted text
> 
> On Thu, May 08, 2008 at 04:53:20PM -0500, Brandon Casey wrote:
> 
> > > Yes. You would have to use the pack mtime. But of course you would have
> > > to actually _leave_ them in a pack, or they would just keep getting
> > > added to the new pack.
> > 
> > I had the impression that unreachable objects would not be packed. Maybe it
> > was more of an assumption.
> 
> Look in builtin-pack-objects.c:1981-1982. We basically just say "if it's
> in a pack now, then it should go into the new pack."
> 
> -Peff
> 


Here's what I was thinking (posted using gmane):

diff --git a/git-repack.sh b/git-repack.sh
index e18eb3f..064c331 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -30,7 +30,7 @@ do
        -n)     no_update_info=t ;;
        -a)     all_into_one=t ;;
        -A)     all_into_one=t
-               keep_unreachable=--keep-unreachable ;;
+               keep_unreachable=t ;;
        -d)     remove_redundant=t ;;
        -q)     quiet=-q ;;
        -f)     no_reuse=--no-reuse-object ;;
@@ -78,9 +78,6 @@ case ",$all_into_one," in
        if test -z "$args"
        then
                args='--unpacked --incremental'
-       elif test -n "$keep_unreachable"
-       then
-               args="$args $keep_unreachable"
        fi
        ;;
 esac
@@ -116,7 +113,15 @@ for name in $names ; do
                echo >&2 "old-pack-$name.{pack,idx} in $PACKDIR."
                exit 1
        }
-       rm -f "$PACKDIR/old-pack-$name.pack" "$PACKDIR/old-pack-$name.idx"
+       rm -f "$PACKDIR/old-pack-$name.idx"
+       test -z "$keep_unreachable" ||
+         ! test -f "$PACKDIR/old-pack-$name.pack" ||
+         git unpack-objects < "$PACKDIR/old-pack-$name.pack" || {
+               echo >&2 "Failed unpacking unreachable objects from old pack"
+               echo >&2 "saved as old-pack-$name.pack in $PACKDIR."
+               exit 1
+       }
+       rm -f "$PACKDIR/old-pack-$name.pack"
 done
 
 if test "$remove_redundant" = t
@@ -130,7 +135,18 @@ then
                  do
                        case " $fullbases " in
                        *" $e "*) ;;
-                       *)      rm -f "$e.pack" "$e.idx" "$e.keep" ;;
+                       *)
+                               rm -f "$e.idx" "$e.keep"
+                               if test -n "$keep_unreachable" &&
+                                  test -f "$e.pack"
+                               then
+                                       git unpack-objects < "$e.pack" || {
+                                               echo >&2 "Fail AVOID GMANE WRAP"
+                                               exit 1
+                                       }
+                               fi
+                               rm -f "$e.pack"
+                       ;;
                        esac
                  done
                )


Is the first invocation of unpack-objects necessary? pack-objects has created
a pack which hashes to the same name of a pack we already have, and we replace
the original with the new one. Is that what is happening? They will be identical
right?

Of course this won't set the timestamp on the created objects based on the
timestamp of the pack file, but this was easy. Setting the timestamp would be
proper, but what's another two weeks. Besides, for those users not manually
running git-gc, this code path won't even be executed until there are enough
pack files for git-gc to add -A to the repack options.

Then, for git-gc it should be enough to just always use -A with repack when
manually running it. Then --prune can be deprecated.

-brandon


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html