{"thread":{"id":"58372","subject":"[PATCH 0/3] fix failing t4301 test and &&-chain breakage","startedAt":"2022-08-28T05:18:08Z","lastAt":"2022-08-30T14:02:51Z","messageCount":12,"participants":["Eric Sunshine via GitGitGadget","Junio C Hamano","Eric Sunshine","Elijah Newren","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"462049","messageId":"pull.1339.git.1661663879.gitgitgadget@gmail.com","threadId":"58372","inReplyTo":null,"subject":"[PATCH 0/3] fix failing t4301 test and &&-chain breakage","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-28T05:17:56Z","receivedAt":"2022-08-28T05:18:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This series fixes a failing test in t4301 due to 'sed' behavioral\ndifferences between implementations. It also fixes a couple broken &&-chains\nand adds missing explicit loop termination.\n\nThe third patch is entirely subjective and can be dropped if unwanted. I\nspent more than a few minutes puzzling over the script's use of 'printf\n\"\\\\n\"' rather than the more typical 'printf \"\\n\"' or even a simple 'echo',\nwondering if there was some subtlety I was missing or whether Elijah had\nencountered an unusual situation in which '\\\\n' was needed over '\\n'. The\nthird patch chooses to replace 'printf \"\\\\n\"' with 'echo' which I find more\nidiomatic, but I can see value in using 'printf \"\\n\"' as perhaps being\nclearer that it is adding a newline where one is missing.\n\nThe series is built atop 'en/t4301-more-merge-tree-tests' which is already\nin 'next'.\n\nEric Sunshine (3):\n  t4301: account for behavior differences between sed implementations\n  t4031: fix broken &&-chains and add missing loop termination\n  t4301: emit blank line in more idiomatic fashion\n\n t/t4301-merge-tree-write-tree.sh | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\n\nbase-commit: 3c4dbf556f425d83f3fbb729dcbecdc719ee4099\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1339%2Fsunshineco%2Fanonhash-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1339/sunshineco/anonhash-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1339\n-- \ngitgitgadget\n"},{"id":"462050","messageId":"a3576ff88226ffbdcc58bf837e4cd97dd299f77b.1661663880.git.gitgitgadget@gmail.com","threadId":"58372","inReplyTo":"pull.1339.git.1661663879.gitgitgadget@gmail.com","subject":"[PATCH 1/3] t4301: account for behavior differences between sed implementations","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-28T05:17:57Z","receivedAt":"2022-08-28T05:18:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nIt is a common pattern in this script to write the result of\n`merge-tree -z` (NUL-termination mode) to an \"actual\" file and then\nmanually append a newline to that file so that it can be diff'd easily\nwith a hand-crafted \"expect\" file which itself ends with a newline since\nit has been created by standard Unix tools which terminate lines by\ndefault. For instance:\n\n    git merge-tree --write-tree -z ... >out &&\n    printf \"\\\\n\" >>out\n    anonymize_hash out >actual &&\n    q_to_nul <<-EOF >expect &&\n    ...\n    EOF\n    test_cmp expect actual\n\nHowever, one test gets this backward:\n\n    git merge-tree --write-tree -z ... >out &&\n    anonymize_hash out >actual &&\n    printf \"\\\\n\" >>actual\n\nwhich means that, unlike all other cases, when anonymize_hash() is\ncalled, the file being anonymized does not end with a newline. As a\nresult, this test fails on some platforms.\n\nanonymize_hash() is implemented like this:\n\n    anonymize_hash() {\n        sed -e \"s/[0-9a-f]\\{40,\\}/HASH/g\" \"$@\"\n    }\n\nThe problem arises due to differences in behavior of various `sed`\nimplementations when fed an incomplete line (lacking a newline).\nAlthough most modern `sed` implementations output such a line\nunmolested (i.e. without a newline), some older `sed` implementations\nforcibly add a newline to the incomplete line (giving the output an\nextra unexpected newline), while other very old implementations simply\nswallow an incomplete line and don't emit it at all (making the output\nshorter than expected).\n\nFix this test by manually adding the newline before passing it through\n`sed`, thus ensuring identical behavior with all `sed` implementation,\nand bringing the test in line with other tests in this script.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t4301-merge-tree-write-tree.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex c5fd56df28f..d44c7767f30 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -760,8 +760,8 @@ test_expect_success 'NUL terminated conflicted file \"lines\"' '\n \tgit commit -m \"Renamed numbers\" &&\n \n \ttest_expect_code 1 git merge-tree --write-tree -z tweak1 side2 >out &&\n+\tprintf \"\\\\n\" >>out &&\n \tanonymize_hash out >actual &&\n-\tprintf \"\\\\n\" >>actual &&\n \n \t# Expected results:\n \t#   \"greeting\" should merge with conflicts\n-- \ngitgitgadget\n\n"},{"id":"462051","messageId":"dce35a47012fecc6edc11c68e91dbb485c5bc36f.1661663880.git.gitgitgadget@gmail.com","threadId":"58372","inReplyTo":"pull.1339.git.1661663879.gitgitgadget@gmail.com","subject":"[PATCH 2/3] t4031: fix broken &&-chains and add missing loop termination","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-28T05:17:58Z","receivedAt":"2022-08-28T05:18:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nFix &&-chain breaks in a couple tests which went unnoticed due to blind\nspots in the &&-chain linters. In particular, the \"magic exit code 117\"\n&&-chain checker built into test-lib.sh only recognizes broken &&-chains\nat the top-level; it does not work within `{...}` groups, `(...)`\nsubshells, `$(...)` substitutions, or within bodies of compound\nstatements, such as `if`, `for`, `while`, `case`, etc. Furthermore,\n`chainlint.sed`, which detects broken &&-chains only in `(...)`\nsubshells, missed these cases (which are in subshells) because it\n(surprisingly) neglects to check for intact &&-chain on single-line\n`for` loops.\n\nWhile at it, explicitly signal failure of commands within the `for`\nloops (which might arise due to the filesystem being full or \"inode\"\nexhaustion). This is important since failures within `for` and `while`\nloops can go unnoticed if not detected and signaled manually since the\nloop itself does not abort when a contained command fails, nor will a\nfailure necessarily be detected when the loop finishes since the loop\nreturns the exit code of the last command it ran on the final iteration,\nwhich may not be the command which failed.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t4301-merge-tree-write-tree.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex d44c7767f30..82a104bcbc9 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -150,7 +150,7 @@ test_expect_success 'directory rename + content conflict' '\n \t\tcd dir-rename-and-content &&\n \t\ttest_write_lines 1 2 3 4 5 >foo &&\n \t\tmkdir olddir &&\n-\t\tfor i in a b c; do echo $i >olddir/$i; done\n+\t\tfor i in a b c; do echo $i >olddir/$i || exit 1; done &&\n \t\tgit add foo olddir &&\n \t\tgit commit -m \"original\" &&\n \n@@ -662,7 +662,7 @@ test_expect_success 'directory rename + rename/delete + modify/delete + director\n \t\tcd 4-stacked-conflict &&\n \t\ttest_write_lines 1 2 3 4 5 >foo &&\n \t\tmkdir olddir &&\n-\t\tfor i in a b c; do echo $i >olddir/$i; done\n+\t\tfor i in a b c; do echo $i >olddir/$i || exit 1; done &&\n \t\tgit add foo olddir &&\n \t\tgit commit -m \"original\" &&\n \n-- \ngitgitgadget\n\n"},{"id":"462052","messageId":"32cb3a23c31ecc2700d14d80b62fcccf40da81a6.1661663880.git.gitgitgadget@gmail.com","threadId":"58372","inReplyTo":"pull.1339.git.1661663879.gitgitgadget@gmail.com","subject":"[PATCH 3/3] t4301: emit blank line in more idiomatic fashion","fromName":"Eric Sunshine via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-28T05:17:59Z","receivedAt":"2022-08-28T05:18:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"From: Eric Sunshine <sunshine@sunshineco.com>\n\nThe unusual use of:\n\n    printf \"\\\\n\" >>file &&\n\nmay give readers pause, making them wonder why this form was chosen over\nthe more typical:\n\n    printf \"\\n\" >>file &&\n\nHowever, even that may give pause since it is a somewhat unusual and\nlong-winded way of saying:\n\n    echo >>file &&\n\nTherefore, replace `printf` with the more idiomatic `echo`, with the\nhope of eliminating a possible stumbling block for those reading the\ncode.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t4301-merge-tree-write-tree.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex 82a104bcbc9..28ca5c38bb5 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -176,7 +176,7 @@ test_expect_success 'directory rename + content conflict' '\n \n \t\ttest_expect_code 1 \\\n \t\t\tgit merge-tree -z A^0 B^0 >out &&\n-\t\tprintf \"\\\\n\" >>out &&\n+\t\techo >>out &&\n \t\tanonymize_hash out >actual &&\n \t\tq_to_tab <<-\\EOF | lf_to_nul >expect &&\n \t\tHASH\n@@ -230,7 +230,7 @@ test_expect_success 'rename/delete handling' '\n \n \t\ttest_expect_code 1 \\\n \t\t\tgit merge-tree -z A^0 B^0 >out &&\n-\t\tprintf \"\\\\n\" >>out &&\n+\t\techo >>out &&\n \t\tanonymize_hash out >actual &&\n \t\tq_to_tab <<-\\EOF | lf_to_nul >expect &&\n \t\tHASH\n@@ -284,7 +284,7 @@ test_expect_success 'rename/add handling' '\n \n \t\ttest_expect_code 1 \\\n \t\t\tgit merge-tree -z A^0 B^0 >out &&\n-\t\tprintf \"\\\\n\" >>out &&\n+\t\techo >>out &&\n \n \t\t#\n \t\t# First, check that the bar that appears at stage 3 does not\n@@ -351,7 +351,7 @@ test_expect_success SYMLINKS 'rename/add, where add is a mode conflict' '\n \n \t\ttest_expect_code 1 \\\n \t\t\tgit merge-tree -z A^0 B^0 >out &&\n-\t\tprintf \"\\\\n\" >>out &&\n+\t\techo >>out &&\n \n \t\t#\n \t\t# First, check that the bar that appears at stage 3 does not\n@@ -417,7 +417,7 @@ test_expect_success 'rename/rename + content conflict' '\n \n \t\ttest_expect_code 1 \\\n \t\t\tgit merge-tree -z A^0 B^0 >out &&\n-\t\tprintf \"\\\\n\" >>out &&\n+\t\techo >>out &&\n \t\tanonymize_hash out >actual &&\n \t\tq_to_tab <<-\\EOF | lf_to_nul >expect &&\n \t\tHASH\n@@ -471,7 +471,7 @@ test_expect_success 'rename/add/delete conflict' '\n \n \t\ttest_expect_code 1 \\\n \t\t\tgit merge-tree -z B^0 A^0 >out &&\n-\t\tprintf \"\\\\n\" >>out &&\n+\t\techo >>out &&\n \t\tanonymize_hash out >actual &&\n \n \t\tq_to_tab <<-\\EOF | lf_to_nul >expect &&\n@@ -528,7 +528,7 @@ test_expect_success 'rename/rename(2to1)/delete/delete conflict' '\n \n \t\ttest_expect_code 1 \\\n \t\t\tgit merge-tree -z A^0 B^0 >out &&\n-\t\tprintf \"\\\\n\" >>out &&\n+\t\techo >>out &&\n \t\tanonymize_hash out >actual &&\n \n \t\tq_to_tab <<-\\EOF | lf_to_nul >expect &&\n@@ -600,7 +600,7 @@ test_expect_success 'mod6: chains of rename/rename(1to2) and add/add via collidi\n \n \t\ttest_expect_code 1 \\\n \t\t\tgit merge-tree -z A^0 B^0 >out &&\n-\t\tprintf \"\\\\n\" >>out &&\n+\t\techo >>out &&\n \n \t\t#\n \t\t# First, check that some of the hashes that appear as stage\n@@ -690,7 +690,7 @@ test_expect_success 'directory rename + rename/delete + modify/delete + director\n \n \t\ttest_expect_code 1 \\\n \t\t\tgit merge-tree -z A^0 B^0 >out &&\n-\t\tprintf \"\\\\n\" >>out &&\n+\t\techo >>out &&\n \t\tanonymize_hash out >actual &&\n \n \t\tq_to_tab <<-\\EOF | lf_to_nul >expect &&\n@@ -760,7 +760,7 @@ test_expect_success 'NUL terminated conflicted file \"lines\"' '\n \tgit commit -m \"Renamed numbers\" &&\n \n \ttest_expect_code 1 git merge-tree --write-tree -z tweak1 side2 >out &&\n-\tprintf \"\\\\n\" >>out &&\n+\techo >>out &&\n \tanonymize_hash out >actual &&\n \n \t# Expected results:\n-- \ngitgitgadget\n"},{"id":"462062","messageId":"xmqq35dgt9ph.fsf@gitster.g","threadId":"58372","inReplyTo":"pull.1339.git.1661663879.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] fix failing t4301 test and &&-chain breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-28T20:05:46Z","receivedAt":"2022-08-28T20:05:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Eric Sunshine via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> Eric Sunshine (3):\n>   t4301: account for behavior differences between sed implementations\n>   t4031: fix broken &&-chains and add missing loop termination\n>   t4301: emit blank line in more idiomatic fashion\n>\n>  t/t4301-merge-tree-write-tree.sh | 24 ++++++++++++------------\n>  1 file changed, 12 insertions(+), 12 deletions(-)\n\nThe second one is off by 270.\n"},{"id":"462063","messageId":"CAPig+cSzQAwQLVXbQRLpOJOC=APP-T0DfCzw87xuXKfM8nzSWw@mail.gmail.com","threadId":"58372","inReplyTo":"xmqq35dgt9ph.fsf@gitster.g","subject":"Re: [PATCH 0/3] fix failing t4301 test and &&-chain breakage","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-08-28T20:46:55Z","receivedAt":"2022-08-28T20:47:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 28, 2022 at 4:05 PM Junio C Hamano <gitster@pobox.com> wrote:\n> \"Eric Sunshine via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> >   t4301: account for behavior differences between sed implementations\n> >   t4031: fix broken &&-chains and add missing loop termination\n> >   t4301: emit blank line in more idiomatic fashion\n>\n> The second one is off by 270.\n\nShall I re-roll or will you fix it while queuing (assuming you queue it)?\n"},{"id":"462067","messageId":"xmqq4jxvsjag.fsf@gitster.g","threadId":"58372","inReplyTo":"CAPig+cSzQAwQLVXbQRLpOJOC=APP-T0DfCzw87xuXKfM8nzSWw@mail.gmail.com","subject":"Re: [PATCH 0/3] fix failing t4301 test and &&-chain breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-29T05:36:23Z","receivedAt":"2022-08-29T05:36:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Sun, Aug 28, 2022 at 4:05 PM Junio C Hamano <gitster@pobox.com> wrote:\n>> \"Eric Sunshine via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>> >   t4301: account for behavior differences between sed implementations\n>> >   t4031: fix broken &&-chains and add missing loop termination\n>> >   t4301: emit blank line in more idiomatic fashion\n>>\n>> The second one is off by 270.\n>\n> Shall I re-roll or will you fix it while queuing (assuming you queue it)?\n\nI plan to fix it up when I queue.\n"},{"id":"462123","messageId":"CABPp-BGBEfFh0z0YHGcHE+rva9JdapXprPn-RSmif0xn6fcxYw@mail.gmail.com","threadId":"58372","inReplyTo":"pull.1339.git.1661663879.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] fix failing t4301 test and &&-chain breakage","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-08-30T02:52:59Z","receivedAt":"2022-08-30T02:53:18Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sat, Aug 27, 2022 at 10:18 PM Eric Sunshine via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> This series fixes a failing test in t4301 due to 'sed' behavioral\n> differences between implementations. It also fixes a couple broken &&-chains\n> and adds missing explicit loop termination.\n>\n> The third patch is entirely subjective and can be dropped if unwanted. I\n> spent more than a few minutes puzzling over the script's use of 'printf\n> \"\\\\n\"' rather than the more typical 'printf \"\\n\"' or even a simple 'echo',\n> wondering if there was some subtlety I was missing or whether Elijah had\n> encountered an unusual situation in which '\\\\n' was needed over '\\n'. The\n> third patch chooses to replace 'printf \"\\\\n\"' with 'echo' which I find more\n> idiomatic, but I can see value in using 'printf \"\\n\"' as perhaps being\n> clearer that it is adding a newline where one is missing.\n\nI can't actually provide the reasoning for it; I took Dscho's testcase\nfrom [1] and used it as a basis for adding several other testcases.\nWhen I was copying & pasting and adjusting, I just didn't notice the\n'printf \"\\\\n\"'.  But using a simple echo makes sense.\n\n[1] https://lore.kernel.org/git/3b4ed8bb1bb615277ee51a7b2af5fc53bae0a6e4.1660892256.git.gitgitgadget@gmail.com/\n\nAnyway, I've read through the patches and your series looks good to me.\n"},{"id":"462124","messageId":"CABPp-BGTdqo9rBoWYeOp+ATU1NS+GFzBcdPnMPGgH0_Jksr7zA@mail.gmail.com","threadId":"58372","inReplyTo":"xmqq35dgt9ph.fsf@gitster.g","subject":"Re: [PATCH 0/3] fix failing t4301 test and &&-chain breakage","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-08-30T02:53:40Z","receivedAt":"2022-08-30T02:53:54Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sun, Aug 28, 2022 at 1:05 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"Eric Sunshine via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > Eric Sunshine (3):\n> >   t4301: account for behavior differences between sed implementations\n> >   t4031: fix broken &&-chains and add missing loop termination\n> >   t4301: emit blank line in more idiomatic fashion\n> >\n> >  t/t4301-merge-tree-write-tree.sh | 24 ++++++++++++------------\n> >  1 file changed, 12 insertions(+), 12 deletions(-)\n>\n> The second one is off by 270.\n\nApparently Eric knew what you meant, but I'm perplexed by this\nstatement and what it means.  What am I missing?\n"},{"id":"462125","messageId":"CAPig+cTNaE8sBRyMqMzRiE7+RMwaFEUNPuArr8dvrOrRzq-QFA@mail.gmail.com","threadId":"58372","inReplyTo":"CABPp-BGTdqo9rBoWYeOp+ATU1NS+GFzBcdPnMPGgH0_Jksr7zA@mail.gmail.com","subject":"Re: [PATCH 0/3] fix failing t4301 test and &&-chain breakage","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-08-30T02:56:33Z","receivedAt":"2022-08-30T02:56:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Aug 29, 2022 at 10:53 PM Elijah Newren <newren@gmail.com> wrote:\n> On Sun, Aug 28, 2022 at 1:05 PM Junio C Hamano <gitster@pobox.com> wrote:\n> > >   t4301: account for behavior differences between sed implementations\n> > >   t4031: fix broken &&-chains and add missing loop termination\n> > >   t4301: emit blank line in more idiomatic fashion\n> >\n> > The second one is off by 270.\n>\n> Apparently Eric knew what you meant, but I'm perplexed by this\n> statement and what it means.  What am I missing?\n\nJunio's comment was opaque to me, as well, and it took several minutes\nto figure it out (especially since I authored the patches, thus I read\nwhat I expected to read, not what was really there). Taking a look at\njust prefixes on the patch subject lines...\n\n    t4301\n    t4031\n    t4301\n\nthere's a transposition in there which, mathematically speaking, makes\none of the test script numbers 270 less than the others.\n"},{"id":"462126","messageId":"CABPp-BEeyyx4GbtYEUk3kXK=-3hneLACmD0_yLJZEhcTN2qpGA@mail.gmail.com","threadId":"58372","inReplyTo":"CAPig+cTNaE8sBRyMqMzRiE7+RMwaFEUNPuArr8dvrOrRzq-QFA@mail.gmail.com","subject":"Re: [PATCH 0/3] fix failing t4301 test and &&-chain breakage","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-08-30T02:59:35Z","receivedAt":"2022-08-30T02:59:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Aug 29, 2022 at 7:56 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Mon, Aug 29, 2022 at 10:53 PM Elijah Newren <newren@gmail.com> wrote:\n> > On Sun, Aug 28, 2022 at 1:05 PM Junio C Hamano <gitster@pobox.com> wrote:\n> > > >   t4301: account for behavior differences between sed implementations\n> > > >   t4031: fix broken &&-chains and add missing loop termination\n> > > >   t4301: emit blank line in more idiomatic fashion\n> > >\n> > > The second one is off by 270.\n> >\n> > Apparently Eric knew what you meant, but I'm perplexed by this\n> > statement and what it means.  What am I missing?\n>\n> Junio's comment was opaque to me, as well, and it took several minutes\n> to figure it out (especially since I authored the patches, thus I read\n> what I expected to read, not what was really there). Taking a look at\n> just prefixes on the patch subject lines...\n>\n>     t4301\n>     t4031\n>     t4301\n>\n> there's a transposition in there which, mathematically speaking, makes\n> one of the test script numbers 270 less than the others.\n\nAh, gotcha.  Yeah, I totally missed both the digit transposition and\nthe attempt to highlight it.  Thanks for the explanation.\n"},{"id":"462196","messageId":"30rq792s-818n-q078-3837-977prpqqprq6@tzk.qr","threadId":"58372","inReplyTo":"CABPp-BGBEfFh0z0YHGcHE+rva9JdapXprPn-RSmif0xn6fcxYw@mail.gmail.com","subject":"Re: [PATCH 0/3] fix failing t4301 test and &&-chain breakage","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-30T14:02:44Z","receivedAt":"2022-08-30T14:02:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah & Eric,\n\nOn Mon, 29 Aug 2022, Elijah Newren wrote:\n\n> On Sat, Aug 27, 2022 at 10:18 PM Eric Sunshine via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> >\n> > This series fixes a failing test in t4301 due to 'sed' behavioral\n> > differences between implementations. It also fixes a couple broken &&-chains\n> > and adds missing explicit loop termination.\n> >\n> > The third patch is entirely subjective and can be dropped if unwanted. I\n> > spent more than a few minutes puzzling over the script's use of 'printf\n> > \"\\\\n\"' rather than the more typical 'printf \"\\n\"' or even a simple 'echo',\n> > wondering if there was some subtlety I was missing or whether Elijah had\n> > encountered an unusual situation in which '\\\\n' was needed over '\\n'. The\n> > third patch chooses to replace 'printf \"\\\\n\"' with 'echo' which I find more\n> > idiomatic, but I can see value in using 'printf \"\\n\"' as perhaps being\n> > clearer that it is adding a newline where one is missing.\n>\n> I can't actually provide the reasoning for it; I took Dscho's testcase\n> from [1] and used it as a basis for adding several other testcases.\n> When I was copying & pasting and adjusting, I just didn't notice the\n> 'printf \"\\\\n\"'.  But using a simple echo makes sense.\n>\n> [1] https://lore.kernel.org/git/3b4ed8bb1bb615277ee51a7b2af5fc53bae0a6e4.1660892256.git.gitgitgadget@gmail.com/\n\nNo other reason than that I _seem_ to recall having run into some issues\nwhere _some_ POSIX shell (was it BusyBox' ash?) did not like the\nsingle-escape form \"\\n\".\n\nI have no firm recollection, though, and am fine with converting all of\nthe double backslashes to single backslashes (read: I am very indifferent\nto this issue).\n\nCiao,\nDscho\n"}]}