| Message ID | 20250204014652.3509928-1-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | thunderbird-patch-inline: avoid bashism | expand |
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. > -e 's/^Signed-off-by: \(.*\)/\1,/gp') > > echo "$SUBJECT" > $1 > [1]: https://github.com/benknoble/echocho
On 2025-02-04 at 02:11:19, D. Ben Knoble wrote: > 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" <sandals@crustytoothpaste.net> writes: > 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. > > 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
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
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