From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 17 Sep 2018 20:53:58 +0000 (-0700)
Subject: Merge branch 'jk/pack-objects-with-bitmap-fix'
X-Git-Tag: v2.20.0-rc0~235
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/b4583001b4387b454b5afffebb8b350fca291393?ds=inline;hp=-c

Merge branch 'jk/pack-objects-with-bitmap-fix'

Hotfix of the base topic.

* jk/pack-objects-with-bitmap-fix:
pack-bitmap: drop "loaded" flag
traverse_bitmap_commit_list(): don't free result
t5310: test delta reuse with bitmaps
bitmap_has_sha1_in_uninteresting(): drop BUG check
---

b4583001b4387b454b5afffebb8b350fca291393
diff --combined pack-bitmap.c
index c7d593c91c,40debd5e20..5848cc93aa
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@@ -91,8 -91,6 +91,6 @@@ struct bitmap_index 
  
  	/* Version of the bitmap index */
  	unsigned int version;
- 
- 	unsigned loaded : 1;
  };
  
  static struct ewah_bitmap *lookup_stored_bitmap(struct stored_bitmap *st)
@@@ -306,7 -304,7 +304,7 @@@ static int open_pack_bitmap_1(struct bi
  
  static int load_pack_bitmap(struct bitmap_index *bitmap_git)
  {
- 	assert(bitmap_git->map && !bitmap_git->loaded);
+ 	assert(bitmap_git->map);
  
  	bitmap_git->bitmaps = kh_init_sha1();
  	bitmap_git->ext_index.positions = kh_init_sha1_pos();
@@@ -321,7 -319,6 +319,6 @@@
  	if (load_bitmap_entries_v1(bitmap_git) < 0)
  		goto failed;
  
- 	bitmap_git->loaded = 1;
  	return 0;
  
  failed:
@@@ -336,9 -333,9 +333,9 @@@ static int open_pack_bitmap(struct bitm
  	struct packed_git *p;
  	int ret = -1;
  
- 	assert(!bitmap_git->map && !bitmap_git->loaded);
+ 	assert(!bitmap_git->map);
  
 -	for (p = get_packed_git(the_repository); p; p = p->next) {
 +	for (p = get_all_packs(the_repository); p; p = p->next) {
  		if (open_pack_bitmap_1(bitmap_git, p) == 0)
  			ret = 0;
  	}
@@@ -738,7 -735,7 +735,7 @@@ struct bitmap_index *prepare_bitmap_wal
  	 * from disk. this is the point of no return; after this the rev_list
  	 * becomes invalidated and we must perform the revwalk through bitmaps
  	 */
- 	if (!bitmap_git->loaded && load_pack_bitmap(bitmap_git) < 0)
+ 	if (load_pack_bitmap(bitmap_git) < 0)
  		goto cleanup;
  
  	object_array_clear(&revs->pending);
@@@ -848,9 -845,6 +845,6 @@@ void traverse_bitmap_commit_list(struc
  		OBJ_TAG, show_reachable);
  
  	show_extended_objects(bitmap_git, show_reachable);
- 
- 	bitmap_free(bitmap_git->result);
- 	bitmap_git->result = NULL;
  }
  
  static uint32_t count_object_type(struct bitmap_index *bitmap_git,
@@@ -1128,8 -1122,6 +1122,6 @@@ int bitmap_has_sha1_in_uninteresting(st
  
  	if (!bitmap_git)
  		return 0; /* no bitmap loaded */
- 	if (!bitmap_git->result)
- 		BUG("failed to perform bitmap walk before querying");
  	if (!bitmap_git->haves)
  		return 0; /* walk had no "haves" */
  
diff --combined t/t5310-pack-bitmaps.sh
index 7bff7923f2,af38776054..1be3459c5b
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@@ -335,11 -335,104 +335,104 @@@ test_expect_success 'truncated bitmap f
  	git rev-list --use-bitmap-index --count --all >expect &&
  	bitmap=$(ls .git/objects/pack/*.bitmap) &&
  	test_when_finished "rm -f $bitmap" &&
 -	head -c 512 <$bitmap >$bitmap.tmp &&
 +	test_copy_bytes 512 <$bitmap >$bitmap.tmp &&
  	mv -f $bitmap.tmp $bitmap &&
  	git rev-list --use-bitmap-index --count --all >actual 2>stderr &&
  	test_cmp expect actual &&
  	test_i18ngrep corrupt stderr
  '
  
+ # have_delta <obj> <expected_base>
+ #
+ # Note that because this relies on cat-file, it might find _any_ copy of an
+ # object in the repository. The caller is responsible for making sure
+ # there's only one (e.g., via "repack -ad", or having just fetched a copy).
+ have_delta () {
+ 	echo $2 >expect &&
+ 	echo $1 | git cat-file --batch-check="%(deltabase)" >actual &&
+ 	test_cmp expect actual
+ }
+ 
+ # Create a state of history with these properties:
+ #
+ #  - refs that allow a client to fetch some new history, while sharing some old
+ #    history with the server; we use branches delta-reuse-old and
+ #    delta-reuse-new here
+ #
+ #  - the new history contains an object that is stored on the server as a delta
+ #    against a base that is in the old history
+ #
+ #  - the base object is not immediately reachable from the tip of the old
+ #    history; finding it would involve digging down through history we know the
+ #    other side has
+ #
+ # This should result in a state where fetching from old->new would not
+ # traditionally reuse the on-disk delta (because we'd have to dig to realize
+ # that the client has it), but we will do so if bitmaps can tell us cheaply
+ # that the other side has it.
+ test_expect_success 'set up thin delta-reuse parent' '
+ 	# This first commit contains the buried base object.
+ 	test-tool genrandom delta 16384 >file &&
+ 	git add file &&
+ 	git commit -m "delta base" &&
+ 	base=$(git rev-parse --verify HEAD:file) &&
+ 
+ 	# These intermediate commits bury the base back in history.
+ 	# This becomes the "old" state.
+ 	for i in 1 2 3 4 5
+ 	do
+ 		echo $i >file &&
+ 		git commit -am "intermediate $i" || return 1
+ 	done &&
+ 	git branch delta-reuse-old &&
+ 
+ 	# And now our new history has a delta against the buried base. Note
+ 	# that this must be smaller than the original file, since pack-objects
+ 	# prefers to create deltas from smaller objects to larger.
+ 	test-tool genrandom delta 16300 >file &&
+ 	git commit -am "delta result" &&
+ 	delta=$(git rev-parse --verify HEAD:file) &&
+ 	git branch delta-reuse-new &&
+ 
+ 	# Repack with bitmaps and double check that we have the expected delta
+ 	# relationship.
+ 	git repack -adb &&
+ 	have_delta $delta $base
+ '
+ 
+ # Now we can sanity-check the non-bitmap behavior (that the server is not able
+ # to reuse the delta). This isn't strictly something we care about, so this
+ # test could be scrapped in the future. But it makes sure that the next test is
+ # actually triggering the feature we want.
+ #
+ # Note that our tools for working with on-the-wire "thin" packs are limited. So
+ # we actually perform the fetch, retain the resulting pack, and inspect the
+ # result.
+ test_expect_success 'fetch without bitmaps ignores delta against old base' '
+ 	test_config pack.usebitmaps false &&
+ 	test_when_finished "rm -rf client.git" &&
+ 	git init --bare client.git &&
+ 	(
+ 		cd client.git &&
+ 		git config transfer.unpackLimit 1 &&
+ 		git fetch .. delta-reuse-old:delta-reuse-old &&
+ 		git fetch .. delta-reuse-new:delta-reuse-new &&
+ 		have_delta $delta $ZERO_OID
+ 	)
+ '
+ 
+ # And do the same for the bitmap case, where we do expect to find the delta.
+ test_expect_success 'fetch with bitmaps can reuse old base' '
+ 	test_config pack.usebitmaps true &&
+ 	test_when_finished "rm -rf client.git" &&
+ 	git init --bare client.git &&
+ 	(
+ 		cd client.git &&
+ 		git config transfer.unpackLimit 1 &&
+ 		git fetch .. delta-reuse-old:delta-reuse-old &&
+ 		git fetch .. delta-reuse-new:delta-reuse-new &&
+ 		have_delta $delta $base
+ 	)
+ '
+ 
  test_done