[PATCH] Avoid broken Solaris tr

[PATCH] Avoid broken Solaris tr

[Ben Walton]

Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) fail to handle the case
where the first argument is a multi-character set and the second is a
single null character. Use perl to perform these substitutions
instead. Now that we're using perl for the transliteration, we might
as well replace the sed invocations with it too.

We make this change globally in t0008-ignores instead of just for the
cases where it matters in order to maintain consistency.

Signed-off-by: Ben Walton <bdwalton@gmail.com>
---
 t/t0008-ignores.sh | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index a56db80..9e4987e 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -552,12 +552,13 @@ cat <<-EOF >expected-verbose
 	$global_excludes:2:!globaltwo	b/globaltwo
 EOF
 
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
-	tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
-	tr "\n" "\0" >expected-default0
-sed -e 's/	"/	/' -e 's/\\//' -e 's/"$//' expected-verbose | \
-	tr ":\t\n" "\0" >expected-verbose0
+perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' stdin >stdin0
+
+perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' expected-default > \
+    expected-default0
+
+perl -pne 's/	"/	/; s/\\//; s/"$//; s/[:\t\n]/\0/g' expected-verbose > \
+    expected-verbose0
 
 test_expect_success '--stdin' '
 	expect_from_stdin <expected-default &&
@@ -638,12 +639,13 @@ EOF
 grep -v '^::	' expected-all >expected-verbose
 sed -e 's/.*	//' expected-verbose >expected-default
 
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
-	tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
-	tr "\n" "\0" >expected-default0
-sed -e 's/	"/	/' -e 's/\\//' -e 's/"$//' expected-verbose | \
-	tr ":\t\n" "\0" >expected-verbose0
+perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' stdin >stdin0
+
+perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' expected-default > \
+    expected-default0
+
+perl -pne 's/	"/	/; s/\\//; s/"$//; s/[:\t\n]/\0/g' expected-verbose > \
+    expected-verbose0
 
 test_expect_success '--stdin from subdirectory' '
 	expect_from_stdin <expected-default &&
-- 
1.8.1.2

Re: [PATCH] Avoid broken Solaris tr

[Junio C Hamano]

Ben Walton <bdwalton@gmail.com> writes:

> Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) fail to handle the case
> where the first argument is a multi-character set and the second is a
> single null character.

Almost all the tr invocations look like converting LF to NUL, except
for two that squash a colon ':', HT and LF all to NUL.  Is Solaris's
tr fine with the former but not the latter?

> We make this change globally in t0008-ignores instead of just for the
> cases where it matters in order to maintain consistency.

I am not suggesting to keep 'tr "\n" "\0"', but just wanted to make
sure I am reading the first paragraph correctly.  If we are
rewriting, we should do so consistently.

> +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' stdin >stdin0

What is -pne?  Is it the same as -pe?

tr/\n/\0/ (or y/\n/\0/) may be more faithful to the original.


> +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' expected-default > \
> +    expected-default0

Ditto.  We may want to give the same script used in the above two
(and twice again in the later hunk) more descriptive name, e.g.

	broken_c_unquote () {
		perl -pe '... that script ...' "$@"
	}

	broken_c_quote stdin >stdin0

Side note: the script is broken as a generic C-unquote function in
multiple ways.  It does not work if it has more than one backslash
quoted characters, it does not understand \t, \b, \015, \\, etc. to
name two.

But the breakage does not matter for the strings used in the test
vector.

Re: [PATCH] Avoid broken Solaris tr

[Ben Walton]

On Tue, Jun 18, 2013 at 11:31 PM, Junio C Hamano <gitster@pobox.com> wrote:

Sorry for the very slow reply. This got lost in my inbox and I forgot about it.

> Ben Walton <bdwalton@gmail.com> writes:
>
>> Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) fail to handle the case
>> where the first argument is a multi-character set and the second is a
>> single null character.
>
> Almost all the tr invocations look like converting LF to NUL, except
> for two that squash a colon ':', HT and LF all to NUL.  Is Solaris's
> tr fine with the former but not the latter?

In retrospect, this isn't brokenness, just a difference in System V vs
BSD semantics for tr, both of which are allowed by POSIX since the
behaviour in question is specifically unspecified by the standard. The
System V behaviour is to require a 1:1 map between string1 and string2
transformations whereas BSD behaviour (when len(string2) <
len(string1)) is to pad string2 with the last character in string2
until the lengths are equal.

>
>> We make this change globally in t0008-ignores instead of just for the
>> cases where it matters in order to maintain consistency.
>
> I am not suggesting to keep 'tr "\n" "\0"', but just wanted to make
> sure I am reading the first paragraph correctly.  If we are
> rewriting, we should do so consistently.
>
>> +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' stdin >stdin0
>
> What is -pne?  Is it the same as -pe?
>
> tr/\n/\0/ (or y/\n/\0/) may be more faithful to the original.
>
>
>> +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' expected-default > \
>> +    expected-default0
>
> Ditto.  We may want to give the same script used in the above two
> (and twice again in the later hunk) more descriptive name, e.g.
>
>         broken_c_unquote () {
>                 perl -pe '... that script ...' "$@"
>         }
>
>         broken_c_quote stdin >stdin0
>
> Side note: the script is broken as a generic C-unquote function in
> multiple ways.  It does not work if it has more than one backslash
> quoted characters, it does not understand \t, \b, \015, \\, etc. to
> name two.
>
> But the breakage does not matter for the strings used in the test
> vector.

I've updated the patch and will forward it shortly.

Thanks
-Ben
--
---------------------------------------------------------------------------------------------------------------------------
Take the risk of thinking for yourself.  Much more happiness,
truth, beauty and wisdom will come to you that way.

-Christopher Hitchens
---------------------------------------------------------------------------------------------------------------------------

[PATCH] Avoid difference in tr semantics between System V and BSD

[Ben Walton]

Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
semantics for tr whereby string1's length is truncated to the length
of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
see string2 padded to the length of string1 using the final character
in string2. POSIX explicitly doesn't specify the correct behavior
here, making both equally valid.

This difference means that Solaris' native tr implementations produce
different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
tests in t0008-ignores.sh.

Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".

Instead, use perl to perform these transliterations which means we
don't need to worry about the difference at all. Since we're replacing
tr with perl, we also use perl to replace the sed invocations used to
transform the files.

Replace four identical transforms with a function named
broken_c_unquote. Replace the other two identical transforms with a
fuction named broken_c_unquote_verbose.

Signed-off-by: Ben Walton <bdwalton@gmail.com>
---
 t/t0008-ignores.sh | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 181513a..4ebda99 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -37,6 +37,14 @@ test_stderr () {
 	test_cmp "$HOME/expected-stderr" "$HOME/stderr"
 }
 
+broken_c_unquote () {
+	perl -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+}
+
+broken_c_unquote_verbose () {
+	perl -pe 's/	"/	/; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+}
+
 stderr_contains () {
 	regexp="$1"
 	if grep "$regexp" "$HOME/stderr"
@@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
 	$global_excludes:2:!globaltwo	b/globaltwo
 EOF
 
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
-	tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
-	tr "\n" "\0" >expected-default0
-sed -e 's/	"/	/' -e 's/\\//' -e 's/"$//' expected-verbose | \
-	tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
 
 test_expect_success '--stdin' '
 	expect_from_stdin <expected-default &&
@@ -692,12 +699,11 @@ EOF
 grep -v '^::	' expected-all >expected-verbose
 sed -e 's/.*	//' expected-verbose >expected-default
 
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
-	tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
-	tr "\n" "\0" >expected-default0
-sed -e 's/	"/	/' -e 's/\\//' -e 's/"$//' expected-verbose | \
-	tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
 
 test_expect_success '--stdin from subdirectory' '
 	expect_from_stdin <expected-default &&
-- 
1.8.1.2

Re: [PATCH] Avoid difference in tr semantics between System V and BSD

[Johannes Sixt]

Am 28.10.2013 10:13, schrieb Ben Walton:
> Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
> semantics for tr whereby string1's length is truncated to the length
> of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
> see string2 padded to the length of string1 using the final character
> in string2. POSIX explicitly doesn't specify the correct behavior
> here, making both equally valid.
> 
> This difference means that Solaris' native tr implementations produce
> different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
> tests in t0008-ignores.sh.
> 
> Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
> 
> Instead, use perl to perform these transliterations which means we
> don't need to worry about the difference at all. Since we're replacing
> tr with perl, we also use perl to replace the sed invocations used to
> transform the files.

In other tests, we check for prerequisite PERL, i.e., we are prepared
that perl is not available. Shouldn't we do that here, too?

But OTOH, I think that we should skip as few test cases as possible in
such a basic test as t0008.

Just some food for thought...
-- Hannes

Re: [PATCH] Avoid difference in tr semantics between System V and BSD

[Jonathan Nieder]

Johannes Sixt wrote:

> In other tests, we check for prerequisite PERL, i.e., we are prepared
> that perl is not available. Shouldn't we do that here, too?

I think the tests assume there's a perl present even when the PERL
prereq isn't present already.  E.g.:

	nul_to_q () {
		"$PERL_PATH" -pe 'y/\000/Q/'
	}

So in practice the PERL prereq just means "NO_PERL wasn't set", or
in other words, "commands that use perl work".  Maybe something
like the following would help?

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

diff --git i/t/README w/t/README
index 2167125..54cd064 100644
--- i/t/README
+++ w/t/README
@@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in the "Test harness
 library" section above and the "test_have_prereq" function for how to
 use these, and "test_set_prereq" for how to define your own.
 
- - PERL & PYTHON
+ - PYTHON
 
-   Git wasn't compiled with NO_PERL=YesPlease or
-   NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
-   these.
+   Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
+   need Python with this.
+
+ - PERL
+
+   Git wasn't compiled with NO_PERL=YesPlease.
+
+   Even without the PERL prerequisite, tests can assume there is a
+   usable perl interpreter at $PERL_PATH, though it need not be
+   particularly modern.
+
+   Wrap tests for commands implemented in Perl with this.
 
  - POSIXPERM
 

Re: [PATCH] Avoid difference in tr semantics between System V and BSD

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Johannes Sixt wrote:
>
>> In other tests, we check for prerequisite PERL, i.e., we are prepared
>> that perl is not available. Shouldn't we do that here, too?
>
> I think the tests assume there's a perl present even when the PERL
> prereq isn't present already.  E.g.:
>
> 	nul_to_q () {
> 		"$PERL_PATH" -pe 'y/\000/Q/'
> 	}
>
> So in practice the PERL prereq just means "NO_PERL wasn't set", or
> in other words, "commands that use perl work".  Maybe something
> like the following would help?
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>
> diff --git i/t/README w/t/README
> index 2167125..54cd064 100644
> --- i/t/README
> +++ w/t/README
> @@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in the "Test harness
>  library" section above and the "test_have_prereq" function for how to
>  use these, and "test_set_prereq" for how to define your own.
>  
> - - PERL & PYTHON
> + - PYTHON
>  
> -   Git wasn't compiled with NO_PERL=YesPlease or
> -   NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
> -   these.
> +   Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
> +   need Python with this.
> +
> + - PERL
> +
> +   Git wasn't compiled with NO_PERL=YesPlease.
> +
> +   Even without the PERL prerequisite, tests can assume there is a
> +   usable perl interpreter at $PERL_PATH, though it need not be
> +   particularly modern.
> +
> +   Wrap tests for commands implemented in Perl with this.

Sounds like a sensible thing to address, but I first parsed it as

    Wrap (tests for (commands implemented in Perl)) with this.

while I think the readers are expected to parse it as

   Wrap ((tests for commands) implemented in Perl) with this.

[PATCH] t/README: tests can use perl even with NO_PERL

[Jonathan Nieder]

The git build system supports a NO_PERL switch to avoid installing
perl bindings or other features (like "git add --patch") that rely on
perl on runtime, but even with NO_PERL it has not been possible for a
long time to run tests without perl.  Helpers such as

	nul_to_q () {
		"$PERL_PATH" -pe 'y/\000/Q/'
	}

use perl as a better tr or sed and are regularly used in tests without
worrying to add a PERL prerequisite.

Perl is portable enough that it seems fine to keep relying on it for
this kind of thing in tests (and more readable than the alternative of
trying to find POSIXy equivalents).  Update the test documentation to
clarify this.

Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> + - PERL
>> +
>> +   Git wasn't compiled with NO_PERL=YesPlease.
>> +
>> +   Even without the PERL prerequisite, tests can assume there is a
>> +   usable perl interpreter at $PERL_PATH, though it need not be
>> +   particularly modern.
>> +
>> +   Wrap tests for commands implemented in Perl with this.
>
> Sounds like a sensible thing to address, but I first parsed it as
>
>     Wrap (tests for (commands implemented in Perl)) with this.
>
> while I think the readers are expected to parse it as
>
>    Wrap ((tests for commands) implemented in Perl) with this.

I meant the former --- that is, tests for commands like "git add -p"
should be skipped in a NO_PERL build.  Probably clearest to leave out
the third paragraph entirely, like this:

 t/README | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/t/README b/t/README
index 2167125..0a939eb 100644
--- a/t/README
+++ b/t/README
@@ -629,11 +629,18 @@ See the prereq argument to the test_* functions in the "Test harness
 library" section above and the "test_have_prereq" function for how to
 use these, and "test_set_prereq" for how to define your own.
 
- - PERL & PYTHON
+ - PYTHON
 
-   Git wasn't compiled with NO_PERL=YesPlease or
-   NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
-   these.
+   Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
+   need Python with this.
+
+ - PERL
+
+   Git wasn't compiled with NO_PERL=YesPlease.
+
+   Even without the PERL prerequisite, tests can assume there is a
+   usable perl interpreter at $PERL_PATH, though it need not be
+   particularly modern.
 
  - POSIXPERM
 
-- 
1.8.4.1

Re: [PATCH] t/README: tests can use perl even with NO_PERL

[Johannes Sixt]

Am 28.10.2013 20:22, schrieb Jonathan Nieder:
> The git build system supports a NO_PERL switch to avoid installing
> perl bindings or other features (like "git add --patch") that rely on
> perl on runtime, but even with NO_PERL it has not been possible for a
> long time to run tests without perl.  Helpers such as
> 
> 	nul_to_q () {
> 		"$PERL_PATH" -pe 'y/\000/Q/'
> 	}
> 
> use perl as a better tr or sed and are regularly used in tests without
> worrying to add a PERL prerequisite.
> 
> Perl is portable enough that it seems fine to keep relying on it for
> this kind of thing in tests (and more readable than the alternative of
> trying to find POSIXy equivalents).  Update the test documentation to
> clarify this.

Thank you for the clarification!

-- Hannes

Re: [PATCH] t/README: tests can use perl even with NO_PERL

[Jeff King]

On Mon, Oct 28, 2013 at 12:22:16PM -0700, Jonathan Nieder wrote:

> The git build system supports a NO_PERL switch to avoid installing
> perl bindings or other features (like "git add --patch") that rely on
> perl on runtime, but even with NO_PERL it has not been possible for a
> long time to run tests without perl.  Helpers such as
> 
> 	nul_to_q () {
> 		"$PERL_PATH" -pe 'y/\000/Q/'
> 	}
> 
> use perl as a better tr or sed and are regularly used in tests without
> worrying to add a PERL prerequisite.
> 
> Perl is portable enough that it seems fine to keep relying on it for
> this kind of thing in tests (and more readable than the alternative of
> trying to find POSIXy equivalents).  Update the test documentation to
> clarify this.
> 
> Reported-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---

Yeah, I think this accurately the conclusions we've come to informally
during review on the list (for a long time we did not even use
$PERL_PATH for such "vanilla" cases, but some people have a broken perl
in their PATH).

Your patch looks good, and I think Ben's patch does not need a PERL
prerequisite. However, it is supposed to use $PERL_PATH, which it does
not.

Speaking of which, is there any reason to use the ugly "$PERL_PATH"
everywhere, and not simply do:

  perl () {
    "$PERL_PATH" "$@"
  }

in test-lib.sh?

-Peff

Re: [PATCH] t/README: tests can use perl even with NO_PERL

[Jonathan Nieder]

Jeff King wrote:

> Speaking of which, is there any reason to use the ugly "$PERL_PATH"
> everywhere, and not simply do:
>
>   perl () {
>     "$PERL_PATH" "$@"
>   }
>
> in test-lib.sh?

Sounds like a nice potential improvement to me. :)

Thanks,
Jonathan

Re: [PATCH] t/README: tests can use perl even with NO_PERL

[Ben Walton]

On Mon, Oct 28, 2013 at 9:04 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Jeff King wrote:
>
>> Speaking of which, is there any reason to use the ugly "$PERL_PATH"
>> everywhere, and not simply do:
>>
>>   perl () {
>>     "$PERL_PATH" "$@"
>>   }
>>
>> in test-lib.sh?
>
> Sounds like a nice potential improvement to me. :)

+1.

-Ben
-- 
---------------------------------------------------------------------------------------------------------------------------
Take the risk of thinking for yourself.  Much more happiness,
truth, beauty and wisdom will come to you that way.

-Christopher Hitchens
---------------------------------------------------------------------------------------------------------------------------

[RFC/PATCH 0/3] perl

[Jeff King]

On Mon, Oct 28, 2013 at 02:04:20PM -0700, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > Speaking of which, is there any reason to use the ugly "$PERL_PATH"
> > everywhere, and not simply do:
> >
> >   perl () {
> >     "$PERL_PATH" "$@"
> >   }
> >
> > in test-lib.sh?
> 
> Sounds like a nice potential improvement to me. :)

One answer to "is there any reason..." is "it will loop infinitely if
you set PERL_PATH=perl". :) However, we can work around that with
"command".

It also may cause problems due to the way one-shot variables are treated
when calling a function versus a command, but we do not seem to set any
variables for invocations perl (and I do not envision it happening
often).

And finally, the other reason I can think of is that we can't apply it
consistently. It only helps where a shell function would activate, which
makes the end result potentially more confusing (especially to somebody
who does not really grok shells and subprocesses). Still, it does not
introduce any _new_ cases that need it, but only helps with a subset of
the cases. So in that sense it is a strict improvement, as we can let
most uses go, but catch only the trickier cases in review.

So I'm on the fence on whether it is a good idea or not, but I wrote up
the patches to play with it. I also noticed that we do not consistently
use $PERL_PATH in some of the built scripts, so I included that fix,
too.

Note that I do not have a system with a broken perl. I simulated a very
broken perl, which is how I found all of the spots to fix. But whether
they are actual bugs that would trigger due to a Windows perl that
handles CRLF differently, I have no clue.

  [1/3]: use @@PERL@@ in built scripts
  [2/3]: t: provide a perl() function which uses $PERL_PATH
  [3/3]: t: use perl instead of "$PERL_PATH" where applicable

-Peff

[PATCH 1/3] use @@PERL@@ in built scripts

[Jeff King]

Several of the built shell commands invoke a bare "perl" to
perform some one-liners. This will use the first perl in the
PATH rather than the one specified by the user's SHELL_PATH.
We are not asking these perl invocations to do anything
exotic, so typically any old system perl will do; however,
in some cases the system perl may have unexpected behavior
(e.g., by handling line endings differently). We should err
on the side of using the perl the user pointed us to.

The downside of this is that on systems with a sane perl
setup, we no longer find the perl at runtime, but instead
point to a static perl (like /usr/bin/perl). That means we
will not handle somebody moving perl without rebuilding git,
whereas before we tracked it just fine. This is probably not
a big deal, though, as the built perl scripts already
suffered from this.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-am.sh           | 4 ++--
 git-instaweb.sh     | 2 +-
 git-request-pull.sh | 2 +-
 git-submodule.sh    | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 7ea40fe..bbea430 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -302,7 +302,7 @@ split_patches () {
 			# not starting with Author, From or Date is the
 			# subject, and the body starts with the next nonempty
 			# line not starting with Author, From or Date
-			perl -ne 'BEGIN { $subject = 0 }
+			@@PERL@@ -ne 'BEGIN { $subject = 0 }
 				if ($subject > 1) { print ; }
 				elsif (/^\s+$/) { next ; }
 				elsif (/^Author:/) { s/Author/From/ ; print ;}
@@ -334,7 +334,7 @@ split_patches () {
 			# Since we cannot guarantee that the commit message is in
 			# git-friendly format, we put no Subject: line and just consume
 			# all of the message as the body
-			LANG=C LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+			LANG=C LC_ALL=C @@PERL@@ -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
 				if ($subject) { print ; }
 				elsif (/^\# User /) { s/\# User/From:/ ; print ; }
 				elsif (/^\# Date /) {
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 01a1b05..e93a238 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -581,7 +581,7 @@ EOF
 
 gitweb_conf() {
 	cat > "$fqgitdir/gitweb/gitweb_config.perl" <<EOF
-#!/usr/bin/perl
+#!@@PERL@@
 our \$projectroot = "$(dirname "$fqgitdir")";
 our \$git_temp = "$fqgitdir/gitweb/tmp";
 our \$projects_list = \$projectroot;
diff --git a/git-request-pull.sh b/git-request-pull.sh
index ebf1269..fe21d5d 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -106,7 +106,7 @@ find_matching_ref='
 	}
 '
 
-ref=$(git ls-remote "$url" | perl -e "$find_matching_ref" "$head" "$headrev" "$tag_name")
+ref=$(git ls-remote "$url" | @@PERL@@ -e "$find_matching_ref" "$head" "$headrev" "$tag_name")
 
 url=$(git ls-remote --get-url "$url")
 
diff --git a/git-submodule.sh b/git-submodule.sh
index 896f1c9..20ebf2e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -156,7 +156,7 @@ module_list()
 		git ls-files -z --error-unmatch --stage -- "$@" ||
 		echo "unmatched pathspec exists"
 	) |
-	perl -e '
+	@@PERL@@ -e '
 	my %unmerged = ();
 	my ($null_sha1) = ("0" x 40);
 	my @out = ();
-- 
1.8.4.1.898.g8bf8a41.dirty

Re: [PATCH 1/3] use @@PERL@@ in built scripts

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Several of the built shell commands invoke a bare "perl" to
> perform some one-liners. This will use the first perl in the
> PATH rather than the one specified by the user's SHELL_PATH.
> We are not asking these perl invocations to do anything
> exotic, so typically any old system perl will do; however,
> in some cases the system perl may have unexpected behavior
> (e.g., by handling line endings differently). We should err
> on the side of using the perl the user pointed us to.

Thanks; this makes sense.

[PATCH 2/3] t: provide a perl() function which uses $PERL_PATH

[Jeff King]

Once upon a time, we assumed that calling a bare "perl" in
the test scripts was OK, because we would find the perl from
the user's PATH, and we were only asking that perl to do
basic operations that work even on old versions of perl.

Later, we found that some systems really prefer to use
$PERL_PATH even for these basic cases, because the system
perl misbehaves in some way (e.g., by handling line endings
differently). We then switched "perl" invocations to
"$PERL_PATH" to respect the user's choice.

Having to use "$PERL_PATH" is ugly and cumbersome, though.
Instead, let's provide a perl() shell function that tests
can use, which will transparently do the right thing.

Unfortunately, test writers still have to use $PERL_PATH in
certain situations, so we still need to keep the advice in
the README.

Note that this may fix test failures in t5004, t5503, t6002,
t6003, t6300, t8001, and t8002, depending on your system's
perl setup. All of these can be detected by running:

  ln -s /bin/false bin-wrappers/perl
  make test

which fails before this patch, and passes after.

Signed-off-by: Jeff King <peff@peff.net>
---
We could always "pollute" bin-wrappers with a broken perl to catch
errors like these, but I think that would also pollute people who put
bin-wrappers into their $PATH. I think we'd need a separate
"test-wrappers" directory, and then put both it and bin-wrappers into
the PATH. I don't know if that is worth it or not.

 t/README                | 12 ++++++++----
 t/test-lib-functions.sh |  4 ++++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/t/README b/t/README
index 2167125..06bc5ed 100644
--- a/t/README
+++ b/t/README
@@ -340,7 +340,11 @@ Don't:
  - use perl without spelling it as "$PERL_PATH". This is to help our
    friends on Windows where the platform Perl often adds CR before
    the end of line, and they bundle Git with a version of Perl that
-   does not do so, whose path is specified with $PERL_PATH.
+   does not do so, whose path is specified with $PERL_PATH. Note that we
+   provide a "perl" function which uses $PERL_PATH under the hood, so
+   you do not need to worry when simply running perl in the test scripts
+   (but you do, for example, on a shebang line or in a sub script
+   created via "write_script").
 
  - use sh without spelling it as "$SHELL_PATH", when the script can
    be misinterpreted by broken platform shell (e.g. Solaris).
@@ -387,7 +391,7 @@ of the test_* functions (see the "Test harness library" section
 below), e.g.:
 
     test_expect_success PERL 'I need Perl' '
-        "$PERL_PATH" -e "hlagh() if unf_unf()"
+        perl -e "hlagh() if unf_unf()"
     '
 
 The advantage of skipping tests like this is that platforms that don't
@@ -520,7 +524,7 @@ library for your script to use.
 
 	test_external \
 	    'GitwebCache::*FileCache*' \
-	    "$PERL_PATH" "$TEST_DIRECTORY"/t9503/test_cache_interface.pl
+	    perl "$TEST_DIRECTORY"/t9503/test_cache_interface.pl
 
    If the test is outputting its own TAP you should set the
    test_external_has_tap variable somewhere before calling the first
@@ -536,7 +540,7 @@ library for your script to use.
 
 	test_external_without_stderr \
 	    'Perl API' \
-	    "$PERL_PATH" "$TEST_DIRECTORY"/t9700/test.pl
+	    perl "$TEST_DIRECTORY"/t9700/test.pl
 
  - test_expect_code <exit-code> <command>
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index a7e9aac..53af452 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -710,3 +710,7 @@ test_ln_s_add () {
 		git update-index --add --cacheinfo 120000 $ln_s_obj "$2"
 	fi
 }
+
+perl () {
+	command "$PERL_PATH" "$@"
+}
-- 
1.8.4.1.898.g8bf8a41.dirty

[PATCH 3/3] t: use perl instead of "$PERL_PATH" where applicable

[Jeff King]

As of the last commit, we can use "perl" instead of
"$PERL_PATH" when running tests, as the former is now a
function which uses the latter. As the shorter "perl" is
easier on the eyes, let's switch to using it everywhere.

This is not quite a mechanical s/$PERL_PATH/perl/
replacement, though. There are some places where we invoke
perl from a script we generate on the fly, and those scripts
do not have access to our internal shell functions. The
result can be double-checked by running:

  ln -s /bin/false bin-wrappers/perl
  make test

which continues to pass even after this patch.

Signed-off-by: Jeff King <peff@peff.net>
---
This one is noisy and optional on top of 2/3. We could also just fix up
cases as we see them going forward.

 t/gitweb-lib.sh                           | 2 +-
 t/lib-git-svn.sh                          | 4 ++--
 t/lib-terminal.sh                         | 4 ++--
 t/t0202-gettext-perl.sh                   | 4 ++--
 t/t1010-mktree.sh                         | 4 ++--
 t/t3300-funny-names.sh                    | 6 +++---
 t/t4014-format-patch.sh                   | 2 +-
 t/t4020-diff-external.sh                  | 2 +-
 t/t4029-diff-trailing-space.sh            | 2 +-
 t/t4103-apply-binary.sh                   | 4 ++--
 t/t4116-apply-reverse.sh                  | 4 ++--
 t/t4200-rerere.sh                         | 8 ++++----
 t/t5300-pack-object.sh                    | 8 ++++----
 t/t5303-pack-corruption-resilience.sh     | 4 ++--
 t/t5551-http-fetch.sh                     | 2 +-
 t/t6011-rev-list-with-bad-commit.sh       | 2 +-
 t/t6013-rev-list-reverse-parents.sh       | 4 ++--
 t/t7508-status.sh                         | 2 +-
 t/t9129-git-svn-i18n-commitencoding.sh    | 2 +-
 t/t9137-git-svn-dcommit-clobber-series.sh | 8 ++++----
 t/t9300-fast-import.sh                    | 2 +-
 t/t9350-fast-export.sh                    | 2 +-
 t/t9400-git-cvsserver-server.sh           | 2 +-
 t/t9401-git-cvsserver-crlf.sh             | 2 +-
 t/t9402-git-cvsserver-refs.sh             | 2 +-
 t/t9700-perl-git.sh                       | 4 ++--
 t/t9810-git-p4-rcs.sh                     | 2 +-
 t/test-lib-functions.sh                   | 6 +++---
 28 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 9e381e0..8cf909a 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -69,7 +69,7 @@ gitweb_run () {
 	# written to web server logs, so we are not interested in that:
 	# we are interested only in properly formatted errors/warnings
 	rm -f gitweb.log &&
-	"$PERL_PATH" -- "$SCRIPT_NAME" \
+	perl -- "$SCRIPT_NAME" \
 		>gitweb.output 2>gitweb.log &&
 	perl -w -e '
 		open O, ">gitweb.headers";
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index c5e55b1..b0ec12f 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -29,7 +29,7 @@ export svnrepo
 svnconf=$PWD/svnconf
 export svnconf
 
-"$PERL_PATH" -w -e "
+perl -w -e "
 use SVN::Core;
 use SVN::Repos;
 \$SVN::Core::VERSION gt '1.1.0' or exit(42);
@@ -146,7 +146,7 @@ stop_httpd () {
 }
 
 convert_to_rev_db () {
-	"$PERL_PATH" -w -- - "$@" <<\EOF
+	perl -w -- - "$@" <<\EOF
 use strict;
 @ARGV == 2 or die "usage: convert_to_rev_db <input> <output>";
 open my $wr, '+>', $ARGV[1] or die "$!: couldn't open: $ARGV[1]";
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 58d911d..737df28 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -19,7 +19,7 @@ test_expect_success PERL 'set up terminal for tests' '
 	then
 		:
 	elif
-		"$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl \
+		perl "$TEST_DIRECTORY"/test-terminal.perl \
 			sh -c "test -t 1 && test -t 2"
 	then
 		test_set_prereq TTY &&
@@ -29,7 +29,7 @@ test_expect_success PERL 'set up terminal for tests' '
 				echo >&4 "test_terminal: need to declare TTY prerequisite"
 				return 127
 			fi
-			"$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl "$@"
+			perl "$TEST_DIRECTORY"/test-terminal.perl "$@"
 		}
 	fi
 '
diff --git a/t/t0202-gettext-perl.sh b/t/t0202-gettext-perl.sh
index 428ebb0..a29d166 100755
--- a/t/t0202-gettext-perl.sh
+++ b/t/t0202-gettext-perl.sh
@@ -12,7 +12,7 @@ if ! test_have_prereq PERL; then
 	test_done
 fi
 
-"$PERL_PATH" -MTest::More -e 0 2>/dev/null || {
+perl -MTest::More -e 0 2>/dev/null || {
 	skip_all="Perl Test::More unavailable, skipping test"
 	test_done
 }
@@ -22,6 +22,6 @@ test_external_has_tap=1
 
 test_external_without_stderr \
     'Perl Git::I18N API' \
-    "$PERL_PATH" "$TEST_DIRECTORY"/t0202/test.pl
+    perl "$TEST_DIRECTORY"/t0202/test.pl
 
 test_done
diff --git a/t/t1010-mktree.sh b/t/t1010-mktree.sh
index df573c4..b946f87 100755
--- a/t/t1010-mktree.sh
+++ b/t/t1010-mktree.sh
@@ -42,13 +42,13 @@ test_expect_success 'ls-tree piped to mktree (2)' '
 '
 
 test_expect_success 'ls-tree output in wrong order given to mktree (1)' '
-	"$PERL_PATH" -e "print reverse <>" <top |
+	perl -e "print reverse <>" <top |
 	git mktree >actual &&
 	test_cmp tree actual
 '
 
 test_expect_success 'ls-tree output in wrong order given to mktree (2)' '
-	"$PERL_PATH" -e "print reverse <>" <top.withsub |
+	perl -e "print reverse <>" <top.withsub |
 	git mktree >actual &&
 	test_cmp tree.withsub actual
 '
diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 7480d6e..9a146f1 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -69,7 +69,7 @@ test_expect_success 'ls-files -z does not quote funny filename' '
 	tabs	," (dq) and spaces
 	EOF
 	git ls-files -z >ls-files.z &&
-	"$PERL_PATH" -pe "y/\000/\012/" <ls-files.z >current &&
+	perl -pe "y/\000/\012/" <ls-files.z >current &&
 	test_cmp expected current
 '
 
@@ -106,7 +106,7 @@ test_expect_success 'diff-index -z does not quote funny filename' '
 	tabs	," (dq) and spaces
 	EOF
 	git diff-index -z --name-status $t0 >diff-index.z &&
-	"$PERL_PATH" -pe "y/\000/\012/" <diff-index.z >current &&
+	perl -pe "y/\000/\012/" <diff-index.z >current &&
 	test_cmp expected current
 '
 
@@ -116,7 +116,7 @@ test_expect_success 'diff-tree -z does not quote funny filename' '
 	tabs	," (dq) and spaces
 	EOF
 	git diff-tree -z --name-status $t0 $t1 >diff-tree.z &&
-	"$PERL_PATH" -pe y/\\000/\\012/ <diff-tree.z >current &&
+	perl -pe y/\\000/\\012/ <diff-tree.z >current &&
 	test_cmp expected current
 '
 
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 8f272bc..73194b2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -293,7 +293,7 @@ check_threading () {
 	(git format-patch --stdout "$@"; echo $? > status.out) |
 	# Prints everything between the Message-ID and In-Reply-To,
 	# and replaces all Message-ID-lookalikes by a sequence number
-	"$PERL_PATH" -ne '
+	perl -ne '
 		if (/^(message-id|references|in-reply-to)/i) {
 			$printing = 1;
 		} elsif (/^\S/) {
diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index 2e7d73f..8a30979 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -177,7 +177,7 @@ test_expect_success 'no diff with -diff' '
 	git diff | grep Binary
 '
 
-echo NULZbetweenZwords | "$PERL_PATH" -pe 'y/Z/\000/' > file
+echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file
 
 test_expect_success 'force diff with "diff"' '
 	echo >.gitattributes "file diff" &&
diff --git a/t/t4029-diff-trailing-space.sh b/t/t4029-diff-trailing-space.sh
index 36e2f07..3ccc237 100755
--- a/t/t4029-diff-trailing-space.sh
+++ b/t/t4029-diff-trailing-space.sh
@@ -27,7 +27,7 @@ test_expect_success \
      git config --bool diff.suppressBlankEmpty true &&
      git diff f > actual &&
      test_cmp exp actual &&
-     "$PERL_PATH" -i.bak -p -e "s/^\$/ /" exp &&
+     perl -i.bak -p -e "s/^\$/ /" exp &&
      git config --bool diff.suppressBlankEmpty false &&
      git diff f > actual &&
      test_cmp exp actual &&
diff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh
index b1b906b..1b420e3 100755
--- a/t/t4103-apply-binary.sh
+++ b/t/t4103-apply-binary.sh
@@ -23,10 +23,10 @@ test_expect_success 'setup' '
 	git commit -m "Initial Version" 2>/dev/null &&
 
 	git checkout -b binary &&
-	"$PERL_PATH" -pe "y/x/\000/" <file1 >file3 &&
+	perl -pe "y/x/\000/" <file1 >file3 &&
 	cat file3 >file4 &&
 	git add file2 &&
-	"$PERL_PATH" -pe "y/\000/v/" <file3 >file1 &&
+	perl -pe "y/\000/v/" <file3 >file1 &&
 	rm -f file2 &&
 	git update-index --add --remove file1 file2 file3 file4 &&
 	git commit -m "Second Version" &&
diff --git a/t/t4116-apply-reverse.sh b/t/t4116-apply-reverse.sh
index fca8153..2298ece 100755
--- a/t/t4116-apply-reverse.sh
+++ b/t/t4116-apply-reverse.sh
@@ -12,14 +12,14 @@ test_description='git apply in reverse
 test_expect_success setup '
 
 	for i in a b c d e f g h i j k l m n; do echo $i; done >file1 &&
-	"$PERL_PATH" -pe "y/ijk/\\000\\001\\002/" <file1 >file2 &&
+	perl -pe "y/ijk/\\000\\001\\002/" <file1 >file2 &&
 
 	git add file1 file2 &&
 	git commit -m initial &&
 	git tag initial &&
 
 	for i in a b c g h i J K L m o n p q; do echo $i; done >file1 &&
-	"$PERL_PATH" -pe "y/mon/\\000\\001\\002/" <file1 >file2 &&
+	perl -pe "y/mon/\\000\\001\\002/" <file1 >file2 &&
 
 	git commit -a -m second &&
 	git tag second &&
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 7f6666f..076e770 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -78,7 +78,7 @@ test_expect_success 'activate rerere, old style (conflicting merge)' '
 	test_might_fail git config --unset rerere.enabled &&
 	test_must_fail git merge first &&
 
-	sha1=$("$PERL_PATH" -pe "s/	.*//" .git/MERGE_RR) &&
+	sha1=$(perl -pe "s/	.*//" .git/MERGE_RR) &&
 	rr=.git/rr-cache/$sha1 &&
 	grep "^=======\$" $rr/preimage &&
 	! test -f $rr/postimage &&
@@ -91,7 +91,7 @@ test_expect_success 'rerere.enabled works, too' '
 	git reset --hard &&
 	test_must_fail git merge first &&
 
-	sha1=$("$PERL_PATH" -pe "s/	.*//" .git/MERGE_RR) &&
+	sha1=$(perl -pe "s/	.*//" .git/MERGE_RR) &&
 	rr=.git/rr-cache/$sha1 &&
 	grep ^=======$ $rr/preimage
 '
@@ -101,7 +101,7 @@ test_expect_success 'set up rr-cache' '
 	git config rerere.enabled true &&
 	git reset --hard &&
 	test_must_fail git merge first &&
-	sha1=$("$PERL_PATH" -pe "s/	.*//" .git/MERGE_RR) &&
+	sha1=$(perl -pe "s/	.*//" .git/MERGE_RR) &&
 	rr=.git/rr-cache/$sha1
 '
 
@@ -185,7 +185,7 @@ test_expect_success 'rerere updates postimage timestamp' '
 
 test_expect_success 'rerere clear' '
 	rm $rr/postimage &&
-	echo "$sha1	a1" | "$PERL_PATH" -pe "y/\012/\000/" >.git/MERGE_RR &&
+	echo "$sha1	a1" | perl -pe "y/\012/\000/" >.git/MERGE_RR &&
 	git rerere clear &&
 	! test -d $rr
 '
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index a07c871..c755c09 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -13,9 +13,9 @@ TRASH=`pwd`
 test_expect_success \
     'setup' \
     'rm -f .git/index* &&
-     "$PERL_PATH" -e "print \"a\" x 4096;" > a &&
-     "$PERL_PATH" -e "print \"b\" x 4096;" > b &&
-     "$PERL_PATH" -e "print \"c\" x 4096;" > c &&
+     perl -e "print \"a\" x 4096;" > a &&
+     perl -e "print \"b\" x 4096;" > b &&
+     perl -e "print \"c\" x 4096;" > c &&
      test-genrandom "seed a" 2097152 > a_big &&
      test-genrandom "seed b" 2097152 > b_big &&
      git update-index --add a a_big b b_big c &&
@@ -129,7 +129,7 @@ test_expect_success \
 cd "$TRASH"
 
 test_expect_success 'compare delta flavors' '
-	"$PERL_PATH" -e '\''
+	perl -e '\''
 		defined($_ = -s $_) or die for @ARGV;
 		exit 1 if $ARGV[0] <= $ARGV[1];
 	'\'' test-2-$packname_2.pack test-3-$packname_3.pack
diff --git a/t/t5303-pack-corruption-resilience.sh b/t/t5303-pack-corruption-resilience.sh
index 35926de..663b02b 100755
--- a/t/t5303-pack-corruption-resilience.sh
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -98,7 +98,7 @@ test_expect_success \
     'create_new_pack &&
      git prune-packed &&
      chmod +w ${pack}.pack &&
-     "$PERL_PATH" -i.bak -pe "s/ base /abcdef/" ${pack}.pack &&
+     perl -i.bak -pe "s/ base /abcdef/" ${pack}.pack &&
      test_must_fail git cat-file blob $blob_1 > /dev/null &&
      test_must_fail git cat-file blob $blob_2 > /dev/null &&
      test_must_fail git cat-file blob $blob_3 > /dev/null'
@@ -155,7 +155,7 @@ test_expect_success \
     'create_new_pack &&
      git prune-packed &&
      chmod +w ${pack}.pack &&
-     "$PERL_PATH" -i.bak -pe "s/ delta1 /abcdefgh/" ${pack}.pack &&
+     perl -i.bak -pe "s/ delta1 /abcdefgh/" ${pack}.pack &&
      git cat-file blob $blob_1 > /dev/null &&
      test_must_fail git cat-file blob $blob_2 > /dev/null &&
      test_must_fail git cat-file blob $blob_3 > /dev/null'
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index 8196af1..f036392 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -224,7 +224,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
 	done | git fast-import --export-marks=marks &&
 
 	# now assign tags to all the dangling commits we created above
-	tag=$("$PERL_PATH" -e "print \"bla\" x 30") &&
+	tag=$(perl -e "print \"bla\" x 30") &&
 	sed -e "s|^:\([^ ]*\) \(.*\)$|\2 refs/tags/$tag-\1|" <marks >>packed-refs
 	)
 '
diff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh
index bbb0581..e51eb41 100755
--- a/t/t6011-rev-list-with-bad-commit.sh
+++ b/t/t6011-rev-list-with-bad-commit.sh
@@ -37,7 +37,7 @@ test_expect_success 'verify number of revisions' \
 
 test_expect_success 'corrupt second commit object' \
    '
-   "$PERL_PATH" -i.bak -pe "s/second commit/socond commit/" .git/objects/pack/*.pack &&
+   perl -i.bak -pe "s/second commit/socond commit/" .git/objects/pack/*.pack &&
    test_must_fail git fsck --full
    '
 
diff --git a/t/t6013-rev-list-reverse-parents.sh b/t/t6013-rev-list-reverse-parents.sh
index 892a537..59fc2f0 100755
--- a/t/t6013-rev-list-reverse-parents.sh
+++ b/t/t6013-rev-list-reverse-parents.sh
@@ -25,7 +25,7 @@ test_expect_success 'set up --reverse example' '
 
 test_expect_success '--reverse --parents --full-history combines correctly' '
 	git rev-list --parents --full-history master -- foo |
-		"$PERL_PATH" -e "print reverse <>" > expected &&
+		perl -e "print reverse <>" > expected &&
 	git rev-list --reverse --parents --full-history master -- foo \
 		> actual &&
 	test_cmp actual expected
@@ -33,7 +33,7 @@ test_expect_success '--reverse --parents --full-history combines correctly' '
 
 test_expect_success '--boundary does too' '
 	git rev-list --boundary --parents --full-history master ^root -- foo |
-		"$PERL_PATH" -e "print reverse <>" > expected &&
+		perl -e "print reverse <>" > expected &&
 	git rev-list --boundary --reverse --parents --full-history \
 		master ^root -- foo > actual &&
 	test_cmp actual expected
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 6fb59f3..c987b5e 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -994,7 +994,7 @@ test_expect_success 'status -s submodule summary (clean submodule)' '
 
 test_expect_success 'status -z implies porcelain' '
 	git status --porcelain |
-	"$PERL_PATH" -pe "s/\012/\000/g" >expect &&
+	perl -pe "s/\012/\000/g" >expect &&
 	git status -z >output &&
 	test_cmp expect output
 '
diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh
index 9a40f1e..8cfdfe7 100755
--- a/t/t9129-git-svn-i18n-commitencoding.sh
+++ b/t/t9129-git-svn-i18n-commitencoding.sh
@@ -29,7 +29,7 @@ fi
 compare_svn_head_with () {
 	# extract just the log message and strip out committer info.
 	# don't use --limit here since svn 1.1.x doesn't have it,
-	LC_ALL="$a_utf8_locale" svn log `git svn info --url` | "$PERL_PATH" -w -e '
+	LC_ALL="$a_utf8_locale" svn log `git svn info --url` | perl -w -e '
 		use bytes;
 		$/ = ("-"x72) . "\n";
 		my @x = <STDIN>;
diff --git a/t/t9137-git-svn-dcommit-clobber-series.sh b/t/t9137-git-svn-dcommit-clobber-series.sh
index c17aa31..d60da63 100755
--- a/t/t9137-git-svn-dcommit-clobber-series.sh
+++ b/t/t9137-git-svn-dcommit-clobber-series.sh
@@ -20,8 +20,8 @@ test_expect_success '(supposedly) non-conflicting change from SVN' '
 	test x"`sed -n -e 61p < file`" = x61 &&
 	svn_cmd co "$svnrepo" tmp &&
 	(cd tmp &&
-		"$PERL_PATH" -i.bak -p -e "s/^58$/5588/" file &&
-		"$PERL_PATH" -i.bak -p -e "s/^61$/6611/" file &&
+		perl -i.bak -p -e "s/^58$/5588/" file &&
+		perl -i.bak -p -e "s/^61$/6611/" file &&
 		poke file &&
 		test x"`sed -n -e 58p < file`" = x5588 &&
 		test x"`sed -n -e 61p < file`" = x6611 &&
@@ -40,8 +40,8 @@ test_expect_success 'some unrelated changes to git' "
 test_expect_success 'change file but in unrelated area' "
 	test x\"\`sed -n -e 4p < file\`\" = x4 &&
 	test x\"\`sed -n -e 7p < file\`\" = x7 &&
-	"$PERL_PATH" -i.bak -p -e 's/^4\$/4444/' file &&
-	"$PERL_PATH" -i.bak -p -e 's/^7\$/7777/' file &&
+	perl -i.bak -p -e 's/^4\$/4444/' file &&
+	perl -i.bak -p -e 's/^7\$/7777/' file &&
 	test x\"\`sed -n -e 4p < file\`\" = x4444 &&
 	test x\"\`sed -n -e 7p < file\`\" = x7777 &&
 	git commit -m '4 => 4444, 7 => 7777' file &&
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 88fc407..27263df 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -12,7 +12,7 @@ test_description='test git fast-import utility'
 # This could be written as "head -c $1", but IRIX "head" does not
 # support the -c option.
 head_c () {
-	"$PERL_PATH" -e '
+	perl -e '
 		my $len = $ARGV[1];
 		while ($len > 0) {
 			my $s;
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 34c2d8f..2312dec 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -429,7 +429,7 @@ test_expect_success 'fast-export quotes pathnames' '
 		--cacheinfo 100644 $blob "path with \\backslash" \
 		--cacheinfo 100644 $blob "path with space" &&
 	 git commit -m addition &&
-	 git ls-files -z -s | "$PERL_PATH" -0pe "s{\\t}{$&subdir/}" >index &&
+	 git ls-files -z -s | perl -0pe "s{\\t}{$&subdir/}" >index &&
 	 git read-tree --empty &&
 	 git update-index -z --index-info <index &&
 	 git commit -m rename &&
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 0431386..3edc408 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -20,7 +20,7 @@ then
     skip_all='skipping git-cvsserver tests, cvs not found'
     test_done
 fi
-"$PERL_PATH" -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     skip_all='skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index 8c3db76..5a4ed28 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -68,7 +68,7 @@ then
     skip_all='skipping git-cvsserver tests, perl not available'
     test_done
 fi
-"$PERL_PATH" -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     skip_all='skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
 }
diff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh
index db69af2..1e266ef 100755
--- a/t/t9402-git-cvsserver-refs.sh
+++ b/t/t9402-git-cvsserver-refs.sh
@@ -76,7 +76,7 @@ then
 	skip_all='skipping git-cvsserver tests, perl not available'
 	test_done
 fi
-"$PERL_PATH" -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
+perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
 	skip_all='skipping git-cvsserver tests, Perl SQLite interface unavailable'
 	test_done
 }
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 435d896..102c133 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -11,7 +11,7 @@ if ! test_have_prereq PERL; then
 	test_done
 fi
 
-"$PERL_PATH" -MTest::More -e 0 2>/dev/null || {
+perl -MTest::More -e 0 2>/dev/null || {
 	skip_all="Perl Test::More unavailable, skipping test"
 	test_done
 }
@@ -55,6 +55,6 @@ test_external_has_tap=1
 
 test_external_without_stderr \
     'Perl API' \
-    "$PERL_PATH" "$TEST_DIRECTORY"/t9700/test.pl
+    perl "$TEST_DIRECTORY"/t9700/test.pl
 
 test_done
diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
index 34fbc90..8134ab4 100755
--- a/t/t9810-git-p4-rcs.sh
+++ b/t/t9810-git-p4-rcs.sh
@@ -263,7 +263,7 @@ test_expect_success 'cope with rcs keyword expansion damage' '
 		git config git-p4.attemptRCSCleanup true &&
 		(cd "$cli" && p4_append_to_file kwfile1.c) &&
 		old_lines=$(wc -l <kwfile1.c) &&
-		"$PERL_PATH" -n -i -e "print unless m/Revision:/" kwfile1.c &&
+		perl -n -i -e "print unless m/Revision:/" kwfile1.c &&
 		new_lines=$(wc -l <kwfile1.c) &&
 		test $new_lines = $(($old_lines - 1)) &&
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 53af452..bb878f3 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -76,11 +76,11 @@ test_decode_color () {
 }
 
 nul_to_q () {
-	"$PERL_PATH" -pe 'y/\000/Q/'
+	perl -pe 'y/\000/Q/'
 }
 
 q_to_nul () {
-	"$PERL_PATH" -pe 'y/Q/\000/'
+	perl -pe 'y/Q/\000/'
 }
 
 q_to_cr () {
@@ -648,7 +648,7 @@ test_seq () {
 	2)	;;
 	*)	error "bug in the test script: not 1 or 2 parameters to test_seq" ;;
 	esac
-	"$PERL_PATH" -le 'print for $ARGV[0]..$ARGV[1]' -- "$@"
+	perl -le 'print for $ARGV[0]..$ARGV[1]' -- "$@"
 }
 
 # This function can be used to schedule some commands to be run
-- 
1.8.4.1.898.g8bf8a41.dirty

Re: [PATCH] Avoid difference in tr semantics between System V and BSD

[Ben Walton]

I'm happy to defer to your judgement on this - If you'd like the tests
wrapped, I'll do so.

Thanks
-Ben

On Mon, Oct 28, 2013 at 7:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Johannes Sixt wrote:
>>
>>> In other tests, we check for prerequisite PERL, i.e., we are prepared
>>> that perl is not available. Shouldn't we do that here, too?
>>
>> I think the tests assume there's a perl present even when the PERL
>> prereq isn't present already.  E.g.:
>>
>>       nul_to_q () {
>>               "$PERL_PATH" -pe 'y/\000/Q/'
>>       }
>>
>> So in practice the PERL prereq just means "NO_PERL wasn't set", or
>> in other words, "commands that use perl work".  Maybe something
>> like the following would help?
>>
>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>>
>> diff --git i/t/README w/t/README
>> index 2167125..54cd064 100644
>> --- i/t/README
>> +++ w/t/README
>> @@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in the "Test harness
>>  library" section above and the "test_have_prereq" function for how to
>>  use these, and "test_set_prereq" for how to define your own.
>>
>> - - PERL & PYTHON
>> + - PYTHON
>>
>> -   Git wasn't compiled with NO_PERL=YesPlease or
>> -   NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
>> -   these.
>> +   Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
>> +   need Python with this.
>> +
>> + - PERL
>> +
>> +   Git wasn't compiled with NO_PERL=YesPlease.
>> +
>> +   Even without the PERL prerequisite, tests can assume there is a
>> +   usable perl interpreter at $PERL_PATH, though it need not be
>> +   particularly modern.
>> +
>> +   Wrap tests for commands implemented in Perl with this.
>
> Sounds like a sensible thing to address, but I first parsed it as
>
>     Wrap (tests for (commands implemented in Perl)) with this.
>
> while I think the readers are expected to parse it as
>
>    Wrap ((tests for commands) implemented in Perl) with this.
>



-- 
---------------------------------------------------------------------------------------------------------------------------
Take the risk of thinking for yourself.  Much more happiness,
truth, beauty and wisdom will come to you that way.

-Christopher Hitchens
---------------------------------------------------------------------------------------------------------------------------

Re: [PATCH] Avoid difference in tr semantics between System V and BSD

[Ben Walton]

On Mon, Oct 28, 2013 at 9:04 PM, Ben Walton <bdwalton@gmail.com> wrote:
> I'm happy to defer to your judgement on this - If you'd like the tests
> wrapped, I'll do so.
>
> Thanks
> -Ben
>
> On Mon, Oct 28, 2013 at 7:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>
>>> Johannes Sixt wrote:
>>>
>>>> In other tests, we check for prerequisite PERL, i.e., we are prepared
>>>> that perl is not available. Shouldn't we do that here, too?
>>>
>>> I think the tests assume there's a perl present even when the PERL
>>> prereq isn't present already.  E.g.:
>>>
>>>       nul_to_q () {
>>>               "$PERL_PATH" -pe 'y/\000/Q/'
>>>       }
>>>
>>> So in practice the PERL prereq just means "NO_PERL wasn't set", or
>>> in other words, "commands that use perl work".  Maybe something
>>> like the following would help?
>>>
>>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>>>
>>> diff --git i/t/README w/t/README
>>> index 2167125..54cd064 100644
>>> --- i/t/README
>>> +++ w/t/README
>>> @@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in the "Test harness
>>>  library" section above and the "test_have_prereq" function for how to
>>>  use these, and "test_set_prereq" for how to define your own.
>>>
>>> - - PERL & PYTHON
>>> + - PYTHON
>>>
>>> -   Git wasn't compiled with NO_PERL=YesPlease or
>>> -   NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
>>> -   these.
>>> +   Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
>>> +   need Python with this.
>>> +
>>> + - PERL
>>> +
>>> +   Git wasn't compiled with NO_PERL=YesPlease.
>>> +
>>> +   Even without the PERL prerequisite, tests can assume there is a
>>> +   usable perl interpreter at $PERL_PATH, though it need not be
>>> +   particularly modern.
>>> +
>>> +   Wrap tests for commands implemented in Perl with this.
>>
>> Sounds like a sensible thing to address, but I first parsed it as
>>
>>     Wrap (tests for (commands implemented in Perl)) with this.
>>
>> while I think the readers are expected to parse it as
>>
>>    Wrap ((tests for commands) implemented in Perl) with this.
>>

Per the other discussion about replacing all PERL_PATH with a shell
function named perl, should I update this patch to use $PERL_PATH in
the meantime so that it can be batch updated when the function is
added in a separate patch?

Thanks
-Ben
-- 
---------------------------------------------------------------------------------------------------------------------------
Take the risk of thinking for yourself.  Much more happiness,
truth, beauty and wisdom will come to you that way.

-Christopher Hitchens
---------------------------------------------------------------------------------------------------------------------------

Re: [PATCH] Avoid difference in tr semantics between System V and BSD

[Junio C Hamano]

Ben Walton <bdwalton@gmail.com> writes:

> Per the other discussion about replacing all PERL_PATH with a shell
> function named perl, should I update this patch to use $PERL_PATH in
> the meantime so that it can be batch updated when the function is
> added in a separate patch?

Yeah, sounds like a good plan, and very much appreciated.

Thanks.

[PATCH] Avoid difference in tr semantics between System V and BSD

[Ben Walton]

Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
semantics for tr whereby string1's length is truncated to the length
of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
see string2 padded to the length of string1 using the final character
in string2. POSIX explicitly doesn't specify the correct behavior
here, making both equally valid.

This difference means that Solaris' native tr implementations produce
different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
tests in t0008-ignores.sh.

Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".

Instead, use perl to perform these transliterations which means we
don't need to worry about the difference at all. Since we're replacing
tr with perl, we also use perl to replace the sed invocations used to
transform the files.

Replace four identical transforms with a function named
broken_c_unquote. Replace the other two identical transforms with a
fuction named broken_c_unquote_verbose.

Signed-off-by: Ben Walton <bdwalton@gmail.com>
---
 t/t0008-ignores.sh | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 181513a..45f9396 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -37,6 +37,14 @@ test_stderr () {
 	test_cmp "$HOME/expected-stderr" "$HOME/stderr"
 }
 
+broken_c_unquote () {
+	$PERL_PATH -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+}
+
+broken_c_unquote_verbose () {
+	$PERL_PATH -pe 's/	"/	/; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+}
+
 stderr_contains () {
 	regexp="$1"
 	if grep "$regexp" "$HOME/stderr"
@@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
 	$global_excludes:2:!globaltwo	b/globaltwo
 EOF
 
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
-	tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
-	tr "\n" "\0" >expected-default0
-sed -e 's/	"/	/' -e 's/\\//' -e 's/"$//' expected-verbose | \
-	tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
 
 test_expect_success '--stdin' '
 	expect_from_stdin <expected-default &&
@@ -692,12 +699,11 @@ EOF
 grep -v '^::	' expected-all >expected-verbose
 sed -e 's/.*	//' expected-verbose >expected-default
 
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
-	tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
-	tr "\n" "\0" >expected-default0
-sed -e 's/	"/	/' -e 's/\\//' -e 's/"$//' expected-verbose | \
-	tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
 
 test_expect_success '--stdin from subdirectory' '
 	expect_from_stdin <expected-default &&
-- 
1.8.1.2

Re: [PATCH] Avoid difference in tr semantics between System V and BSD

[Ben Walton]

Ignore this version. The immediate followup quotes PERL_PATH.

On Mon, Oct 28, 2013 at 9:40 PM, Ben Walton <bdwalton@gmail.com> wrote:
> Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
> semantics for tr whereby string1's length is truncated to the length
> of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
> see string2 padded to the length of string1 using the final character
> in string2. POSIX explicitly doesn't specify the correct behavior
> here, making both equally valid.
>
> This difference means that Solaris' native tr implementations produce
> different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
> tests in t0008-ignores.sh.
>
> Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
>
> Instead, use perl to perform these transliterations which means we
> don't need to worry about the difference at all. Since we're replacing
> tr with perl, we also use perl to replace the sed invocations used to
> transform the files.
>
> Replace four identical transforms with a function named
> broken_c_unquote. Replace the other two identical transforms with a
> fuction named broken_c_unquote_verbose.
>
> Signed-off-by: Ben Walton <bdwalton@gmail.com>
> ---
>  t/t0008-ignores.sh | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> index 181513a..45f9396 100755
> --- a/t/t0008-ignores.sh
> +++ b/t/t0008-ignores.sh
> @@ -37,6 +37,14 @@ test_stderr () {
>         test_cmp "$HOME/expected-stderr" "$HOME/stderr"
>  }
>
> +broken_c_unquote () {
> +       $PERL_PATH -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
> +}
> +
> +broken_c_unquote_verbose () {
> +       $PERL_PATH -pe 's/      "/      /; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
> +}
> +
>  stderr_contains () {
>         regexp="$1"
>         if grep "$regexp" "$HOME/stderr"
> @@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
>         $global_excludes:2:!globaltwo   b/globaltwo
>  EOF
>
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
> -       tr "\n" "\0" >stdin0
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
> -       tr "\n" "\0" >expected-default0
> -sed -e 's/     "/      /' -e 's/\\//' -e 's/"$//' expected-verbose | \
> -       tr ":\t\n" "\0" >expected-verbose0
> +broken_c_unquote stdin >stdin0
> +
> +broken_c_unquote expected-default >expected-default0
> +
> +broken_c_unquote_verbose expected-verbose >expected-verbose0
>
>  test_expect_success '--stdin' '
>         expect_from_stdin <expected-default &&
> @@ -692,12 +699,11 @@ EOF
>  grep -v '^::   ' expected-all >expected-verbose
>  sed -e 's/.*   //' expected-verbose >expected-default
>
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
> -       tr "\n" "\0" >stdin0
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
> -       tr "\n" "\0" >expected-default0
> -sed -e 's/     "/      /' -e 's/\\//' -e 's/"$//' expected-verbose | \
> -       tr ":\t\n" "\0" >expected-verbose0
> +broken_c_unquote stdin >stdin0
> +
> +broken_c_unquote expected-default >expected-default0
> +
> +broken_c_unquote_verbose expected-verbose >expected-verbose0
>
>  test_expect_success '--stdin from subdirectory' '
>         expect_from_stdin <expected-default &&
> --
> 1.8.1.2
>



-- 
---------------------------------------------------------------------------------------------------------------------------
Take the risk of thinking for yourself.  Much more happiness,
truth, beauty and wisdom will come to you that way.

-Christopher Hitchens
---------------------------------------------------------------------------------------------------------------------------

[PATCH] Avoid difference in tr semantics between System V and BSD

[Ben Walton]

Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
semantics for tr whereby string1's length is truncated to the length
of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
see string2 padded to the length of string1 using the final character
in string2. POSIX explicitly doesn't specify the correct behavior
here, making both equally valid.

This difference means that Solaris' native tr implementations produce
different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
tests in t0008-ignores.sh.

Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".

Instead, use perl to perform these transliterations which means we
don't need to worry about the difference at all. Since we're replacing
tr with perl, we also use perl to replace the sed invocations used to
transform the files.

Replace four identical transforms with a function named
broken_c_unquote. Replace the other two identical transforms with a
fuction named broken_c_unquote_verbose.

Signed-off-by: Ben Walton <bdwalton@gmail.com>
---

...Forgot to quote PERL_PATH in the previous iteration.

 t/t0008-ignores.sh | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 181513a..b4d98e6 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -37,6 +37,14 @@ test_stderr () {
 	test_cmp "$HOME/expected-stderr" "$HOME/stderr"
 }
 
+broken_c_unquote () {
+	"$PERL_PATH" -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+}
+
+broken_c_unquote_verbose () {
+	"$PERL_PATH" -pe 's/	"/	/; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+}
+
 stderr_contains () {
 	regexp="$1"
 	if grep "$regexp" "$HOME/stderr"
@@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
 	$global_excludes:2:!globaltwo	b/globaltwo
 EOF
 
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
-	tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
-	tr "\n" "\0" >expected-default0
-sed -e 's/	"/	/' -e 's/\\//' -e 's/"$//' expected-verbose | \
-	tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
 
 test_expect_success '--stdin' '
 	expect_from_stdin <expected-default &&
@@ -692,12 +699,11 @@ EOF
 grep -v '^::	' expected-all >expected-verbose
 sed -e 's/.*	//' expected-verbose >expected-default
 
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
-	tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
-	tr "\n" "\0" >expected-default0
-sed -e 's/	"/	/' -e 's/\\//' -e 's/"$//' expected-verbose | \
-	tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
 
 test_expect_success '--stdin from subdirectory' '
 	expect_from_stdin <expected-default &&
-- 
1.8.1.2

Re: [PATCH] Avoid difference in tr semantics between System V and BSD

[Junio C Hamano]

Ben Walton <bdwalton@gmail.com> writes:

> Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
> semantics for tr whereby string1's length is truncated to the length
> of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
> see string2 padded to the length of string1 using the final character
> in string2. POSIX explicitly doesn't specify the correct behavior
> here, making both equally valid.
>
> This difference means that Solaris' native tr implementations produce
> different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
> tests in t0008-ignores.sh.
>
> Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
>
> Instead, use perl to perform these transliterations which means we
> don't need to worry about the difference at all. Since we're replacing
> tr with perl, we also use perl to replace the sed invocations used to
> transform the files.
>
> Replace four identical transforms with a function named
> broken_c_unquote. Replace the other two identical transforms with a
> fuction named broken_c_unquote_verbose.
>
> Signed-off-by: Ben Walton <bdwalton@gmail.com>
> ---
>
> ...Forgot to quote PERL_PATH in the previous iteration.

Thanks; will requeue.