GNU bug report logs - #25629
Automake output non-deterministic when used with later Perls

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25629 in the body.
You can then email your comments to 25629 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: demerphq <demerphq <at> gmail.com>
To: bug-automake <at> gnu.org
Subject: Automake output non-deterministic when used with later Perls
Date: Mon, 6 Feb 2017 03:32:11 +0100

Some time back I switched Perl to using a randomly seeded hash
function. (I am a core perl committer, and I do a lot of work on its
hash function.)

A consequence of this is that default output from tools like
Data::Dumper is non-deterministic, meaning output files under source
control, etc, change unnecessarily every time.

At least part of the automake toolset is affected, for instance autom4te.

For autom4te a simple solution is to add:

$Data::Dumper::Sortkeys = 1;

to the script. This seems to fix the problem I was observing, but an
audit of the use of  the "keys" function to ensure that any such use
for output is sorted will eliminate this class of errors.

I would be happy to put together a pull request, or a patch if you
could point me to the relevant repository. (Which does not appear to
be documented in the installed versions of automake, although I may
have overlooked it.)

I am sorry I cannot give a better reproduction description than this:

while hacking the "dieharder" package I observed that after running
"autoreconf -i" that the file ./autom4te.cache/requests file changed
in ways that it should not have. Opening the file I observed it
contains output clearly from the Perl Data::Dumper module, I then made
the patch to autom4te described above, and observed that after running
"autoreconf -i" again the output was sorted, and that after running it
yet again the file was unchanged.

Classic unsorted use of undefined hash order bug that we had to do a
lot of cleanup in the Perl world around the time of Perl 5.10

Thanks,
Yves



-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Message #8 received at 25629 <at> debbugs.gnu.org (full text, mbox):

From: demerphq <demerphq <at> gmail.com>
To: 25629 <at> debbugs.gnu.org
Subject: Hrm, actually autom4te isnt part of automake, but rather autoconf.
Date: Mon, 6 Feb 2017 04:33:30 +0100

[Message part 1 (text/plain, inline)]

Also I observe that there were previous patches to ensure most uses of
keys() were sorted. However not all of them, including diagnostics,
and various other places where the key order could be exposed to a
user.

Attached is a patch that I believe fixes any remaining uses of unsorted keys.

I took the policy that automake is not performance sensitive, at least
at the level of sorting keys, and that sorting something that is not
strictly necessary does no harm.

I admit I have not been able to properly test all of these changes,
but superficial testing does not reveal any issues.

Anyway, sorry for the misleading original bug-report.

Cheers,
Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

[automake.patch (text/x-patch, attachment)]

Message #11 received at 25629 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Lirzin <mthl <at> gnu.org>
To: demerphq <demerphq <at> gmail.com>
Cc: 25629 <at> debbugs.gnu.org
Subject: Re: bug#25629: Hrm, actually autom4te isnt part of automake,
 but rather autoconf.
Date: Sun, 16 Jul 2017 01:49:50 +0200

Hello Yves,

demerphq <demerphq <at> gmail.com> writes:

> Also I observe that there were previous patches to ensure most uses of
> keys() were sorted. However not all of them, including diagnostics,
> and various other places where the key order could be exposed to a
> user.
>
> Attached is a patch that I believe fixes any remaining uses of unsorted keys.
>
> I took the policy that automake is not performance sensitive, at least
> at the level of sorting keys, and that sorting something that is not
> strictly necessary does no harm.
>
> I admit I have not been able to properly test all of these changes,
> but superficial testing does not reveal any issues.
>
> Anyway, sorry for the misleading original bug-report.
>
> Cheers,
> Yves

Can you explain what would be the benefit for Automake to have such
deterministic behavior?

Thanks for the report.

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

Message #16 received at 25629 <at> debbugs.gnu.org (full text, mbox):

From: demerphq <demerphq <at> gmail.com>
To: Mathieu Lirzin <mthl <at> gnu.org>
Cc: 25629 <at> debbugs.gnu.org
Subject: Re: bug#25629: Hrm, actually autom4te isnt part of automake, but
 rather autoconf.
Date: Sun, 16 Jul 2017 12:02:12 +0200

[Message part 1 (text/plain, inline)]

On 16 Jul 2017 01:50, "Mathieu Lirzin" <mthl <at> gnu.org> wrote:

Hello Yves,

demerphq <demerphq <at> gmail.com> writes:

> Also I observe that there were previous patches to ensure most uses of
> keys() were sorted. However not all of them, including diagnostics,
> and various other places where the key order could be exposed to a
> user.
>
> Attached is a patch that I believe fixes any remaining uses of unsorted
keys.
>
> I took the policy that automake is not performance sensitive, at least
> at the level of sorting keys, and that sorting something that is not
> strictly necessary does no harm.
>
> I admit I have not been able to properly test all of these changes,
> but superficial testing does not reveal any issues.
>
> Anyway, sorry for the misleading original bug-report.
>
> Cheers,
> Yves

Can you explain what would be the benefit for Automake to have such
deterministic behavior?

Thanks for the report.


The most obvious reason is when debugging with something like diff
deterministic behaviour eliminates spurious changes. Also having a
deterministic and well understood ordering rule eliminates any concern that
order might matter, and that a bug or whatnot was due to a different order
(regardless of whether such a concern actually makes sense.) also sorting
output lists makes them more readable.  I don't know how well tested
Automake is but I would assume deterministic hash order would help there
too.

Also note that Prior to hash randomization the key order would have been
consistent between multiple runs. Thus when randomization was introduced
the behaviour of Automake changed and effectively regressed. Sorting keys
effectively restores the original behaviour.

Cheers
Yves

[Message part 2 (text/html, inline)]

Message #19 received at 25629 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Lirzin <mthl <at> gnu.org>
To: demerphq <demerphq <at> gmail.com>
Cc: 25629 <at> debbugs.gnu.org
Subject: Re: bug#25629: Hrm, actually autom4te isnt part of automake,
 but rather autoconf.
Date: Sun, 16 Jul 2017 19:24:18 +0200

demerphq <demerphq <at> gmail.com> writes:

> On 16 Jul 2017 01:50, "Mathieu Lirzin" <mthl <at> gnu.org> wrote:
>
>  Can you explain what would be the benefit for Automake to have such
>  deterministic behavior?
>
>  Thanks for the report.
>
> The most obvious reason is when debugging with something like diff
> deterministic behaviour eliminates spurious changes. Also having a
> deterministic and well understood ordering rule eliminates any concern
> that order might matter, and that a bug or whatnot was due to a
> different order (regardless of whether such a concern actually makes
> sense.) also sorting output lists makes them more readable.  I don't
> know how well tested Automake is but I would assume deterministic hash
> order would help there too.
>
> Also note that Prior to hash randomization the key order would have
> been consistent between multiple runs. Thus when randomization was
> introduced the behaviour of Automake changed and effectively
> regressed. Sorting keys effectively restores the original behaviour.

IMO if the hash randomization introduces bugs in Automake this is the
sign of bad code on Automake side that need to be fixed.  I think it
would be better to fix those bugs in a case by case manner instead of
preemptively imposing an order on data structure models that are foreign
to the notion of order.

Could you run Automake test suite with those hash randomizations applied
and report the actual failures?

Thanks.

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

Message #22 received at 25629 <at> debbugs.gnu.org (full text, mbox):

From: Sam James <sam <at> gentoo.org>
To: 25629 <at> debbugs.gnu.org
Subject: Sorting updates in automake + autoconf
Date: Fri, 07 Jun 2024 13:45:41 +0100

See also https://git.savannah.gnu.org/cgit/automake.git/commit/?id=4a12201292d9f114549911cf5aaad15e1e5dce3c
and https://bugs.gnu.org/46744 linked therein.

On the autoconf side, see
http://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=commit;h=c2ab755698db245898a4cc89149eb5df256e4bd0.

Might be done here?

Message #25 received at 25629 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: demerphq <at> gmail.com
Cc: 25629 <at> debbugs.gnu.org
Subject: Re: bug#25629: Hrm, actually autom4te isnt part of automake,
 but rather autoconf.
Date: Sat, 15 Jun 2024 19:18:43 -0600

Hi Yves - back in 2017 you sent a patch to sort keys in automake:

    https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25629
    Attached is a patch that I believe fixes any remaining uses of
    unsorted keys.

I've now applied this, with the exception of
a) the cases that had already been done in the meantime,

b) gendocs.sh and gitlog-to-changelog, which are maintained in gnulib
(I forwarded the changes there and they have been applied), and

c) help2man, which is maintained separately; I would have forwarded that
change to bug-help2man <at> gnu.org except that as far as I can see this is a
case where truly no sorting is needed. It's purely an internal
assignment:
--- a/doc/help2man
+++ b/doc/help2man
@@ -204,7 +204,7 @@ while (@opt_include)
 # Compress trailing blank lines.
 for my $hash (\(%include, %append))
 {
-    for (keys %$hash) { $hash->{$_} =~ s/\n+$/\n/ }
+    for (sort keys %$hash) { $hash->{$_} =~ s/\n+$/\n/ }
 }
 
 sub get_option_value;

So I'm closing the bug.

Belated thanks for the contribution.
(And Sam, thanks for unearthing this and bringing it up again.)

Happy hashing,
karl

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.