threads / patch / 39885

patch, 2 partsFix sed usage in tests to work around broken xpg4/sed on Solaris

Subject: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris

## tl;dr

7 messages between Jul 18, 2015 and Jul 22, 2015. Diffs are folded; open one to read it.

replies: 6people: 5as markdown or json

Ben Walton· Jul 18, 2015, 15:21 UTC · lore

The space following the last / in a sed command caused Solaris' xpg4/sed to fail, claiming the program was garbled and exit with status 2:

% echo 'foo' | /usr/xpg4/bin/sed -e 's/foo/bar/ ' sed: command garbled: s/foo/bar/ % echo $? 2

Fix this by simply removing the unnecessary space.

Additionally, in 99094a7a, a trivial && breakage was fixed. This exposed a problem with the test when run on Solaris with xpg4/sed that had gone silently undetected since its introduction in e4bd10b2. Solaris' sed executes the requested substitution but prints a warning about the missing newline at the end of the file and exits with status 2.

% echo "CHANGE_ME" | \ tr -d "\\012" | /usr/xpg4/bin/sed -e 's/CHANGE_ME/change_me/' sed: Missing newline at end of file standard input. change_me % echo $? 2

To work around this, use perl to execute the substitution instead. By using inplace replacement, we can subsequently drop the mv command.

Signed-off-by: Ben Walton <bdwalton@gmail.com>
---
 t/t5601-clone.sh                       |    2 +-
 t/t9500-gitweb-standalone-no-errors.sh |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)
Show changes to 2 files +2 −3

t/t5601-clone.sh, t/t9500-gitweb-standalone-no-errors.sh

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index fa6be3c..2583f84 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' '
 #IPv6
 for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]:
 do
-	ehost=$(echo $tuah | sed -e "s/1]:/1]/ " | tr -d "\133\135")
+	ehost=$(echo $tuah | sed -e "s/1]:/1]/" | tr -d "\133\135")
 	test_expect_success "clone ssh://$tuah/home/user/repo" "
 	  test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo
 	"
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index e94b2f1..eb264f9 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -290,8 +290,7 @@ test_expect_success 'setup incomplete lines' '
 	echo "incomplete" | tr -d "\\012" >>file &&
 	git commit -a -m "Add incomplete line" &&
 	git tag incomplete_lines_add &&
-	sed -e s/CHANGE_ME/change_me/ <file >file+ &&
-	mv -f file+ file &&
+	perl -pi -e "s/CHANGE_ME/change_me/" file &&
 	git commit -a -m "Incomplete context line" &&
 	git tag incomplete_lines_ctx &&
 	echo "Dominus regit me," >file &&

-- 
1.7.10.4
Eric Sunshine· Jul 19, 2015, 03:39 UTC · re: Ben Walton · lore

Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris

On Sat, Jul 18, 2015 at 11:21 AM, Ben Walton <bdwalton@gmail.com> wrote:
Show 27 quoted lines
> The space following the last / in a sed command caused Solaris'
> xpg4/sed to fail, claiming the program was garbled and exit with
> status 2:
>
> % echo 'foo' | /usr/xpg4/bin/sed -e 's/foo/bar/ '
> sed: command garbled: s/foo/bar/
> % echo $?
> 2
>
> Fix this by simply removing the unnecessary space.
>
> Additionally, in 99094a7a, a trivial && breakage was fixed. This
> exposed a problem with the test when run on Solaris with xpg4/sed that
> had gone silently undetected since its introduction in
> e4bd10b2. Solaris' sed executes the requested substitution but prints
> a warning about the missing newline at the end of the file and exits
> with status 2.
>
> % echo "CHANGE_ME" | \
> tr -d "\\012" | /usr/xpg4/bin/sed -e 's/CHANGE_ME/change_me/'
> sed: Missing newline at end of file standard input.
> change_me
> % echo $?
> 2
>
> To work around this, use perl to execute the substitution instead. By
> using inplace replacement, we can subsequently drop the mv command.
This two unrelated fixes could be separate patches...
Show 31 quoted lines
> Signed-off-by: Ben Walton <bdwalton@gmail.com>
> ---
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index fa6be3c..2583f84 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' '
>  #IPv6
>  for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]:
>  do
> -       ehost=$(echo $tuah | sed -e "s/1]:/1]/ " | tr -d "\133\135")
> +       ehost=$(echo $tuah | sed -e "s/1]:/1]/" | tr -d "\133\135")
>         test_expect_success "clone ssh://$tuah/home/user/repo" "
>           test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo
>         "
> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> index e94b2f1..eb264f9 100755
> --- a/t/t9500-gitweb-standalone-no-errors.sh
> +++ b/t/t9500-gitweb-standalone-no-errors.sh
> @@ -290,8 +290,7 @@ test_expect_success 'setup incomplete lines' '
>         echo "incomplete" | tr -d "\\012" >>file &&
>         git commit -a -m "Add incomplete line" &&
>         git tag incomplete_lines_add &&
> -       sed -e s/CHANGE_ME/change_me/ <file >file+ &&
> -       mv -f file+ file &&
> +       perl -pi -e "s/CHANGE_ME/change_me/" file &&
>         git commit -a -m "Incomplete context line" &&
>         git tag incomplete_lines_ctx &&
>         echo "Dominus regit me," >file &&
> --
> 1.7.10.4
Johannes Sixt· Jul 19, 2015, 06:54 UTC · re: Ben Walton · lore

Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris

Am 18.07.2015 um 17:21 schrieb Ben Walton:
Show 44 quoted lines
> The space following the last / in a sed command caused Solaris'
> xpg4/sed to fail, claiming the program was garbled and exit with
> status 2:
>
> % echo 'foo' | /usr/xpg4/bin/sed -e 's/foo/bar/ '
> sed: command garbled: s/foo/bar/
> % echo $?
> 2
>
> Fix this by simply removing the unnecessary space.
>
> Additionally, in 99094a7a, a trivial && breakage was fixed. This
> exposed a problem with the test when run on Solaris with xpg4/sed that
> had gone silently undetected since its introduction in
> e4bd10b2. Solaris' sed executes the requested substitution but prints
> a warning about the missing newline at the end of the file and exits
> with status 2.
>
> % echo "CHANGE_ME" | \
> tr -d "\\012" | /usr/xpg4/bin/sed -e 's/CHANGE_ME/change_me/'
> sed: Missing newline at end of file standard input.
> change_me
> % echo $?
> 2
>
> To work around this, use perl to execute the substitution instead. By
> using inplace replacement, we can subsequently drop the mv command.
>
> Signed-off-by: Ben Walton <bdwalton@gmail.com>
> ---
>   t/t5601-clone.sh                       |    2 +-
>   t/t9500-gitweb-standalone-no-errors.sh |    3 +--
>   2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index fa6be3c..2583f84 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' '
>   #IPv6
>   for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]:
>   do
> -	ehost=$(echo $tuah | sed -e "s/1]:/1]/ " | tr -d "\133\135")
> +	ehost=$(echo $tuah | sed -e "s/1]:/1]/" | tr -d "\133\135")
Can this not be rewritten as
	ehost=$(echo $tuah | sed -e "s/1]:/1]/" -e "s/[][]//g")
But I admit that it looks like black magic without a comment...
Show 14 quoted lines
>   	test_expect_success "clone ssh://$tuah/home/user/repo" "
>   	  test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo
>   	"
> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> index e94b2f1..eb264f9 100755
> --- a/t/t9500-gitweb-standalone-no-errors.sh
> +++ b/t/t9500-gitweb-standalone-no-errors.sh
> @@ -290,8 +290,7 @@ test_expect_success 'setup incomplete lines' '
>   	echo "incomplete" | tr -d "\\012" >>file &&
>   	git commit -a -m "Add incomplete line" &&
>   	git tag incomplete_lines_add &&
> -	sed -e s/CHANGE_ME/change_me/ <file >file+ &&
> -	mv -f file+ file &&
> +	perl -pi -e "s/CHANGE_ME/change_me/" file &&

This is problematic. On Windows, perl -i fails when no backup file extension is specified because perl attempts to replace a file that is still open; that does not work on Windows. This should work, but I haven't tested, yet:

	perl -pi.bak -e "s/CHANGE_ME/change_me/" file &&
>   	git commit -a -m "Incomplete context line" &&
>   	git tag incomplete_lines_ctx &&
>   	echo "Dominus regit me," >file &&
>
-- Hannes
Johannes Schindelin· Jul 19, 2015, 07:37 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris

Hi,
On 2015-07-19 08:54, Johannes Sixt wrote:
Show 19 quoted lines
> Am 18.07.2015 um 17:21 schrieb Ben Walton:
>>   	test_expect_success "clone ssh://$tuah/home/user/repo" "
>>   	  test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo
>>   	"
>> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
>> index e94b2f1..eb264f9 100755
>> --- a/t/t9500-gitweb-standalone-no-errors.sh
>> +++ b/t/t9500-gitweb-standalone-no-errors.sh
>> @@ -290,8 +290,7 @@ test_expect_success 'setup incomplete lines' '
>>   	echo "incomplete" | tr -d "\\012" >>file &&
>>   	git commit -a -m "Add incomplete line" &&
>>   	git tag incomplete_lines_add &&
>> -	sed -e s/CHANGE_ME/change_me/ <file >file+ &&
>> -	mv -f file+ file &&
>> +	perl -pi -e "s/CHANGE_ME/change_me/" file &&
> 
> This is problematic. On Windows, perl -i fails when no backup file
> extension is specified because perl attempts to replace a file that is
> still open; that does not work on Windows.
Let's qualify this a bit better: it actually works with the SDK of Git for Windows 2.x. It is therefore incomplete and partially incorrect to say "that does not work on Windows". It is true that Git for Windows 1.x' perl bails out with "Can't do inplace edit".
> This should work, but I haven't tested, yet:
> 
> 	perl -pi.bak -e "s/CHANGE_ME/change_me/" file &&
This works, of course, but it leaves an extra file behind.
I really wonder why the previous ">file+ && mv -f file+ file" dance needs to be replaced?

Ciao, Johannes

Johannes Sixt· Jul 19, 2015, 08:40 UTC · re: Johannes Schindelin · lore

Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris

Am 19.07.2015 um 09:37 schrieb Johannes Schindelin:
Show 12 quoted lines
> On 2015-07-19 08:54, Johannes Sixt wrote:
>> Am 18.07.2015 um 17:21 schrieb Ben Walton:
>>> -	sed -e s/CHANGE_ME/change_me/ <file >file+ &&
>>> -	mv -f file+ file &&
>>> +	perl -pi -e "s/CHANGE_ME/change_me/" file &&
>>
>> This is problematic. On Windows, perl -i fails when no backup file
>> extension is specified because perl attempts to replace a file that is
>> still open; that does not work on Windows.
>
> Let's qualify this a bit better: it actually works with the SDK of
> Git  for Windows 2.x.
Good to know!
> I really wonder why the previous ">file+ && mv -f file+ file" dance
> needs to be replaced?

The sed must be replaced because some versions on Solaris choke on the incomplete last line in the file.

-- Hannes
Junio C Hamano· Jul 20, 2015, 16:07 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris

Johannes Sixt <j6t@kdbg.org> writes:
Show 5 quoted lines
>> I really wonder why the previous ">file+ && mv -f file+ file" dance
>> needs to be replaced?
>
> The sed must be replaced because some versions on Solaris choke on the
> incomplete last line in the file.
Switching from sed to perl is not being questioned.

I think Dscho is asking about the use of "-i", when the original idiom ">target+ && mv -f target+ target" worked perfectly fine.

I.e.
	perl -p -e '...' file >file+ &&
        mv file+ file
Johannes Schindelin· Jul 22, 2015, 09:52 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris

Hi,
On 2015-07-20 18:07, Junio C Hamano wrote:
Show 17 quoted lines
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>>> I really wonder why the previous ">file+ && mv -f file+ file" dance
>>> needs to be replaced?
>>
>> The sed must be replaced because some versions on Solaris choke on the
>> incomplete last line in the file.
> 
> Switching from sed to perl is not being questioned.
> 
> I think Dscho is asking about the use of "-i", when the original
> idiom ">target+ && mv -f target+ target" worked perfectly fine.
> 
> I.e.
> 
> 	perl -p -e '...' file >file+ &&
>         mv file+ file
Thanks for translating from Dscho to English.

Ciao, Dscho

← back to recent threads