Skip Menu |
 

This queue is for tickets about the ExtUtils-MakeMaker CPAN distribution.

Report information
The Basics
Id: 68613
Status: new
Priority: 0/
Queue: ExtUtils-MakeMaker

People
Owner: Nobody in particular
Requestors: ntyni [...] iki.fi
Cc: dom [...] cpan.org
jquelin [...] cpan.org
AdminCc:

Bug Information
Severity: Normal
Broken in:
  • 6.57_05
  • 6.57_11
Fixed in: (no value)

Attachments
0001-Append-CCFLAGS-to-Config-ccflags-instead-of-overridi.patch
0001-Document-that-CCFLAGS-should-include-Config-ccflags.patch
0002-Add-a-new-CCEXTRAFLAGS-setting.patch



Subject: CCFLAGS should not override $Config{ccflags}
Download (untitled) / with headers
text/plain 1.6k
As seen in http://bugs.debian.org/628522 and possibly also http://rt.cpan.org/Ticket/Display.html?id=67990 : Compiling XS extensions without $Config{ccflags} can break the binary interface on some platforms as preprocessor definitions like -D_FILE_OFFSET_BITS=64 are not used for the build. This has become a practical problem for us (Debian) with 5.14.0 where such extensions actually stop working on the x86 (32-bit) architecture; see also the recent similar EU::CBuilder issue at http://rt.perl.org/rt3/Public/Bug/Display.html?id=89478 Unless a correct use case exists for compiling an XS module without $Config{ccflags} (I can't think of any), it seems to me that any extra CCFLAGS should be appended to those rather than overriding them. The alternative to such a fix seems to be that the modules overriding CCFLAGS should include $Config{ccflags} themselves. In that case this requirement should probably be mentioned in the EU::MM documentation. The attached naive patch implements the first option. Please note that I had to change an existing test to accomodate the new behaviour. Based on the comment the aim of the test is not to check that CCFLAGS was completely overridden, just that the contents are not deleted. This potentially affects modules that already explicitly set CCFLAGS to a value that includes $Config{ccflags}. I'm only aware of DBD::Oracle doing this if it's building with 'bcc32' (probably a win32 thing?). FWIW, Dominic Hargreaves has tested a rebuild of about 400 XS modules on Debian/5.14.0 with the patch without any regressions, as mentioned in the Debian bug report. Please let me know what you think. Many thanks for your work on ExtUtils::MakeMaker! -- Niko Tyni ntyni@debian.org
Subject: 0001-Append-CCFLAGS-to-Config-ccflags-instead-of-overridi.patch
From bb5cff55e52477be757b73d915bda76b4c5e088f Mon Sep 17 00:00:00 2001 From: Niko Tyni <ntyni@debian.org> Date: Mon, 30 May 2011 22:54:24 +0300 Subject: [PATCH] Append CCFLAGS to $Config{ccflags} instead of overriding it As seen in http://bugs.debian.org/628522 and possibly also http://rt.cpan.org/Ticket/Display.html?id=67990 compiling XS extensions without $Config{ccflags} can break the binary interface on some platforms. There does not seem to be any correct use case for compiling a module without $Config{ccflags}, so any extra CCFLAGS should be appended to those rather than overriding them. --- lib/ExtUtils/MM_Unix.pm | 6 +++++- t/MM_Unix.t | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/ExtUtils/MM_Unix.pm b/lib/ExtUtils/MM_Unix.pm index a3b7e9d..5e0758a 100644 --- a/lib/ExtUtils/MM_Unix.pm +++ b/lib/ExtUtils/MM_Unix.pm @@ -250,7 +250,11 @@ sub cflags { $cflags{$_} =~ s/^\s+//; $cflags{$_} =~ s/\s+/ /g; $cflags{$_} =~ s/\s+$//; - $self->{uc $_} ||= $cflags{$_}; + if (/ccflags/ && $self->{uc $_}) { + $self->{uc $_} = "$cflags{$_} " . $self->{uc $_}; + } else { + $self->{uc $_} ||= $cflags{$_}; + } } if ($self->{POLLUTE}) { diff --git a/t/MM_Unix.t b/t/MM_Unix.t index 55c29e3..5f76e08 100644 --- a/t/MM_Unix.t +++ b/t/MM_Unix.t @@ -12,7 +12,7 @@ BEGIN { plan skip_all => 'Non-Unix platform'; } else { - plan tests => 110; + plan tests => 112; } } @@ -20,6 +20,7 @@ BEGIN { use_ok( 'ExtUtils::MM_Unix' ); } use strict; use File::Spec; +use Config; my $class = 'ExtUtils::MM_Unix'; @@ -220,6 +221,9 @@ foreach (qw/ EXPORT_LIST PERL_ARCHIVE PERL_ARCHIVE_AFTER /) $t->cflags(); # Brief bug where CCFLAGS was being blown away - is( $t->{CCFLAGS}, '-DMY_THING', 'cflags retains CCFLAGS' ); + like( $t->{CCFLAGS}, "/-DMY_THING/", 'cflags retains CCFLAGS' ); + + like( $t->{CCFLAGS}, "/\Q$Config{ccflags}\E/", 'cflags does not override $Config{ccflags} with CCFLAGS' ); + like( $t->{CCFLAGS}, '/ -DMY_THING$/', 'cflags appends CCFLAGS to $Config{ccflags}' ); } -- 1.7.5.1
Subject: Re: [rt.cpan.org #68613] AutoReply: CCFLAGS should not override $Config{ccflags}
Date: Fri, 3 Jun 2011 08:22:21 +0300
To: Bugs in ExtUtils-MakeMaker via RT <bug-ExtUtils-MakeMaker [...] rt.cpan.org>
From: Niko Tyni <ntyni [...] iki.fi>
Download (untitled) / with headers
text/plain 2.3k
On Thu, Jun 02, 2011 at 03:01:27PM -0400, Bugs in ExtUtils-MakeMaker via RT wrote: Show quoted text
> Compiling XS extensions without $Config{ccflags} can break the > binary interface on some platforms as preprocessor definitions like > -D_FILE_OFFSET_BITS=64 are not used for the build. > > This has become a practical problem for us (Debian) with 5.14.0 where > such extensions actually stop working on the x86 (32-bit) architecture; > see also the recent similar EU::CBuilder issue at > http://rt.perl.org/rt3/Public/Bug/Display.html?id=89478 > > Unless a correct use case exists for compiling an XS module without > $Config{ccflags} (I can't think of any), it seems to me that any extra > CCFLAGS should be appended to those rather than overriding them. > > The alternative to such a fix seems to be that the modules overriding > CCFLAGS should include $Config{ccflags} themselves. In that case this > requirement should probably be mentioned in the EU::MM documentation.
It turns out that some modules currently modify $Config{ccflags} settings. For instance, the Verilog-Perl CPAN module has this: # Grr; some flags cause warnings in g++ (my $ccflags = $Config{ccflags}) =~ s/ *-Wdeclaration-after-statement//; and Search-Xapian (also on CPAN) has this: # Filter out some gcc options which g++ doesn't support. my $CCFLAGS = $Config{'ccflags'}; # Perl is built with -Wdeclaration-after-statement on RHEL5 - this isn't # meaningful for C++ - it only emits a warning but it's easy to fix. $CCFLAGS =~ s/(?:^|\s+)-Wdeclaration-after-statement(?:\s+|$)/ /; # The generated code causes "variable may be used uninitialized" warnings # if Perl was built with -Wall. $CCFLAGS =~ s/(^|\s+)-Wall(\s+|$)/$1-Wall -Wno-uninitialized$2/; Also, a few modules (DBD::Pg, Algorith::Permute) already use $Config{ccflags} unmodified and would get them twice with the patch. This seems to imply that appending CCFLAGS to $Config{ccflags} isn't quite the right thing to do after all. A better approach might be to update the EU::MM documentation, optionally add a new CCEXTRAFLAGS that automatically includes $Config{ccflags}, and fix the modules that currently don't use $Config{ccflags} at all (this includes at least GD, OIS, Ogre on CPAN). See the attached patches. They may need some tweaking for non-MM_Unix platforms. Again, thanks for your work on EU::MM. -- Niko Tyni ntyni@debian.org

Message body is not shown because sender requested not to inline it.

Message body is not shown because sender requested not to inline it.



This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.