This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] GNU_AR_DETERMINISTIC env var, --enable-deterministic-archives


On Tue, Dec 13, 2011 at 5:29 AM, nick clifton <nickc@redhat.com> wrote:
> 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 agree with this as a general principle.  (But it's worth pointing out
that GCC is indeed affected by various environment variables--which, at
least, are documented in their own section of the GCC manual.)  However, I
don't think that this is worse than the royal pain in the ass of trying to
modify lots of packages.

> 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.

Frankly, I think this makes a mountain of a mole-hill.  (Anyone ever using
the "u" option is probably a far more difficult and subtle problem today.)

> 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.

I don't think I have to tell you what a monumental pain in the ass this is
going to be in practice.  I agree it would eventually result in a better
situation, bringing both uniformity and configurability to the use of ar in
many packages.  But for effort to effect ratio, it's just beyond the pale.

> 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.

Well, it's just not very true at all about GCC.  But I take your point.
However, you didn't say anything about the build-time configure option.

> 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.

Understood.

>> 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.

I'd be happy to add such an option.

> 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.

What do you have in mind?  It's easy enough to add the information to
--help output, but that is not programmatically friendly.  But I don't
really see why someone really needs to know the default.  If they care
especially about the behavior, having options to explicitly set it one way
or the other seems like the most useful thing.

> 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.

I have no problem with adding the warning.

> So setting "GNU_AR_DETERMINISTIC=no" will enable determinism... ?I admit
> that it matches your proposed new documentation, but it still seems counter
> intuitive.

Well, I didn't want to get into a lot of value-parsing.  I wouldn't object
to a more precise requirement on the values, with an unrecognized value
causing a warning or error.


Thanks,
Roland


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]