Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test failure in t/run/switchM.t with -DPERL_DISABLE_PMC #13736

Closed
p5pRT opened this issue Apr 14, 2014 · 10 comments
Closed

Test failure in t/run/switchM.t with -DPERL_DISABLE_PMC #13736

p5pRT opened this issue Apr 14, 2014 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Apr 14, 2014

Migrated from rt.perl.org#121662 (status was 'resolved')

Searchable as RT121662$

@p5pRT
Copy link
Author

p5pRT commented Apr 14, 2014

From @tomhukins

It's unusual to compile Perl with -DPERL_DISABLE_PMC, but it's
something that has worked in previous versions of perl.

On blead, tests in t/run/switchM.t fail. The attached patch skips
those tests using a crude regular expression​: a better approach that I
don't know of might exist.

Please apply this patch, or something that does the job better, before
5.20 so everyone can continue to compile and use Perl 5 without PMC
support.

Thanks,
Tom

@p5pRT
Copy link
Author

p5pRT commented Apr 14, 2014

From @tomhukins

0001-Make-the-test-suite-pass-with-DPERL_DISABLE_PMC.patch
From 800d2bc336a68e5d0038cc1039d35b8fcaa32e14 Mon Sep 17 00:00:00 2001
From: Tom Hukins <tom@eborcom.com>
Date: Mon, 14 Apr 2014 18:38:46 +0100
Subject: [PATCH] Make the test suite pass with -DPERL_DISABLE_PMC

Commit 9fdd5a7ac74817cfaab6 introduced new tests that fail when building
perl without PMC support.  In such cases, skip these new tests.
---
 t/run/switchM.t |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/t/run/switchM.t b/t/run/switchM.t
index 6a75100..6ec3669 100644
--- a/t/run/switchM.t
+++ b/t/run/switchM.t
@@ -3,6 +3,9 @@
 BEGIN {
     chdir 't' if -d 't';
     @INC = '../lib';
+    require Config;
+    import Config;
+
 }
 use strict;
 
@@ -18,10 +21,16 @@ like(runperl(switches => ['-Irun/flib/', '-Mbroken'], stderr => 1),
      qr/^Global symbol "\$x" requires explicit package name at run\/flib\/broken.pm line 6\./,
      "Ensure -Irun/flib/ produces correct filename in warnings");
 
-like(runperl(switches => ['-Irun/flib', '-Mt2', '-e "print t2::id()"'], stderr => 1),
-     qr/^t2pmc$/,
-     "Ensure -Irun/flib loads pmc");
+SKIP: {
+    if ( $Config{ccflags} =~ /-DPERL_DISABLE_PMC/ ) {
+        skip('Tests fail without PMC support', 2);
+    }
+
+    like(runperl(switches => ['-Irun/flib', '-Mt2', '-e "print t2::id()"'], stderr => 1),
+         qr/^t2pmc$/,
+         "Ensure -Irun/flib loads pmc");
 
-like(runperl(switches => ['-Irun/flib/', '-Mt2', '-e "print t2::id()"'], stderr => 1),
-     qr/^t2pmc$/,
-     "Ensure -Irun/flib/ loads pmc");
+    like(runperl(switches => ['-Irun/flib/', '-Mt2', '-e "print t2::id()"'], stderr => 1),
+         qr/^t2pmc$/,
+         "Ensure -Irun/flib/ loads pmc");
+}
-- 
1.7.7

@p5pRT
Copy link
Author

p5pRT commented Apr 16, 2014

From @tonycoz

On Mon Apr 14 13​:02​:02 2014, tomhukins wrote​:

It's unusual to compile Perl with -DPERL_DISABLE_PMC, but it's
something that has worked in previous versions of perl.

On blead, tests in t/run/switchM.t fail. The attached patch skips
those tests using a crude regular expression​: a better approach that I
don't know of might exist.

Please apply this patch, or something that does the job better, before
5.20 so everyone can continue to compile and use Perl 5 without PMC
support.

I'd at least add a \b to the end of the match.

t/comp/require.t uses​:

  $ccflags =~ /(?​:^|\s)-DPERL_DISABLE_PMC\b/

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 16, 2014

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Apr 16, 2014

From @tomhukins

On Wed, Apr 16, 2014 at 12​:00​:56AM -0700, Tony Cook via RT wrote​:

I'd at least add a \b to the end of the match.

t/comp/require.t uses​:

$ccflags =~ /(?​:^|\s)-DPERL_DISABLE_PMC\b/

Thank you for the helpful review, Tony. I've just tested the
attached patch both with and without -DPERL_DISABLE_PMC and it works
fine.

Tom

@p5pRT
Copy link
Author

p5pRT commented Apr 16, 2014

From @tomhukins

0001-Make-the-test-suite-pass-with-DPERL_DISABLE_PMC.patch
From c785429ee541cd3d98bfb1a3f5499e93a043884e Mon Sep 17 00:00:00 2001
From: Tom Hukins <tom@eborcom.com>
Date: Wed, 16 Apr 2014 13:36:45 +0100
Subject: [PATCH] Make the test suite pass with -DPERL_DISABLE_PMC

Commit 9fdd5a7ac74817cfaab6 introduced new tests that fail when building
perl without PMC support.  In such cases, skip these new tests.
---
 t/run/switchM.t |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/t/run/switchM.t b/t/run/switchM.t
index 6a75100..8d87581 100644
--- a/t/run/switchM.t
+++ b/t/run/switchM.t
@@ -3,6 +3,9 @@
 BEGIN {
     chdir 't' if -d 't';
     @INC = '../lib';
+    require Config;
+    import Config;
+
 }
 use strict;
 
@@ -18,10 +21,16 @@ like(runperl(switches => ['-Irun/flib/', '-Mbroken'], stderr => 1),
      qr/^Global symbol "\$x" requires explicit package name at run\/flib\/broken.pm line 6\./,
      "Ensure -Irun/flib/ produces correct filename in warnings");
 
-like(runperl(switches => ['-Irun/flib', '-Mt2', '-e "print t2::id()"'], stderr => 1),
-     qr/^t2pmc$/,
-     "Ensure -Irun/flib loads pmc");
+SKIP: {
+    if ( $Config{ccflags} =~ /(?:^|\s)-DPERL_DISABLE_PMC\b/ ) {
+        skip('Tests fail without PMC support', 2);
+    }
+
+    like(runperl(switches => ['-Irun/flib', '-Mt2', '-e "print t2::id()"'], stderr => 1),
+         qr/^t2pmc$/,
+         "Ensure -Irun/flib loads pmc");
 
-like(runperl(switches => ['-Irun/flib/', '-Mt2', '-e "print t2::id()"'], stderr => 1),
-     qr/^t2pmc$/,
-     "Ensure -Irun/flib/ loads pmc");
+    like(runperl(switches => ['-Irun/flib/', '-Mt2', '-e "print t2::id()"'], stderr => 1),
+         qr/^t2pmc$/,
+         "Ensure -Irun/flib/ loads pmc");
+}
-- 
1.7.7

@p5pRT
Copy link
Author

p5pRT commented Apr 21, 2014

From @tonycoz

On Mon Apr 14 13​:02​:02 2014, tomhukins wrote​:

It's unusual to compile Perl with -DPERL_DISABLE_PMC, but it's
something that has worked in previous versions of perl.

On blead, tests in t/run/switchM.t fail. The attached patch skips
those tests using a crude regular expression​: a better approach that I
don't know of might exist.

Please apply this patch, or something that does the job better, before
5.20 so everyone can continue to compile and use Perl 5 without PMC
support.

I think this belongs in blead, and plan to apply it in a couple of days if no-one objects.

This fixes a new test introduced during the code freeze which I think makes it fair game.

The patch will probably require some adjustment to avoid a conflict with the patch in 121672.

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 21, 2014

From @rjbs

I agree​: this should be applied.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Apr 23, 2014

From @tonycoz

On Mon Apr 21 07​:41​:14 2014, rjbs wrote​:

I agree​: this should be applied.

Applied, though I​:

- had to re-work the patch, since it conflicted with Craig's in 121672

- based my work on the wrong patch <sigh>, which has the less stringent -DPERL_DISABLE_PMC test.

So applied as d9c544b and adding back the stringent check with 006200b.

Tony

@p5pRT p5pRT closed this as completed Apr 23, 2014
@p5pRT
Copy link
Author

p5pRT commented Apr 23, 2014

@tonycoz - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant