On Wed, Mar 23, 2022 at 10:55:37AM -0400, Derrick Stolee wrote: > > So, perhaps #3 ;-)? > > I'll default to #3 (do nothing), but if this shows up again > I'll plan on adding a comment to the helper to be clear on > how "inexact" the helper really is. I wonder if we could sidestep the whole issue with test_subcommand_inexact by testing this behavior by looking at the contents of the packs themselves. If we have a kept pack, and then add some new objects, and run "git repack --write-midx -adb", the new pack should not contain any of the objects found in the old (kept) pack. And that's the case after this patch, but was broken before it. Here's a test which constructs that scenario and then asserts that there isn't any overlap between the newly created pack and the old, kept pack. --- 8< --- diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 5922fb5bdd..96812fa226 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -370,9 +370,31 @@ test_expect_success '--write-midx with preferred bitmap tips' ' ' test_expect_success '--write-midx -b packs non-kept objects' ' - GIT_TRACE2_EVENT="$(pwd)/trace.txt" \ - git repack --write-midx -a -b && - test_subcommand_inexact git pack-objects --honor-pack-keep <trace.txt + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + + test_commit base && + git repack -ad && + + find $objdir/pack -name "*.idx" >before && + >$objdir/pack/$(basename $(cat before) .idx).keep && + + test_commit other && + git repack --write-midx -a -b -d && + + find $objdir/pack -name "*.idx" | sort >after && + + git show-index <$(cat before) >old.raw && + git show-index <$(comm -13 before after) >new.raw && + + cut -d" " -f2 old.raw | sort >old.objects && + cut -d" " -f2 new.raw | sort >new.objects && + + comm -12 old.objects new.objects >shared.objects && + test_must_be_empty shared.objects + ) ' test_expect_success TTY '--quiet disables progress' ' --- >8 --- It does seem a little word-y to me, but I think you could clean it up a little bit, too. If you want to take that patch, I think we could reasonably use the above diff as a first patch, and then remove the declaration of test_subcommand_inexact in a second patch. (Feel free to forge my s-o-b if you do want to pick that up). Thanks, Taylor
