{"thread":{"id":"39885","subject":"[PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris","startedAt":"2015-07-18T15:21:32Z","lastAt":"2015-07-22T09:52:32Z","messageCount":7,"participants":["Ben Walton","Eric Sunshine","Johannes Sixt","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"266331","messageId":"1437232892-27978-1-git-send-email-bdwalton@gmail.com","threadId":"39885","inReplyTo":null,"subject":"[PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris","fromName":"Ben Walton","fromEmail":"bdwalton@gmail.com","sentAt":"2015-07-18T15:21:32Z","receivedAt":"2015-07-18T15:21:32Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"The space following the last / in a sed command caused Solaris'\nxpg4/sed to fail, claiming the program was garbled and exit with\nstatus 2:\n\n% echo 'foo' | /usr/xpg4/bin/sed -e 's/foo/bar/ '\nsed: command garbled: s/foo/bar/\n% echo $?\n2\n\nFix this by simply removing the unnecessary space.\n\nAdditionally, in 99094a7a, a trivial && breakage was fixed. This\nexposed a problem with the test when run on Solaris with xpg4/sed that\nhad gone silently undetected since its introduction in\ne4bd10b2. Solaris' sed executes the requested substitution but prints\na warning about the missing newline at the end of the file and exits\nwith status 2.\n\n% echo \"CHANGE_ME\" | \\\ntr -d \"\\\\012\" | /usr/xpg4/bin/sed -e 's/CHANGE_ME/change_me/'\nsed: Missing newline at end of file standard input.\nchange_me\n% echo $?\n2\n\nTo work around this, use perl to execute the substitution instead. By\nusing inplace replacement, we can subsequently drop the mv command.\n\nSigned-off-by: Ben Walton \n---\n t/t5601-clone.sh | 2 +-\n t/t9500-gitweb-standalone-no-errors.sh | 3 +--\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex fa6be3c..2583f84 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' '\n #IPv6\n for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]:\n do\n-\tehost=$(echo $tuah | sed -e \"s/1]:/1]/ \" | tr -d \"\\133\\135\")\n+\tehost=$(echo $tuah | sed -e \"s/1]:/1]/\" | tr -d \"\\133\\135\")\n \ttest_expect_success \"clone ssh://$tuah/home/user/repo\" \"\n \t test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo\n \t\"\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex e94b2f1..eb264f9 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -290,8 +290,7 @@ test_expect_success 'setup incomplete lines' '\n \techo \"incomplete\" | tr -d \"\\\\012\" >>file &&\n \tgit commit -a -m \"Add incomplete line\" &&\n \tgit tag incomplete_lines_add &&\n-\tsed -e s/CHANGE_ME/change_me/ file+ &&\n-\tmv -f file+ file &&\n+\tperl -pi -e \"s/CHANGE_ME/change_me/\" file &&\n \tgit commit -a -m \"Incomplete context line\" &&\n \tgit tag incomplete_lines_ctx &&\n \techo \"Dominus regit me,\" >file &&\n-- \n1.7.10.4\n"},{"id":"266365","messageId":"CAPig+cSXUoMniWABY-LrH=Z-A6josuAfqW7BVBTXPKG=vd7ouA@mail.gmail.com","threadId":"39885","inReplyTo":"1437232892-27978-1-git-send-email-bdwalton@gmail.com","subject":"Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-19T03:39:26Z","receivedAt":"2015-07-19T03:39:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jul 18, 2015 at 11:21 AM, Ben Walton wrote:\n> The space following the last / in a sed command caused Solaris'\n> xpg4/sed to fail, claiming the program was garbled and exit with\n> status 2:\n>\n> % echo 'foo' | /usr/xpg4/bin/sed -e 's/foo/bar/ '\n> sed: command garbled: s/foo/bar/\n> % echo $?\n> 2\n>\n> Fix this by simply removing the unnecessary space.\n>\n> Additionally, in 99094a7a, a trivial && breakage was fixed. This\n> exposed a problem with the test when run on Solaris with xpg4/sed that\n> had gone silently undetected since its introduction in\n> e4bd10b2. Solaris' sed executes the requested substitution but prints\n> a warning about the missing newline at the end of the file and exits\n> with status 2.\n>\n> % echo \"CHANGE_ME\" | \\\n> tr -d \"\\\\012\" | /usr/xpg4/bin/sed -e 's/CHANGE_ME/change_me/'\n> sed: Missing newline at end of file standard input.\n> change_me\n> % echo $?\n> 2\n>\n> To work around this, use perl to execute the substitution instead. By\n> using inplace replacement, we can subsequently drop the mv command.\n\nThis two unrelated fixes could be separate patches...\n\n> Signed-off-by: Ben Walton \n> ---\n> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> index fa6be3c..2583f84 100755\n> --- a/t/t5601-clone.sh\n> +++ b/t/t5601-clone.sh\n> @@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' '\n> #IPv6\n> for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]:\n> do\n> - ehost=$(echo $tuah | sed -e \"s/1]:/1]/ \" | tr -d \"\\133\\135\")\n> + ehost=$(echo $tuah | sed -e \"s/1]:/1]/\" | tr -d \"\\133\\135\")\n> test_expect_success \"clone ssh://$tuah/home/user/repo\" \"\n> test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo\n> \"\n> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\n> index e94b2f1..eb264f9 100755\n> --- a/t/t9500-gitweb-standalone-no-errors.sh\n> +++ b/t/t9500-gitweb-standalone-no-errors.sh\n> @@ -290,8 +290,7 @@ test_expect_success 'setup incomplete lines' '\n> echo \"incomplete\" | tr -d \"\\\\012\" >>file &&\n> git commit -a -m \"Add incomplete line\" &&\n> git tag incomplete_lines_add &&\n> - sed -e s/CHANGE_ME/change_me/ file+ &&\n> - mv -f file+ file &&\n> + perl -pi -e \"s/CHANGE_ME/change_me/\" file &&\n> git commit -a -m \"Incomplete context line\" &&\n> git tag incomplete_lines_ctx &&\n> echo \"Dominus regit me,\" >file &&\n> --\n> 1.7.10.4\n"},{"id":"266367","messageId":"55AB49C1.8010105@kdbg.org","threadId":"39885","inReplyTo":"1437232892-27978-1-git-send-email-bdwalton@gmail.com","subject":"Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-07-19T06:54:57Z","receivedAt":"2015-07-19T06:54:57Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 18.07.2015 um 17:21 schrieb Ben Walton:\n> The space following the last / in a sed command caused Solaris'\n> xpg4/sed to fail, claiming the program was garbled and exit with\n> status 2:\n>\n> % echo 'foo' | /usr/xpg4/bin/sed -e 's/foo/bar/ '\n> sed: command garbled: s/foo/bar/\n> % echo $?\n> 2\n>\n> Fix this by simply removing the unnecessary space.\n>\n> Additionally, in 99094a7a, a trivial && breakage was fixed. This\n> exposed a problem with the test when run on Solaris with xpg4/sed that\n> had gone silently undetected since its introduction in\n> e4bd10b2. Solaris' sed executes the requested substitution but prints\n> a warning about the missing newline at the end of the file and exits\n> with status 2.\n>\n> % echo \"CHANGE_ME\" | \\\n> tr -d \"\\\\012\" | /usr/xpg4/bin/sed -e 's/CHANGE_ME/change_me/'\n> sed: Missing newline at end of file standard input.\n> change_me\n> % echo $?\n> 2\n>\n> To work around this, use perl to execute the substitution instead. By\n> using inplace replacement, we can subsequently drop the mv command.\n>\n> Signed-off-by: Ben Walton \n> ---\n> t/t5601-clone.sh | 2 +-\n> t/t9500-gitweb-standalone-no-errors.sh | 3 +--\n> 2 files changed, 2 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> index fa6be3c..2583f84 100755\n> --- a/t/t5601-clone.sh\n> +++ b/t/t5601-clone.sh\n> @@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' '\n> #IPv6\n> for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]:\n> do\n> -\tehost=$(echo $tuah | sed -e \"s/1]:/1]/ \" | tr -d \"\\133\\135\")\n> +\tehost=$(echo $tuah | sed -e \"s/1]:/1]/\" | tr -d \"\\133\\135\")\n\nCan this not be rewritten as\n\n\tehost=$(echo $tuah | sed -e \"s/1]:/1]/\" -e \"s/[][]//g\")\n\nBut I admit that it looks like black magic without a comment...\n\n> \ttest_expect_success \"clone ssh://$tuah/home/user/repo\" \"\n> \t test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo\n> \t\"\n> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\n> index e94b2f1..eb264f9 100755\n> --- a/t/t9500-gitweb-standalone-no-errors.sh\n> +++ b/t/t9500-gitweb-standalone-no-errors.sh\n> @@ -290,8 +290,7 @@ test_expect_success 'setup incomplete lines' '\n> \techo \"incomplete\" | tr -d \"\\\\012\" >>file &&\n> \tgit commit -a -m \"Add incomplete line\" &&\n> \tgit tag incomplete_lines_add &&\n> -\tsed -e s/CHANGE_ME/change_me/ file+ &&\n> -\tmv -f file+ file &&\n> +\tperl -pi -e \"s/CHANGE_ME/change_me/\" file &&\n\nThis is problematic. On Windows, perl -i fails when no backup file \nextension is specified because perl attempts to replace a file that is \nstill open; that does not work on Windows. This should work, but I \nhaven't tested, yet:\n\n\tperl -pi.bak -e \"s/CHANGE_ME/change_me/\" file &&\n\n> \tgit commit -a -m \"Incomplete context line\" &&\n> \tgit tag incomplete_lines_ctx &&\n> \techo \"Dominus regit me,\" >file &&\n>\n\n-- Hannes\n"},{"id":"266368","messageId":"fadc4ff7e755913a4c6076165556b56c@www.dscho.org","threadId":"39885","inReplyTo":"55AB49C1.8010105@kdbg.org","subject":"Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-07-19T07:37:53Z","receivedAt":"2015-07-19T07:37:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn 2015-07-19 08:54, Johannes Sixt wrote:\n> Am 18.07.2015 um 17:21 schrieb Ben Walton:\n>> \ttest_expect_success \"clone ssh://$tuah/home/user/repo\" \"\n>> \t test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo\n>> \t\"\n>> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\n>> index e94b2f1..eb264f9 100755\n>> --- a/t/t9500-gitweb-standalone-no-errors.sh\n>> +++ b/t/t9500-gitweb-standalone-no-errors.sh\n>> @@ -290,8 +290,7 @@ test_expect_success 'setup incomplete lines' '\n>> \techo \"incomplete\" | tr -d \"\\\\012\" >>file &&\n>> \tgit commit -a -m \"Add incomplete line\" &&\n>> \tgit tag incomplete_lines_add &&\n>> -\tsed -e s/CHANGE_ME/change_me/ file+ &&\n>> -\tmv -f file+ file &&\n>> +\tperl -pi -e \"s/CHANGE_ME/change_me/\" file &&\n> \n> This is problematic. On Windows, perl -i fails when no backup file\n> extension is specified because perl attempts to replace a file that is\n> still open; that does not work on Windows.\n\nLet'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\".\n\n> This should work, but I haven't tested, yet:\n> \n> \tperl -pi.bak -e \"s/CHANGE_ME/change_me/\" file &&\n\nThis works, of course, but it leaves an extra file behind.\n\nI really wonder why the previous \">file+ && mv -f file+ file\" dance needs to be replaced?\n\nCiao,\nJohannes\n"},{"id":"266371","messageId":"55AB6290.2090003@kdbg.org","threadId":"39885","inReplyTo":"fadc4ff7e755913a4c6076165556b56c@www.dscho.org","subject":"Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-07-19T08:40:48Z","receivedAt":"2015-07-19T08:40:48Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.07.2015 um 09:37 schrieb Johannes Schindelin:\n> On 2015-07-19 08:54, Johannes Sixt wrote:\n>> Am 18.07.2015 um 17:21 schrieb Ben Walton:\n>>> -\tsed -e s/CHANGE_ME/change_me/ file+ &&\n>>> -\tmv -f file+ file &&\n>>> +\tperl -pi -e \"s/CHANGE_ME/change_me/\" file &&\n>>\n>> This is problematic. On Windows, perl -i fails when no backup file\n>> extension is specified because perl attempts to replace a file that is\n>> still open; that does not work on Windows.\n>\n> Let's qualify this a bit better: it actually works with the SDK of\n> Git for Windows 2.x.\n\nGood to know!\n\n> I really wonder why the previous \">file+ && mv -f file+ file\" dance\n> needs to be replaced?\n\nThe sed must be replaced because some versions on Solaris choke on the \nincomplete last line in the file.\n\n-- Hannes\n"},{"id":"266492","messageId":"xmqqd1zmzuc7.fsf@gitster.dls.corp.google.com","threadId":"39885","inReplyTo":"55AB6290.2090003@kdbg.org","subject":"Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-20T16:07:04Z","receivedAt":"2015-07-20T16:07:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n>> I really wonder why the previous \">file+ && mv -f file+ file\" dance\n>> needs to be replaced?\n>\n> The sed must be replaced because some versions on Solaris choke on the\n> incomplete last line in the file.\n\nSwitching from sed to perl is not being questioned.\n\nI think Dscho is asking about the use of \"-i\", when the original\nidiom \">target+ && mv -f target+ target\" worked perfectly fine.\n\nI.e.\n\n\tperl -p -e '...' file >file+ &&\n mv file+ file\n"},{"id":"266603","messageId":"9b162fbacc41120c8c6edc053a017ba5@www.dscho.org","threadId":"39885","inReplyTo":"xmqqd1zmzuc7.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-07-22T09:52:32Z","receivedAt":"2015-07-22T09:52:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn 2015-07-20 18:07, Junio C Hamano wrote:\n> Johannes Sixt writes:\n> \n>>> I really wonder why the previous \">file+ && mv -f file+ file\" dance\n>>> needs to be replaced?\n>>\n>> The sed must be replaced because some versions on Solaris choke on the\n>> incomplete last line in the file.\n> \n> Switching from sed to perl is not being questioned.\n> \n> I think Dscho is asking about the use of \"-i\", when the original\n> idiom \">target+ && mv -f target+ target\" worked perfectly fine.\n> \n> I.e.\n> \n> \tperl -p -e '...' file >file+ &&\n> mv file+ file\n\nThanks for translating from Dscho to English.\n\nCiao,\nDscho\n"}]}