From b4a14394af96b6a87a722ffb57666bb1edc001b9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 5 Apr 2019 14:04:56 -0400 Subject: t5319: fix bogus cat-file argument There's no such argument as "--unsorted"; it's spelled "--unordered". But our test failed to notice that cat-file didn't run at all because: 1. It lost the exit code of git on the left-hand side of a pipe. 2. It was comparing two runs of the broken invocation with and without a particular config variable (and indeed, both cases produced no output!). Let's fix the option, but also tweak the helper function to check the exit code. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5319-multi-pack-index.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 't/t5319-multi-pack-index.sh') diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 70926b5bc0..42f4d6cd01 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -86,13 +86,14 @@ test_expect_success 'write midx with one v1 pack' ' ' midx_git_two_modes () { + git -c core.multiPackIndex=false $1 >expect && + git -c core.multiPackIndex=true $1 >actual && if [ "$2" = "sorted" ] then - git -c core.multiPackIndex=false $1 | sort >expect && - git -c core.multiPackIndex=true $1 | sort >actual - else - git -c core.multiPackIndex=false $1 >expect && - git -c core.multiPackIndex=true $1 >actual + sort expect.sorted && + mv expect.sorted expect && + sort actual.sorted && + mv actual.sorted actual fi && test_cmp expect actual } @@ -104,7 +105,7 @@ compare_results_with_midx () { midx_git_two_modes "log --raw" && midx_git_two_modes "count-objects --verbose" && midx_git_two_modes "cat-file --batch-all-objects --buffer --batch-check" && - midx_git_two_modes "cat-file --batch-all-objects --buffer --batch-check --unsorted" sorted + midx_git_two_modes "cat-file --batch-all-objects --buffer --batch-check --unordered" sorted ' } -- cgit v1.2.3 From 5670ad98a8c0b5d7cabb5931225c759ec977600f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 5 Apr 2019 14:05:03 -0400 Subject: t5319: drop useless --buffer from cat-file The cat-file --buffer option is the default already when using --batch-all-objects. It doesn't hurt to specify it, but it's nice for the test scripts to model good usage. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5319-multi-pack-index.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 't/t5319-multi-pack-index.sh') diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 42f4d6cd01..8c4d2bd849 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -104,8 +104,8 @@ compare_results_with_midx () { midx_git_two_modes "rev-list --objects --all" && midx_git_two_modes "log --raw" && midx_git_two_modes "count-objects --verbose" && - midx_git_two_modes "cat-file --batch-all-objects --buffer --batch-check" && - midx_git_two_modes "cat-file --batch-all-objects --buffer --batch-check --unordered" sorted + midx_git_two_modes "cat-file --batch-all-objects --batch-check" && + midx_git_two_modes "cat-file --batch-all-objects --batch-check --unordered" sorted ' } -- cgit v1.2.3 From fc78915674ff1bca1726348d7c434dfa0048a5d7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 5 Apr 2019 14:06:22 -0400 Subject: packfile: fix pack basename computation When we have a multi-pack-index that covers many packfiles, we try to avoid opening the .idx for those packfiles. To do that we feed the pack name to midx_contains_pack(). But that function wants to see only the basename, which we compute using strrchr() to find the final slash. But that leaves an extra "/" at the start of our string. We can fix this by incrementing the pointer. That also raises the question of what to do when the name does not have a '/' at all. This should generally not happen (we always find files in "pack/"), but it doesn't hurt to be defensive here. Let's wrap all of that up in a helper function and make it publicly available, since a later patch will need to use it, too. The tests don't notice because there's nothing about opening those .idx files that would cause us to give incorrect output. It's just a little slower. The new test checks this case by corrupting the covered .idx, and then making sure we don't complain about it. We also have to tweak t5570, which intentionally corrupts a .idx file and expects us to notice it. When run with GIT_TEST_MULTI_PACK_INDEX, this will fail since we now will (correctly) not bother opening the .idx at all. We can fix that by unconditionally dropping any midx that's there, which ensures we'll have to read the .idx. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5319-multi-pack-index.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 't/t5319-multi-pack-index.sh') diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 8c4d2bd849..1ebf19ec3c 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -117,6 +117,20 @@ test_expect_success 'write midx with one v2 pack' ' compare_results_with_midx "one v2 pack" +test_expect_success 'corrupt idx not opened' ' + idx=$(test-tool read-midx $objdir | grep "\.idx\$") && + mv $objdir/pack/$idx backup-$idx && + test_when_finished "mv backup-\$idx \$objdir/pack/\$idx" && + + # This is the minimum size for a sha-1 based .idx; this lets + # us pass perfunctory tests, but anything that actually opens and reads + # the idx file will complain. + test_copy_bytes 1064 $objdir/pack/$idx && + + git -c core.multiPackIndex=true rev-list --objects --all 2>err && + test_must_be_empty err +' + test_expect_success 'add more objects' ' for i in $(test_seq 6 10) do -- cgit v1.2.3