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

[Jim Meyering]

Stefano Lattarini wrote:
> While I don't feel qualified to discuss the merits of the matter here,
> I have a couple of nits below...

Hi Stefano,

Thanks for the review!

> On Monday 04 October 2010, Jim Meyering wrote:
...
>>  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(-)
...
>> +      | BZIP2=${BZIP2--9} bzip2 -c >$(distdir).tar.bz2
> Typo here: should be "$${BZIP2--9}".

Well caught.  Fixed.
I should have tested with dist-bzip2, too.

> But then: why not follow consistently the example of the gzip
> compression?  As in:
>   GZIP=$(GZIP_ENV) gzip -c >$(distdir).tar.gz
> with GZIP_ENV set to a proper default in the generated makefile,
> and overridable through the command line:
>   $ make dist GZIP_ENV=--fast

To minimize namespace impact, and to keep the change compact/local.
With my proposed approach, we do not impinge on
user name space at all.  With the above, we'd usurp
BZIP2_ENV and XZ_ENV if you want to be consistent.
Also, with the _ENV variables, there would have to be
added definitions.  No fundamental objection, but I do
prefer the smaller patch.

>> diff --git a/aclocal.m4 b/aclocal.m4
>> index c43a368..4a22182 100644
>> --- a/aclocal.m4
>> +++ b/aclocal.m4
> Why have you regenerated this file with a unreleased autoconf version?

I regularly test unreleased versions.  Sorry to have included that.
I was obviously asleep at the wheel.
I'm no longer accustomed to submitting patches in projects
that version-control such generated files.

>> diff --git a/configure
>> b/configure
>> index 169d82d..2940276 100755
>> --- a/configure
>> +++ b/configure
> Ditto.

Likewise.

>> 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
> BZIP2 environment variable
>> +For example, @samp{make dist-bzip2 XZ_OPT=-7}.
> Ditto.

Thank you.  Fixed.

>From ebb9b294cf878ed7083376bd15a0269a0583836c 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.
---
 NEWS              |    5 +++++
 doc/automake.texi |    7 +++++++
 lib/am/distdir.am |    6 ++++--
 3 files changed, 16 insertions(+), 2 deletions(-)

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/doc/automake.texi b/doc/automake.texi
index 22c2f27..f8f2d8f 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{BZIP2} environment variable.
+For example, @samp{make dist-bzip2 BZIP2=-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..ad789df 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