diff --git a/pack-bitmap.c b/pack-bitmap.c index c7d593c91c7a76497407a703d9f0c50925e2475a..5848cc93aa254b8549f4c569882b5d781984ca47 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -91,8 +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 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git 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 @@ static int load_pack_bitmap(struct bitmap_index *bitmap_git) if (load_bitmap_entries_v1(bitmap_git) < 0) goto failed; - bitmap_git->loaded = 1; return 0; failed: @@ -336,7 +333,7 @@ static int open_pack_bitmap(struct bitmap_index *bitmap_git) struct packed_git *p; int ret = -1; - assert(!bitmap_git->map && !bitmap_git->loaded); + assert(!bitmap_git->map); for (p = get_all_packs(the_repository); p; p = p->next) { if (open_pack_bitmap_1(bitmap_git, p) == 0) @@ -738,7 +735,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs) * 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 @@ void traverse_bitmap_commit_list(struct bitmap_index *bitmap_git, 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 @@ int bitmap_has_sha1_in_uninteresting(struct bitmap_index *bitmap_git, 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 --git a/pack-bitmap.h b/pack-bitmap.h index c633bf523898db8d261f5c3f8098c3e843ca9c21..189dd68ad30b14281b42ea43bb21739006ea6061 100644 --- a/pack-bitmap.h +++ b/pack-bitmap.h @@ -54,7 +54,7 @@ int rebuild_existing_bitmaps(struct bitmap_index *, struct packing_data *mapping void free_bitmap_index(struct bitmap_index *); /* - * After a traversal has been performed on the bitmap_index, this can be + * After a traversal has been performed by prepare_bitmap_walk(), this can be * queried to see if a particular object was reachable from any of the * objects flagged as UNINTERESTING. */ diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 7bff7923f2a91165b5a0b5251351798519d63270..1be3459c5b8aa331a646e3ead24f7422110cdc08 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -342,4 +342,97 @@ test_expect_success 'truncated bitmap fails gracefully' ' test_i18ngrep corrupt stderr ' +# have_delta +# +# 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