{"thread":{"id":"62897","subject":"[PATCH] thunderbird-patch-inline: avoid bashism","startedAt":"2025-02-04T01:55:31Z","lastAt":"2025-02-11T00:19:14Z","messageCount":6,"participants":["brian m. carlson","D. Ben Knoble","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"511773","messageId":"20250204014652.3509928-1-sandals@crustytoothpaste.net","threadId":"62897","inReplyTo":null,"subject":"[PATCH] thunderbird-patch-inline: avoid bashism","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-02-04T01:46:52Z","receivedAt":"2025-02-04T01:55:31Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The use of \"echo -e\" is not portable and not specified by POSIX.  dash\ndoes not support any options except \"-n\", and so this script will not\nwork on operating systems which use that as /bin/sh.\n\nFortunately, the solution is easy: switch to printf(1), which is\nspecified by POSIX and allows the escape sequences we want to use.  This\nwill allow the script to work with any POSIX shell.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n contrib/thunderbird-patch-inline/appp.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\nI noticed this in Debian bug 772238[0], while looking for any bug\nreports that I might be able to fix.  It was reported in 2014 and has\ngone unfixed since then, so possibly this script is seeing relatively\nlittle use on Debian and Ubuntu.\n\nI have not CC'd any of the authors because nobody's touched this in over\n9 years and none of those people are still active.\n\n[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772238\n\ndiff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh\nindex 1053872eea..c55c2caa41 100755\n--- a/contrib/thunderbird-patch-inline/appp.sh\n+++ b/contrib/thunderbird-patch-inline/appp.sh\n@@ -31,7 +31,7 @@ BODY=$(sed -e \"1,/${SEP}/d\" $1)\n CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' \"${PATCH}\")\n DIFF=$(sed -e '1,/^---$/d' \"${PATCH}\")\n \n-CCS=$(echo -e \"$CMT_MSG\\n$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n+CCS=$(printf '%s\\n%s' \"$CMT_MSG\" \"$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n \t-e 's/^Signed-off-by: \\(.*\\)/\\1,/gp')\n \n echo \"$SUBJECT\" > $1\n"},{"id":"511775","messageId":"CALnO6CAib-p1+1VHMfqo4VGLVvWs63z_xCjkCANJS-NLK5FpdQ@mail.gmail.com","threadId":"62897","inReplyTo":"20250204014652.3509928-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] thunderbird-patch-inline: avoid bashism","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-02-04T02:11:19Z","receivedAt":"2025-02-04T02:11:32Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Mon, Feb 3, 2025 at 8:55 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> The use of \"echo -e\" is not portable and not specified by POSIX.  dash\n> does not support any options except \"-n\", and so this script will not\n> work on operating systems which use that as /bin/sh.\n>\n> Fortunately, the solution is easy: switch to printf(1), which is\n> specified by POSIX and allows the escape sequences we want to use.  This\n> will allow the script to work with any POSIX shell.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  contrib/thunderbird-patch-inline/appp.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> I noticed this in Debian bug 772238[0], while looking for any bug\n> reports that I might be able to fix.  It was reported in 2014 and has\n> gone unfixed since then, so possibly this script is seeing relatively\n> little use on Debian and Ubuntu.\n>\n> I have not CC'd any of the authors because nobody's touched this in over\n> 9 years and none of those people are still active.\n>\n> [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772238\n>\n> diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh\n> index 1053872eea..c55c2caa41 100755\n> --- a/contrib/thunderbird-patch-inline/appp.sh\n> +++ b/contrib/thunderbird-patch-inline/appp.sh\n> @@ -31,7 +31,7 @@ BODY=$(sed -e \"1,/${SEP}/d\" $1)\n>  CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' \"${PATCH}\")\n>  DIFF=$(sed -e '1,/^---$/d' \"${PATCH}\")\n>\n> -CCS=$(echo -e \"$CMT_MSG\\n$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n> +CCS=$(printf '%s\\n%s' \"$CMT_MSG\" \"$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n\nLooks obviously correct to me (I once wrote POSIX-compatible echos\njust to see how hard it was [1]), though I find it interesting that\n`sed` can process input lacking a final newline.\n\n>         -e 's/^Signed-off-by: \\(.*\\)/\\1,/gp')\n>\n>  echo \"$SUBJECT\" > $1\n>\n\n[1]: https://github.com/benknoble/echocho\n\n-- \nD. Ben Knoble\n"},{"id":"511779","messageId":"Z6F-ySoYbDJxJ3xs@tapette.crustytoothpaste.net","threadId":"62897","inReplyTo":"CALnO6CAib-p1+1VHMfqo4VGLVvWs63z_xCjkCANJS-NLK5FpdQ@mail.gmail.com","subject":"Re: [PATCH] thunderbird-patch-inline: avoid bashism","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-02-04T02:43:21Z","receivedAt":"2025-02-04T02:43:26Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-02-04 at 02:11:19, D. Ben Knoble wrote:\n> On Mon, Feb 3, 2025 at 8:55 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> >\n> > The use of \"echo -e\" is not portable and not specified by POSIX.  dash\n> > does not support any options except \"-n\", and so this script will not\n> > work on operating systems which use that as /bin/sh.\n> >\n> > Fortunately, the solution is easy: switch to printf(1), which is\n> > specified by POSIX and allows the escape sequences we want to use.  This\n> > will allow the script to work with any POSIX shell.\n> >\n> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> > ---\n> >  contrib/thunderbird-patch-inline/appp.sh | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > I noticed this in Debian bug 772238[0], while looking for any bug\n> > reports that I might be able to fix.  It was reported in 2014 and has\n> > gone unfixed since then, so possibly this script is seeing relatively\n> > little use on Debian and Ubuntu.\n> >\n> > I have not CC'd any of the authors because nobody's touched this in over\n> > 9 years and none of those people are still active.\n> >\n> > [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772238\n> >\n> > diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh\n> > index 1053872eea..c55c2caa41 100755\n> > --- a/contrib/thunderbird-patch-inline/appp.sh\n> > +++ b/contrib/thunderbird-patch-inline/appp.sh\n> > @@ -31,7 +31,7 @@ BODY=$(sed -e \"1,/${SEP}/d\" $1)\n> >  CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' \"${PATCH}\")\n> >  DIFF=$(sed -e '1,/^---$/d' \"${PATCH}\")\n> >\n> > -CCS=$(echo -e \"$CMT_MSG\\n$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n> > +CCS=$(printf '%s\\n%s' \"$CMT_MSG\" \"$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n> \n> Looks obviously correct to me (I once wrote POSIX-compatible echos\n> just to see how hard it was [1]), though I find it interesting that\n> `sed` can process input lacking a final newline.\n\nThat's actually a good point, which means we probably need to put\nanother newline there.  The shell will have stripped off the newline from\nthe command substitution, so we'll need to re-add it.\n\nGNU and BSD sed have no problem with this, but POSIX doesn't require\nthat sed work here.  (Of course, the likelihood that anyone is actually\nrunning this on a system with such a rigid sed is extremely unlikely,\nbut we might as well fix all of the portability problems.)\n\nI'll try to get a reroll out later this week.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"511804","messageId":"xmqqed0dx2jc.fsf@gitster.g","threadId":"62897","inReplyTo":"20250204014652.3509928-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] thunderbird-patch-inline: avoid bashism","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-04T13:28:07Z","receivedAt":"2025-02-04T13:28:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> The use of \"echo -e\" is not portable and not specified by POSIX.  dash\n> does not support any options except \"-n\", and so this script will not\n> work on operating systems which use that as /bin/sh.\n>\n> Fortunately, the solution is easy: switch to printf(1), which is\n> specified by POSIX and allows the escape sequences we want to use.  This\n> will allow the script to work with any POSIX shell.\n\nMakes sense.  Will queue.  Thanks.\n\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  contrib/thunderbird-patch-inline/appp.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> I noticed this in Debian bug 772238[0], while looking for any bug\n> reports that I might be able to fix.  It was reported in 2014 and has\n> gone unfixed since then, so possibly this script is seeing relatively\n> little use on Debian and Ubuntu.\n>\n> I have not CC'd any of the authors because nobody's touched this in over\n> 9 years and none of those people are still active.\n>\n> [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772238\n>\n> diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh\n> index 1053872eea..c55c2caa41 100755\n> --- a/contrib/thunderbird-patch-inline/appp.sh\n> +++ b/contrib/thunderbird-patch-inline/appp.sh\n> @@ -31,7 +31,7 @@ BODY=$(sed -e \"1,/${SEP}/d\" $1)\n>  CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' \"${PATCH}\")\n>  DIFF=$(sed -e '1,/^---$/d' \"${PATCH}\")\n>  \n> -CCS=$(echo -e \"$CMT_MSG\\n$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n> +CCS=$(printf '%s\\n%s' \"$CMT_MSG\" \"$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n>  \t-e 's/^Signed-off-by: \\(.*\\)/\\1,/gp')\n>  \n>  echo \"$SUBJECT\" > $1\n"},{"id":"512203","messageId":"20250210234947.1317056-1-sandals@crustytoothpaste.net","threadId":"62897","inReplyTo":"20250204014652.3509928-1-sandals@crustytoothpaste.net","subject":"[PATCH v2] thunderbird-patch-inline: avoid bashism","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-02-10T23:49:47Z","receivedAt":"2025-02-10T23:50:56Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The use of \"echo -e\" is not portable and not specified by POSIX.  dash\ndoes not support any options except \"-n\", and so this script will not\nwork on operating systems which use that as /bin/sh.\n\nFortunately, the solution is easy: switch to printf(1), which is\nspecified by POSIX and allows the escape sequences we want to use.  This\nwill allow the script to work with any POSIX shell.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n contrib/thunderbird-patch-inline/appp.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\nChanges from v1:\n* Add a missing newline.\n\nI'll note that I could have just written '%s\\n' here, but I think this\nis a little easier to reason about, so I didn't.\n\ndiff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh\nindex 1053872eea..fdcc948352 100755\n--- a/contrib/thunderbird-patch-inline/appp.sh\n+++ b/contrib/thunderbird-patch-inline/appp.sh\n@@ -31,7 +31,7 @@ BODY=$(sed -e \"1,/${SEP}/d\" $1)\n CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' \"${PATCH}\")\n DIFF=$(sed -e '1,/^---$/d' \"${PATCH}\")\n \n-CCS=$(echo -e \"$CMT_MSG\\n$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n+CCS=$(printf '%s\\n%s\\n' \"$CMT_MSG\" \"$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n \t-e 's/^Signed-off-by: \\(.*\\)/\\1,/gp')\n \n echo \"$SUBJECT\" > $1\n"},{"id":"512204","messageId":"xmqqa5at9vuo.fsf@gitster.g","threadId":"62897","inReplyTo":"20250210234947.1317056-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2] thunderbird-patch-inline: avoid bashism","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-11T00:19:11Z","receivedAt":"2025-02-11T00:19:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> I'll note that I could have just written '%s\\n' here, but I think this\n> is a little easier to reason about, so I didn't.\n\nYes, I actually was wondering why you didn't, as I find the \"short\nformat string makes the command iterate over its arguments\" easier\nto understand, than how you wrote it.  Either is fine, but that\nwould also have been shorter.\n\n> diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh\n> index 1053872eea..fdcc948352 100755\n> --- a/contrib/thunderbird-patch-inline/appp.sh\n> +++ b/contrib/thunderbird-patch-inline/appp.sh\n> @@ -31,7 +31,7 @@ BODY=$(sed -e \"1,/${SEP}/d\" $1)\n>  CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' \"${PATCH}\")\n>  DIFF=$(sed -e '1,/^---$/d' \"${PATCH}\")\n>  \n> -CCS=$(echo -e \"$CMT_MSG\\n$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n> +CCS=$(printf '%s\\n%s\\n' \"$CMT_MSG\" \"$HEADERS\" | sed -n -e 's/^Cc: \\(.*\\)$/\\1,/gp' \\\n>  \t-e 's/^Signed-off-by: \\(.*\\)/\\1,/gp')\n>  \n>  echo \"$SUBJECT\" > $1\n"}]}