Re: [PATCH] dist: add dist-zstd option

On Mon, Nov 11, 2019 at 12:35 PM Giuseppe Scrivano <[email protected]> wrote:
> Hi Jim,
>
> Jim Meyering <[email protected]> writes:
>
> > On Fri, Oct 4, 2019 at 8:03 AM Giuseppe Scrivano <[email protected]> wrote:
> >> add support for using the zstd compression algorithm.
> >
> > Hi Giuseppe,
> > Thank you for that patch.
> > I've adjusted it and propose the attached, which makes these changes:
> > - add tests
> > - that exposed the need for a correction, s/-d/-dc/ in distdir.am
> > - extend documentation
> > - use the 3-byte suffix, .zst, not .zstd
> > - use -19 as the default compression level
> >
> > We must use -19 as the default, not the aggressive --ultra -22 -- the
> > package maintainer can always override with ZSTD_OPT if they know all
> > clients will always have sufficient memory. In the early days, some
> > reported failure to decompress a "xz -9e"-compressed coreutils tarball
> > on tiny-memory routers. Like zstd's --ultra settings, xz's -9 requires
> > more RAM when DEcompressing -- so automake defaults to xz's "-e" (use
> > extra CPU only) and used -e8 for coreutils
> > (https://git.sv.gnu.org/cgit/coreutils.git/commit/?id=v8.15-61-gc1d07237a):
> > i.e., still require 32MiB more RAM, but not the 64MiB that "-9" would
> > require.
> >
> > Cc'd the zstd author, Yann Collet, in case he'd like to add something.
> >
> > Giuseppe, please re-review this diff and its updated commit log.
>
> is there anything more holding the patch?

Sorry about the delay.
On suggestion from Yann, I propose one additional change. Barring
objection, I will push the combined result tomorrow.

diff --git a/doc/automake.texi b/doc/automake.texi
index 09bbfa998..09374a8aa 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -8751,12 +8751,14 @@ The Types of Distributions
 @trindex dist-shar

 @vindex ZSTD_OPT
+@vindex ZSTD_CLEVEL
 @item @code{dist-zstd}
 Generate a @samp{zstd} tar archive of the distribution.  By default, this
 rule makes @samp{zstd} use a compression option of @option{-19}.  To
 make it use a different one, set the @env{ZSTD_OPT} environment variable.
 For example, run this command to use the default compression ratio,
 but with a progress indicator: @samp{make dist-zstd ZSTD_OPT=-19v}.
+However, note that for compatibility with zstd itself, you may instead set the @env{ZSTD_CLEVEL} environment variable, in which case, any @env{ZSTD_OPT} setting is ignored.
 @trindex dist-zstd

 @end table
diff --git a/lib/am/distdir.am b/lib/am/distdir.am
index 197942829..cdd77eb83 100644
--- a/lib/am/distdir.am
+++ b/lib/am/distdir.am
@@ -353,7 +353,7 @@ dist-xz: distdir
 ?ZSTD?DIST_ARCHIVES += $(distdir).tar.zst
 .PHONY: dist-zstd
 dist-zstd: distdir
-	tardir=$(distdir) && $(am__tar) | zstd -c $${ZSTD_OPT--19} >$(distdir).tar.zst
+	tardir=$(distdir) && $(am__tar) | zstd -c $${ZSTD_CLEVEL-$${ZSTD_OPT--19}} >$(distdir).tar.zst
 	$(am__post_remove_distdir)

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