* [PATCH] thunderbird-patch-inline: avoid bashism
@ 2025-02-04 1:46 brian m. carlson
2025-02-04 2:11 ` D. Ben Knoble
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: brian m. carlson @ 2025-02-04 1:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
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' \
-e 's/^Signed-off-by: \(.*\)/\1,/gp')
echo "$SUBJECT" > $1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] thunderbird-patch-inline: avoid bashism
2025-02-04 1:46 [PATCH] thunderbird-patch-inline: avoid bashism brian m. carlson
@ 2025-02-04 2:11 ` D. Ben Knoble
2025-02-04 2:43 ` brian m. carlson
2025-02-04 13:28 ` Junio C Hamano
2025-02-10 23:49 ` [PATCH v2] " brian m. carlson
2 siblings, 1 reply; 6+ messages in thread
From: D. Ben Knoble @ 2025-02-04 2:11 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Junio C Hamano
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
--
D. Ben Knoble
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] thunderbird-patch-inline: avoid bashism
2025-02-04 2:11 ` D. Ben Knoble
@ 2025-02-04 2:43 ` brian m. carlson
0 siblings, 0 replies; 6+ messages in thread
From: brian m. carlson @ 2025-02-04 2:43 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: git, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]
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 (they/them or he/him)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] thunderbird-patch-inline: avoid bashism
2025-02-04 1:46 [PATCH] thunderbird-patch-inline: avoid bashism brian m. carlson
2025-02-04 2:11 ` D. Ben Knoble
@ 2025-02-04 13:28 ` Junio C Hamano
2025-02-10 23:49 ` [PATCH v2] " brian m. carlson
2 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2025-02-04 13:28 UTC (permalink / raw)
To: brian m. carlson; +Cc: git
"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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] thunderbird-patch-inline: avoid bashism
2025-02-04 1:46 [PATCH] thunderbird-patch-inline: avoid bashism brian m. carlson
2025-02-04 2:11 ` D. Ben Knoble
2025-02-04 13:28 ` Junio C Hamano
@ 2025-02-10 23:49 ` brian m. carlson
2025-02-11 0:19 ` Junio C Hamano
2 siblings, 1 reply; 6+ messages in thread
From: brian m. carlson @ 2025-02-10 23:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, D. Ben Knoble
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.
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] thunderbird-patch-inline: avoid bashism
2025-02-10 23:49 ` [PATCH v2] " brian m. carlson
@ 2025-02-11 0:19 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2025-02-11 0:19 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, D. Ben Knoble
"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.
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-11 0:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-04 1:46 [PATCH] thunderbird-patch-inline: avoid bashism brian m. carlson
2025-02-04 2:11 ` D. Ben Knoble
2025-02-04 2:43 ` brian m. carlson
2025-02-04 13:28 ` Junio C Hamano
2025-02-10 23:49 ` [PATCH v2] " brian m. carlson
2025-02-11 0:19 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).