threads / patch / 13524

patchFix sed syntax in t7502-commit for OSX

Subject: [PATCH] Fix sed syntax in t7502-commit for OSX

## tl;dr

7 messages between May 15, 2008 and May 16, 2008. Diffs are folded; open one to read it.

replies: 6people: 5as markdown or json

Marcel Koeppen· May 15, 2008, 02:19 UTC · lore
The OSX version of sed interprets the command as argument to the -i option.
Signed-off-by: Marcel Koeppen <git-dev@marzelpan.de>
---
 t/t7502-commit.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to t/t7502-commit.sh +1 −1
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 018060c..9a43104 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -166,7 +166,7 @@ test_expect_success 'author different from committer' '
 	test_cmp expect actual
 '
 
-sed -i '$d' expect
+sed -i -e '$d' expect
 echo "# Committer:
 #" >> expect
 unset GIT_COMMITTER_EMAIL

-- 
1.5.5.1.215.g64c0d
Junio C Hamano· May 15, 2008, 08:37 UTC · re: Marcel Koeppen · lore

Re: [PATCH] Fix sed syntax in t7502-commit for OSX

Marcel Koeppen <git-dev@marzelpan.de> writes:
Show 20 quoted lines
> The OSX version of sed interprets the command as argument to the -i option.
>
> Signed-off-by: Marcel Koeppen <git-dev@marzelpan.de>
> ---
>  t/t7502-commit.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
> index 018060c..9a43104 100755
> --- a/t/t7502-commit.sh
> +++ b/t/t7502-commit.sh
> @@ -166,7 +166,7 @@ test_expect_success 'author different from committer' '
>  	test_cmp expect actual
>  '
>  
> -sed -i '$d' expect
> +sed -i -e '$d' expect
>  echo "# Committer:
>  #" >> expect
>  unset GIT_COMMITTER_EMAIL
Two comments and a half.
This patch does not make things worse, so I do not have much against it.

However, "sed -i $cmd" is also used in templates/hooks--prepare-commit-msg and you would want to fix it as well.

But more importantly, as we are serious about portability, we should not be using "sed -i" which is not even in POSIX to begin with. Can we rewrite these places without using the in-place extension?

Marcel Koeppen· May 15, 2008, 11:06 UTC · re: Holger Schurig · lore

Re: [PATCH] Fix sed syntax in t7502-commit for OSX

Am 15.05.2008 um 10:58 schrieb Holger Schurig:
>>> -sed -i '$d' expect
>>> +sed -i -e '$d' expect
>
> perl -pi -e 's/foo/bar/' ???

This does not work with '$d', so we could use sed with a temp instead of the -i extension. Alternatively we could specify the full test fixture instead of editing an old one. This would make the expected results more obvious and the tests would work without assumptions about running order.

Marcel
Santi Béjar· May 15, 2008, 09:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] Fix sed syntax in t7502-commit for OSX

On Thu, May 15, 2008 at 10:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> But more importantly, as we are serious about portability, we should not
> be using "sed -i" which is not even in POSIX to begin with.

Is there a way (environment variable, flag) to test if it is POSIX? Or we just have to know?

I'll send a patch to document the "sed -i" issue in Documentation/CodingGuidelines.

>  Can we
> rewrite these places without using the in-place extension?
Yes, I'll do it soon if nobody beats me.
Santi
Marcel Koeppen· May 16, 2008, 00:21 UTC · re: Junio C Hamano · lore

[PATCH] Replace in-place sed in t7502-commit

The in-place mode of sed used in t7502-commit is a non-POSIX extension. That call of sed is replaced by a more portable version using a temporary file.

Signed-off-by: Marcel Koeppen <git-dev@marzelpan.de>
---
 t/t7502-commit.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
Show changes to t/t7502-commit.sh +3 −1
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 018060c..3531a99 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -166,7 +166,9 @@ test_expect_success 'author different from committer' '
 	test_cmp expect actual
 '
 
-sed -i '$d' expect
+mv expect expect.tmp
+sed '$d' < expect.tmp > expect
+rm -f expect.tmp
 echo "# Committer:
 #" >> expect
 unset GIT_COMMITTER_EMAIL

-- 
1.5.5.1.215.g64c0d
Jeff King· May 16, 2008, 09:56 UTC · re: Marcel Koeppen · lore

Re: [PATCH] Replace in-place sed in t7502-commit

On Fri, May 16, 2008 at 02:21:43AM +0200, Marcel Koeppen wrote:
> The in-place mode of sed used in t7502-commit is a non-POSIX extension.
> That call of sed is replaced by a more portable version using a
> temporary file.

Thanks, this fixes a similar issue on FreeBSD (which is probably the same sed).

-Peff

← back to recent threads