This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Impose deterministic order on cpp-definition options.
authorZefram <zefram@fysh.org>
Wed, 18 Mar 2015 23:49:28 +0000 (19:49 -0400)
committerJames E Keenan <jkeenan@cpan.org>
Tue, 2 Jun 2015 00:07:16 +0000 (20:07 -0400)
Heretofore, ExtUtils::CBuilder put cpp-definition options into the cc command line in
non-deterministic order. This produced noise when diffing build logs.  Make
this order deterministic.

Add tests for ascii-betical order in t/04-base.t.  Increment $VERSION in three
packages.  Remove commented-out line.

For: RT #124106

dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm
dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/VMS.pm
dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Platform/Windows.pm
dist/ExtUtils-CBuilder/t/04-base.t

index 7df61e4..a077cfe 100644 (file)
@@ -1,5 +1,5 @@
 package ExtUtils::CBuilder::Base;
-$ExtUtils::CBuilder::Base::VERSION = '0.280221';
+$ExtUtils::CBuilder::Base::VERSION = '0.280222';
 use strict;
 use File::Spec;
 use File::Basename;
@@ -128,20 +128,20 @@ sub arg_exec_file {
 
 sub arg_defines {
   my ($self, %args) = @_;
-  return map "-D$_=$args{$_}", keys %args;
+  return map "-D$_=$args{$_}", sort keys %args;
 }
 
 sub compile {
   my ($self, %args) = @_;
   die "Missing 'source' argument to compile()" unless defined $args{source};
-  
+
   my $cf = $self->{config}; # For convenience
-  
+
   my $object_file = $args{object_file}
     ? $args{object_file}
     : $self->object_file($args{source});
 
-  my $include_dirs_ref = 
+  my $include_dirs_ref =
     (exists($args{include_dirs}) && ref($args{include_dirs}) ne "ARRAY")
       ? [ $args{include_dirs} ]
       : $args{include_dirs};
@@ -149,9 +149,9 @@ sub compile {
     @{ $include_dirs_ref || [] },
     $self->perl_inc(),
   );
-  
+
   my @defines = $self->arg_defines( %{$args{defines} || {}} );
-  
+
   my @extra_compiler_flags =
     $self->split_like_shell($args{extra_compiler_flags});
   my @cccdlflags = $self->split_like_shell($cf->{cccdlflags});
@@ -168,7 +168,7 @@ sub compile {
     $self->arg_object_file($object_file),
   );
   my @cc = $self->split_like_shell($args{'C++'} ? $cf->{cxx} : $cf->{cc});
-  
+
   $self->do_system(@cc, @flags, $args{source})
     or die "error building $object_file from '$args{source}'";
 
@@ -222,7 +222,7 @@ sub lib_file {
   my ($self, $dl_file, %args) = @_;
   $dl_file =~ s/\.[^.]+$//;
   $dl_file =~ tr/"//d;
-  
+
   if (defined $args{module_name} and length $args{module_name}) {
     # Need to create with the same name as DynaLoader will load with.
     require DynaLoader;
@@ -232,7 +232,7 @@ sub lib_file {
       $dl_file = File::Spec->catpath($dev, $lib_dir, $lib);
     }
   }
-  
+
   $dl_file .= ".$self->{config}{dlext}";
 
   return $dl_file;
@@ -266,7 +266,7 @@ sub prelink {
 sub _prepare_mksymlists_args {
   my $args = shift;
   ($args->{dl_file} = $args->{dl_name}) =~ s/.*::// unless $args->{dl_file};
-  
+
   my %mksymlists_args = (
     DL_VARS  => $args->{dl_vars}      || [],
     DL_FUNCS => $args->{dl_funcs}     || {},
@@ -294,16 +294,16 @@ sub _do_link {
   my ($self, $type, %args) = @_;
 
   my $cf = $self->{config}; # For convenience
-  
+
   my $objects = delete $args{objects};
   $objects = [$objects] unless ref $objects;
   my $out = $args{$type} || $self->$type($objects->[0], %args);
-  
+
   my @temp_files;
   @temp_files =
     $self->prelink(%args, dl_name => $args{module_name})
       if $args{lddl} && $self->need_prelink;
-  
+
   my @linker_flags = (
     $self->split_like_shell($args{extra_linker_flags}),
     $self->extra_link_args_after_prelink(
@@ -316,10 +316,10 @@ sub _do_link {
     : $self->arg_exec_file($out);
   my @shrp = $self->split_like_shell($cf->{shrpenv});
   my @ld = $self->split_like_shell($cf->{ld});
-  
+
   $self->do_system(@shrp, @ld, @output, @$objects, @linker_flags)
     or die "error building $out from @$objects";
-  
+
   return wantarray ? ($out, @temp_files) : $out;
 }
 
@@ -332,17 +332,17 @@ sub do_system {
 
 sub split_like_shell {
   my ($self, $string) = @_;
-  
+
   return () unless defined($string);
   return @$string if UNIVERSAL::isa($string, 'ARRAY');
   $string =~ s/^\s+|\s+$//g;
   return () unless length($string);
-  
+
   # Text::ParseWords replaces all 'escaped' characters with themselves, which completely
   # breaks paths under windows. As such, we forcibly replace backwards slashes with forward
   # slashes on windows.
   $string =~ s@\\@/@g if $^O eq 'MSWin32';
-  
+
   return Text::ParseWords::shellwords($string);
 }
 
index 6285e33..940dc50 100644 (file)
@@ -1,5 +1,5 @@
 package ExtUtils::CBuilder::Platform::VMS;
-$ExtUtils::CBuilder::Platform::VMS::VERSION = '0.280221';
+$ExtUtils::CBuilder::Platform::VMS::VERSION = '0.280222';
 use strict;
 use ExtUtils::CBuilder::Base;
 
@@ -27,11 +27,11 @@ sub arg_defines {
 
   return '' unless keys(%args) || @config_defines;
 
-  return ('/define=(' 
-          . join(',', 
+  return ('/define=('
+          . join(',',
                 @config_defines,
-                 map "\"$_" . ( length($args{$_}) ? "=$args{$_}" : '') . "\"", 
-                     keys %args) 
+                 map "\"$_" . ( length($args{$_}) ? "=$args{$_}" : '') . "\"",
+                     sort keys %args)
           . ')');
 }
 
@@ -50,7 +50,7 @@ sub arg_include_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.  
+# operation.
 
 sub compile {
   my ($self, %args) = @_;
@@ -63,10 +63,10 @@ sub compile {
 
 sub _do_link {
   my ($self, $type, %args) = @_;
-  
+
   my $objects = delete $args{objects};
   $objects = [$objects] unless ref $objects;
-  
+
   if ($args{lddl}) {
 
     # prelink will call Mksymlists, which creates the extension-specific
@@ -77,7 +77,7 @@ sub _do_link {
     # We now add the rest of what we need to the linker options file.  We
     # should replicate the functionality of C<ExtUtils::MM_VMS::dlsyms>,
     # but there is as yet no infrastructure for handling object libraries,
-    # so for now we depend on object files being listed individually on the 
+    # so for now we depend on object files being listed individually on the
     # command line, which should work for simple cases.  We do bring in our
     # own version of C<ExtUtils::Liblist::Kid::ext> so that any additional
     # libraries (including PERLSHR) can be added to the options file.
@@ -85,7 +85,7 @@ sub _do_link {
     my @optlibs = $self->_liblist_ext( $args{'libs'} );
 
     my $optfile = 'sys$disk:[]' . $temp_files[0];
-    open my $opt_fh, '>>', $optfile 
+    open my $opt_fh, '>>', $optfile
         or die "_do_link: Unable to open $optfile: $!";
     for my $lib (@optlibs) {print $opt_fh "$lib\n" if length $lib }
     close $opt_fh;
@@ -136,7 +136,7 @@ sub _liblist_ext {
   # which a system-wide logical may point.
   if ($self->perl_src) {
     my($lib,$locspec,$type);
-    foreach $lib (@crtls) { 
+    foreach $lib (@crtls) {
       if (($locspec,$type) = $lib =~ m{^([\w\$-]+)(/\w+)?} and $locspec =~ /perl/i) {
         if    (lc $type eq '/share')   { $locspec .= $self->{'config'}{'exe_ext'}; }
         elsif (lc $type eq '/library') { $locspec .= $self->{'config'}{'lib_ext'}; }
@@ -188,8 +188,8 @@ sub _liblist_ext {
       next;
     }
     warn "Resolving directory $dir\n" if $verbose;
-    if (!File::Spec->file_name_is_absolute($dir)) { 
-        $dir = catdir($cwd,$dir); 
+    if (!File::Spec->file_name_is_absolute($dir)) {
+        $dir = catdir($cwd,$dir);
     }
   }
   @dirs = grep { length($_) } @dirs;
@@ -243,14 +243,14 @@ sub _liblist_ext {
           $type = 'SHR';
           $name = $fullname unless $fullname =~ /exe;?\d*$/i;
         }
-        elsif (not length($ctype) and  # If we've got a lib already, 
+        elsif (not length($ctype) and  # If we've got a lib already,
                                        # don't bother
                ( -f ($fullname = VMS::Filespec::rmsexpand($name,$lib_ext)) or
                  -f ($fullname = VMS::Filespec::rmsexpand($name,'.olb'))))  {
           $type = 'OLB';
           $name = $fullname unless $fullname =~ /olb;?\d*$/i;
         }
-        elsif (not length($ctype) and  # If we've got a lib already, 
+        elsif (not length($ctype) and  # If we've got a lib already,
                                        # don't bother
                ( -f ($fullname = VMS::Filespec::rmsexpand($name,$obj_ext)) or
                  -f ($fullname = VMS::Filespec::rmsexpand($name,'.obj'))))  {
@@ -264,9 +264,9 @@ sub _liblist_ext {
           last if $ctype eq 'SHR';
         }
       }
-      if ($ctype) { 
+      if ($ctype) {
         push @{$found{$ctype}}, $cand;
-        warn "\tFound as $cand (really $fullname), type $ctype\n" 
+        warn "\tFound as $cand (really $fullname), type $ctype\n"
           if $verbose > 1;
        push @flibs, $name unless $libs_seen{$fullname}++;
         next LIB;
index 472c801..6c77fd4 100644 (file)
@@ -1,5 +1,5 @@
 package ExtUtils::CBuilder::Platform::Windows;
-$ExtUtils::CBuilder::Platform::Windows::VERSION = '0.280221';
+$ExtUtils::CBuilder::Platform::Windows::VERSION = '0.280222';
 use strict;
 use warnings;
 
@@ -57,7 +57,7 @@ sub split_like_shell {
   # array) to the target program and make the program parse it itself,
   # we don't actually need to do any processing here.
   (my $self, local $_) = @_;
-  
+
   return @$_ if defined() && UNIVERSAL::isa($_, 'ARRAY');
   return unless defined() && length();
   return ($_);
@@ -76,7 +76,7 @@ sub do_system {
 sub arg_defines {
   my ($self, %args) = @_;
   s/"/\\"/g foreach values %args;
-  return map qq{"-D$_=$args{$_}"}, keys %args;
+  return map qq{"-D$_=$args{$_}"}, sort keys %args;
 }
 
 sub compile {
@@ -85,7 +85,7 @@ sub compile {
 
   die "Missing 'source' argument to compile()" unless defined $args{source};
 
-  $args{include_dirs} = [ $args{include_dirs} ] 
+  $args{include_dirs} = [ $args{include_dirs} ]
     if exists($args{include_dirs}) && ref($args{include_dirs}) ne "ARRAY";
 
   my ($basename, $srcdir) =
index 3b525b7..5daed5f 100644 (file)
@@ -1,7 +1,7 @@
 #! perl -w
 
 use strict;
-use Test::More tests => 64;
+use Test::More tests => 65;
 use Config;
 use Cwd;
 use File::Path qw( mkpath );
@@ -152,7 +152,6 @@ my ($lib, @temps);
     ok( -d $include_dir, "perl_inc() returned directory" );
 }
 
-#
 $base = ExtUtils::CBuilder::Base->new( quiet => 1 );
 ok( $base, "ExtUtils::CBuilder::Base->new() returned true value" );
 isa_ok( $base, 'ExtUtils::CBuilder::Base' );
@@ -165,13 +164,27 @@ my %args = ();
 my @defines = $base->arg_defines( %args );
 ok( ! @defines, "Empty hash passed to arg_defines() returns empty list" );
 
-%args = ( alpha => 'beta', gamma => 'delta' );
-my $defines_seen_ref = { map { $_ => 1 } $base->arg_defines( %args ) };
+my @epsilon = ( epsilon => 'zeta' );
+my @eta     = ( eta => 'theta' );
+my @alpha   = ( alpha => 'beta' );
+my @gamma   = ( gamma => 'delta' );
+my @all = (\@epsilon, \@eta, \@alpha, \@gamma);
+
+%args = map { @{$_} } @all;
+@defines = $base->arg_defines( %args );
+my $defines_seen_ref = { map { $_ => 1 } @defines };
+my $defines_expected_ref;
+for my $r (@all) {
+    $defines_expected_ref->{"-D$r->[0]=$r->[1]"} = 1;
+}
 is_deeply(
     $defines_seen_ref,
-    { '-Dalpha=beta' => 1, '-Dgamma=delta' => 1 },
+    $defines_expected_ref,
     "arg_defines(): got expected defines",
 );
+my $ordered_defines_expected_ref = [ sort keys %{$defines_expected_ref} ];
+is_deeply(\@defines, $ordered_defines_expected_ref,
+    "Got expected order of defines: RT #124106");
 
 my $include_dirs_seen_ref =
     { map {$_ => 1} $base->arg_include_dirs( qw| alpha beta gamma | ) };