On 08/07/2015 06:45 AM, Johannes Schauer wrote:
> Hi,
> 
> just some (hopefully helpful) comments about your freeipmi patch. Not CC-ing
> the bug so that it's completely up to you whether you want to address my
> comments or not.
> 
> Quoting Dhole (2015-08-06 19:13:22)
>> The attached patch replaces the timestamp in the docs with the latest
>> debian/changelog entry timestamp. Once applied, freeipmi can be built
>> reproducibly in our current experimental framework.
>>
>> [...]
>>
>> --- freeipmi-1.4.9/debian/patches/honour-SOURCE_DATE_EPOCH.patch        
>> 1970-01-01 01:00:00.000000000 +0100
>> +++ freeipmi-1.4.9/debian/patches/honour-SOURCE_DATE_EPOCH.patch        
>> 2015-08-06 17:18:43.000000000 +0200
>> @@ -0,0 +1,21 @@
>> +Description: Honour SOURCE_DATE_EPOCH in manpages
>> + .
>> + freeipmi (1.4.9-1.1) UNRELEASED; urgency=medium
>> + .
>> +   * Non-maintainer upload.
>> +   * Replace man pages dates with the timestamp from the latest
>> +     debian/changelog entry using the variable SOURCE_DATE_EPOCH to make the
>> +     package build reproducibly.
>> +Author: Eduard Sanou <dh...@openmailbox.org>
> 
> Usually, you would not put the debian/changelog entry as a patch Description.
> See here for samples of how to describe patches best and which headers you can
> and/or should use: http://dep.debian.net/deps/dep3/
> 

I just took the lazy approach to leave the template message generated by
`dpkg-source --commit`. I've written a properly formatted description now.

>> +--- freeipmi-1.4.9.orig/configure
>> ++++ freeipmi-1.4.9/configure
>> +@@ -2723,7 +2723,7 @@ ac_config_headers="$ac_config_headers co
>> + ac_config_files="$ac_config_files freeipmi.spec Makefile 
>> bmc-device/Makefile bmc-info/Makefile bmc-watchdog/Makefile common/Makefile 
>> common/debugutil/Makefile common/miscutil/Makefile 
>> common/parsecommon/Makefile common/pingtool/Makefile 
>> common/portability/Makefile common/toolcommon/Makefile contrib/Makefile 
>> doc/Makefile etc/Makefile ipmi-chassis/Makefile ipmi-config/Makefile 
>> ipmi-dcmi/Makefile ipmi-fru/Makefile ipmi-locate/Makefile ipmi-oem/Makefile 
>> ipmi-pet/Makefile ipmi-raw/Makefile ipmi-sel/Makefile ipmi-sensors/Makefile 
>> ipmiconsole/Makefile ipmidetect/Makefile ipmidetectd/Makefile 
>> ipmiping/Makefile ipmipower/Makefile ipmiseld/Makefile libfreeipmi/Makefile 
>> libfreeipmi/libfreeipmi.pc libfreeipmi/include/Makefile 
>> libfreeipmi/include/freeipmi/freeipmi.h libipmiconsole/Makefile 
>> libipmiconsole/ipmiconsole.h libipmiconsole/libipmiconsole.pc 
>> libipmidetect/Makefile libipmidetect/ipmidetect.h 
>> libipmidetect/libipmidetect.pc libipmimonitoring/Makefile 
>> libipmimonitoring/ipmi_monitoring.h 
libipmimonitoring/libipmimonitoring.pc man/Makefile man/bmc-device.8.pre 
man/bmc-info.8.pre man/bmc-watchdog.8.pre man/freeipmi.conf.5.pre 
man/freeipmi.7.pre man/ipmi-chassis.8.pre man/ipmi-config.8.pre 
man/ipmi-config.conf.5.pre man/ipmi-dcmi.8.pre man/ipmi-fru.8.pre 
man/ipmi-locate.8.pre man/ipmi-oem.8.pre man/ipmi-pet.8.pre man/ipmi-raw.8.pre 
man/ipmi-sel.8.pre man/ipmi-sensors.8.pre man/ipmiconsole.8.pre 
man/ipmidetect.8.pre man/ipmidetect.conf.5.pre man/ipmidetectd.8.pre 
man/ipmidetectd.conf.5.pre man/ipmiping.8.pre man/ipmipower.8.pre 
man/ipmiseld.8.pre man/ipmiseld.conf.5.pre man/libfreeipmi.3.pre 
man/freeipmi_interpret_sensor.conf.5.pre man/freeipmi_interpret_sel.conf.5.pre 
man/libipmiconsole.3.pre man/libipmiconsole.conf.5.pre man/libipmidetect.3.pre 
man/libipmimonitoring.3.pre man/rmcpping.8.pre rmcpping/Makefile"
>> + 
>> + 
>> +-ISODATE=`date +%Y-%m-%d`
>> ++ISODATE=`date -u -d @$SOURCE_DATE_EPOCH +%Y-%m-%d`
>> + 
>> + 
>> + # libfreeipmi libtool versioning
> 
> The file you are patching here, ./configure, is autogenerated from another 
> file
> called ./configure.ac in the freeimpi sources. Maintainers are encouraged to
> rebuild their ./configure file from ./configure.ac at build time for various
> reasons but even if the freeimpi maintainer does not decide to do that in the
> near future, your changes will probably be overwritten by the next upstream
> release which changes the ./configure.ac and thus regenerates ./configure. It
> is thus better to patch the source of ./configure in ./configure.ac about 
> here:
> http://sources.debian.net/src/freeipmi/1.4.9-1/configure.ac/#L109 and suggest
> to upstream to call dh with `--with --autoreconf` instead of their current
> `--with autotools_dev` in debian/rules.
> 

I actually missed the configure.ac! Thanks for noticing. I've written
the patch targeting configure.ac now, and changed the dh from autotools
to autoreconf (Otherwise only autoconfigure is used and the change
doesn't get processed). I'll suggest the maintainer to do this change too.

> Secondly, you might want to consider to write the patch such that it can be
> accepted by upstream. The first part of that is to make the changes against
> ./configure.ac instead of ./configure as described above. The other 
> requirement
> would be that you conditionally use $SOURCE_DATE_EPOCH depending on whether it
> has been set or not. In its current state, upstream would not be able to 
> accept
> your patch, even if it was against ./configure.ac, because nobody would set
> $SOURCE_DATE_EPOCH and thus their build would fail. It is in the interest of
> the Debian maintainer that they can forward your patches to upstream and get
> them accepted there so that they don't have to carry and maintain your patch
> forever.
> 

You're right on that! I've just followed your suggestion :)

>> --- freeipmi-1.4.9/debian/rules 2015-07-01 16:09:12.000000000 +0200
>> +++ freeipmi-1.4.9/debian/rules 2015-08-06 17:12:52.000000000 +0200
>> @@ -1,6 +1,8 @@
>>  #!/usr/bin/make -f
>>  # -*- makefile -*-
>>  
>> +export SOURCE_DATE_EPOCH = $(shell date -d "$$(dpkg-parsechangelog --count 
>> 1 -SDate)" +%s)
>> +
>>  # --fail-missing for dh_install
>>  # --link-doc for dh_installdocs
>>  %:
> 
> It looks to me as if freeipmi was using dh for building. Are you sure that you
> have to manually export SOURCE_DATE_EPOCH? What is the condition under which
> one has to export SOURCE_DATE_EPOCH even in dh-based debian/rules files?
> 

You're right, exporting SOURCE_DATE_EPOCH is not needed here, I just
added that line mechanically. The build happens under dh so it will pick
the SOURCE_DATE_ECPOH exported by dh.

> Thanks for your work!
> 
> cheers, josch
> 

Thanks for the comments! I've attached a patch with all your comments
addressed. Let me know if you find it good, and I'll send it to the bug
I opened.

PS: I had to modify the Build-Depends: removing autotools-dev and adding
dh-autoreconf. I hope that's the correct way.


-- 
Dhole
diff -Nru freeipmi-1.4.9/debian/changelog freeipmi-1.4.9/debian/changelog
--- freeipmi-1.4.9/debian/changelog     2015-07-01 16:09:12.000000000 +0200
+++ freeipmi-1.4.9/debian/changelog     2015-08-07 17:06:34.000000000 +0200
@@ -1,3 +1,12 @@
+freeipmi (1.4.9-1.1) UNRELEASED; urgency=medium
+
+  * Non-maintainer upload.
+  * Replace man pages dates with the timestamp from the latest
+    debian/changelog entry using the variable SOURCE_DATE_EPOCH to make the
+    package build reproducibly. Move from autotools to autoreconf.
+
+ -- Eduard Sanou <dh...@openmailbox.org>  Fri, 07 Aug 2015 17:06:03 +0200
+
 freeipmi (1.4.9-1) unstable; urgency=medium
 
   * Fresh upstream release
diff -Nru freeipmi-1.4.9/debian/control freeipmi-1.4.9/debian/control
--- freeipmi-1.4.9/debian/control       2015-07-01 16:09:12.000000000 +0200
+++ freeipmi-1.4.9/debian/control       2015-08-07 17:32:01.000000000 +0200
@@ -4,7 +4,7 @@
 Maintainer: Debian FreeIPMI Maintainers 
<pkg-freeipmi-de...@lists.alioth.debian.org>
 Uploaders: Yaroslav Halchenko <deb...@onerussian.com>, Ferenc W√°gner 
<wf...@niif.hu>, Bernd Zeimetz <b...@debian.org>
 Build-Depends: debhelper (>= 9),
- autotools-dev (>= 20100122.1~),
+ dh-autoreconf,
  libgcrypt11-dev,
  chrpath
 Standards-Version: 3.9.3
diff -Nru freeipmi-1.4.9/debian/patches/0002-Honour-SOURCE_DATE_EPOCH.patch 
freeipmi-1.4.9/debian/patches/0002-Honour-SOURCE_DATE_EPOCH.patch
--- freeipmi-1.4.9/debian/patches/0002-Honour-SOURCE_DATE_EPOCH.patch   
1970-01-01 01:00:00.000000000 +0100
+++ freeipmi-1.4.9/debian/patches/0002-Honour-SOURCE_DATE_EPOCH.patch   
2015-08-07 18:07:12.000000000 +0200
@@ -0,0 +1,23 @@
+Description: Honour SOURCE_DATE_EPOCH in manpages
+ Replace man pages dates with the timestamp from the environment variable
+ SOURCE_DATE_EPOCH (latest debian/changelog tiestamp entry in Debian) in case 
+ it is set to make the package build reproducibly.
+Author: Eduard Sanou <dh...@openmailbox.org>
+
+Index: freeipmi-1.4.9/configure.ac
+===================================================================
+--- freeipmi-1.4.9.orig/configure.ac
++++ freeipmi-1.4.9/configure.ac
+@@ -106,7 +106,11 @@ AC_CONFIG_FILES([
+       man/rmcpping.8.pre
+         rmcpping/Makefile])
+ 
+-ISODATE=`date +%Y-%m-%d`
++if test -z "${SOURCE_DATE_EPOCH}"; then
++  ISODATE=`date +%Y-%m-%d`
++else
++  ISODATE=`date -u -d @$SOURCE_DATE_EPOCH +%Y-%m-%d`
++fi
+ AC_SUBST([ISODATE])
+ 
+ # libfreeipmi libtool versioning
diff -Nru freeipmi-1.4.9/debian/patches/series 
freeipmi-1.4.9/debian/patches/series
--- freeipmi-1.4.9/debian/patches/series        2015-07-01 16:09:12.000000000 
+0200
+++ freeipmi-1.4.9/debian/patches/series        2015-08-07 17:08:36.000000000 
+0200
@@ -1 +1,2 @@
 deb_bmc-watchdog_noRUN
+0002-Honour-SOURCE_DATE_EPOCH.patch
diff -Nru freeipmi-1.4.9/debian/rules freeipmi-1.4.9/debian/rules
--- freeipmi-1.4.9/debian/rules 2015-07-01 16:09:12.000000000 +0200
+++ freeipmi-1.4.9/debian/rules 2015-08-07 17:27:03.000000000 +0200
@@ -4,7 +4,7 @@
 # --fail-missing for dh_install
 # --link-doc for dh_installdocs
 %:
-       dh $@ --with autotools_dev --fail-missing --link-doc=freeipmi-common
+       dh $@ --with autoreconf --fail-missing --link-doc=freeipmi-common
 
 override_dh_auto_configure:
        : # suppress multiarch support for now

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds

Reply via email to