threads / patch / 62897

patchthunderbird-patch-inline: avoid bashism

Subject: [PATCH] thunderbird-patch-inline: avoid bashism

## tl;dr

6 messages between Feb 4, 2025 and Feb 11, 2025. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

brian m. carlson· Feb 4, 2025, 01:46 UTC · lore

The use of "echo -e" is not portable and not specified by POSIX. dash does not support any options except "-n", and so this script will not work on operating systems which use that as /bin/sh.

Fortunately, the solution is easy: switch to printf(1), which is specified by POSIX and allows the escape sequences we want to use. This will allow the script to work with any POSIX shell.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 contrib/thunderbird-patch-inline/appp.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

I noticed this in Debian bug 772238[0], while looking for any bug reports that I might be able to fix. It was reported in 2014 and has gone unfixed since then, so possibly this script is seeing relatively little use on Debian and Ubuntu.

I have not CC'd any of the authors because nobody's touched this in over 9 years and none of those people are still active.

[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772238

Show changes to contrib/thunderbird-patch-inline/appp.sh +1 −1

diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh
index 1053872eea..c55c2caa41 100755
--- a/contrib/thunderbird-patch-inline/appp.sh
+++ b/contrib/thunderbird-patch-inline/appp.sh
@@ -31,7 +31,7 @@ BODY=$(sed -e "1,/${SEP}/d" $1)
 CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' "${PATCH}")
 DIFF=$(sed -e '1,/^---$/d' "${PATCH}")
 
-CCS=$(echo -e "$CMT_MSG\n$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
+CCS=$(printf '%s\n%s' "$CMT_MSG" "$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
 	-e 's/^Signed-off-by: \(.*\)/\1,/gp')
 
 echo "$SUBJECT" > $1

D. Ben Knoble· Feb 4, 2025, 02:11 UTC · re: brian m. carlson · lore

Re: [PATCH] thunderbird-patch-inline: avoid bashism

On Mon, Feb 3, 2025 at 8:55 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 34 quoted lines

>
> The use of "echo -e" is not portable and not specified by POSIX.  dash
> does not support any options except "-n", and so this script will not
> work on operating systems which use that as /bin/sh.
>
> Fortunately, the solution is easy: switch to printf(1), which is
> specified by POSIX and allows the escape sequences we want to use.  This
> will allow the script to work with any POSIX shell.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  contrib/thunderbird-patch-inline/appp.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> I noticed this in Debian bug 772238[0], while looking for any bug
> reports that I might be able to fix.  It was reported in 2014 and has
> gone unfixed since then, so possibly this script is seeing relatively
> little use on Debian and Ubuntu.
>
> I have not CC'd any of the authors because nobody's touched this in over
> 9 years and none of those people are still active.
>
> [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772238
>
> diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh
> index 1053872eea..c55c2caa41 100755
> --- a/contrib/thunderbird-patch-inline/appp.sh
> +++ b/contrib/thunderbird-patch-inline/appp.sh
> @@ -31,7 +31,7 @@ BODY=$(sed -e "1,/${SEP}/d" $1)
>  CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' "${PATCH}")
>  DIFF=$(sed -e '1,/^---$/d' "${PATCH}")
>
> -CCS=$(echo -e "$CMT_MSG\n$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
> +CCS=$(printf '%s\n%s' "$CMT_MSG" "$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \

Looks obviously correct to me (I once wrote POSIX-compatible echos just to see how hard it was [1]), though I find it interesting that `sed` can process input lacking a final newline.

>         -e 's/^Signed-off-by: \(.*\)/\1,/gp')
>
>  echo "$SUBJECT" > $1
>

[1]: https://github.com/benknoble/echocho

-- 
D. Ben Knoble

brian m. carlson· Feb 4, 2025, 02:43 UTC · re: D. Ben Knoble · lore

Re: [PATCH] thunderbird-patch-inline: avoid bashism

On 2025-02-04 at 02:11:19, D. Ben Knoble wrote:

Show 40 quoted lines

> On Mon, Feb 3, 2025 at 8:55 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > The use of "echo -e" is not portable and not specified by POSIX.  dash
> > does not support any options except "-n", and so this script will not
> > work on operating systems which use that as /bin/sh.
> >
> > Fortunately, the solution is easy: switch to printf(1), which is
> > specified by POSIX and allows the escape sequences we want to use.  This
> > will allow the script to work with any POSIX shell.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  contrib/thunderbird-patch-inline/appp.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > I noticed this in Debian bug 772238[0], while looking for any bug
> > reports that I might be able to fix.  It was reported in 2014 and has
> > gone unfixed since then, so possibly this script is seeing relatively
> > little use on Debian and Ubuntu.
> >
> > I have not CC'd any of the authors because nobody's touched this in over
> > 9 years and none of those people are still active.
> >
> > [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772238
> >
> > diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh
> > index 1053872eea..c55c2caa41 100755
> > --- a/contrib/thunderbird-patch-inline/appp.sh
> > +++ b/contrib/thunderbird-patch-inline/appp.sh
> > @@ -31,7 +31,7 @@ BODY=$(sed -e "1,/${SEP}/d" $1)
> >  CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' "${PATCH}")
> >  DIFF=$(sed -e '1,/^---$/d' "${PATCH}")
> >
> > -CCS=$(echo -e "$CMT_MSG\n$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
> > +CCS=$(printf '%s\n%s' "$CMT_MSG" "$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
> 
> Looks obviously correct to me (I once wrote POSIX-compatible echos
> just to see how hard it was [1]), though I find it interesting that
> `sed` can process input lacking a final newline.

That's actually a good point, which means we probably need to put another newline there. The shell will have stripped off the newline from the command substitution, so we'll need to re-add it.

GNU and BSD sed have no problem with this, but POSIX doesn't require that sed work here. (Of course, the likelihood that anyone is actually running this on a system with such a rigid sed is extremely unlikely, but we might as well fix all of the portability problems.)

I'll try to get a reroll out later this week.

-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

Junio C Hamano· Feb 4, 2025, 13:28 UTC · re: brian m. carlson · lore

Re: [PATCH] thunderbird-patch-inline: avoid bashism

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Show 7 quoted lines

> The use of "echo -e" is not portable and not specified by POSIX.  dash
> does not support any options except "-n", and so this script will not
> work on operating systems which use that as /bin/sh.
>
> Fortunately, the solution is easy: switch to printf(1), which is
> specified by POSIX and allows the escape sequences we want to use.  This
> will allow the script to work with any POSIX shell.

Makes sense.  Will queue.  Thanks.

Show 29 quoted lines

>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  contrib/thunderbird-patch-inline/appp.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> I noticed this in Debian bug 772238[0], while looking for any bug
> reports that I might be able to fix.  It was reported in 2014 and has
> gone unfixed since then, so possibly this script is seeing relatively
> little use on Debian and Ubuntu.
>
> I have not CC'd any of the authors because nobody's touched this in over
> 9 years and none of those people are still active.
>
> [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772238
>
> diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh
> index 1053872eea..c55c2caa41 100755
> --- a/contrib/thunderbird-patch-inline/appp.sh
> +++ b/contrib/thunderbird-patch-inline/appp.sh
> @@ -31,7 +31,7 @@ BODY=$(sed -e "1,/${SEP}/d" $1)
>  CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' "${PATCH}")
>  DIFF=$(sed -e '1,/^---$/d' "${PATCH}")
>  
> -CCS=$(echo -e "$CMT_MSG\n$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
> +CCS=$(printf '%s\n%s' "$CMT_MSG" "$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
>  	-e 's/^Signed-off-by: \(.*\)/\1,/gp')
>  
>  echo "$SUBJECT" > $1

brian m. carlson· Feb 10, 2025, 23:49 UTC · re: brian m. carlson · lore

[PATCH v2] thunderbird-patch-inline: avoid bashism

The use of "echo -e" is not portable and not specified by POSIX. dash does not support any options except "-n", and so this script will not work on operating systems which use that as /bin/sh.

Fortunately, the solution is easy: switch to printf(1), which is specified by POSIX and allows the escape sequences we want to use. This will allow the script to work with any POSIX shell.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 contrib/thunderbird-patch-inline/appp.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Changes from v1:
* Add a missing newline.

I'll note that I could have just written '%s\n' here, but I think this is a little easier to reason about, so I didn't.

Show changes to contrib/thunderbird-patch-inline/appp.sh +1 −1

diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh
index 1053872eea..fdcc948352 100755
--- a/contrib/thunderbird-patch-inline/appp.sh
+++ b/contrib/thunderbird-patch-inline/appp.sh
@@ -31,7 +31,7 @@ BODY=$(sed -e "1,/${SEP}/d" $1)
 CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' "${PATCH}")
 DIFF=$(sed -e '1,/^---$/d' "${PATCH}")
 
-CCS=$(echo -e "$CMT_MSG\n$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
+CCS=$(printf '%s\n%s\n' "$CMT_MSG" "$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
 	-e 's/^Signed-off-by: \(.*\)/\1,/gp')
 
 echo "$SUBJECT" > $1

Junio C Hamano· Feb 11, 2025, 00:19 UTC · re: brian m. carlson · lore

Re: [PATCH v2] thunderbird-patch-inline: avoid bashism

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> I'll note that I could have just written '%s\n' here, but I think this
> is a little easier to reason about, so I didn't.

Yes, I actually was wondering why you didn't, as I find the "short format string makes the command iterate over its arguments" easier to understand, than how you wrote it. Either is fine, but that would also have been shorter.

Show 13 quoted lines

> diff --git a/contrib/thunderbird-patch-inline/appp.sh b/contrib/thunderbird-patch-inline/appp.sh
> index 1053872eea..fdcc948352 100755
> --- a/contrib/thunderbird-patch-inline/appp.sh
> +++ b/contrib/thunderbird-patch-inline/appp.sh
> @@ -31,7 +31,7 @@ BODY=$(sed -e "1,/${SEP}/d" $1)
>  CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' "${PATCH}")
>  DIFF=$(sed -e '1,/^---$/d' "${PATCH}")
>  
> -CCS=$(echo -e "$CMT_MSG\n$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
> +CCS=$(printf '%s\n%s\n' "$CMT_MSG" "$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
>  	-e 's/^Signed-off-by: \(.*\)/\1,/gp')
>  
>  echo "$SUBJECT" > $1