This is the mail archive of the 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

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

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