Re: [PATCH] dist-xz: don't hard-code -9: honor setting of XZ_OPT

[Jim Meyering]

Ralf Wildenhues wrote:
...
>> > and NEWS update, was already on my list of things to do.  (There are
>> > some prior bug reports too.)
>>
>> The name I used there, XZ_OPT, is dictated by the tool.  An alternative
>> is to invoke xz with no explicit XZ_OPT setting, but that would result
>> in a semantic change: using the default compression level of -7 rather
>> than what was hard-coded as -9.  Perhaps that would be better, because
>> some people have complained that using -9 makes it harder to decompress
>> (requires more memory) on extremely low-memory (embedded) systems.
>
> The change to use -9 for xz is fairly recent, new in 1.11.1.

To give you an idea of the scale, I believe that the failure
arose on an embedded system with only 64MB or 96MB of RAM.
That was because the larger dictionary size selected by -9
at compression time induced a requirement for more RAM at
decompression than was available.

However, people distributing packages for such small systems can
accommodate by repackaging.  The extreme constraints of those
fringe users/systems should not make us dumb-down the methods that
save bandwidth/space for everyone else.

>> Hence, my using an explicit XZ_OPT setting served three purposes:
>> - preserved the existing -9 (perhaps not desirable)
>> - allowed an override
>> - documented in the Makefile that xz's behavior can be overridden
>>     through that envvar.
>
> Sounds good.
>
>> There is no point in changing lzma, since it's deprecated.
>
> Except that the legacy lzma Utils that I tested on one MSYS install
> failed with OOM due to the -9, causing a spurious testsuite failure.
>
> Wrt. xz: I think the general approach for 'dist-*' should be: by default
> use highest compression that is portable.  Rationale: the default
> behavior is one compression plus upload, and lots of downloads and
> decompressions.  That it makes testing the package take longer is a QoI
> issue which the developer can tune with the XZ_OPT knob then.  OTOH,
> when we find out that -9 fails due to OOM on some systems (and I have no
> idea whether the lzma utils failure corresponds to a similar xz failure)
> then we should consider backing down to something lower.  I really
> prefer the defaults to work OOTB.
>
>> bzip2 and lzip hard-code the -9, just like xz does/did.
>> bzip2 honors BZIP2, but lzip appears to honor no envvar.
>> gzip doesn't hard-code a compression level and besides,
>> it already honors the $(GZIP_ENV) makefile variable.
>>
>> With all that, trying to make an across-the-board, consistent change
>> appears to be a lost cause.
>
> :-/
>
>> As a half measure, I could change bzip2 in the same way as xz, but
>> hesitated to do even that, since I feel that bzip2 has been subsumed.
>> Tell me what you'd like.
>
> I would be fine with an analogous bzip2 change.

I've done that.

> If you see chance for testsuite additions anywhere and have time for it,
> all the better.  Sorry I'm a bit out of time right now.

I've tested it by using it in coreutils and comparing
the sizes of the resulting .xz files:

  4532 KiB    make dist-xz XZ_OPT=-9e
  4544 KiB    make dist-xz XZ_OPT=-8e
  4624 KiB    make dist-xz XZ_OPT=-9
  4632 KiB    make dist-xz XZ_OPT=-7e
  4636 KiB    make dist-xz XZ_OPT=-8
  4724 KiB    make dist-xz XZ_OPT=-7

Thus, for coreutils, I expect to use -8e, to require a smaller
memory footprint on decompression, at the cost of a mere 12 bytes.

> I'm not quite sure if I want to see all these changes in branch-1.11 any
> more though, but am certainly open to input/comments here.

Just "master" is fine with me.
Here's what I have now (yes, I'll add an entry to ChangeLog if you ACK this):

>From d2c29e15a77ab2a073cf64919ab320bfec001833 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 2 Oct 2010 22:30:02 +0200
Subject: [PATCH] dist-xz: don't hard-code -9: honor setting of XZ_OPT

* lib/am/distdir.am (dist-xz): Do not hard-code xz's -9: that
made it impossible to override.  Instead, use its XZ_OPT envvar,
defaulting to -9 if not defined.  Thus no change in behavior
when XZ_OPT is not set, and now, this rule honors the setting
of that envvar when it is set.  Suggested by Lasse Collin.
* NEWS (Miscellaneous changes): Mention it.
* doc/automake.texi (The Types of Distributions): Describe the newly
enabled environment variables.
---
 Makefile.in       |    6 ++++--
 NEWS              |    5 +++++
 aclocal.m4        |    4 ++--
 configure         |   10 +++++-----
 doc/automake.texi |    7 +++++++
 lib/am/distdir.am |    6 ++++--
 6 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index 232fbf7..194b784 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -651,7 +651,8 @@ dist-gzip: distdir
        tardir=$(distdir) && $(am__tar) | GZIP=$(GZIP_ENV) gzip -c 
>$(distdir).tar.gz
        $(am__post_remove_distdir)
 dist-bzip2: distdir
-       tardir=$(distdir) && $(am__tar) | bzip2 -9 -c >$(distdir).tar.bz2
+       tardir=$(distdir) && $(am__tar) \
+         | BZIP2=${BZIP2--9} bzip2 -c >$(distdir).tar.bz2
        $(am__post_remove_distdir)

 dist-lzip: distdir
@@ -663,7 +664,8 @@ dist-lzma: distdir
        $(am__post_remove_distdir)

 dist-xz: distdir
-       tardir=$(distdir) && $(am__tar) | xz -9 -c >$(distdir).tar.xz
+       tardir=$(distdir) && $(am__tar) \
+         | XZ_OPT=$${XZ_OPT--9} xz -c >$(distdir).tar.xz
        $(am__post_remove_distdir)

 dist-tarZ: distdir
diff --git a/NEWS b/NEWS
index 121989f..c64ec14 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,11 @@ New in 1.11a:

   - "make dist" can now create lzip-compressed tarballs.

+  - You may adjust the compression options used in dist-xz and dist-bzip2.
+    The default is still -9 for each, but you may specify a different
+    level via the XZ_OPT and BZIP2 envvars respectively.  E.g.,
+    "make dist-xz XZ_OPT=-7" or "make dist-xz BZIP2=-5"
+
   - Messages of types warning or error from `automake' and `aclocal' are now
     prefixed with the respective type, and presence of -Werror is noted.

diff --git a/aclocal.m4 b/aclocal.m4
index c43a368..4a22182 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -13,8 +13,8 @@

 m4_ifndef([AC_AUTOCONF_VERSION],
   [m4_copy([m4_PACKAGE_VERSION], [AC_AUTOCONF_VERSION])])dnl
-m4_if(m4_defn([AC_AUTOCONF_VERSION]), [2.68],,
-[m4_warning([this file was generated for autoconf 2.68.
+m4_if(m4_defn([AC_AUTOCONF_VERSION]), [2.68.3-de12b],,
+[m4_warning([this file was generated for autoconf 2.68.3-de12b.
 You have another version of autoconf.  It may work, but is not guaranteed to.
 If you have problems, you may need to regenerate the build system entirely.
 To do so, use the procedure documented by the package, typically 
`autoreconf'.])])
diff --git a/configure b/configure
index 169d82d..2940276 100755
--- a/configure
+++ b/configure
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.68 for GNU Automake 1.11a.
+# Generated by GNU Autoconf 2.68.3-de12b for GNU Automake 1.11a.
 #
 # Report bugs to <address@hidden>.
 #
@@ -1345,7 +1345,7 @@ test -n "$ac_init_help" && exit $ac_status
 if $ac_init_version; then
   cat <<\_ACEOF
 GNU Automake configure 1.11a
-generated by GNU Autoconf 2.68
+generated by GNU Autoconf 2.68.3-de12b

 Copyright (C) 2010 Free Software Foundation, Inc.
 This configure script is free software; the Free Software Foundation
@@ -1362,7 +1362,7 @@ This file contains any messages produced by compilers 
while
 running configure, to aid debugging if configure makes a mistake.

 It was created by GNU Automake $as_me 1.11a, which was
-generated by GNU Autoconf 2.68.  Invocation command line was
+generated by GNU Autoconf 2.68.3-de12b.  Invocation command line was

   $ $0 $@

@@ -3353,7 +3353,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1
 # values after options handling.
 ac_log="
 This file was extended by GNU Automake $as_me 1.11a, which was
-generated by GNU Autoconf 2.68.  Invocation command line was
+generated by GNU Autoconf 2.68.3-de12b.  Invocation command line was

   CONFIG_FILES    = $CONFIG_FILES
   CONFIG_HEADERS  = $CONFIG_HEADERS
@@ -3408,7 +3408,7 @@ cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; 
s/[\\""\`\$]/\\\\&/g'`"
 ac_cs_version="\\
 GNU Automake config.status 1.11a
-configured by $0, generated by GNU Autoconf 2.68,
+configured by $0, generated by GNU Autoconf 2.68.3-de12b,
   with options \\"\$ac_cs_config\\"

 Copyright (C) 2010 Free Software Foundation, Inc.
diff --git a/doc/automake.texi b/doc/automake.texi
index 22c2f27..0652aa8 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -8461,6 +8461,9 @@ The Types of Distributions
 @item @code{dist-bzip2}
 Generate a bzip2 tar archive of the distribution.  bzip2 archives are
 frequently smaller than gzipped archives.
+By default, this rule makes @samp{bzip2} use a compression option of 
@option{-9}.
+To make it use a different one, set the @env{XZ_OPT} environment variable.
+For example, @samp{make dist-bzip2 XZ_OPT=-7}.
 @trindex dist-bzip2

 @item @code{dist-gzip}
@@ -8487,6 +8490,10 @@ The Types of Distributions
 Generate an @samp{xz} tar archive of the distribution.  @command{xz}
 archives are frequently smaller than @command{bzip2}-compressed archives.
 The @samp{xz} format displaces the obsolete @samp{lzma} format.
+By default, this rule makes @samp{xz} use a compression option of @option{-9}.
+To make it use a different one, set the @env{XZ_OPT} environment variable.
+For example, run this command to use the default compression ratio, but
+with a progress indicator: @samp{make dist-xz XZ_OPT=-7e}.
 @trindex dist-xz

 @item @code{dist-zip}
diff --git a/lib/am/distdir.am b/lib/am/distdir.am
index a11d3a4..5b9b89f 100644
--- a/lib/am/distdir.am
+++ b/lib/am/distdir.am
@@ -345,7 +345,8 @@ dist-gzip: distdir
 ?BZIP2?DIST_ARCHIVES += $(distdir).tar.bz2
 .PHONY: dist-bzip2
 dist-bzip2: distdir
-       tardir=$(distdir) && $(am__tar) | bzip2 -9 -c >$(distdir).tar.bz2
+       tardir=$(distdir) && $(am__tar) \
+         | BZIP2=${BZIP2--9} bzip2 -c >$(distdir).tar.bz2
        $(am__post_remove_distdir)

 ?LZIP?DIST_ARCHIVES += $(distdir).tar.lz
@@ -363,7 +364,8 @@ dist-lzma: distdir
 ?XZ?DIST_ARCHIVES += $(distdir).tar.xz
 .PHONY: dist-xz
 dist-xz: distdir
-       tardir=$(distdir) && $(am__tar) | xz -9 -c >$(distdir).tar.xz
+       tardir=$(distdir) && $(am__tar) \
+         | XZ_OPT=$${XZ_OPT--9} xz -c >$(distdir).tar.xz
        $(am__post_remove_distdir)

 ?COMPRESS?DIST_ARCHIVES += $(distdir).tar.Z
--
1.7.3.1.45.g9855b