[PATCH v2] tests: force non-deterministic mode in non-deterministic tests

[Stephen Kitt]

On Wed, 20 Dec 2023 09:03:37 +0100, Jan Beulich <jbeulich@suse.com> wrote:
> On 19.12.2023 22:53, Stephen Kitt wrote:
> > Since ar can be built defaulting to deterministic mode, tests which
> > expect non-deterministic behaviour need to explicitly set the U flag.
> > They also need to run without SOURCE_DATE_EPOCH since that also
> > enables deterministic mode.  
> 
> Not quite - the env var controls only time stamps, not UID/GID or
> permissions, aiui. That's merely a matter of changing the wording of
> course.

Right, I’ll update the wording.

> > --- a/binutils/testsuite/binutils-all/ar.exp
> > +++ b/binutils/testsuite/binutils-all/ar.exp
> > @@ -571,6 +571,8 @@ proc replacing_non_deterministic_member { } {
> >  	return
> >      }
> >  
> > +    unsetenv SOURCE_DATE_EPOCH
> > +
> >      set archive tmpdir/artest.a
> >      set older_objfile tmpdir/bintest.${obj}
> >      set newer_objfile tmpdir/ar/bintest.${obj}  
> 
> I think it would be nice if this was done in the place where,
> respectively, replacing_sde_deterministic_member has its setenv
> (and then, like that, if it also had a brief comment). Unless of
> course there's an issue with moving this down by a few lines.

Yes, that makes sense.

> Furthermore I'm afraid I'm less comfortable approving this, as the
> correctness of the unconditional unsetenv in
> replacing_sde_deterministic_member isn't really clear to me: If that
> variable was indeed set in the environment up front, it's not clear
> to me whether it really is okay to permanently alter the environment.
> Otoh of course your change merely moves the (possible) problem ahead
> by a tiny bit, so yes - with the cosmetic adjustments done the change
> is probably still okay.

The scenario I’m trying to address is indeed when SOURCE_DATE_EPOCH is set in
the environment on entry to the tests. This is what happens for example in
Debian builds, and that’s where I ran into this.
https://buildd.debian.org/status/fetch.php?pkg=binutils-mingw-w64&arch=amd64&ver=11.1&stamp=1702977480&raw=0
provides an example.

I think it’s fine for tests to make sure their environment is appropriate for
their own tests, but I agree it would be better for them to restore the
environment as it was on entry.

The Linaro build bot is complaining about this too; it’s failing because
unsetenv is called on a variable which isn’t present in the environment. It
seems I should check for the variable’s presence before unsetting it, or
perhaps just set it to empty...

Incidentally, builds with this patch in Debian all pass these tests (with
SOURCE_DATE_EPOCH set in the environment), but fail on i386 tests on
big-endian systems; see
https://buildd.debian.org/status/package.php?p=binutils-mingw-w64 for logs.
git bisect tells me that the culprit is
734dfd1cc966aff736eaeda68bfa4807ee4b50c1 but I haven’t figured out why yet.

Regards,

Stephen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://sourceware.org/pipermail/binutils/attachments/20231220/1729242c/attachment.sig>