From: Jeff King <peff@peff.net>
To: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Cc: Christian Couder <christian.couder@gmail.com>,
git <git@vger.kernel.org>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] t9700: fix test for perl older than 5.14
Date: Fri, 4 Mar 2016 06:43:21 -0500 [thread overview]
Message-ID: <20160304114321.GA569@sigill.intra.peff.net> (raw)
In-Reply-To: <1457089104.2660.79.camel@kaarsemaker.net>
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.
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");
--
2.8.0.rc0.309.g6677de9
next prev parent reply other threads:[~2016-03-04 11:43 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-04 8:13 t9700-perl-git.sh is broken on some configurations Christian Couder
2016-03-04 8:56 ` Jeff King
2016-03-04 10:30 ` Christian Couder
2016-03-04 11:45 ` Jeff King
2016-03-04 10:58 ` Dennis Kaarsemaker
2016-03-04 11:43 ` Jeff King [this message]
2016-03-04 12:21 ` [PATCH] t9700: fix test for perl older than 5.14 Dennis Kaarsemaker
2016-03-04 20:12 ` Christian Couder
2016-03-04 16:21 ` Johannes Schindelin
2016-03-04 18:15 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160304114321.GA569@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=Johannes.Schindelin@gmx.de \
--cc=christian.couder@gmail.com \
--cc=dennis@kaarsemaker.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).