git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
Feb 4, 2025, 02:43 UTC
Message-ID
<Z6F-ySoYbDJxJ3xs@tapette.crustytoothpaste.net>
In-Reply-To
<CALnO6CAib-p1+1VHMfqo4VGLVvWs63z_xCjkCANJS-NLK5FpdQ@mail.gmail.com>
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
Previous: D. Ben KnobleNext: Junio C Hamano
Message 3 of 6 in “thunderbird-patch-inline: avoid bashism”
  1. thunderbird-patch-inline: avoid bashismbrian m. carlson, Feb 4, 2025
  2. D. Ben KnobleFeb 4, 2025
  3. brian m. carlsonFeb 4, 2025
  4. Junio C HamanoFeb 4, 2025
  5. thunderbird-patch-inline: avoid bashismbrian m. carlson, Feb 10, 2025
  6. Junio C HamanoFeb 11, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.