{"thread":{"id":"56258","subject":"t5607 fail with GIT_TEST_FAIL_PREREQS enabled","startedAt":"2021-08-11T13:03:07Z","lastAt":"2021-08-11T23:30:25Z","messageCount":5,"participants":["Son Luong Ngoc","Jeff King","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"432479","messageId":"CAL3xRKcMAkfaoAWo5VfQ_Q6nYkR94spSN4Ko6xfpSRKa1cEuAg@mail.gmail.com","threadId":"56258","inReplyTo":null,"subject":"t5607 fail with GIT_TEST_FAIL_PREREQS enabled","fromName":"Son Luong Ngoc","fromEmail":"sluongng@gmail.com","sentAt":"2021-08-11T13:02:52Z","receivedAt":"2021-08-11T13:03:07Z","isPatch":false,"sender":{"key":"sluongng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26684313?v=4"},"body":"Hi folks,\n\nOur internal CI spotted a failing test when build from 'next' and\n'master' branch\n\n    git/t% GIT_TEST_FAIL_PREREQS=1 ./t5607-clone-bundle.sh\n    ok 1 - setup\n    ok 2 - \"verify\" needs a worktree\n    ok 3 - annotated tags can be excluded by rev-list options\n    ok 4 - die if bundle file cannot be created\n    ok 5 - bundle --stdin\n    ok 6 - bundle --stdin <rev-list options>\n    ok 7 - empty bundle file is rejected\n    not ok 8 - ridiculously long subject in boundary\n    #\n    #               >file4 &&\n    #               test_tick &&\n    #               git add file4 &&\n    #               printf \"%01200d\\n\" 0 | git commit -F - &&\n    #               test_commit fifth &&\n    #               git bundle create long-subject-bundle.bdl HEAD^..HEAD &&\n    #               cat >expect <<-EOF &&\n    #               $(git rev-parse main) HEAD\n    #               EOF\n    #               git bundle list-heads long-subject-bundle.bdl >actual &&\n    #               test_cmp expect actual &&\n    #\n    #               git fetch long-subject-bundle.bdl &&\n    #\n    #               if ! test_have_prereq SHA1\n    #               then\n    #                       echo \"@object-format=sha256\"\n    #               fi >expect &&\n    #               cat >>expect <<-EOF &&\n    #               -$(git log --pretty=format:\"%H %s\" -1 HEAD^)\n    #               $(git rev-parse HEAD) HEAD\n    #               EOF\n    #\n    #               if test_have_prereq SHA1\n    #               then\n    #                       head -n 3 long-subject-bundle.bdl\n    #               else\n    #                       head -n 4 long-subject-bundle.bdl\n    #               fi | grep -v \"^#\" >actual &&\n    #\n    #               test_cmp expect actual\n    #\n    ok 9 - prerequisites with an empty commit message\n    ok 10 - failed bundle creation does not leave cruft\n    ok 11 - fetch SHA-1 from bundle\n    ok 12 - git bundle uses expected default format\n    ok 13 - git bundle v3 has expected contents\n    ok 14 - git bundle v3 rejects unknown capabilities\n    # failed 1 among 14 test(s)\n    1..14\n\nCheers,\nSon Luong.\n"},{"id":"432484","messageId":"YRPYrXYwmwVnligD@coredump.intra.peff.net","threadId":"56258","inReplyTo":"CAL3xRKcMAkfaoAWo5VfQ_Q6nYkR94spSN4Ko6xfpSRKa1cEuAg@mail.gmail.com","subject":"Re: t5607 fail with GIT_TEST_FAIL_PREREQS enabled","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-11T14:03:25Z","receivedAt":"2021-08-11T14:03:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 11, 2021 at 03:02:52PM +0200, Son Luong Ngoc wrote:\n\n>     git/t% GIT_TEST_FAIL_PREREQS=1 ./t5607-clone-bundle.sh\n> [...]\n>     #               if ! test_have_prereq SHA1\n>     #               then\n>     #                       echo \"@object-format=sha256\"\n>     #               fi >expect &&\n\nThe problem is presumably here. If test_have_prereq lies and say \"no, we\nare using sha256\" then we cannot expect what the built binary does to\nmatch that lie.\n\nPerhaps that is a sign that test_have_prereq is not the right tool to\ncheck \"which hash format are we using\", but I don't think we have\nanother convenient mechanism to do so currently.\n\nI also think that the FAIL_PREREQS system may be mis-designed a bit. We\nhad a similar problem a few months ago, and I think Junio's response\nhere points in a good direction:\n\n  https://lore.kernel.org/git/xmqqblbgrwkg.fsf@gitster.g/\n\n-Peff\n"},{"id":"432540","messageId":"YRRZ7E6W+xV2f/CG@camp.crustytoothpaste.net","threadId":"56258","inReplyTo":"YRPYrXYwmwVnligD@coredump.intra.peff.net","subject":"Re: t5607 fail with GIT_TEST_FAIL_PREREQS enabled","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-08-11T23:14:52Z","receivedAt":"2021-08-11T23:15:28Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-08-11 at 14:03:25, Jeff King wrote:\n> On Wed, Aug 11, 2021 at 03:02:52PM +0200, Son Luong Ngoc wrote:\n> \n> >     git/t% GIT_TEST_FAIL_PREREQS=1 ./t5607-clone-bundle.sh\n> > [...]\n> >     #               if ! test_have_prereq SHA1\n> >     #               then\n> >     #                       echo \"@object-format=sha256\"\n> >     #               fi >expect &&\n> \n> The problem is presumably here. If test_have_prereq lies and say \"no, we\n> are using sha256\" then we cannot expect what the built binary does to\n> match that lie.\n> \n> Perhaps that is a sign that test_have_prereq is not the right tool to\n> check \"which hash format are we using\", but I don't think we have\n> another convenient mechanism to do so currently.\n\nWe can use something like this:\n\n  if \"$(test_oid algo)\" != sha1\n\n> I also think that the FAIL_PREREQS system may be mis-designed a bit. We\n> had a similar problem a few months ago, and I think Junio's response\n> here points in a good direction:\n> \n>   https://lore.kernel.org/git/xmqqblbgrwkg.fsf@gitster.g/\n\nI take no position on this, but I'll send a patch to do something\nsimilar to the above in a few minutes in case someone feels like picking\nit up.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"432541","messageId":"20210811231644.570030-1-sandals@crustytoothpaste.net","threadId":"56258","inReplyTo":"YRRZ7E6W+xV2f/CG@camp.crustytoothpaste.net","subject":"[PATCH v2] t5607: avoid using prerequisites to select algorithm","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-08-11T23:16:44Z","receivedAt":"2021-08-11T23:17:02Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In this test, we currently use the SHA1 prerequisite to specify the\nalgorithm we're using to test, since SHA-256 bundles are always v3,\nwhereas SHA-1 bundles default to v2, and as a result the default output\ndiffers.\n\nHowever, this causes a problem if we run with GIT_TEST_FAIL_PREREQS set,\nsince that means that we'll unexpectedly fail the SHA1 prerequisite,\nresulting in incorrect expected output.  Let's fix this by checking\nagainst the built-in data called \"algo\", which tells us which algorithm\nis in use.  This should work in any situation, making our test a little\nmore robust.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t5607-clone-bundle.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh\nindex ed0d911e95..51705aa86a 100755\n--- a/t/t5607-clone-bundle.sh\n+++ b/t/t5607-clone-bundle.sh\n@@ -91,7 +91,8 @@ test_expect_success 'ridiculously long subject in boundary' '\n \n \tgit fetch long-subject-bundle.bdl &&\n \n-\tif ! test_have_prereq SHA1\n+\talgo=$(test_oid algo) &&\n+\tif test \"$algo\" != sha1\n \tthen\n \t\techo \"@object-format=sha256\"\n \tfi >expect &&\n@@ -100,7 +101,7 @@ test_expect_success 'ridiculously long subject in boundary' '\n \t$(git rev-parse HEAD) HEAD\n \tEOF\n \n-\tif test_have_prereq SHA1\n+\tif test \"$algo\" = sha1\n \tthen\n \t\thead -n 3 long-subject-bundle.bdl\n \telse\n"},{"id":"432545","messageId":"YRRdjgzjokT1nQmG@coredump.intra.peff.net","threadId":"56258","inReplyTo":"20210811231644.570030-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2] t5607: avoid using prerequisites to select algorithm","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-08-11T23:30:22Z","receivedAt":"2021-08-11T23:30:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 11, 2021 at 11:16:44PM +0000, brian m. carlson wrote:\n\n> In this test, we currently use the SHA1 prerequisite to specify the\n> algorithm we're using to test, since SHA-256 bundles are always v3,\n> whereas SHA-1 bundles default to v2, and as a result the default output\n> differs.\n> \n> However, this causes a problem if we run with GIT_TEST_FAIL_PREREQS set,\n> since that means that we'll unexpectedly fail the SHA1 prerequisite,\n> resulting in incorrect expected output.  Let's fix this by checking\n> against the built-in data called \"algo\", which tells us which algorithm\n> is in use.  This should work in any situation, making our test a little\n> more robust.\n\nThanks, this seems like a reasonable step, and fixes the test for me.\n\nI still get tons of other failures because I set GIT_TEST_HTTPD=yes,\nwhich implies to me we should still be fixing GIT_TEST_FAIL_PREREQS. But\nI am happy to take this in the meantime for people who do care.\n\n-Peff\n"}]}