{"thread":{"id":"59702","subject":"[PATCH] t9800: correct misuse of 'show -s --raw' in a test","startedAt":"2023-05-06T21:30:01Z","lastAt":"2023-05-08T20:53:29Z","messageCount":3,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"476682","messageId":"xmqqild5rvvw.fsf@gitster.g","threadId":"59702","inReplyTo":null,"subject":"[PATCH] t9800: correct misuse of 'show -s --raw' in a test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-06T21:29:55Z","receivedAt":"2023-05-06T21:30:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There is $(git show -s --raw --pretty=format:%at HEAD) in this test\nthat is meant to grab the author time of the commit.  We used to\nhave a bug in the command line option parser of the diff family of\ncommands, where \"show -s --raw\" was identical to \"show -s\".\n\nWith the \"-s\" bug fixed, \"show -s --raw\" would mean the same thing\nas \"show --raw\", i.e. show the output from the diff machinery in the\n\"raw\" format.  And this test will start failing, so fix it before\nthat happens.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n* It appears pretty obvious that the original wanted to just grab\n  the %at timestamp of the HEAD, and with the fix to allow output\n  format options to take effect after \"-s\" is given, the existing\n  misuse of \"--raw\" after \"-s\" that ought to be a no-op breaks the\n  test.\n\n  https://github.com/git/git/actions/runs/4897931294/jobs/8755228452#step:4:1847\n\n  is CI run with the \"-s\" fix merged into 'seen'.\n\n t/t9800-git-p4-basic.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\nindex dc88d0e064..a4b3cb9492 100755\n--- a/t/t9800-git-p4-basic.sh\n+++ b/t/t9800-git-p4-basic.sh\n@@ -330,7 +330,7 @@ test_expect_success 'initial import time from top change time' '\n \ttest_when_finished cleanup_git &&\n \t(\n \t\tcd \"$git\" &&\n-\t\tgittime=$(git show -s --raw --pretty=format:%at HEAD) &&\n+\t\tgittime=$(git show -s --pretty=format:%at HEAD) &&\n \t\techo $p4time $gittime &&\n \t\ttest $p4time = $gittime\n \t)\n-- \n2.40.1-476-g69c786637d\n\n"},{"id":"476788","messageId":"20230508185545.GA1893355@coredump.intra.peff.net","threadId":"59702","inReplyTo":"xmqqild5rvvw.fsf@gitster.g","subject":"Re: [PATCH] t9800: correct misuse of 'show -s --raw' in a test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-08T18:55:45Z","receivedAt":"2023-05-08T18:55:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 06, 2023 at 02:29:55PM -0700, Junio C Hamano wrote:\n\n> There is $(git show -s --raw --pretty=format:%at HEAD) in this test\n> that is meant to grab the author time of the commit.  We used to\n> have a bug in the command line option parser of the diff family of\n> commands, where \"show -s --raw\" was identical to \"show -s\".\n> \n> With the \"-s\" bug fixed, \"show -s --raw\" would mean the same thing\n> as \"show --raw\", i.e. show the output from the diff machinery in the\n> \"raw\" format.  And this test will start failing, so fix it before\n> that happens.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n> * It appears pretty obvious that the original wanted to just grab\n>   the %at timestamp of the HEAD, and with the fix to allow output\n>   format options to take effect after \"-s\" is given, the existing\n>   misuse of \"--raw\" after \"-s\" that ought to be a no-op breaks the\n>   test.\n\nYes, I think this is matching what the original code was trying to do.\nThe patch looks good to me.\n\n-Peff\n"},{"id":"476809","messageId":"xmqqild2mtod.fsf@gitster.g","threadId":"59702","inReplyTo":"20230508185545.GA1893355@coredump.intra.peff.net","subject":"Re: [PATCH] t9800: correct misuse of 'show -s --raw' in a test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-08T20:53:22Z","receivedAt":"2023-05-08T20:53:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> * It appears pretty obvious that the original wanted to just grab\n>>   the %at timestamp of the HEAD, and with the fix to allow output\n>>   format options to take effect after \"-s\" is given, the existing\n>>   misuse of \"--raw\" after \"-s\" that ought to be a no-op breaks the\n>>   test.\n>\n> Yes, I think this is matching what the original code was trying to do.\n> The patch looks good to me.\n\nThanks.\n\nThis still can be a cause for backward compatibility worry, though.\nI do not think anybody would have lifted $(git show -s --raw --format=%ad)\nas a way to get the author timestamp from this test script, but it\nis entirely possible that this was written by the author who saw it\ndone (incorrectly) that way elsewhere, and who knows how many others\ncopied that (incorrect) way of using \"-s --raw\" from that original\nsource we have no control over.\n\nAnyway.\n\n\n"}]}