t9700-perl-git.sh is broken on some configurations

> Indeed on the command line I get:
> 
> ------------------------
> $ t/t9700/test.pl
> ok 2 - use Git;
> Bareword found where operator expected at t/t9700/test.pl line 36,
> near "s/\\/\//gr"
> syntax error at t/t9700/test.pl line 36, near "s/\\/\//gr"
> Execution of t/t9700/test.pl aborted due to compilation errors.
> ------------------------
> 
> A quick look at t/t9700/test.pl line 36 was not enough for me to spot
> the problem.
> 
> Perl version is: perl 5, version 18, subversion 2 (v5.18.2) built for
> x86_64-linux
> 
> The machine is running CentOS 6.5.

> On Fri, Mar 04, 2016 at 09:13:51AM +0100, Christian Couder wrote:
>
>> Indeed on the command line I get:
>>
>> ------------------------
>> $ t/t9700/test.pl
>> ok 2 - use Git;
>> Bareword found where operator expected at t/t9700/test.pl line 36,
>> near "s/\\/\//gr"
>> syntax error at t/t9700/test.pl line 36, near "s/\\/\//gr"
>> Execution of t/t9700/test.pl aborted due to compilation errors.
>> ------------------------
>>
>> A quick look at t/t9700/test.pl line 36 was not enough for me to spot
>> the problem.
>>
>> Perl version is: perl 5, version 18, subversion 2 (v5.18.2) built for
>> x86_64-linux
>>
>> The machine is running CentOS 6.5.
>
> I can't reproduce on any of the machines I have handy (perl 5.14, 5.20,
> and 5.22). I don't have 5.18 handy. The line in question looks fine to
> me, so perhaps it is a temporary regression in 5.18.

> Does it help to wrap it in parentheses, like:
>
> diff --git a/t/t9700/test.pl b/t/t9700/test.pl
> index 7e8c40b..edeeb0e 100755
> --- a/t/t9700/test.pl
> +++ b/t/t9700/test.pl
> @@ -33,7 +33,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
>  is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
>  ok($r->config_bool("test.booltrue"), "config_bool: true");
>  ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
> -is($r->config_path("test.path") =~ s/\\/\//gr, $r->config("test.pathexpanded"),
> +is(($r->config_path("test.path") =~ s/\\/\//gr), $r->config("test.pathexpanded"),
>     "config_path: ~/foo expansion");
>  is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"],
>     "config_path: multiple values");

> or even write it out longhand without "/r":
>
> diff --git a/t/t9700/test.pl b/t/t9700/test.pl
> index 7e8c40b..52471cf 100755
> --- a/t/t9700/test.pl
> +++ b/t/t9700/test.pl
> @@ -33,7 +33,9 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
>  is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
>  ok($r->config_bool("test.booltrue"), "config_bool: true");
>  ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
> -is($r->config_path("test.path") =~ s/\\/\//gr, $r->config("test.pathexpanded"),
> +my $test_path = $r->config_path("test.path");
> +$test_path =~ s/\\/\//g;
> +is($test_path, $r->config("test.pathexpanded"),
>     "config_path: ~/foo expansion");
>  is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"],
>     "config_path: multiple values");
>
> ?

> > Those are just guesses, but if we are tickling a bug in perl's parser,
> > this might avoid them. I also wondered when "/r" appeared. It was in
> > 5.14, so you're presumably good there.
> 
> If I just remove the "r" at the end of "s/\\/\//gr", I get with both
> Perl versions:
> 
> Can't modify non-lvalue subroutine call at t/t9700/test.pl line 36.

> > The "use" statement at the top of
> > the script says "5.008", so perhaps we should be writing it out longhand
> > anyway (that version is "only" 5 years old, so I suspect there are still
> > systems around with 5.12 or older).
> 
> Yeah, it would work.

> ? Those are just guesses, but if we are tickling a bug in perl's parser,
> this might avoid them. I also wondered when "/r" appeared. It was in
> 5.14, so you're presumably good there. The "use" statement at the top of
> the script says "5.008", so perhaps we should be writing it out longhand
> anyway (that version is "only" 5 years old, so I suspect there are still
> systems around with 5.12 or older).

> On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote:
> > ? Those are just guesses, but if we are tickling a bug in perl's parser,
> > this might avoid them. I also wondered when "/r" appeared. It was in
> > 5.14, so you're presumably good there. The "use" statement at the top of
> > the script says "5.008", so perhaps we should be writing it out longhand
> > anyway (that version is "only" 5 years old, so I suspect there are still
> > systems around with 5.12 or older).
> 
> Knowing the system Christian is testing on, I think the problem is that
> the tests are actually being run against perl 5.10, which RHEL 6 ships
> as system perl. As that's still a supported OS, writing tests in a form
> compatible with it would be a good thing :)

> On Fri, Mar 04, 2016 at 11:58:24AM +0100, Dennis Kaarsemaker wrote:
> 
> > On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote:
> > > ? Those are just guesses, but if we are tickling a bug in perl's parser,
> > > this might avoid them. I also wondered when "/r" appeared. It was in
> > > 5.14, so you're presumably good there. The "use" statement at the top of
> > > the script says "5.008", so perhaps we should be writing it out longhand
> > > anyway (that version is "only" 5 years old, so I suspect there are still
> > > systems around with 5.12 or older).
> > 
> > Knowing the system Christian is testing on, I think the problem is that
> > the tests are actually being run against perl 5.10, which RHEL 6 ships
> > as system perl. As that's still a supported OS, writing tests in a form
> > compatible with it would be a good thing :)
> 
> That would make sense. `perl` in t9700-perl-git.sh (and all of our
> scripts) is actually a shell function:
> 
>   perl () {
>           command "$PERL_PATH" "$@"
>   }
> 
> to make sure we respect PERL_PATH everywhere. And that defaults in the
> Makefile to /usr/bin/perl. Christian presumably has 5.14 in his $PATH,
> but /usr/bin/perl is the system 5.10.

> On vr, 2016-03-04 at 06:43 -0500, Jeff King wrote:
>> On Fri, Mar 04, 2016 at 11:58:24AM +0100, Dennis Kaarsemaker wrote:
>>
>> > On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote:
>> > > ? Those are just guesses, but if we are tickling a bug in perl's parser,
>> > > this might avoid them. I also wondered when "/r" appeared. It was in
>> > > 5.14, so you're presumably good there. The "use" statement at the top of
>> > > the script says "5.008", so perhaps we should be writing it out longhand
>> > > anyway (that version is "only" 5 years old, so I suspect there are still
>> > > systems around with 5.12 or older).
>> >
>> > Knowing the system Christian is testing on, I think the problem is that
>> > the tests are actually being run against perl 5.10, which RHEL 6 ships
>> > as system perl. As that's still a supported OS, writing tests in a form
>> > compatible with it would be a good thing :)
>>
>> That would make sense. `perl` in t9700-perl-git.sh (and all of our
>> scripts) is actually a shell function:
>>
>>   perl () {
>>           command "$PERL_PATH" "$@"
>>   }
>>
>> to make sure we respect PERL_PATH everywhere. And that defaults in the
>> Makefile to /usr/bin/perl. Christian presumably has 5.14 in his $PATH,
>> but /usr/bin/perl is the system 5.10.
>
> Yeah, that's how our systems are set up.

> Subject: t9700: fix test for perl older than 5.14
> 
> Commit d53c2c6 (mingw: fix t9700's assumption about
> directory separators, 2016-01-27) uses perl's "/r" regex
> modifier to do a non-destructive replacement on a string,
> leaving the original unmodified and returning the result.
> 
> This feature was introduced in perl 5.14, but systems with
> older perl are still common (e.g., CentOS 6.5 still has perl
> 5.10). Let's work around it by providing a helper function
> that does the same thing using older syntax.
> 
> While we're at it, let's switch to using an alternate regex
> separator, which is slightly more readable.

> One workaround would therefore be for him to tweak PERL_PATH, but
> obviously that does not help anyone else. I think we should do this:
>
> -- >8 --
> Subject: t9700: fix test for perl older than 5.14
>
> Commit d53c2c6 (mingw: fix t9700's assumption about
> directory separators, 2016-01-27) uses perl's "/r" regex
> modifier to do a non-destructive replacement on a string,
> leaving the original unmodified and returning the result.

>
> This feature was introduced in perl 5.14, but systems with
> older perl are still common (e.g., CentOS 6.5 still has perl
> 5.10). Let's work around it by providing a helper function
> that does the same thing using older syntax.
>
> While we're at it, let's switch to using an alternate regex
> separator, which is slightly more readable.
>
> Reported-by: Christian Couder <christian.couder@gmail.com>
> Helped-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> And yes, before anyone checks, the alternate separators are available
> even in ancient versions of perl. :)
>
>  t/t9700/test.pl | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/t/t9700/test.pl b/t/t9700/test.pl
> index 7e8c40b..1b75c91 100755
> --- a/t/t9700/test.pl
> +++ b/t/t9700/test.pl
> @@ -17,6 +17,12 @@ BEGIN {
>  use Cwd;
>  use File::Basename;
>  
> +sub adjust_dirsep {
> +	my $path = shift;
> +	$path =~ s{\\}{/}g;
> +	return $path;
> +}
> +
>  BEGIN { use_ok('Git') }
>  
>  # set up
> @@ -33,7 +39,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
>  is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
>  ok($r->config_bool("test.booltrue"), "config_bool: true");
>  ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
> -is($r->config_path("test.path") =~ s/\\/\//gr, $r->config("test.pathexpanded"),
> +is(adjust_dirsep($r->config_path("test.path")), $r->config("test.pathexpanded"),
>     "config_path: ~/foo expansion");
>  is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"],
>     "config_path: multiple values");