PackWriter: match git's pack names

As reported by Jamie Wyrick, git appears to use the sha1 of
the entire pack file as the (default) name for it, not the
sha1(sorted-object-list) as the git-index-pack man page seems
to imply, since it says:

  Once the index has been created, the list of object names is
  sorted and the SHA-1 hash of that list is printed to stdout.
  If --stdin was also used then this is prefixed by either
  "pack\t", or "keep\t" if a new .keep file was successfully
  created. This is useful to remove a .keep file used as a lock
  to prevent the race with git repack mentioned above.

while also saying:

  If <pack-file> is not specified, the pack is written to
  objects/pack/ directory of the current Git repository with a
  default name determined from the pack content.

Originally, git-index-pack was used by bup, and when that was
changed, the naming convention was kept, presumably according
to this documentation; see commit 4cab9ab71fff ("Write idxs
directly rather than using git-index-pack.")

Change things to use the pack's entire content sha1, that we
calculate anyway, as the name.

This requires updating the 'gc' test to not compare by name
but by (index) content.

Also add a test that our behaviour matches git's.

Reported-by: Jamie Wyrick <terrifiedquack80@gmail.com>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

diff --git a/lib/bup/git.py b/lib/bup/git.py
index c8ebc88e64504230eca669ff37b6e24ae13bdcbf..61238bd250f42c72cef19914286a0d4e9ed3d7a6 100644 (file)
--- a/lib/bup/git.py
+++ b/lib/bup/git.py
@@ -879,9 +879,9 @@ class PackWriter:
         finally:
             f.close()
 
-        obj_list_sha = idx.write(self.filename + b'.idx', packbin)
+        idx.write(self.filename + b'.idx', packbin)
         nameprefix = os.path.join(self.repo_dir,
-                                  b'objects/pack/pack-' +  obj_list_sha)
+                                  b'objects/pack/pack-' +  hexlify(packbin))
         if os.path.exists(self.filename + b'.map'):
             os.unlink(self.filename + b'.map')
         os.rename(self.filename + b'.pack', nameprefix + b'.pack')
@@ -947,17 +947,13 @@ class PackIdxV2Writer:
             b = idx_f.read(8 + 4*256)
             idx_sum.update(b)
 
-            obj_list_sum = Sha1()
             for b in chunkyreader(idx_f, 20 * self.count):
                 idx_sum.update(b)
-                obj_list_sum.update(b)
-            namebase = hexlify(obj_list_sum.digest())
 
             for b in chunkyreader(idx_f):
                 idx_sum.update(b)
             idx_f.write(idx_sum.digest())
             fdatasync(idx_f.fileno())
-            return namebase
         finally:
             idx_f.close()
 

diff --git a/test/ext/test-gc b/test/ext/test-gc
index e29370d627fc82f2ac790053f8f34b8f0cf4fccb..9a4b0b00eec3a5610b6ef8d28fd06064b395fc93 100755 (executable)
--- a/test/ext/test-gc
+++ b/test/ext/test-gc
@@ -230,12 +230,15 @@ WVPASS echo 1 > src/1
 WVPASS bup index src
 WVPASS bup save -n src-1 src
 
-packs_before="$(ls "$BUP_DIR/objects/pack/"*.pack)" || exit $?
+pack_contents_before="$(git show-index < "$BUP_DIR/objects/pack/"*.idx | cut -d' ' -f2- | sort)" || exit $?
 WVPASS bup gc -v $GC_OPTS --threshold 0 2>&1 | tee gc.log
-packs_after="$(ls "$BUP_DIR/objects/pack/"*.pack)" || exit $?
-# Check that the pack was rewritten, but not removed (since the
-# result-pack is equal to the source pack)
+pack_contents_after="$(git show-index < "$BUP_DIR/objects/pack/"*.idx | cut -d' ' -f2- | sort)" || exit $?
+# Check that the pack was rewritten or a new pack written, but
+# with the same objects. Note that the name of the pack will
+# likely change as the *order* of objects is different. The
+# "git show-index | cut | sort" ignores the offsets but checks
+# the object and their crc.
 WVPASSEQ 1 "$(grep -cE '^rewriting ' gc.log)"
-WVPASSEQ "$packs_before" "$packs_after"
+WVPASSEQ "$pack_contents_before" "$pack_contents_after"
 
 WVPASS rm -rf "$tmpdir"

diff --git a/test/ext/test-misc b/test/ext/test-misc
index 6f3b2f377512ddd8708b0e0cac3b2d1c641f64ed..1b5833297b0810ac2437120e18dbc548552551f0 100755 (executable)
--- a/test/ext/test-misc
+++ b/test/ext/test-misc
@@ -60,6 +60,19 @@ WVSTART "save/git-fsck"
 ) || exit $?
 
 
+WVSTART "pack name same as git"
+(
+    # reuse packs from previous test
+    WVPASS cd "$BUP_DIR"/objects/pack/
+    WVPASS ls *.pack
+    for pack in *.pack ; do
+       gitname=$(git index-pack $pack) || exit $?
+       # make sure we named it correctly (like git)
+       WVPASSEQ pack-$gitname.pack $pack
+    done
+) || exit $?
+
+
 WVSTART "ftp"
 WVPASS bup ftp "cat /master/latest/$tmpdir/$D/b" >$D/b.new
 WVPASS bup ftp "cat /master/latest/$tmpdir/$D/f" >$D/f.new