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 wrote: > 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... > Signed-off-by: Ben Walton > --- > 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+ && > - 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: 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 --- 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... 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+ && - 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: > 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+ && >> -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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: 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+ && - 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris
Johannes Sixt 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: > Johannes Sixt 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html