Re: [PATCH v3 07/10] t: prepare for GIT_TEST_WRITE_REV_INDEX
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 29, 2021, 02:42 UTC
- Message-ID
- <xmqqsg6ktqe1.fsf@gitster.c.googlers.com>
- In-Reply-To
- <YBNasXXZc1BEkMxH@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 16 quoted lines
>> # Ensure that there are exactly 6 files (3 .pack and 3 .idx).
>> - ls http_child/.git/objects/pack/* >filelist &&
>> - test_line_count = 6 filelist
>> + ls http_child/.git/objects/pack/*.pack >packlist &&
>> + ls http_child/.git/objects/pack/*.idx >idxlist &&
>> + test_line_count = 3 idxlist &&
>> + test_line_count = 3 packlist
>> '
>
> Hmm. Too bad we can't rely on shell brace expansion, as:
>
> ls http_child/.git/objects/pack/*.{pack,idx}
>
> would be more readable. You could still do it in a single "ls" by
> writing out both arguments manually, but it's probably not that
> important.This part looks a bit familiar as I had to fix the interaction with Jonathan's topic, IIRC. We need to update the comment. It is not ensuring "exact 6"---it merely is interested in having three pack/idx pair, and carefully expressing that by preparing for the presence of other cruft in objects/pack/ directory other people may create (like ".rev", but we may gain more).
I wonder if we even _care_ about .idx. Why not just count .pack, to prepare for a possible distant future where we do not even write .idx but append to existing multi-pack-index as we download a new pack stream and store it in a .pack, or something?
Show 25 quoted lines
>> - ls .git/objects/pack | sort >existing_packs && >> + ls .git/objects/pack/pack-*.pack | sort >existing_packs && >> test_commit "$(test_oid obj3)" && >> test_commit "$(test_oid obj4)" && >> >> git gc --auto 2>err && >> test_i18ngrep ! "^warning:" err && >> - ls .git/objects/pack/ | sort >post_packs && >> + ls .git/objects/pack/pack-*.pack | sort >post_packs && >> comm -1 -3 existing_packs post_packs >new && >> comm -2 -3 existing_packs post_packs >del && >> test_line_count = 0 del && # No packs are deleted >> - test_line_count = 2 new # There is one new pack and its .idx >> + test_line_count = 1 new # There is one new pack >> ' > > This one is making the test a bit looser (it would miss a case where we > somehow failed to generate the .idx). That seems like an unlikely bug, > but I wonder if we can keep the original behavior. I guess: > > ls .git/objects/pack/*.pack \ > .git/objects/pack/*.idx | > sort >post_packs > > would work?
I guess we are looking at the same issue from opposite angle. I suspect that it might even be a good thing to only care about .pack and ignore everything else in the longer run.