Skip to contentSkip to conversation

t9001: avoid not portable '\n' with sed

PATCH In review Torsten Bögershausen 5 participants 8 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 t9001: avoid not portable '\n' with sed

t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'.

As the test already requires perl as a prerequisite, use perl instead of sed.

---
 t/t9001-send-email.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
t/t9001-send-email.sh +1 -1
index 64d9434..2bf48d1 100755
@@ -1342,7 +1342,7 @@ test_cover_addresses () {
 	git format-patch --cover-letter -2 -o outdir &&
 	cover=`echo outdir/0000-*.patch` &&
 	mv $cover cover-to-edit.patch &&
-	sed "s/^From:/$header: [email protected]\nFrom:/" cover-to-edit.patch >"$cover" &&
+	"$PERL_PATH" -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch | tr Q "$LF" >"$cover" &&
 	git send-email \
 	  --force \
 	  --from="Example <[email protected]>" \
Signature · 1 line
2.0.0.553.ged01b91
#1 lore raw
Quoted 8 lines · Torsten Bögershausen

t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'.

As the test already requires perl as a prerequisite, use perl instead of sed.

---

Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html

The escape sequence '\n' shall match a <newline> embedded in the pattern space.

so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people.

t/t9001-send-email.sh test_cover_addresses () { · line 1345

Quoted 13 lines
 t/t9001-send-email.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
t/t9001-send-email.sh +1 -1
index 64d9434..2bf48d1 100755
@@ -1342,7 +1342,7 @@ test_cover_addresses () {
 	git format-patch --cover-letter -2 -o outdir &&
 	cover=`echo outdir/0000-*.patch` &&
 	mv $cover cover-to-edit.patch &&
-	sed "s/^From:/$header: [email protected]\nFrom:/" cover-to-edit.patch >"$cover" &&
+	"$PERL_PATH" -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch | tr Q "$LF" >"$cover" &&

We have a shell function "perl" in test-lib-function.sh these days so that you do not have to write "$PERL_PATH" yourself in tests ;-)

t/t9001-send-email.sh test_cover_addresses () { · line 1348

Quoted 3 lines
t/t9001-send-email.sh +0 -0
 	git send-email \
 	  --force \
 	  --from="Example <[email protected]>" \

Thanks.

#2 lore raw
Quoted 13 lines · Junio C Hamano
Quoted 8 lines · Torsten Bögershausen

t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'.

As the test already requires perl as a prerequisite, use perl instead of sed.

---

Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html

Quoted 8 lines

The escape sequence '\n' shall match a <newline> embedded in the pattern space.

so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people.

t/t9001-send-email.sh test_cover_addresses () { · line 1345

Quoted 2 lines
t/t9001-send-email.sh +1 -1
-	sed "s/^From:/$header: [email protected]\nFrom:/" cover-to-edit.patch >"$cover" &&
+	"$PERL_PATH" -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch | tr Q "$LF" >"$cover" &&

Note that quoted section of POSIX says "embedded in the pattern space"; under the description of the "s" command, it says:

The replacement string shall be scanned from beginning to end. [...]
The meaning of a <backslash> immediately followed by any character other than '&', <backslash>, a digit, or the delimiter character used for this command, is unspecified.

A line can be split by substituting a <newline> into it. The application shall escape the <newline> in the replacement by preceding it by a <backslash>.

So the portable way to do it is:

	sed "s/^From:/$header: [email protected]\

From:/" cover-to-edit.patch >"$cover" &&

but that requires the continuation to start in column 0, so the Perl variant is probably neater.

#3 lore raw
Quoted 4 lines · John Keeping

So the portable way to do it is:

	sed "s/^From:/$header: [email protected]\

From:/" cover-to-edit.patch >"$cover" &&

That wouldn't work as \<newline> is removed in double quotes. You either need to double the backslash or put it in single quotes.

Andreas.

Signature · 3 lines
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
#4 lore raw
In reply to John Keeping
Quoted 8 lines · John Keeping

Note that quoted section of POSIX says "embedded in the pattern space"; under the description of the "s" command, it says:

The replacement string shall be scanned from beginning to end. [...]
The meaning of a <backslash> immediately followed by any character other than '&', <backslash>, a digit, or the delimiter character used for this command, is unspecified.

Very true.

#5 lore raw
In reply to Junio C Hamano
Quoted 11 lines · Junio C Hamano
Quoted 8 lines · Torsten Bögershausen

t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'.

As the test already requires perl as a prerequisite, use perl instead of sed.

---

Quoted 7 lines

Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html

The escape sequence '\n' shall match a <newline> embedded in the pattern space.

so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people.

"shall match" talks about the match expression, not the replacement.

Signature · 1 line
David Kastrup
#6 lore raw
In reply to Junio C Hamano
Quoted 22 lines · Junio C Hamano
Quoted 8 lines · Torsten Bögershausen

t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'.

As the test already requires perl as a prerequisite, use perl instead of sed.

---

Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html

The escape sequence '\n' shall match a <newline> embedded in the pattern space.

so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people.

Quoted 3 lines
 t/t9001-send-email.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 13 lines

t/t9001-send-email.sh test_cover_addresses () { · line 1345

Quoted 10 lines
t/t9001-send-email.sh +1 -1
index 64d9434..2bf48d1 100755
@@ -1342,7 +1342,7 @@ test_cover_addresses () {
 	git format-patch --cover-letter -2 -o outdir &&
 	cover=`echo outdir/0000-*.patch` &&
 	mv $cover cover-to-edit.patch &&
-	sed "s/^From:/$header: [email protected]\nFrom:/" cover-to-edit.patch >"$cover" &&
+	"$PERL_PATH" -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch | tr Q "$LF" >"$cover" &&

We have a shell function "perl" in test-lib-function.sh these days so that you do not have to write "$PERL_PATH" yourself in tests ;-)

Also, piping output from perl to tr feels somewhat suboptimal. I do not see where in the test material we use "Q to LF", and we may want to remove that altogether, but without that removal, an updated patch may look like this.

 t/t9001-send-email.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
t/t9001-send-email.sh +4 -1
index 64d9434..9f06b8c 100755
@@ -1342,7 +1342,10 @@ test_cover_addresses () {
 	git format-patch --cover-letter -2 -o outdir &&
 	cover=`echo outdir/0000-*.patch` &&
 	mv $cover cover-to-edit.patch &&
-	sed "s/^From:/$header: [email protected]\nFrom:/" cover-to-edit.patch >"$cover" &&
+	perl -pe "
+		s/^From:/$header: extra\@address.com\nFrom:/;
+		y/Q/\n/;
+	" cover-to-edit.patch >"$cover" &&
 	git send-email \
 	  --force \
 	  --from="Example <[email protected]>" \
#7 lore raw
Quoted 52 lines · Junio C Hamano
Quoted 22 lines · Junio C Hamano
Quoted 8 lines · Torsten Bögershausen

t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'.

As the test already requires perl as a prerequisite, use perl instead of sed.

---

Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html

The escape sequence '\n' shall match a <newline> embedded in the pattern space.

so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people.

Quoted 3 lines
 t/t9001-send-email.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 13 lines

t/t9001-send-email.sh test_cover_addresses () { · line 1345

Quoted 10 lines
t/t9001-send-email.sh +1 -1
index 64d9434..2bf48d1 100755
@@ -1342,7 +1342,7 @@ test_cover_addresses () {
 	git format-patch --cover-letter -2 -o outdir &&
 	cover=`echo outdir/0000-*.patch` &&
 	mv $cover cover-to-edit.patch &&
-	sed "s/^From:/$header: [email protected]\nFrom:/" cover-to-edit.patch >"$cover" &&
+	"$PERL_PATH" -pe "s/^From:/$header: extra\@address.com\nFrom:/" cover-to-edit.patch | tr Q "$LF" >"$cover" &&

We have a shell function "perl" in test-lib-function.sh these days so that you do not have to write "$PERL_PATH" yourself in tests ;-)

Also, piping output from perl to tr feels somewhat suboptimal. I do not see where in the test material we use "Q to LF", and we may want to remove that altogether, but without that removal, an updated patch may look like this.

 t/t9001-send-email.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
t/t9001-send-email.sh +0 -0
index 64d9434..9f06b8c 100755
@@ -1342,7 +1342,10 @@ test_cover_addresses () {
 	git format-patch --cover-letter -2 -o outdir &&

t/t9001-send-email.sh test_cover_addresses () { · line 1351

Quoted 10 lines
t/t9001-send-email.sh +4 -1
 	cover=`echo outdir/0000-*.patch` &&
 	mv $cover cover-to-edit.patch &&
-	sed "s/^From:/$header: [email protected]\nFrom:/" cover-to-edit.patch >"$cover" &&
+	perl -pe "
+		s/^From:/$header: extra\@address.com\nFrom:/;
+		y/Q/\n/;
+	" cover-to-edit.patch >"$cover" &&
 	git send-email \
 	  --force \
 	  --from="Example <[email protected]>" \

Good catch, the "tr" should had been removed.
My first version used
sed "s/^From:/$header: [email protected]:/"
and the Q was replaced by tr with a literal LF.
So I think the 'Q' -> '\n' conversion should be removed completely :-)

The sed in question is /usr/bin/sed under Mac OS X. Then we have the question: What exactly is the pattern space?

Quoted 1 line

In default operation, sed cyclically shall append a line of input, less its terminating <newline> >character, into the pattern space....

Isn't that the stuff from the input?
But that doesn't make too much sence to me, since "input lines" are terminated by \n. So pattern space seems to mean output when they talk about the \n
Anyway, the \n (to insert a newline into the output) works under Linux, but not Mac OS.