{"thread":{"id":"42400","subject":"[PATCH] t0008: 4 tests fail with ksh88","startedAt":"2016-05-20T14:31:30Z","lastAt":"2016-05-22T00:07:58Z","messageCount":7,"participants":["Armin Kunaschik","Junio C Hamano","Christian Couder","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"287092","messageId":"CALR6jEhviK9KZxR6R6xzkZ5EAO-RjWj3xYah_DOSDXhEjYsT-A@mail.gmail.com","threadId":"42400","inReplyTo":null,"subject":"[PATCH] t0008: 4 tests fail with ksh88","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-05-20T14:31:30Z","receivedAt":"2016-05-20T14:31:30Z","isPatch":true,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"From: Armin Kunaschik \n\n\\\" in the test t0008 is not treated the same way in bash and in ksh.\nIn ksh the \\ disappears and generates false expect data to\ncompare with.\nUsing \\\\\" works portable, the same way in bash and in ksh and\nis less ambigous.\n\nAcked-by: Jeff King \nSigned-off-by: Armin Kunaschik \n---\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 89544dd..b425f3a 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -605,7 +605,7 @@ cat <<-EOF >expected-verbose\n a/b/.gitignore:8:!on* a/b/one\n a/b/.gitignore:8:!on* a/b/one one\n a/b/.gitignore:8:!on* a/b/one two\n- a/b/.gitignore:8:!on* \"a/b/one\\\"three\"\n+ a/b/.gitignore:8:!on* \"a/b/one\\\\\"three\"\n a/b/.gitignore:9:!two a/b/two\n a/.gitignore:1:two* a/b/twooo\n $global_excludes:2:!globaltwo globaltwo\n@@ -686,7 +686,7 @@ cat <<-EOF >expected-all\n a/b/.gitignore:8:!on* b/one\n a/b/.gitignore:8:!on* b/one one\n a/b/.gitignore:8:!on* b/one two\n- a/b/.gitignore:8:!on* \"b/one\\\"three\"\n+ a/b/.gitignore:8:!on* \"b/one\\\\\"three\"\n a/b/.gitignore:9:!two b/two\n :: b/not-ignored\n a/.gitignore:1:two* b/twooo\n"},{"id":"287095","messageId":"xmqq37pchi90.fsf@gitster.mtv.corp.google.com","threadId":"42400","inReplyTo":"CALR6jEhviK9KZxR6R6xzkZ5EAO-RjWj3xYah_DOSDXhEjYsT-A@mail.gmail.com","subject":"Re: [PATCH] t0008: 4 tests fail with ksh88","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-20T15:16:43Z","receivedAt":"2016-05-20T15:16:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Armin Kunaschik writes:\n\n> From: Armin Kunaschik \n>\n> \\\" in the test t0008 is not treated the same way in bash and in ksh.\n\nCould you refrain from singling out \"bash\"? We don't write for\n\"bash\" specifically (and the test I ran are with \"dash\" before I\npush things out).\n\nIdeally, if you can try ksh93 and if you find out that ksh93 works,\nthen the above can be made in line with your \"Subject\" to mark ksh88\nas broken (as opposed to other POSIX shells)? That would help us by\nreminding that running test fine with ksh93 is not a sufficient\ncheck to make sure we didn't break ksh88 users.\n\n> In ksh the \\ disappears and generates false expect data to\n> compare with.\n> Using \\\\\" works portable, the same way in bash and in ksh and\n> is less ambigous.\n\nAll of the above would need s/ksh/&88/g; I'd think. I just tried\n\n\tmake SHELL_PATH=/bin/ksh93\n cd t && /bin/ksh93 t0008-*.sh\n\nand this patch is not necessary for ksh93.\n\n> Acked-by: Jeff King \n\nI didn't see him acking this exact version, so if you didn't include\nthis line here, I would have missed it. Thanks.\n\n> Signed-off-by: Armin Kunaschik \n> ---\n> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\n> index 89544dd..b425f3a 100755\n> --- a/t/t0008-ignores.sh\n> +++ b/t/t0008-ignores.sh\n> @@ -605,7 +605,7 @@ cat <<-EOF >expected-verbose\n> a/b/.gitignore:8:!on* a/b/one\n> a/b/.gitignore:8:!on* a/b/one one\n> a/b/.gitignore:8:!on* a/b/one two\n> - a/b/.gitignore:8:!on* \"a/b/one\\\"three\"\n> + a/b/.gitignore:8:!on* \"a/b/one\\\\\"three\"\n> a/b/.gitignore:9:!two a/b/two\n> a/.gitignore:1:two* a/b/twooo\n> $global_excludes:2:!globaltwo globaltwo\n> @@ -686,7 +686,7 @@ cat <<-EOF >expected-all\n> a/b/.gitignore:8:!on* b/one\n> a/b/.gitignore:8:!on* b/one one\n> a/b/.gitignore:8:!on* b/one two\n> - a/b/.gitignore:8:!on* \"b/one\\\"three\"\n> + a/b/.gitignore:8:!on* \"b/one\\\\\"three\"\n> a/b/.gitignore:9:!two b/two\n> :: b/not-ignored\n> a/.gitignore:1:two* b/twooo\n"},{"id":"287100","messageId":"xmqqk2iog16g.fsf@gitster.mtv.corp.google.com","threadId":"42400","inReplyTo":"xmqq37pchi90.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0008: 4 tests fail with ksh88","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-20T16:10:47Z","receivedAt":"2016-05-20T16:10:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\n>> index 89544dd..b425f3a 100755\n>> --- a/t/t0008-ignores.sh\n>> +++ b/t/t0008-ignores.sh\n>> @@ -605,7 +605,7 @@ cat <<-EOF >expected-verbose\n>> a/b/.gitignore:8:!on* a/b/one\n>> a/b/.gitignore:8:!on* a/b/one one\n\nThe patch was whitespace-damaged and didn't apply, so I had to redo\nit from scratch while updating the log message. If this looks good\nto you, there is no need to resend.\n\nThanks.\n\n-- >8 --\nFrom: Armin Kunaschik \nDate: Fri, 20 May 2016 16:31:30 +0200\nSubject: [PATCH] t0008: 4 tests fail with ksh88\n\nIn t0008, we have\n\n\tcat <<-EOF\n\t...\n\ta/b/.gitignore:8:!on*\t\"a/b/one\\\"three\"\n\t...\n\tEOF\n\nane expect that the backslash-dq is passed through literally.\n\nksh88 eats \\ and generates a wrong expect data to compare with.\n\nUsing \\\\\" works this around without breaking other POSIX shells\n(which collapse backslash-backslash to a single backslash), and\nksh88 does so, too.\n\nIt makes it easier to read, too, because the reason why we are\nwriting backslash there is *not* because we think dq is special and\nwant to quote it (if that were the case we would have two more\nbackslashes on that line). It is simply because we want a single\nliteral backslash there. Since backslash is treated specially in\nunquoted here-document, explicitly doubling it to quote it expresses\nour intent better than relying on the character that immediately\ncomes after it (i.e. '\"') not being a special character.\n\nSigned-off-by: Armin Kunaschik \nAcked-by: Jeff King \nSigned-off-by: Junio C Hamano \n---\n t/t0008-ignores.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 89544dd..b425f3a 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -605,7 +605,7 @@ cat <<-EOF >expected-verbose\n \ta/b/.gitignore:8:!on*\ta/b/one\n \ta/b/.gitignore:8:!on*\ta/b/one one\n \ta/b/.gitignore:8:!on*\ta/b/one two\n-\ta/b/.gitignore:8:!on*\t\"a/b/one\\\"three\"\n+\ta/b/.gitignore:8:!on*\t\"a/b/one\\\\\"three\"\n \ta/b/.gitignore:9:!two\ta/b/two\n \ta/.gitignore:1:two*\ta/b/twooo\n \t$global_excludes:2:!globaltwo\tglobaltwo\n@@ -686,7 +686,7 @@ cat <<-EOF >expected-all\n \ta/b/.gitignore:8:!on*\tb/one\n \ta/b/.gitignore:8:!on*\tb/one one\n \ta/b/.gitignore:8:!on*\tb/one two\n-\ta/b/.gitignore:8:!on*\t\"b/one\\\"three\"\n+\ta/b/.gitignore:8:!on*\t\"b/one\\\\\"three\"\n \ta/b/.gitignore:9:!two\tb/two\n \t::\tb/not-ignored\n \ta/.gitignore:1:two*\tb/twooo\n-- \n2.8.3-625-g89e3711\n"},{"id":"287101","messageId":"CALR6jEj=Nd6K8poP=G+n_5dZfAGaZLqmnOV0EuSJ9aE5LNniEQ@mail.gmail.com","threadId":"42400","inReplyTo":"xmqq37pchi90.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0008: 4 tests fail with ksh88","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-05-20T16:11:45Z","receivedAt":"2016-05-20T16:11:45Z","isPatch":true,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"On Fri, May 20, 2016 at 5:16 PM, Junio C Hamano wrote:\n> Armin Kunaschik writes:\n>\n>> From: Armin Kunaschik \n>>\n>> \\\" in the test t0008 is not treated the same way in bash and in ksh.\n>\n> Could you refrain from singling out \"bash\"? We don't write for\n> \"bash\" specifically (and the test I ran are with \"dash\" before I\n> push things out).\nI can name it \"other shells\" if this is more comfortable. But I tested\nthis only with bash and ksh88 on AIX.\n\n> Ideally, if you can try ksh93 and if you find out that ksh93 works,\n> then the above can be made in line with your \"Subject\" to mark ksh88\n> as broken (as opposed to other POSIX shells)? That would help us by\n> reminding that running test fine with ksh93 is not a sufficient\n> check to make sure we didn't break ksh88 users.\n>\n>> In ksh the \\ disappears and generates false expect data to\n>> compare with.\n>> Using \\\\\" works portable, the same way in bash and in ksh and\n>> is less ambigous.\n>\n> All of the above would need s/ksh/&88/g; I'd think. I just tried\n>\n> make SHELL_PATH=/bin/ksh93\n> cd t && /bin/ksh93 t0008-*.sh\n>\n> and this patch is not necessary for ksh93.\nYes, the patch is not necessary with ksh93 on AIX, but it works :-)\nThe patch is targeting \"ksh\" on AIX (which actually is a ksh88).\n\nIn the discussion Jeff took a look into the POSIX specification\nand described the behavior like this:\n\n\nI think either is reasonable (there is no need to backslash-escape a\ndouble-quote inside a here-doc, but one assumes that backslash would\ngenerally have its usual behavior). I'm not quite sure how to interpret\nPOSIX here (see below), but it seems clear that spelling it with two\nbackslashes as you suggest is the best bet.\n\n\nI'd not declare ksh88 on AIX broken just because of this ambiguity\nsince it is not 100% clear in the POSIX description.\n\nArmin\n"},{"id":"287103","messageId":"CALR6jEgaHXAfh6137XYfouFbNL7SYSdrgE_+PSX=xiS8u-mNjQ@mail.gmail.com","threadId":"42400","inReplyTo":"xmqqk2iog16g.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0008: 4 tests fail with ksh88","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-05-20T16:14:36Z","receivedAt":"2016-05-20T16:14:36Z","isPatch":true,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"On Fri, May 20, 2016 at 6:10 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>>> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\n>>> index 89544dd..b425f3a 100755\n>>> --- a/t/t0008-ignores.sh\n>>> +++ b/t/t0008-ignores.sh\n>>> @@ -605,7 +605,7 @@ cat <<-EOF >expected-verbose\n>>> a/b/.gitignore:8:!on* a/b/one\n>>> a/b/.gitignore:8:!on* a/b/one one\n>\n> The patch was whitespace-damaged and didn't apply, so I had to redo\n> it from scratch while updating the log message. If this looks good\n> to you, there is no need to resend.\n\nThanks. Looks like even Google Mail interface in plain text mode eats\nwhite spaces :-(\n"},{"id":"287131","messageId":"CAP8UFD3-B9SiBN479RQwjmtJsTns5Q2XUepdf9_NS0NVoQPGGw@mail.gmail.com","threadId":"42400","inReplyTo":"xmqqk2iog16g.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0008: 4 tests fail with ksh88","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-05-20T21:01:48Z","receivedAt":"2016-05-20T21:01:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, May 20, 2016 at 6:10 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n> From: Armin Kunaschik \n> Date: Fri, 20 May 2016 16:31:30 +0200\n> Subject: [PATCH] t0008: 4 tests fail with ksh88\n>\n> In t0008, we have\n>\n> cat <<-EOF\n> ...\n> a/b/.gitignore:8:!on* \"a/b/one\\\"three\"\n> ...\n> EOF\n>\n> ane expect that the backslash-dq is passed through literally.\n\ns/ane/and/\n\n> ksh88 eats \\ and generates a wrong expect data to compare with.\n>\n> Using \\\\\" works this around without breaking other POSIX shells\n> (which collapse backslash-backslash to a single backslash), and\n> ksh88 does so, too.\n"},{"id":"287174","messageId":"20160522000756.GA9591@sigill.intra.peff.net","threadId":"42400","inReplyTo":"xmqq37pchi90.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t0008: 4 tests fail with ksh88","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-22T00:07:58Z","receivedAt":"2016-05-22T00:07:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 20, 2016 at 08:16:43AM -0700, Junio C Hamano wrote:\n\n> > Acked-by: Jeff King \n> \n> I didn't see him acking this exact version, so if you didn't include\n> this line here, I would have missed it. Thanks.\n\nI don't think I ever saw an actual patch to ack until now; I just said\nthe idea seemed good.\n\nThat being said, the patch _does_ look good, and I am fine to have my\nack/reviewed-by on it (though I agree with your points regarding the\ncommit message).\n\n-Peff\n"}]}