In Perl 5.18 I changed Perl to use a randomly chosen hash seed each
invocation (I am p5p committer). This means that any code that depends
on the native key order of a hash is non-deterministic.
autoconf includes various places where this is the case, which makes
it annoying to see what *actually* changed from run to run.
The attached patch ensures that all use of keys() in list context is
sorted, including indirectly places like Data::Dumper.
I took the approach that autoconf is not performance sensitive and
that sorting things that strictly speaking do not need to be sorted is
not an issue, especially if it reduces cognitive load in trying to
tell if such use *could* be an issue.
This patch is not well tested, however I believe it should be safe. It
passes make check.
Thanks,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
commit e78a11581b51254a19c0b2d8994af8fec4ba1bfd
Author: Yves Orton <demer...@gmail.com>
Date: Mon Feb 6 04:21:02 2017 +0100
Ensure deterministic behavior, sort all keys()
In Perl 5.18 I changed how Perls hash function was initialized
so that every process start it would use a different seed. This
results in guaranteed non-deterministic behavior of functions like
keys() (as opposed to the possibly non-deterministic behavior
previously), this means that various bits of autoconf were producing
inconsistent output from run to run.
This patch corrects and documents all list-context uses of keys() to
ensure that the code is fully deterministic.
diff --git a/bin/autom4te.in b/bin/autom4te.in
index 964ac1a..1a203e5 100644
--- a/bin/autom4te.in
+++ b/bin/autom4te.in
@@ -512,6 +512,13 @@ EOF
}
}
+sub _keys_to_pattern {
+ my ($hash) = @_;
+ return '\b(' . join ('|',
+ sort { length($b) <=> length($a) || $a cmp $b }
+ keys %$hash) . ')\b';
+}
+
# handle_output ($REQ, $OUTPUT)
# -----------------------------
@@ -598,7 +605,7 @@ sub handle_output ($$)
$exit_code = 1;
if ($ARGV[$#ARGV] ne '-')
{
- my $prohibited = '\b(' . join ('|', keys %prohibited) . ')\b';
+ my $prohibited = _keys_to_pattern(\%prohibited);
my $file = new Autom4te::XFile ($ARGV[$#ARGV], "<");
while ($_ = $file->getline)
@@ -617,12 +624,13 @@ sub handle_output ($$)
# If we're done, exit.
return
if ! %prohibited;
- $prohibited = '\b(' . join ('|', keys %prohibited) . ')\b';
+ $prohibited = _keys_to_pattern(\%prohibited);
}
}
}
warn_forbidden ("$output:$prohibited{$_}", $_, %forbidden)
- foreach (sort { $prohibited{$a} <=> $prohibited{$b} } keys %prohibited);
+ foreach (sort { $prohibited{$a} <=> $prohibited{$b} || $a cmp $b }
+ keys %prohibited);
}
@@ -1000,7 +1008,7 @@ Autom4te::C4che->load ($icache_file)
# Add the new trace requests.
my $req = Autom4te::C4che->request ('input' => \@ARGV,
'path' => \@include,
- 'macro' => [keys %trace, @preselect]);
+ 'macro' => [sort keys %trace, @preselect]);
# If $REQ's cache files are not up to date, or simply if the user
# discarded them (-f), declare it invalid.
@@ -1012,7 +1020,7 @@ verb "the trace request object is:\n" . $req->marshall;
# We need to run M4 if (i) the user wants it (--force), (ii) $REQ is
# invalid.
-handle_m4 ($req, keys %{$req->macro})
+handle_m4 ($req, sort keys %{$req->macro})
if $force || ! $req->valid;
# Issue the warnings each time autom4te was run.
diff --git a/bin/autoupdate.in b/bin/autoupdate.in
index 274d52e..d54a127 100644
--- a/bin/autoupdate.in
+++ b/bin/autoupdate.in
@@ -166,15 +166,15 @@ sub handle_autoconf_macros ()
# Don't keep AU macros in @AC_MACROS.
delete $ac_macros{$_}
- foreach (keys %au_macros);
+ foreach (keys %au_macros); # sort keys not needed
# Don't keep M4sugar macros which are redefined by Autoconf,
# such as 'builtin', 'changequote' etc. See autoconf/autoconf.m4.
delete $ac_macros{$_}
- foreach (keys %m4_builtins);
+ foreach (keys %m4_builtins); # sort keys not needed
error "no current Autoconf macros found"
- unless keys %ac_macros;
+ unless keys %ac_macros; # sort keys not needed
error "no obsolete Autoconf macros found"
- unless keys %au_macros;
+ unless keys %au_macros; # sort keys not needed
if ($debug)
{
@@ -229,8 +229,9 @@ mktmpdir ('au');
handle_autoconf_macros;
# $au_changequote -- enable the quote '[', ']' right before any AU macro.
-my $au_changequote =
- 's/\b(' . join ('|', keys %au_macros) . ')\b/_au_m4_changequote([,])$1/g';
+my $au_changequote = 's/\b('
+ . join ('|', sort { length($b) <=> length($a) || $a cmp $b } keys %au_macros)
+ . ')\b/_au_m4_changequote([,])$1/g';
# au.m4 -- definitions the AU macros.
xsystem ("$autoconf --trace AU_DEFINE:'_au_defun(\@<:\@\$1\@:>\@,
diff --git a/build-aux/gendocs.sh b/build-aux/gendocs.sh
index 1d1e1e7..666a829 100755
--- a/build-aux/gendocs.sh
+++ b/build-aux/gendocs.sh
@@ -250,8 +250,8 @@ BEGIN {
/<img src="(.*?)"/g && ++$need{$1};
END {
- #print "$me: @{[keys %need]}\n"; # for debugging, show images found.
- FILE: for my $f (keys %need) {
+ #print "$me: @{[sort keys %need]}\n"; # for debugging, show images found.
+ FILE: for my $f (sort keys %need) {
for my $d (@dirs) {
if (-f "$d/$f") {
use File::Basename;
diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
index a2513d0..04d291f 100755
--- a/build-aux/gitlog-to-changelog
+++ b/build-aux/gitlog-to-changelog
@@ -472,7 +472,7 @@ sub git_dir_option($)
# Complain about any unused entry in the --amend=F specified file.
my $fail = 0;
- foreach my $sha (keys %$amend_code)
+ foreach my $sha (sort keys %$amend_code)
{
warn "$ME:$amend_file: unused entry: $sha\n";
$fail = 1;
diff --git a/lib/Autom4te/C4che.pm b/lib/Autom4te/C4che.pm
index 4f7ce93..66fd685 100644
--- a/lib/Autom4te/C4che.pm
+++ b/lib/Autom4te/C4che.pm
@@ -155,7 +155,7 @@ sub marshall ($)
my $res = '';
my $marshall = Data::Dumper->new ([\@request], [qw (*request)]);
- $marshall->Indent(2)->Terse(0);
+ $marshall->Sortkeys(1)->Indent(2)->Terse(0);
$res = $marshall->Dump . "\n";
return $res;
diff --git a/lib/Autom4te/Channels.pm b/lib/Autom4te/Channels.pm
index 67601a0..5fcf144 100644
--- a/lib/Autom4te/Channels.pm
+++ b/lib/Autom4te/Channels.pm
@@ -290,7 +290,7 @@ sub _reset_duplicates (\%)
{
my ($ref) = @_;
my $dup = 0;
- foreach my $k (keys %$ref)
+ foreach my $k (sort keys %$ref)
{
$dup += $ref->{$k};
}
@@ -332,7 +332,7 @@ sub _merge_options (\%%)
my ($hash, %options) = @_;
local $_;
- foreach (keys %options)
+ foreach (sort keys %options)
{
if (exists $hash->{$_})
{
@@ -693,7 +693,7 @@ with those specified by C<%options>.
sub setup_channel_type ($%)
{
my ($type, %opts) = @_;
- foreach my $channel (keys %channels)
+ foreach my $channel (sort keys %channels)
{
setup_channel $channel, %opts
if $channels{$channel}{'type'} eq $type;
@@ -722,7 +722,7 @@ use vars qw (@_saved_channels @_saved_werrors);
sub dup_channel_setup ()
{
my %channels_copy;
- foreach my $k1 (keys %channels)
+ foreach my $k1 (sort keys %channels)
{
$channels_copy{$k1} = {%{$channels{$k1}}};
}
@@ -786,7 +786,7 @@ and the key to use for serialization.
sub setup_channel_queue ($$)
{
my ($queue, $key) = @_;
- foreach my $channel (keys %channels)
+ foreach my $channel (sort keys %channels)
{
setup_channel $channel, queue => $queue, queue_key => $key
if $channels{$channel}{'ordered'};
diff --git a/lib/Autom4te/Getopt.pm b/lib/Autom4te/Getopt.pm
index 7080325..56f7223 100644
--- a/lib/Autom4te/Getopt.pm
+++ b/lib/Autom4te/Getopt.pm
@@ -62,7 +62,7 @@ sub parse_options (%)
if (@ARGV && $ARGV[0] =~ /^-./)
{
my %argopts;
- for my $k (keys %option)
+ for my $k (sort keys %option)
{
if ($k =~ /(.*)=s$/)
{
