This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] GNU_AR_DETERMINISTIC env var, --enable-deterministic-archives
- From: nick clifton <nickc at redhat dot com>
- To: Roland McGrath <mcgrathr at google dot com>
- Cc: binutils at sourceware dot org
- Date: Tue, 13 Dec 2011 13:29:49 +0000
- Subject: Re: [PATCH] GNU_AR_DETERMINISTIC env var, --enable-deterministic-archives
- References: <x57jsjkxfl0x.fsf@frobland.mtv.corp.google.com>
Hi Roland,
Sorry for the delay in responding. I was waiting to see if any of
the other maintainers had anything to say on this issue.
ar member headers are one of the last remaining common barriers to
reproducible builds (of development packages). Hence we have the D option.
But it is devilishly difficult to get the option override--or even AR and
RANLIB settings pointing to a wrapper script--through configure et al into
all corners of e.g. the gcc build.
Instead, this change lets the user set the environment variable
Eww, I hate using environment variables like this. It makes debugging
so much harder when you cannot rely upon the command line to describe
how a program was invoked.
I appreciate that the main thrust of this change is to make reproducible
builds possible without having to do a lot of work changing package
build scripts. But I think that that is a poor reason to introduce a
debugging nightmare like this.
In my opinion it is better to take on the work of changing all of the
packages one at a time, even if this will be a lot of effort. On the
plus side if you do this you can also simplify the package build scripts
at the same time. Perhaps by providing a library of useful command
lines that the scripts can access. Then future changes like this can be
simply made by just updating the command line in the library.
We have always resisted using environment variables to control program
behaviour in projects like gcc and the binutils and I see no reason why
this policy should change.
That said I have some other comments on the patch (see below). They are
more along the lines of commentary on the code for possible future
reference, rather than acceptance of the patch.
Users can still set GNU_AR_DETERMINISTIC=0 to reverse
the build-time default.
But they cannot use a command line option to do this, which strikes me
as wrong. If we are going to allow deterministic behaviour to be a
default then we must also provide a command line option to disable it.
Also - there needs to be some way for the user to find out how the
particular ar/ranlib that they are using was configured. Ie they need
to be able to find out if the default behaviour is for deterministic or
non-deterministic archives.
Since D is incompatible with u and some packages run "ar cru",
when the environment variable or build-time default sets D, we
just silently ignore u.
I am not happy with this either. Since "u" has been explicitly
requested by the user (well the package build script) we should not
silently ignore it. If "D" and "u" have both been specified on the
command line then a fatal warning makes sense. If on the other hand "D"
is just the default behaviour then a warning message would make more
sense. Then if the user really does want the "u" behaviour and no
warnings they can add the "not-D" command line option to their build script.
Ok for trunk?
No, sorry.
+ const char *env = getenv ("GNU_AR_DETERMINISTIC");
+ if (env != NULL&& env[0] != '\0')
+ {
+ if (isdigit(env[0]))
+ {
+ char *endp = NULL;
+ long int value = strtol (env,&endp, 0);
+ if (endp != NULL)
+ deterministic = value != 0;
+ }
+ else
+ deterministic = 1;
So setting "GNU_AR_DETERMINISTIC=no" will enable determinism... I admit
that it matches your proposed new documentation, but it still seems
counter intuitive.
Cheers
Nick