Communicating about the change in behaviour for ar

[Manoj Srivastava]

Hi,

I was trying to import the new version of make into unstable, and
 I discovered that t/10 tests about the archive related part of makes
 test suite failed. The reason was a change in the behaviour of ar,
 which is now configured with --enable-deterministic-archives. When
 adding files and the archive index use zero for UIDs, GIDs, timestamps,
 and use consistent file modes for all files.  When this option is used,
 if ar is used with identical options and identical input files,
 multiple runs will create identical output files regardless of the
 input files' owners, groups, file modes, or modification times. This
 seems like good news for reproducible builds.

Unfortunately, when using makes libxx(*.a) syntax for creating
 archives, make needs the timestamp of the file in order to decide to
 update it or not. With the current deterministic behavior of ar, the
 timestamp is always 0. This is behaviour that is not backwards
 compatible (as can be seen in the bug report #798804 and
 #798913). Some operations, instead of being no-ops, now rebuild theg/Lunar
 archive.

T think the change, because of the benefits of the
 reproducible builds, are a good thing, but I also think that they are
 not visible to the general user base (not all the users of ar and make
 are debian developers, and the release is not the only thing built
 using ar and make).  My recommendations is a NEWS file entry for
 binutils and make; and a mention in the release announcement for
 reproducible builds.

I have uploaded a version of make the defaults to adding U to the
 ARFLAGS, but, on research and reflection, I would be open to reverting
 that and adding a NEWS entry.

Manoj
-- 
Anything worth doing is worth overdoing.
Manoj Srivastava    
4096R/C5779A1C E37E 5EC5 2A01 DA25 AD20  05B6 CF48 9438 C577 9A1C

Re: Communicating about the change in behaviour for ar

[Jérémy Bobbio]

Manoj Srivastava:
> I was trying to import the new version of make into unstable, and
>  I discovered that t/10 tests about the archive related part of makes
>  test suite failed. The reason was a change in the behaviour of ar,
>  which is now configured with --enable-deterministic-archives. When
>  adding files and the archive index use zero for UIDs, GIDs, timestamps,
>  and use consistent file modes for all files.  When this option is used,
>  if ar is used with identical options and identical input files,
>  multiple runs will create identical output files regardless of the
>  input files' owners, groups, file modes, or modification times. This
>  seems like good news for reproducible builds.
> 
> Unfortunately, when using makes libxx(*.a) syntax for creating
>  archives, make needs the timestamp of the file in order to decide to
>  update it or not. With the current deterministic behavior of ar, the
>  timestamp is always 0. This is behaviour that is not backwards
>  compatible (as can be seen in the bug report #798804 and
>  #798913). Some operations, instead of being no-ops, now rebuild theg/Lunar
>  archive.
> 
> T think the change, because of the benefits of the
>  reproducible builds, are a good thing, but I also think that they are
>  not visible to the general user base (not all the users of ar and make
>  are debian developers, and the release is not the only thing built
>  using ar and make).  My recommendations is a NEWS file entry for
>  binutils and make; and a mention in the release announcement for
>  reproducible builds.
> 
> I have uploaded a version of make the defaults to adding U to the
>  ARFLAGS, but, on research and reflection, I would be open to reverting
>  that and adding a NEWS entry.

In any case, I think we should communicate to users that something
unexpected (from the point view of make) is happening so they can adapt
their Makefile. What would you think of the attached patch series?

The warning is actually implemented in the second patch, but the first
one is required to be able to differentiate a non-existent archive or
member from a member with a timestamp set to 0.

-- 
Lunar.''`. 
lu...@debian.org: :Ⓐ  :  # apt-get install anarchism
`. `'` 
  `-   
From a40d198fb5a3a0387ce5b4f0f40e23cab2a535bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Bobbio?= 
Date: Sun, 17 Jan 2016 10:55:40 +
Subject: [PATCH 1/2] Make ar_member_date compatible with archives with
 timestamps set to 0

ar_scan() scanning function uses 0 to indicate that scanning should continue.
This made ar_member_date() unable to differentiate when it was unable to find
the requested member from a member with a timestamp set to 0.

We thus change its prototype to have its return value indicate if it has been
able to find the requested member. The timestamp is set through the newly given
pointer.
---
 ar.c   | 33 +
 commands.c |  5 -
 dir.c  |  7 +--
 makeint.h  |  2 +-
 remake.c   |  7 +++
 5 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/ar.c b/ar.c
index 675572a..d10a8df 100644
--- a/ar.c
+++ b/ar.c
@@ -68,25 +68,39 @@ ar_parse_name (const char *name, char **arname_p, char **memname_p)
 
 /* This function is called by 'ar_scan' to find which member to look at.  */
 
+struct member_date_lookup
+{
+  const char *name;
+  time_t *member_date;
+};
+
 /* ARGSUSED */
 static long int
 ar_member_date_1 (int desc UNUSED, const char *mem, int truncated,
   long int hdrpos UNUSED, long int datapos UNUSED,
   long int size UNUSED, long int date,
   int uid UNUSED, int gid UNUSED, int mode UNUSED,
-  const void *name)
+  const void *data)
 {
-  return ar_name_equal (name, mem, truncated) ? date : 0;
+  const struct member_date_lookup *lookup_data = data;
+  if (ar_name_equal (lookup_data->name, mem, truncated))
+{
+  *lookup_data->member_date = date;
+  return 1;
+}
+  return 0;
 }
 
-/* Return the modtime of NAME.  */
+/* Read the modtime of NAME in MEMBER_DATE.
+   Returns 1 if NAME exists, 0 otherwise.  */
 
-time_t
-ar_member_date (const char *name)
+int
+ar_member_date (const char *name, time_t *member_date)
 {
   char *arname;
   char *memname;
-  long int val;
+  int found;
+  struct member_date_lookup lookup_data;
 
   ar_parse_name (name, , );
 
@@ -107,11 +121,14 @@ ar_member_date (const char *name)
   (void) f_mtime (arfile, 0);
   }
 
-  val = ar_scan (arname, ar_member_date_1, memname);
+  lookup_data.name = memname;
+  lookup_data.member_date = member_date;
+  found = ar_scan (arname, ar_member_date_1, _data);
 
   free (arname);
 
-  return (val <= 0 ? (time_t) -1 : (time_t) val);
+  /* return 0 (not found) if the archive does not exist or has invalid