[bug#42123] [PATCH] linux-libre: Enable module compression.

["Ludovic Courtès" <ludo@gnu.org>]

Hi!

Mathieu Othacehe <othacehe@gnu.org> skribis:

> So as you suggested I created "guile-zlib" and "guile-lzlib" which was
> the easy part. The other part is the hard to digest, attached patch.

Thanks for doing that!

(I figured David Thompson had one wrapping the low-level bits of zlib:
<https://github.com/davexunit/guile-zlib>.  We should probably
incorporate that.)

> I tested it running "make check", "make as-derivation", "./pre-inst-env
> guix build guix". Hope I didn't miss something.

Nice!

> Please, tell me what you think :)

Just a quick review because I’m headed for a break.  :-)

> From d0f23078d1963f9aa8facda0fb3ae40e1e8c3cf2 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <othacehe@gnu.org>
> Date: Mon, 27 Jul 2020 16:36:39 +0200
> Subject: [PATCH] Use "guile-zlib" and "guile-lzlib" instead of (guix config).
>
> * Makefile.am (MODULES): Remove guix/zlib.scm and guix/lzlib.scm,
> (SCM_TESTS): remove tests/zlib.scm, tests/lzlib.scm.
> * build-aux/build-self.scm (make-config.scm): Remove unused %libz variable.
> * configure.ac: Remove LIBZ and LIBLZ variables and check instead for
> Guile-zlib and Guile-lzlib.
> * doc/guix.texi ("Requirements"): Remove zlib requirement and add Guile-zlib
> and Guile-lzlib instead.
> * gnu/packages/package-management.scm (guix)[native-inputs]: Add "guile-zlib"
> and "guile-lzlib",
> [inputs]: remove "zlib" and "lzlib",
> [propagated-inputs]: ditto,
> [arguments]: add "guile-zlib" and "guile-lzlib" to Guile load path.
> * guix/build/download-nar.scm: Use (zlib) instead of (guix zlib).
> * guix/config.scm.in (%libz, %liblz): Remove them.
> * guix/cvs-download.scm (cvs-fetch): Do not stub (guix config) in imported
> modules list, instead add "guile-zlib" to the extension list.
> * guix/git-download.scm (git-fetch): Ditto.
> * guix/gnu-maintenance.scm: Use (zlib) instead of (guix zlib).
> * guix/hg-download.scm (hg-fetch): Do not stub (guix config) in imported
> modules list, instead add "guile-zlib" to the extension list.
> * guix/lzlib.scm: Remove it.
> * guix/man-db.scm: Use (zlib) instead of (guix zlib).
> * guix/profiles.scm (manual-database): Do not stub (guix config) in imported
> modules list, instead add "guile-zlib" to the extension list.
> * guix/scripts/publish.scm: Use (zlib) instead of (guix zlib) and (lzlib)
> instead of (guix lzlib),
> (string->compression-type, effective-compression): do not check for zlib and
> lzlib availability.
> * guix/scripts/substitute.scm (%compression-methods): Do not check for lzlib
> availability.
> * guix/self.scm (specification->package): Add "guile-zlib" and "guile-lzlib"
> and remove "zlib" and "lzlib",
> (compiled-guix): remove "zlib" and "lzlib" arguments and add guile-zlib and
> guile-lzlib to the dependencies, also do not pass "zlib" and "lzlib" to
> "make-config.scm" procedure,
> (make-config.scm): remove "zlib" and "lzlib" arguments as well as %libz and
> %liblz variables.
> * guix/utils.scm (lzip-port): Use (lzlib) instead of (guix lzlib) and do not
> check for lzlib availability.
> * guix/zlib.scm: Remove it.
> * m4/guix.m4 (GUIX_LIBZ_LIBDIR, GUIX_LIBLZ_FILE_NAME): Remove them.
> * tests/lzlib.scm: Use (zlib) instead of (guix zlib) and (lzlib)
> instead of (guix lzlib), and do not check for zlib and lzlib availability.
> * tests/publish.scm: Ditto.
> * tests/substitute.scm: Do not check for lzlib availability.
> * tests/utils.scm: Ditto.
> * tests/zlib.scm: Remove it.

This can be decomposed in several steps:

  1. We can start using ‘guile-zlib’ as extensions for gexps: in (guix
     scripts pack), (guix download), etc.  Easy, no risk.

  2. Use guile-zlib & co. in Guix itself: (guix scripts substitute),
     (guix scripts publish), etc.  Keep (guix zlib) and (guix lzlib) in
     parallel.

  3. Update ‘guix’ package with these two new dependencies.

  4. Remove uses (guix zlib) and (guix lzlib), adjust build machinery
     and doc.

  5. Adjust (guix self) and related code.  This is a bit touchy.

Perhaps there’s a couple of steps that can be merged, but having a split
along these lines would be clearer and better for our peace of mind.

> --- a/build-aux/build-self.scm
> +++ b/build-aux/build-self.scm
> @@ -71,7 +71,7 @@
>                                        (variables rest ...))))))
>      (variables %localstatedir %storedir %sysconfdir %system)))
>  
> -(define* (make-config.scm #:key zlib gzip xz bzip2
> +(define* (make-config.scm #:key gzip xz bzip2
>                            (package-name "GNU Guix")
>                            (package-version "0")
>                            (bug-report-address "bug-guix@gnu.org")

This is OK.

> @@ -133,11 +133,7 @@
>                       (define %bzip2
>                         #+(and bzip2 (file-append bzip2 "/bin/bzip2")))
>                       (define %xz
> -                       #+(and xz (file-append xz "/bin/xz")))
> -
> -                     (define %libz
> -                       #+(and zlib
> -                              (file-append zlib "/lib/libz")))))))
> +                       #+(and xz (file-append xz "/bin/xz")))))))

I think it’s OK too, but we have to keep in mind that this code can be
run by a past Guix that expects ‘%libz’.  Normally it’s OK because the
modules needed by ‘compute-guix-derivation’ do not rely on ‘%libz’ and
(guix zlib).

One test would be something like:

  guix time-machine --commit=6298c3ffd9654d3231a6f25390b056483e8f407c -- \
    pull -p /tmp/test --url=/path/to/local/repo --branch=the-branch

This will check that 1.0.0 can indeed pull this new repo.

The rest LGTM.  Thanks a lot for all the work!

Ludo’, who’s going to be away the coming weeks.