{"thread":{"id":"36833","subject":"[PATCH] t9001: avoid not portable '\\n' with sed","startedAt":"2014-06-04T08:20:47Z","lastAt":"2014-06-04T19:16:03Z","messageCount":8,"participants":["Torsten Bögershausen","Junio C Hamano","John Keeping","David Kastrup","Andreas Schwab"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"243280","messageId":"538ED6DF.5020505@web.de","threadId":"36833","inReplyTo":null,"subject":"[PATCH] t9001: avoid not portable '\\n' with sed","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-06-04T08:20:47Z","receivedAt":"2014-06-04T08:20:47Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"t9001 used a '\\n' in a sed expression to split one line into two lines.\nSome versions of sed simply ignore the '\\' before the 'n', treating\n'\\n' as 'n'.\n\nAs the test already requires perl as a prerequisite, use perl instead of sed.\n\nSigned-off-by: Torsten Bögershausen \n---\n t/t9001-send-email.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 64d9434..2bf48d1 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1342,7 +1342,7 @@ test_cover_addresses () {\n \tgit format-patch --cover-letter -2 -o outdir &&\n \tcover=`echo outdir/0000-*.patch` &&\n \tmv $cover cover-to-edit.patch &&\n-\tsed \"s/^From:/$header: extra@address.com\\nFrom:/\" cover-to-edit.patch >\"$cover\" &&\n+\t\"$PERL_PATH\" -pe \"s/^From:/$header: extra\\@address.com\\nFrom:/\" cover-to-edit.patch | tr Q \"$LF\" >\"$cover\" &&\n \tgit send-email \\\n \t --force \\\n \t --from=\"Example \" \\\n-- \n2.0.0.553.ged01b91\n"},{"id":"243310","messageId":"xmqqr434vaeh.fsf@gitster.dls.corp.google.com","threadId":"36833","inReplyTo":"538ED6DF.5020505@web.de","subject":"Re: [PATCH] t9001: avoid not portable '\\n' with sed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-04T17:42:46Z","receivedAt":"2014-06-04T17:42:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> t9001 used a '\\n' in a sed expression to split one line into two lines.\n> Some versions of sed simply ignore the '\\' before the 'n', treating\n> '\\n' as 'n'.\n>\n> As the test already requires perl as a prerequisite, use perl instead of sed.\n>\n> Signed-off-by: Torsten Bögershausen \n> ---\n\nHmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n\n The escape sequence '\\n' shall match a embedded in the\n pattern space.\n\nso it may be better to be a bit more explicit in the log message to\nsay whose implementation has this issue to warn people.\n\n> t/t9001-send-email.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 64d9434..2bf48d1 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -1342,7 +1342,7 @@ test_cover_addresses () {\n> \tgit format-patch --cover-letter -2 -o outdir &&\n> \tcover=`echo outdir/0000-*.patch` &&\n> \tmv $cover cover-to-edit.patch &&\n> -\tsed \"s/^From:/$header: extra@address.com\\nFrom:/\" cover-to-edit.patch >\"$cover\" &&\n> +\t\"$PERL_PATH\" -pe \"s/^From:/$header: extra\\@address.com\\nFrom:/\" cover-to-edit.patch | tr Q \"$LF\" >\"$cover\" &&\n\nWe have a shell function \"perl\" in test-lib-function.sh these days\nso that you do not have to write \"$PERL_PATH\" yourself in tests ;-)\n\n> \tgit send-email \\\n> \t --force \\\n> \t --from=\"Example \" \\\n\nThanks.\n"},{"id":"243313","messageId":"xmqqd2eov8ys.fsf@gitster.dls.corp.google.com","threadId":"36833","inReplyTo":"xmqqr434vaeh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] t9001: avoid not portable '\\n' with sed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-04T18:13:47Z","receivedAt":"2014-06-04T18:13:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Torsten Bögershausen writes:\n>\n>> t9001 used a '\\n' in a sed expression to split one line into two lines.\n>> Some versions of sed simply ignore the '\\' before the 'n', treating\n>> '\\n' as 'n'.\n>>\n>> As the test already requires perl as a prerequisite, use perl instead of sed.\n>>\n>> Signed-off-by: Torsten Bögershausen \n>> ---\n>\n> Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n>\n> The escape sequence '\\n' shall match a embedded in the\n> pattern space.\n>\n> so it may be better to be a bit more explicit in the log message to\n> say whose implementation has this issue to warn people.\n>\n>> t/t9001-send-email.sh | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n>> index 64d9434..2bf48d1 100755\n>> --- a/t/t9001-send-email.sh\n>> +++ b/t/t9001-send-email.sh\n>> @@ -1342,7 +1342,7 @@ test_cover_addresses () {\n>> \tgit format-patch --cover-letter -2 -o outdir &&\n>> \tcover=`echo outdir/0000-*.patch` &&\n>> \tmv $cover cover-to-edit.patch &&\n>> -\tsed \"s/^From:/$header: extra@address.com\\nFrom:/\" cover-to-edit.patch >\"$cover\" &&\n>> +\t\"$PERL_PATH\" -pe \"s/^From:/$header: extra\\@address.com\\nFrom:/\" cover-to-edit.patch | tr Q \"$LF\" >\"$cover\" &&\n>\n> We have a shell function \"perl\" in test-lib-function.sh these days\n> so that you do not have to write \"$PERL_PATH\" yourself in tests ;-)\n\nAlso, piping output from perl to tr feels somewhat suboptimal. I do\nnot see where in the test material we use \"Q to LF\", and we may want\nto remove that altogether, but without that removal, an updated\npatch may look like this.\n\n t/t9001-send-email.sh | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 64d9434..9f06b8c 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1342,7 +1342,10 @@ test_cover_addresses () {\n \tgit format-patch --cover-letter -2 -o outdir &&\n \tcover=`echo outdir/0000-*.patch` &&\n \tmv $cover cover-to-edit.patch &&\n-\tsed \"s/^From:/$header: extra@address.com\\nFrom:/\" cover-to-edit.patch >\"$cover\" &&\n+\tperl -pe \"\n+\t\ts/^From:/$header: extra\\@address.com\\nFrom:/;\n+\t\ty/Q/\\n/;\n+\t\" cover-to-edit.patch >\"$cover\" &&\n \tgit send-email \\\n \t --force \\\n \t --from=\"Example \" \\\n"},{"id":"243316","messageId":"538F689B.1020804@web.de","threadId":"36833","inReplyTo":"xmqqd2eov8ys.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] t9001: avoid not portable '\\n' with sed","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-06-04T18:42:35Z","receivedAt":"2014-06-04T18:42:35Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2014-06-04 20.13, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>> Torsten Bögershausen writes:\n>>\n>>> t9001 used a '\\n' in a sed expression to split one line into two lines.\n>>> Some versions of sed simply ignore the '\\' before the 'n', treating\n>>> '\\n' as 'n'.\n>>>\n>>> As the test already requires perl as a prerequisite, use perl instead of sed.\n>>>\n>>> Signed-off-by: Torsten Bögershausen \n>>> ---\n>>\n>> Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n>>\n>> The escape sequence '\\n' shall match a embedded in the\n>> pattern space.\n>>\n>> so it may be better to be a bit more explicit in the log message to\n>> say whose implementation has this issue to warn people.\n>>\n>>> t/t9001-send-email.sh | 2 +-\n>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>\n>>> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n>>> index 64d9434..2bf48d1 100755\n>>> --- a/t/t9001-send-email.sh\n>>> +++ b/t/t9001-send-email.sh\n>>> @@ -1342,7 +1342,7 @@ test_cover_addresses () {\n>>> \tgit format-patch --cover-letter -2 -o outdir &&\n>>> \tcover=`echo outdir/0000-*.patch` &&\n>>> \tmv $cover cover-to-edit.patch &&\n>>> -\tsed \"s/^From:/$header: extra@address.com\\nFrom:/\" cover-to-edit.patch >\"$cover\" &&\n>>> +\t\"$PERL_PATH\" -pe \"s/^From:/$header: extra\\@address.com\\nFrom:/\" cover-to-edit.patch | tr Q \"$LF\" >\"$cover\" &&\n>>\n>> We have a shell function \"perl\" in test-lib-function.sh these days\n>> so that you do not have to write \"$PERL_PATH\" yourself in tests ;-)\n> \n> Also, piping output from perl to tr feels somewhat suboptimal. I do\n> not see where in the test material we use \"Q to LF\", and we may want\n> to remove that altogether, but without that removal, an updated\n> patch may look like this.\n> \n> t/t9001-send-email.sh | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 64d9434..9f06b8c 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -1342,7 +1342,10 @@ test_cover_addresses () {\n> \tgit format-patch --cover-letter -2 -o outdir &&\n> \tcover=`echo outdir/0000-*.patch` &&\n> \tmv $cover cover-to-edit.patch &&\n> -\tsed \"s/^From:/$header: extra@address.com\\nFrom:/\" cover-to-edit.patch >\"$cover\" &&\n> +\tperl -pe \"\n> +\t\ts/^From:/$header: extra\\@address.com\\nFrom:/;\n> +\t\ty/Q/\\n/;\n> +\t\" cover-to-edit.patch >\"$cover\" &&\n> \tgit send-email \\\n> \t --force \\\n> \t --from=\"Example \" \\\nGood catch, the \"tr\" should had been removed.\nMy first version used\nsed \"s/^From:/$header: extra@address.comQFrom:/\"\nand the Q was replaced by tr with a literal LF.\nSo I think the 'Q' -> '\\n' conversion should be removed completely :-)\n\nThe sed in question is /usr/bin/sed under Mac OS X.\nThen we have the question: What exactly is the pattern space?\n\n>In default operation, sed cyclically shall append a line of input, less its terminating >character, into the pattern space....\n\nIsn't that the stuff from the input?\nBut that doesn't make too much sence to me, since \"input lines\" are terminated by \\n.\nSo pattern space seems to mean output when they talk about the \\n\nAnyway, the \\n (to insert a newline into the output) works under Linux, but not Mac OS.\n\n\n\n\n \n"},{"id":"243317","messageId":"20140604184604.GC14457@serenity.lan","threadId":"36833","inReplyTo":"xmqqr434vaeh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] t9001: avoid not portable '\\n' with sed","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2014-06-04T18:46:04Z","receivedAt":"2014-06-04T18:46:04Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Wed, Jun 04, 2014 at 10:42:46AM -0700, Junio C Hamano wrote:\n> Torsten Bögershausen writes:\n> \n> > t9001 used a '\\n' in a sed expression to split one line into two lines.\n> > Some versions of sed simply ignore the '\\' before the 'n', treating\n> > '\\n' as 'n'.\n> >\n> > As the test already requires perl as a prerequisite, use perl instead of sed.\n> >\n> > Signed-off-by: Torsten Bögershausen \n> > ---\n> \n> Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n> \n> The escape sequence '\\n' shall match a embedded in the\n> pattern space.\n> \n> so it may be better to be a bit more explicit in the log message to\n> say whose implementation has this issue to warn people.\n> \n> > -\tsed \"s/^From:/$header: extra@address.com\\nFrom:/\" cover-to-edit.patch >\"$cover\" &&\n> > +\t\"$PERL_PATH\" -pe \"s/^From:/$header: extra\\@address.com\\nFrom:/\" cover-to-edit.patch | tr Q \"$LF\" >\"$cover\" &&\n\nNote that quoted section of POSIX says \"embedded in the pattern space\";\nunder the description of the \"s\" command, it says:\n\n\tThe replacement string shall be scanned from beginning to end.\n\t[...]\n\tThe meaning of a immediately followed by any\n\tcharacter other than '&', , a digit, or the delimiter\n\tcharacter used for this command, is unspecified.\n\n\tA line can be split by substituting a into it. The\n\tapplication shall escape the in the replacement by\n\tpreceding it by a .\n\nSo the portable way to do it is:\n\n\tsed \"s/^From:/$header: extra@address.com\\\nFrom:/\" cover-to-edit.patch >\"$cover\" &&\n\nbut that requires the continuation to start in column 0, so the Perl\nvariant is probably neater.\n"},{"id":"243318","messageId":"878upc7bre.fsf@fencepost.gnu.org","threadId":"36833","inReplyTo":"xmqqr434vaeh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] t9001: avoid not portable '\\n' with sed","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-06-04T18:47:17Z","receivedAt":"2014-06-04T18:47:17Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> Torsten Bögershausen writes:\n>\n>> t9001 used a '\\n' in a sed expression to split one line into two lines.\n>> Some versions of sed simply ignore the '\\' before the 'n', treating\n>> '\\n' as 'n'.\n>>\n>> As the test already requires perl as a prerequisite, use perl instead of sed.\n>>\n>> Signed-off-by: Torsten Bögershausen \n>> ---\n>\n> Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n>\n> The escape sequence '\\n' shall match a embedded in the\n> pattern space.\n>\n> so it may be better to be a bit more explicit in the log message to\n> say whose implementation has this issue to warn people.\n\n\"shall match\" talks about the match expression, not the replacement.\n\n-- \nDavid Kastrup\n"},{"id":"243332","messageId":"xmqq1tv4v6qg.fsf@gitster.dls.corp.google.com","threadId":"36833","inReplyTo":"20140604184604.GC14457@serenity.lan","subject":"Re: [PATCH] t9001: avoid not portable '\\n' with sed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-04T19:01:59Z","receivedAt":"2014-06-04T19:01:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> Note that quoted section of POSIX says \"embedded in the pattern space\";\n> under the description of the \"s\" command, it says:\n>\n> \tThe replacement string shall be scanned from beginning to end.\n> \t[...]\n> \tThe meaning of a immediately followed by any\n> \tcharacter other than '&', , a digit, or the delimiter\n> \tcharacter used for this command, is unspecified.\n\nVery true.\n"},{"id":"243335","messageId":"87wqcw1o5o.fsf@igel.home","threadId":"36833","inReplyTo":"20140604184604.GC14457@serenity.lan","subject":"Re: [PATCH] t9001: avoid not portable '\\n' with sed","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-06-04T19:16:03Z","receivedAt":"2014-06-04T19:16:03Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"John Keeping writes:\n\n> So the portable way to do it is:\n>\n> \tsed \"s/^From:/$header: extra@address.com\\\n> From:/\" cover-to-edit.patch >\"$cover\" &&\n\nThat wouldn't work as \\ is removed in double quotes. You\neither need to double the backslash or put it in single quotes.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"}]}