From @ntyni

On Wed, Apr 27, 2011 at 05​:37​:56PM -0500, Craig A. Berry wrote​:

On Wed, Apr 27, 2011 at 4​:39 PM, Niko Tyni <ntyni@​debian.org> wrote​:

Failed 4 tests out of 2037, 99.80% okay.
       ../cpan/Module-Build/t/xs.t
       ../dist/ExtUtils-ParseXS/t/basic.t
       ../dist/ExtUtils-ParseXS/t/more.t
       ../dist/ExtUtils-ParseXS/t/usage.t

What's happening seems to be that when CFLAGS is set, it overrides
other flags in ExtUtils​::CBuilder. One of these, -D_FILE_OFFSET_BITS=64,
is apparently crucial on this platform for some reason.

Bisecting shows the crucial thing that breaks this is
-D_FILE_OFFSET_BITS=64 missing. Don't know why.

It looks like this is a change in ExtUtils​::CBuilder, maybe
 Perl-Toolchain-Gang/ExtUtils-CBuilder@e653d24a

I doubt there's anything crucial about the particular flag, but rather
it's the fact that you're building extensions using flags that give
you code that is binary incompatible with the perl binary it's being
built against. It looks like your expectation is that you can add an
optimization flag by specifying only that flag in CFLAGS. Sounds
reasonable, but clearly what EU​::CB is doing is dropping everything it
got from $Config{ccflags} and replacing rather than supplementing it
with what you gave it in CFLAGS.

Yes, I see. Thanks. Perlbugging this with the attached proposed patch.
FWIW, I think LDFLAGS should be treated in the same way.

With options like -D_LARGEFILE_SOURCE and -D_FILE_OFFSET_BITS=64' used
to build Perl but dropped when testing extension building, you could
be getting a different and incompatible stat structure or other binary
incompatible differences between the extension and the Perl core.

It seems like the principle of least surprise might be to make EU​::CB
take CFLAGS as a supplement to $Config{ccflags} rather than as a
replacement. It would still be possible to create binary
incompatibilities, but perhaps less likely.
--
Niko Tyni ntyni@​debian.org

From @ntyni

From bb249b0b26c2e79a6f55355ef94889070f07fd21 Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Thu, 28 Apr 2011 09:18:54 +0300
Subject: [PATCH] Append CFLAGS and LDFLAGS to their Config.pm counterparts in
 EU::CBuilder

Since ExtUtils::CBuilder 0.27_04 (bleadperl commit 06e8058f27e4),
CFLAGS and LDFLAGS from the environment have overridden the Config.pm
ccflags and ldflags settings. This can cause binary incompatibilities
between the core Perl and extensions built with EU::CBuilder.

Append to the Config.pm values rather than overriding them.
---
 .../lib/ExtUtils/CBuilder/Base.pm                  |    6 +++-
 dist/ExtUtils-CBuilder/t/04-base.t                 |   25 +++++++++++++++++++-
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm
index b572312..2255c51 100644
--- a/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm
+++ b/dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm
@@ -40,11 +40,13 @@ sub new {
     $self->{config}{$k} = $v unless exists $self->{config}{$k};
   }
   $self->{config}{cc} = $ENV{CC} if defined $ENV{CC};
-  $self->{config}{ccflags} = $ENV{CFLAGS} if defined $ENV{CFLAGS};
+  $self->{config}{ccflags} = join(" ", $self->{config}{ccflags}, $ENV{CFLAGS})
+     if defined $ENV{CFLAGS};
   $self->{config}{cxx} = $ENV{CXX} if defined $ENV{CXX};
   $self->{config}{cxxflags} = $ENV{CXXFLAGS} if defined $ENV{CXXFLAGS};
   $self->{config}{ld} = $ENV{LD} if defined $ENV{LD};
-  $self->{config}{ldflags} = $ENV{LDFLAGS} if defined $ENV{LDFLAGS};
+  $self->{config}{ldflags} = join(" ", $self->{config}{ldflags}, $ENV{LDFLAGS})
+     if defined $ENV{LDFLAGS};
 
   unless ( exists $self->{config}{cxx} ) {
     my ($ccpath, $ccbase, $ccsfx ) = fileparse($self->{config}{cc}, qr/\.[^.]*/);
diff --git a/dist/ExtUtils-CBuilder/t/04-base.t b/dist/ExtUtils-CBuilder/t/04-base.t
index c3bf6b5..1bb15aa 100644
--- a/dist/ExtUtils-CBuilder/t/04-base.t
+++ b/dist/ExtUtils-CBuilder/t/04-base.t
@@ -1,7 +1,7 @@
 #! perl -w
 
 use strict;
-use Test::More tests => 50;
+use Test::More tests => 64;
 use Config;
 use Cwd;
 use File::Path qw( mkpath );
@@ -326,6 +326,29 @@ is_deeply( $mksymlists_args,
     "_prepare_mksymlists_args(): got expected arguments for Mksymlists",
 );
 
+my %testvars = (
+    CFLAGS  => 'ccflags',
+    LDFLAGS => 'ldflags',
+);
+
+while (my ($VAR, $var) = each %testvars) {
+    local $ENV{$VAR};
+    $base = ExtUtils::CBuilder::Base->new( quiet => 1 );
+    ok( $base, "ExtUtils::CBuilder::Base->new() returned true value" );
+    isa_ok( $base, 'ExtUtils::CBuilder::Base' );
+    like($base->{config}{$var}, qr/\Q$Config{$var}/,
+        "honours $var from Config.pm");
+
+    $ENV{$VAR} = "-foo -bar";
+    $base = ExtUtils::CBuilder::Base->new( quiet => 1 );
+    ok( $base, "ExtUtils::CBuilder::Base->new() returned true value" );
+    isa_ok( $base, 'ExtUtils::CBuilder::Base' );
+    like($base->{config}{$var}, qr/\Q$ENV{$VAR}/,
+        "honours $VAR from the environment");
+    like($base->{config}{$var}, qr/\Q$Config{$var}/,
+        "doesn't override $var from Config.pm with $VAR from the environment");
+}
+
 #####
 
 for ($source_file, $object_file, $lib_file) {
-- 
1.7.4.4

From @cpansprout

On Thu Apr 28 08​:02​:02 2011, ntyni@​debian.org wrote​:

On Wed, Apr 27, 2011 at 05​:37​:56PM -0500, Craig A. Berry wrote​:

On Wed, Apr 27, 2011 at 4​:39 PM, Niko Tyni <ntyni@​debian.org> wrote​:

Failed 4 tests out of 2037, 99.80% okay.
� � � �../cpan/Module-Build/t/xs.t
� � � �../dist/ExtUtils-ParseXS/t/basic.t
� � � �../dist/ExtUtils-ParseXS/t/more.t
� � � �../dist/ExtUtils-ParseXS/t/usage.t

What's happening seems to be that when CFLAGS is set, it overrides
other flags in ExtUtils​::CBuilder. One of these,
-D_FILE_OFFSET_BITS=64,
is apparently crucial on this platform for some reason.

Bisecting shows the crucial thing that breaks this is
-D_FILE_OFFSET_BITS=64 missing. Don't know why.

It looks like this is a change in ExtUtils​::CBuilder, maybe
�Perl-Toolchain-Gang/ExtUtils-CBuilder@e653d24a

I doubt there's anything crucial about the particular flag, but rather
it's the fact that you're building extensions using flags that give
you code that is binary incompatible with the perl binary it's being
built against. It looks like your expectation is that you can add an
optimization flag by specifying only that flag in CFLAGS. Sounds
reasonable, but clearly what EU​::CB is doing is dropping everything it
got from $Config{ccflags} and replacing rather than supplementing it
with what you gave it in CFLAGS.

Yes, I see. Thanks. Perlbugging this with the attached proposed patch.
FWIW, I think LDFLAGS should be treated in the same way.

Thank you. Applied as 011e8fb.

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

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