{"thread":{"id":"64296","subject":"[PATCH] t1016: make sure to use specified GPG","startedAt":"2025-10-10T21:14:03Z","lastAt":"2025-10-24T01:52:32Z","messageCount":8,"participants":["Junio C Hamano","Todd Zullinger","Andrew Kreimer"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"528538","messageId":"xmqqsefq7947.fsf@gitster.g","threadId":"64296","inReplyTo":null,"subject":"[PATCH] t1016: make sure to use specified GPG","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-10T21:14:00Z","receivedAt":"2025-10-10T21:14:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"c348192a (t1016: clean up style, 2024-10-22) fixed a coding style\nviolation that has an extra space between redirection operator \">\"\nand the redirection target, but at the same time, replaced the use\nof \"git config\" to set a configuration variable to be used by the\nremainder of tests with \"test_config\".  The pattern employed here is\nthat the first set-up test prepares the environment to be used by\nsubsequent tests, which then use the settings left by this set-up\ntest to perform their tasks.  Using test_config in the first set-up\ntest means the config setting made by the set-up test is reverted at\nthe end of the first set-up test, which totally misses the point.\n\nGo back to use \"git config\" to fix this.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * The commit in question was from October last year, and I didn't\n   notice it until I looked at how the test script evolved.  It is a\n   bit embarrassing that we didn't catch it during review.\n\n t/t1016-compatObjectFormat.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh\nindex 8341a2fe83..cb6d308f1d 100755\n--- a/t/t1016-compatObjectFormat.sh\n+++ b/t/t1016-compatObjectFormat.sh\n@@ -116,7 +116,7 @@ do\n \t\tgit config core.repositoryformatversion 1 &&\n \t\tgit config extensions.objectformat $hash &&\n \t\tgit config extensions.compatobjectformat $(compat_hash $hash) &&\n-\t\ttest_config gpg.program $TEST_DIRECTORY/t1016/gpg &&\n+\t\tgit config gpg.program $TEST_DIRECTORY/t1016/gpg &&\n \t\techo \"Hello World!\" >hello &&\n \t\teval hello_${hash}_oid=$(git hash-object hello) &&\n \t\tgit update-index --add hello &&\n-- \n2.51.0-616-gc7d8d4fd8b\n\n"},{"id":"528582","messageId":"aOu59eVs7tK6pCoF@teonanacatl.net","threadId":"64296","inReplyTo":"xmqqsefq7947.fsf@gitster.g","subject":"Re: [PATCH] t1016: make sure to use specified GPG","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2025-10-12T14:23:49Z","receivedAt":"2025-10-12T14:23:52Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Junio C Hamano wrote:\n> c348192a (t1016: clean up style, 2024-10-22) fixed a coding style\n> violation that has an extra space between redirection operator \">\"\n> and the redirection target, but at the same time, replaced the use\n> of \"git config\" to set a configuration variable to be used by the\n> remainder of tests with \"test_config\".  The pattern employed here is\n> that the first set-up test prepares the environment to be used by\n> subsequent tests, which then use the settings left by this set-up\n> test to perform their tasks.  Using test_config in the first set-up\n> test means the config setting made by the set-up test is reverted at\n> the end of the first set-up test, which totally misses the point.\n> \n> Go back to use \"git config\" to fix this.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  * The commit in question was from October last year, and I didn't\n>    notice it until I looked at how the test script evolved.  It is a\n>    bit embarrassing that we didn't catch it during review.\n\nInteresting.  And well-spotted.\n\nThis _does_ seem to resolve the failures in our CI and in\nthe Fedora build system.  I was able to run a few test\nbuilds.  With this fix, the tests were successful where they\nwere not without it.\n\nI remember suspecting the gpg calls were not using the\nwrapper command in gpg.program.  I even tried forcing the\n--faked-system-time for all the tests to check that theory,\nunsuccessfully.\n\nOddly, I ran into test failures after fixing the GPG2 prereq\nlong before c348192afe (t1016: clean up style, 2024-10-22)\nwas in place.  Perhaps I was hitting a different issue\ninitially?  Then, when I looked at it again I didn't think\nabout gpg.program again, since I'd already tried to force\nthe gpg wrapper which sets --faked-system-time.\n\nIt's both annoying and embarrassing if it is that simple and\nI missed it after looking a few times, to be sure.  But I'll\nbe happy with the end result all the same. :)\n\n>  t/t1016-compatObjectFormat.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh\n> index 8341a2fe83..cb6d308f1d 100755\n> --- a/t/t1016-compatObjectFormat.sh\n> +++ b/t/t1016-compatObjectFormat.sh\n> @@ -116,7 +116,7 @@ do\n>  \t\tgit config core.repositoryformatversion 1 &&\n>  \t\tgit config extensions.objectformat $hash &&\n>  \t\tgit config extensions.compatobjectformat $(compat_hash $hash) &&\n> -\t\ttest_config gpg.program $TEST_DIRECTORY/t1016/gpg &&\n> +\t\tgit config gpg.program $TEST_DIRECTORY/t1016/gpg &&\n>  \t\techo \"Hello World!\" >hello &&\n>  \t\teval hello_${hash}_oid=$(git hash-object hello) &&\n>  \t\tgit update-index --add hello &&\n\n-- \nTodd\n"},{"id":"528584","messageId":"xmqq1pn85f5i.fsf@gitster.g","threadId":"64296","inReplyTo":"aOu59eVs7tK6pCoF@teonanacatl.net","subject":"Re: [PATCH] t1016: make sure to use specified GPG","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-12T15:11:05Z","receivedAt":"2025-10-12T15:11:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> Interesting.  And well-spotted.\n>\n> This _does_ seem to resolve the failures in our CI and in\n> the Fedora build system.  I was able to run a few test\n> builds.  With this fix, the tests were successful where they\n> were not without it.\n>\n> I remember suspecting the gpg calls were not using the\n> wrapper command in gpg.program.  I even tried forcing the\n> --faked-system-time for all the tests to check that theory,\n> unsuccessfully.\n>\n> Oddly, I ran into test failures after fixing the GPG2 prereq\n> long before c348192afe (t1016: clean up style, 2024-10-22)\n> was in place.  Perhaps I was hitting a different issue\n> initially?  Then, when I looked at it again I didn't think\n> about gpg.program again, since I'd already tried to force\n> the gpg wrapper which sets --faked-system-time.\n>\n> It's both annoying and embarrassing if it is that simple and\n> I missed it after looking a few times, to be sure.  But I'll\n> be happy with the end result all the same. :)\n\nFWIW, GitHub CI jobs are failing t1016 at the tip of 'seen' (which\nhas this change), but only some and not all the jobs, which may\nindicate there are timeing-dependent flakes involved.  I didn't dig\nfurther, though.\n\n>>  t/t1016-compatObjectFormat.sh | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>> \n>> diff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh\n>> index 8341a2fe83..cb6d308f1d 100755\n>> --- a/t/t1016-compatObjectFormat.sh\n>> +++ b/t/t1016-compatObjectFormat.sh\n>> @@ -116,7 +116,7 @@ do\n>>  \t\tgit config core.repositoryformatversion 1 &&\n>>  \t\tgit config extensions.objectformat $hash &&\n>>  \t\tgit config extensions.compatobjectformat $(compat_hash $hash) &&\n>> -\t\ttest_config gpg.program $TEST_DIRECTORY/t1016/gpg &&\n>> +\t\tgit config gpg.program $TEST_DIRECTORY/t1016/gpg &&\n>>  \t\techo \"Hello World!\" >hello &&\n>>  \t\teval hello_${hash}_oid=$(git hash-object hello) &&\n>>  \t\tgit update-index --add hello &&\n"},{"id":"528586","messageId":"aOvz-ac8JTCaRwIm@teonanacatl.net","threadId":"64296","inReplyTo":"xmqq1pn85f5i.fsf@gitster.g","subject":"Re: [PATCH] t1016: make sure to use specified GPG","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2025-10-12T18:31:21Z","receivedAt":"2025-10-12T18:31:23Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Junio C Hamano wrote:\n> Todd Zullinger <tmz@pobox.com> writes:\n> \n>> Interesting.  And well-spotted.\n>>\n>> This _does_ seem to resolve the failures in our CI and in\n>> the Fedora build system.  I was able to run a few test\n>> builds.  With this fix, the tests were successful where they\n>> were not without it.\n>>\n>> I remember suspecting the gpg calls were not using the\n>> wrapper command in gpg.program.  I even tried forcing the\n>> --faked-system-time for all the tests to check that theory,\n>> unsuccessfully.\n>>\n>> Oddly, I ran into test failures after fixing the GPG2 prereq\n>> long before c348192afe (t1016: clean up style, 2024-10-22)\n>> was in place.  Perhaps I was hitting a different issue\n>> initially?  Then, when I looked at it again I didn't think\n>> about gpg.program again, since I'd already tried to force\n>> the gpg wrapper which sets --faked-system-time.\n>>\n>> It's both annoying and embarrassing if it is that simple and\n>> I missed it after looking a few times, to be sure.  But I'll\n>> be happy with the end result all the same. :)\n> \n> FWIW, GitHub CI jobs are failing t1016 at the tip of 'seen' (which\n> has this change), but only some and not all the jobs, which may\n> indicate there are timeing-dependent flakes involved.  I didn't dig\n> further, though.\n\nAhh.  When I saw this patch and checked the actions, I\nlooked at this job:\n\n    https://github.com/git/git/actions/runs/18418984778\n\nand the failures all seemed to be unrelated to the test.\nLooking again, I see the ps/ci-avoid-broken-sudo-on-ubuntu\nbranch was merged to seen to fix that problem.\n\nBetween that initial \"success other than unrelated failures\"\nand my multiple successful runs in the previously-failing\nFedora build system, I thought it might have been fixed.  :/\n\nWhen I poked at these failures in the Fedora builds many\nmonths ago, I was able to get the full test-results output\nvia a bit of a gross hack to tar it up and print it to the\nbuild log as base64 text (only when there are failures):\n\n    # tar up test-results & $testdir, then print base64 encoded output\n    #\n    # copy $testdir contents to test-results to avoid absolute paths with tar\n    cp -a $testdir/* t/test-results/\n    begin='-----BEGIN BASE64 MESSAGE-----'\n    end='-----END BASE64 MESSAGE-----'\n    printf '\\n%s\\n' 'test-results and trash directory output follows; decode via:'\n    printf '%s\\n' \"sed -n '/^${begin}$/,/^${end}$/{/^${begin}$/!{/^${end}$/!p}}' \\\n        build.log | base64 -d >output.tar.zst\"\n    printf '%s\\n' \"$begin\"\n    tar --warning=no-file-ignored -C t -cf - test-results/ | zstdmt -17 | base64\n    printf '%s\\n' \"$end\"\n\nI don't know if we have or might want something like that\nfor the Github/Gitlab CI.\n\nEven with the test-results directory contents, I didn't\nmanage to work out what the issue was.  Maybe some\nadditional debug output from the code and tests to show the\nenvironment that is being used when gpg gets called to\nmake/verify the signatures could help.  I didn't try to\nadjust the code.  I did add some debugging to the gpg\nwrapper and used that wrapper by default, without success.\n\nBut I imagine that's mostly an issue with my unfamiliarity\nwith the code being tested. (The hacker version of \"it's not\nyou, it's me.\")\n\n-- \nTodd\n"},{"id":"528589","messageId":"aOwSIgswyQwr1FGy@void","threadId":"64296","inReplyTo":"xmqqsefq7947.fsf@gitster.g","subject":"Re: [PATCH] t1016: make sure to use specified GPG","fromName":"Andrew Kreimer","fromEmail":"algonell@gmail.com","sentAt":"2025-10-12T20:40:02Z","receivedAt":"2025-10-12T20:40:07Z","isPatch":true,"sender":{"key":"algonell@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4669001?v=4"},"body":"On Fri, Oct 10, 2025 at 02:14:00PM -0700, Junio C Hamano wrote:\n> Go back to use \"git config\" to fix this.\n\nApologies for the mess!\n"},{"id":"528634","messageId":"xmqqms5u3j8e.fsf@gitster.g","threadId":"64296","inReplyTo":"aOwSIgswyQwr1FGy@void","subject":"Re: [PATCH] t1016: make sure to use specified GPG","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-13T15:38:09Z","receivedAt":"2025-10-13T15:38:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Kreimer <algonell@gmail.com> writes:\n\n> On Fri, Oct 10, 2025 at 02:14:00PM -0700, Junio C Hamano wrote:\n>> Go back to use \"git config\" to fix this.\n>\n> Apologies for the mess!\n\nNo need to apologize.  A bug in reviewed patch is our collective\nfailing, and the author does not have sole ownership of it.\n\nThanks.\n"},{"id":"529526","messageId":"xmqq3479tk78.fsf@gitster.g","threadId":"64296","inReplyTo":"aOvz-ac8JTCaRwIm@teonanacatl.net","subject":"Re: [PATCH] t1016: make sure to use specified GPG","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-23T20:52:11Z","receivedAt":"2025-10-23T20:52:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> Junio C Hamano wrote:\n>> Todd Zullinger <tmz@pobox.com> writes:\n>> \n>>> Interesting.  And well-spotted.\n>>>\n>>> This _does_ seem to resolve the failures in our CI and in\n>>> the Fedora build system.  I was able to run a few test\n>>> builds.  With this fix, the tests were successful where they\n>>> were not without it.\n>> ...\n>> FWIW, GitHub CI jobs are failing t1016 at the tip of 'seen' (which\n>> has this change), but only some and not all the jobs, which may\n>> indicate there are timeing-dependent flakes involved.  I didn't dig\n>> further, though.\n\nLet's merge this fix down, even though it does not seem to have\nany effect improving the situation of flaky tests here.\n\n"},{"id":"529545","messageId":"aPrb3QGTNE7IBClk@teonanacatl.net","threadId":"64296","inReplyTo":"xmqq3479tk78.fsf@gitster.g","subject":"Re: [PATCH] t1016: make sure to use specified GPG","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2025-10-24T01:52:29Z","receivedAt":"2025-10-24T01:52:32Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Junio C Hamano wrote:\n> Todd Zullinger <tmz@pobox.com> writes:\n> \n>> Junio C Hamano wrote:\n>>> Todd Zullinger <tmz@pobox.com> writes:\n>>> \n>>>> Interesting.  And well-spotted.\n>>>>\n>>>> This _does_ seem to resolve the failures in our CI and in\n>>>> the Fedora build system.  I was able to run a few test\n>>>> builds.  With this fix, the tests were successful where they\n>>>> were not without it.\n>>> ...\n>>> FWIW, GitHub CI jobs are failing t1016 at the tip of 'seen' (which\n>>> has this change), but only some and not all the jobs, which may\n>>> indicate there are timeing-dependent flakes involved.  I didn't dig\n>>> further, though.\n> \n> Let's merge this fix down, even though it does not seem to have\n> any effect improving the situation of flaky tests here.\n\nSounds good, thanks.  It's the right thing to do, even if it\ndidn't clear up the issues which led to you noticing.\n\n-- \nTodd\n"}]}