Hello Ian Jackson,
TL;DR alternative patch attached. Long story below.
On Sat, Dec 08, 2018 at 09:00:53PM +0000, Ian Jackson wrote:
[...]
> I'm sorry to say that while I think that this would be nice to fix in
> principle, I really don't like your patch. I don't think the appraoch
> you suggest is a good one in general.
It is the documented autotools solution, so basically what you're
saying is that you don't like autotools. I'm not a big fan of it myself
so won't argue with you on that.
My impression is that autotools was designed in an era where proprietary
unix systems was the norm and you could not expect system tools like
grep, sed, awk to behave sanely. You thus had to try to find (and
hardcode in your binary) the path to the alternative gnu version of the
tool. I don't think legacy unix systems are relevant anymore, but
ofcourse different upstream projects has different targets they want to
support. (The vm indirection of ls and rm makes me curious though, where
there really systems where these could not be expected to behave
sanely or why was this indirection added in the first place?)
Projects who still want to stick with autotools should today probably
consider to not use the result of AC_PROG_* and AC_PATH_PROG to
embed in their shipped files, but to instead just rely on PATH at
runtime. I spotted atleast a package which had a "package" configure
flag to indicate the built binaries where intended for redistribution
(rather than just for running on the build system). Maybe you can
convince your upstream to do something similar, but I guess in general
it'll be not an easy sell.
The alternative I'd suggest you look at would be if the Makefile
you ship is useful to include in the package. (I have no idea
how emacs plugins works.)
Maybe you can just stop distributing this file as an alternative
solution to making the package build reproducibly:
/usr/share/emacs/site-lisp/vm.in/Makefile
The above file is the only one that embeds the paths as can be seen at:
https://tests.reproducible-builds.org/debian/dbdtxt/unstable/amd64/vm_8.2.0b-4.diffoscope.txt.gz
OTOH, vm do declare a dependency on 'make' so maybe the file is shipped
for a good reason. Maybe you instead need to convince your upstream to
remove the indirection used in the Makefile(.in). (Patch attached.)
> I think it is a bug that anything tries to hardcode the path but that
> this needs to be fixed in one place, in autoconf, rather than by
> having every package which uses autoconf override invididual things.
> And this is complicated by the sad fact that some programs probably do
> need a hardcoded path for some things (for good or bad reasons).
>
> Was this bug report part of an MBF ? If so I think it was ill-advised.
I've worked on several packages individually and filed individual bug
reports, but they can ofcourse be seen as having a pattern and thus
be called a MBF.
All packages that have been identified as having reproducible build
problems on merged-usr vs non-merged systems should now have patches
(or are irrelevant, eg. because they're up for removal). The vm package
was one of the last ones I looked at because of its 0.00% vote score on
popcon.
>
> > Even more importantly you want to look at what lintian has to say about
> > your package.
>
> Thanks for the hint. I guess I should fix at least some of those but
> they dono't seem like crises...
In general missing eg. #DEBHELPER# token in maintscripts can generate
some pretty serious bugs, but I haven't spent any time looking at what
the actual implications are for this packages. They just seemed worth
looking at...
Regards,
Andreas Henriksson
diff -u vm-8.2.0b/debian/changelog vm-8.2.0b/debian/changelog
--- vm-8.2.0b/debian/changelog
+++ vm-8.2.0b/debian/changelog
@@ -1,3 +1,13 @@
+vm (8.2.0b-4.1) UNRELEASED; urgency=medium
+
+ * Non-maintainer upload.
+ * Add debian/patches/reproducible-build.patch
+ - removes useless indirections of rm and ls as embedding their
+ full paths could lead to embedding undesired paths in shipped
+ files (and thus a reproducibly build problem).
+
+ -- Andreas Henriksson <andr...@fatal.se> Mon, 10 Dec 2018 18:39:03 +0100
+
vm (8.2.0b-4) unstable; urgency=medium
* Do not rm version.txt (Closes: #914818).
--- vm-8.2.0b.orig/debian/patches/reproducible-build.patch
+++ vm-8.2.0b/debian/patches/reproducible-build.patch
@@ -0,0 +1,85 @@
+From: Andreas Henriksson <andr...@fatal.se>
+Subject: reproducible build - remove useless indirections
+
+Embedding the full path of tools from the build environment can lead
+to reproducible build problems (and incorrect paths embedded in shipped
+files) when building on a local dirty environment.
+
+Please note that this patch does *not* do anything about @INSTALL@ which
+could also be affected by this problem in some cases.
+
+Index: vm-8.2.0b/lisp/Makefile.in
+===================================================================
+--- vm-8.2.0b.orig/lisp/Makefile.in
++++ vm-8.2.0b/lisp/Makefile.in
+@@ -82,8 +82,6 @@ AUTOLOAD_FILE = (setq generated-autoload
+
+ ##############################################################################
+ # location of required programms
+-RM = @RM@
+-LS = @LS@
+ XARGS = @XARGS@
+ INSTALL = @INSTALL@
+ INSTALL_PROGRAM = @INSTALL_PROGRAM@
+@@ -133,7 +131,7 @@ version.txt:
+ # Since Debian compiles the files on the fly on the target machine,
+ # Do not depend on the abslute file path of the source directory to exist
+ vm-autoloads.el: $(SOURCES:%=@srcdir@/%)
+- -$(RM) -f $@
++ -rm -f $@
+ echo > $@
+ (build_dir="`pwd`"; cd "@srcdir@"; \
+ "$(EMACS_PROG)" $(FLAGS) -l autoload \
+@@ -150,13 +148,13 @@ vm-cus-load.el: $(SOURCES:%=@srcdir@/%)
+ "$(EMACS_PROG)" $(FLAGS) -f vm-custom-make-dependencies .
+ ifeq (@EMACS_VERSION@,21)
+ sed -e "s/provide 'cus-load/provide 'vm-cus-load/" cus-load.el > $@
+- $(RM) cus-load.el
++ rm cus-load.el
+ endif
+
+ ##############################################################################
+ # XEmacs#s auto-autoloads and custom-load file
+ auto-autoloads.el: $(SOURCES:%=@srcdir@/%)
+- -$(RM) -f $@
++ -rm -f $@
+ # (build_dir=`pwd`; cd "@srcdir@"; \
+ # $(EMACS_PROG) $(FLAGS) -l autoload \
+ # -f vm-built-autoloads "`pwd`/$@" "`pwd`")
+@@ -172,7 +170,7 @@ auto-autoloads.el: $(SOURCES:%=@srcdir@/
+ echo "(require 'vm-vars)" >> $@
+ echo "(setq vm-configure-datadir \"${datadir}\")" >> $@
+ echo "(setq vm-configure-pixmapdir \"${pixmapdir}\")" >> $@
+- $(RM) $@.tmp
++ rm $@.tmp
+
+
+ custom-load.el: $(SOURCES:%=@srcdir@/%)
+@@ -205,7 +203,7 @@ install-el: all $(INSTALL_FILES)
+ $(INSTALL_DATA) "${srcdir}/$$el" "$(DESTDIR)$(lispdir)/"; \
+ fi; \
+ done;
+- if $(LS) $(contrib)/*.elc > /dev/null 2>&1; then \
++ if ls $(contrib)/*.elc > /dev/null 2>&1; then \
+ for elc in $(contribdir)/*.elc; do \
+ el=`basename $$elc c`; \
+ if test -f "${srcdir}/$(contribdir)/$$el"; then \
+@@ -221,7 +219,7 @@ install-elc: all $(INSTALL_FILES)
+ echo "Install $$elc in $(DESTDIR)$(lispdir)/"; \
+ $(INSTALL_DATA) $$elc "$(DESTDIR)$(lispdir)/"; \
+ done;
+- if $(LS) $(contribdir)/*.elc > /dev/null 2>&1; then \
++ if ls $(contribdir)/*.elc > /dev/null 2>&1; then \
+ for elc in $(contribdir)/*.elc; do \
+ echo "Install $$elc in $(DESTDIR)$(lispdir)"; \
+ $(INSTALL_DATA) $$elc "$(DESTDIR)$(lispdir)"; \
+@@ -241,7 +239,7 @@ Makefile: @srcdir@/Makefile.in
+
+ ##############################################################################
+ clean:
+- -$(RM) -f version.txt *.elc vm-autoloads.el auto-autoloads.el
custom-load.el
++ -rm -f version.txt *.elc vm-autoloads.el auto-autoloads.el
custom-load.el
+
+ distclean: clean
+- -$(RM) -f Makefile
++ -rm -f Makefile
--- vm-8.2.0b.orig/debian/patches/series
+++ vm-8.2.0b/debian/patches/series
@@ -0,0 +1 @@
+reproducible-build.patch
