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

[PATCH] Make ExtUtils::CBuilder reset ccflags on compile for VMS. #11224

Closed
p5pRT opened this issue Mar 31, 2011 · 7 comments
Closed

[PATCH] Make ExtUtils::CBuilder reset ccflags on compile for VMS. #11224

p5pRT opened this issue Mar 31, 2011 · 7 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 31, 2011

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

Searchable as RT87444$

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2011

From @craigberry

I think the attached is reasonable to go into 5.14 because it fixes a smoke failure (all of the ExtUtils​::ParseXS tests have failed on VMS under threads ever since they were created) and because it only affects one platform and because the module in question now has blead as upstream. But I might be too late for RC0, so not pushing.

________________________________________
Craig A. Berry
mailto​:craigberry@​mac.com

"... getting out of a sonnet is much more
difficult than getting in."
  Brad Leithauser

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2011

From @craigberry

0001-Make-ExtUtils-CBuilder-reset-ccflags-on-compile-for-.patch
From 63341a3b54e15e60c4cbffc80457ac2c0ca3d68c Mon Sep 17 00:00:00 2001
From: Craig A. Berry <craigberry@mac.com>
Date: Thu, 31 Mar 2011 17:09:31 -0500
Subject: [PATCH] Make ExtUtils::CBuilder reset ccflags on compile for VMS.

On VMS only, the /DEFINE and /INCLUDE qualifiers are parsed off the
local copy of $Config{ccflags} and consumed in the process.  This is
necessary because you're only allowed one of each of these clauses
in the compile command, so to add whatever has been requested for a
specific compile, we have to combine them with whatever Perl was
built with.

But since they are consumed, multiple compiles on the same EU::CB
object were only using the correct flags for the first one.  Even
calling the have_compiler() check before compile() would make the
latter miss the defines and includes that were used to build Perl.

The solution is add a platform override that resets the local copy
of $Config{ccflags} from its original in %Config every time a
compiler operation is initiated.

Fixes smoke failures in ExtUtils::ParseXS.
---
 .../lib/ExtUtils/CBuilder/Platform/VMS.pm          |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/VMS.pm b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/VMS.pm
index 4f6a36c..92f17c0 100644
--- a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/VMS.pm
+++ b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/VMS.pm
@@ -8,6 +8,7 @@ $VERSION = '0.280202';
 @ISA = qw(ExtUtils::CBuilder::Base);
 
 use File::Spec::Functions qw(catfile catdir);
+use Config;
 
 # We do prelink, but don't want the parent to redo it.
 
@@ -47,6 +48,20 @@ sub arg_include_dirs {
   return ('/include=(' . join(',', @dirs) . ')');
 }
 
+# We override the compile method because we consume the includes and defines
+# parts of ccflags in the process of compiling but don't save those parts
+# anywhere, so $self->{config}{ccflags} needs to be reset for each compile
+# operation.  
+
+sub compile {
+  my ($self, %args) = @_;
+
+  $self->{config}{ccflags} = $Config{ccflags};
+  $self->{config}{ccflags} = $ENV{CFLAGS} if defined $ENV{CFLAGS};
+
+  return $self->SUPER::compile(%args);
+}
+
 sub _do_link {
   my ($self, $type, %args) = @_;
   
-- 
1.7.3.5

@p5pRT
Copy link
Author

p5pRT commented Apr 1, 2011

From @craigberry

On Thu, Mar 31, 2011 at 5​:37 PM, Craig A. Berry
<perlbug-followup@​perl.org> wrote​:

# New Ticket Created by  Craig A. Berry
# Please include the string​:  [perl #87444]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=87444 >

And the patch inline as well for easier review​:

From 63341a3b54e15e60c4cbffc80457ac2c0ca3d68c Mon Sep 17 00​:00​:00 2001
From​: Craig A. Berry <craigberry@​mac.com>
Date​: Thu, 31 Mar 2011 17​:09​:31 -0500
Subject​: [PATCH] Make ExtUtils​::CBuilder reset ccflags on compile for VMS.

On VMS only, the /DEFINE and /INCLUDE qualifiers are parsed off the
local copy of $Config{ccflags} and consumed in the process. This is
necessary because you're only allowed one of each of these clauses
in the compile command, so to add whatever has been requested for a
specific compile, we have to combine them with whatever Perl was
built with.

But since they are consumed, multiple compiles on the same EU​::CB
object were only using the correct flags for the first one. Even
calling the have_compiler() check before compile() would make the
latter miss the defines and includes that were used to build Perl.

The solution is add a platform override that resets the local copy
of $Config{ccflags} from its original in %Config every time a
compiler operation is initiated.

Fixes smoke failures in ExtUtils​::ParseXS.


.../lib/ExtUtils/CBuilder/Platform/VMS.pm | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/VMS.pm
b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/VMS.pm
index 4f6a36c..92f17c0 100644
--- a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/VMS.pm
+++ b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/VMS.pm
@​@​ -8,6 +8,7 @​@​ $VERSION = '0.280202';
@​ISA = qw(ExtUtils​::CBuilder​::Base);

use File​::Spec​::Functions qw(catfile catdir);
+use Config;

# We do prelink, but don't want the parent to redo it.

@​@​ -47,6 +48,20 @​@​ sub arg_include_dirs {
  return ('/include=(' . join(',', @​dirs) . ')');
}

+# We override the compile method because we consume the includes and defines
+# parts of ccflags in the process of compiling but don't save those parts
+# anywhere, so $self->{config}{ccflags} needs to be reset for each compile
+# operation.
+
+sub compile {
+ my ($self, %args) = @​_;
+
+ $self->{config}{ccflags} = $Config{ccflags};
+ $self->{config}{ccflags} = $ENV{CFLAGS} if defined $ENV{CFLAGS};
+
+ return $self->SUPER​::compile(%args);
+}
+
sub _do_link {
  my ($self, $type, %args) = @​_;

--
1.7.3.5

@p5pRT
Copy link
Author

p5pRT commented Apr 1, 2011

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

@p5pRT
Copy link
Author

p5pRT commented Apr 4, 2011

From @obra

I've made this a 5.14 blocker. Patch away.

@p5pRT
Copy link
Author

p5pRT commented Apr 4, 2011

From @craigberry

On Apr 3, 2011, at 11​:28 PM, Jesse Vincent via RT wrote​:

I've made this a 5.14 blocker. Patch away.

Thanks, I've pushed it as a24b897, so the ticket can be closed.

________________________________________
Craig A. Berry
mailto​:craigberry@​mac.com

"... getting out of a sonnet is much more
difficult than getting in."
  Brad Leithauser

@p5pRT
Copy link
Author

p5pRT commented Apr 4, 2011

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

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

No branches or pull requests

1 participant