Hello list,

attached is a patch that addresses several minor issues - in my
opinion - of make dist.

First, it removes the special flags used for the compressors. Why?
Because:
: xz -e
unreasonably slow for little to no benefit
: bzip2 -9
is the default, no need to set it explicitly
: xz/lzip -9
hinders decompression of tarballs on low RAM systems (my experience is
that with less than 100MB free RAM, it starts swapping.
: gzip -9
Much slower, very little benefit over the default -6. Overall it might
be the only case where it's acceptable, as gzip has low requirements in
general so "much slower" is not *that* slow. But we have better
algorithms for achieving much better compression ratio than gzip -9.

Overall what I'm suggesting is to trust the author defaults, unless
overriden by the user.

Another issue fixed here is that I try to respect the environment passed
to make. If GZIP is set, now it is not overriden in any way. Options to
gzip can be passed by setting the environment variable desribed in its
man page, either in the environment, or in the makefile. Similar for the
other compressors (except lzip that does not support environment
settings). Side effect is that the makefile variable GZIP_ENV is removed
as I tried to keep things in symmetry, but this may break weird
pre-existing use cases.

Additionally I completely removed passing of flags to decompressors. A
decompressor always generates the same result.

Regarding "tar", I followed the same logic as with the compressors and
I'm utilising TAR_OPTIONS to affect GNU tar's behaviour. I also added a
couple of options as defaults, namely "--owner=0 --group=0" to avoid
disclosing buildsystem's UIDs and GIDs and make tarballs closer to
identical even if generated from different users. Again this might break
pre-existing uses that need this info, but variable can be easily set to
empty when calling "make dist".

Finally some variables have been quoted to ensure they will work with
spaces. This was already the case in other parts of the file. It still
is mixed though for important variables, notably "distdir", which is
a separate issue, I believe.

So what do you think?
Regards,
Dimitris

P.S. Please keep me CC'd as I'm not subscribed.
--- /usr/share/automake-1.15/am/distdir.am	2016-03-30 01:57:21.000000000 +0200
+++ distdir.patched.am	2017-01-20 01:17:20.606940705 +0100
@@ -300,73 +300,88 @@
 ## interpret '-z' differently.
 ##
 ## The -o option of GNU tar used to exclude empty directories.  This
 ## behavior was fixed in tar 1.12 (released on 1997-04-25).  But older
 ## versions of tar are still used (for instance NetBSD 1.6.1 ships
 ## with tar 1.11.2).  We do not do anything specific w.r.t. this
 ## incompatibility since packages where empty directories need to be
 ## present in the archive are really unusual.
-##
-## We order DIST_TARGETS by expected duration of the compressors,
-## slowest first, for better parallelism in "make dist".  Do not
-## reorder DIST_ARCHIVES, users may expect gzip to be first.
 
 if %?TOPDIR_P%
 
-?GZIP?DIST_ARCHIVES += $(distdir).tar.gz
-GZIP_ENV = --best
+# Ensure GNU tar does not record the buildsystem's usernames and
+# groupnames. Can be overriden by setting TAR_OPTIONS in the
+# environment.
+TAR_OPTIONS ?= --owner=0 --group=0
+
+# To override compression options, set the respective environment or
+# Makefile variable. Variable names, taken from the man pages: GZIP,
+# BZIP2, XZ_OPT.
+
+# Do not reorder DIST_ARCHIVES, the first one (gzip) is the default for
+# make dist.
+
+?GZIP?DIST_ARCHIVES += "$(distdir)".tar.gz
 .PHONY: dist-gzip
 dist-gzip: distdir
-	tardir=$(distdir) && $(am__tar) | GZIP=$(GZIP_ENV) gzip -c >$(distdir).tar.gz
+	tardir="$(distdir)" && TAR_OPTIONS="$(TAR_OPTIONS)" $(am__tar) \
+	    | GZIP="$(GZIP)" gzip -c > "$(distdir)".tar.gz
 	$(am__post_remove_distdir)
 
 ?BZIP2?DIST_ARCHIVES += $(distdir).tar.bz2
 .PHONY: dist-bzip2
 dist-bzip2: distdir
-	tardir=$(distdir) && $(am__tar) | BZIP2=$${BZIP2--9} bzip2 -c >$(distdir).tar.bz2
+	tardir="$(distdir)" && TAR_OPTIONS="$(TAR_OPTIONS)" $(am__tar) \
+	    | BZIP2="$(BZIP2)" bzip2 -c > "$(distdir)".tar.bz2
 	$(am__post_remove_distdir)
 
 ?LZIP?DIST_ARCHIVES += $(distdir).tar.lz
 .PHONY: dist-lzip
 dist-lzip: distdir
-	tardir=$(distdir) && $(am__tar) | lzip -c $${LZIP_OPT--9} >$(distdir).tar.lz
+	tardir="$(distdir)" && TAR_OPTIONS="$(TAR_OPTIONS)" $(am__tar) \
+	    | lzip -c > "$(distdir)".tar.lz
 	$(am__post_remove_distdir)
 
 ?XZ?DIST_ARCHIVES += $(distdir).tar.xz
 .PHONY: dist-xz
 dist-xz: distdir
-	tardir=$(distdir) && $(am__tar) | XZ_OPT=$${XZ_OPT--e} xz -c >$(distdir).tar.xz
+	tardir="$(distdir)" && TAR_OPTIONS="$(TAR_OPTIONS)" $(am__tar) \
+	    | XZ_OPT="$(XZ_OPT)" xz -c > "$(distdir)".tar.xz
 	$(am__post_remove_distdir)
 
 ?COMPRESS?DIST_ARCHIVES += $(distdir).tar.Z
 .PHONY: dist-tarZ
 dist-tarZ: distdir
 	@echo WARNING: "Support for distribution archives compressed with" \
 		       "legacy program 'compress' is deprecated." >&2
 	@echo WARNING: "It will be removed altogether in Automake 2.0" >&2
-	tardir=$(distdir) && $(am__tar) | compress -c >$(distdir).tar.Z
+	tardir="$(distdir)" && TAR_OPTIONS="$(TAR_OPTIONS)" $(am__tar) \
+	    | compress -c >$(distdir).tar.Z
 	$(am__post_remove_distdir)
 
 ?SHAR?DIST_ARCHIVES += $(distdir).shar.gz
 .PHONY: dist-shar
 dist-shar: distdir
 	@echo WARNING: "Support for shar distribution archives is" \
 	               "deprecated." >&2
 	@echo WARNING: "It will be removed altogether in Automake 2.0" >&2
-	shar $(distdir) | GZIP=$(GZIP_ENV) gzip -c >$(distdir).shar.gz
+	shar $(distdir) | GZIP="$(GZIP)" gzip -c >$(distdir).shar.gz
 	$(am__post_remove_distdir)
 
 ?ZIP?DIST_ARCHIVES += $(distdir).zip
 .PHONY: dist-zip
 dist-zip: distdir
 	-rm -f $(distdir).zip
 	zip -rq $(distdir).zip $(distdir)
 	$(am__post_remove_distdir)
 
+## We order DIST_TARGETS by expected duration of the compressors,
+## slowest first, for better parallelism in "make dist".
+
 ?LZIP?DIST_TARGETS += dist-lzip
 ?XZ?DIST_TARGETS += dist-xz
 ?SHAR?DIST_TARGETS += dist-shar
 ?BZIP2?DIST_TARGETS += dist-bzip2
 ?GZIP?DIST_TARGETS += dist-gzip
 ?ZIP?DIST_TARGETS += dist-zip
 ?COMPRESS?DIST_TARGETS += dist-tarZ
 
@@ -407,27 +422,27 @@
 
 # This target untars the dist file and tries a VPATH configuration.  Then
 # it guarantees that the distribution is self-contained by making another
 # tarfile.
 .PHONY: distcheck
 distcheck: dist
 	case '$(DIST_ARCHIVES)' in \
 	*.tar.gz*) \
-	  GZIP=$(GZIP_ENV) gzip -dc $(distdir).tar.gz | $(am__untar) ;;\
+	  gzip -dc $(distdir).tar.gz | $(am__untar) ;;\
 	*.tar.bz2*) \
 	  bzip2 -dc $(distdir).tar.bz2 | $(am__untar) ;;\
 	*.tar.lz*) \
 	  lzip -dc $(distdir).tar.lz | $(am__untar) ;;\
 	*.tar.xz*) \
 	  xz -dc $(distdir).tar.xz | $(am__untar) ;;\
 	*.tar.Z*) \
 	  uncompress -c $(distdir).tar.Z | $(am__untar) ;;\
 	*.shar.gz*) \
-	  GZIP=$(GZIP_ENV) gzip -dc $(distdir).shar.gz | unshar ;;\
+	  gzip -dc $(distdir).shar.gz | unshar ;;\
 	*.zip*) \
 	  unzip $(distdir).zip ;;\
 	esac
 ## Make the new source tree read-only.  Distributions ought to work in
 ## this case.  However, make the top-level directory writable so we
 ## can make our new subdirs.
 	chmod -R a-w $(distdir)
 	chmod u+w $(distdir)

Reply via email to