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

From: Ludovic Courtès <[email-masked]>
Date: Mon, 06 Jul 2020 22:13:50 +0200

Hi,

Mathieu Othacehe <[email-masked]> skribis:

>> I don’t have other ideas, but both solutions sound good to me.  Using
>> (guix zlib) is slightly more “elegant” IMO, but no big deal.  I don’t
>> expect any significant difference from the use of in-process
>> decompression, unless we really have to go and decompress many modules
>> in a row.
>
> Creating the initrd implies to create the module name database, and it
> ends-up decompressing every single module. Using the in-process method
> it takes 2 seconds, using the second method 30 seconds.

Woow, interesting.

> So, I opted for the first solution as you suggested. Here's an attached
> patch that fixes the situation.

[...]

> From 8bbf343510091fad4a08758e0115a70410c1c8d7 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <[email-masked]>
> Date: Mon, 6 Jul 2020 16:04:21 +0200
> Subject: [PATCH] self: Add with-imported-modules+config and use it.
>
> Introduce "with-imported-modules+config" and use it to replace every call to
> "with-imported-modules" that would trigger an import of (guix config) module.
>
> * guix/self.scm (not-config?): New procedure,
> (with-imported-modules+config): new macro.
> * guix/profiles.scm (linux-module-database): Replace with-imported-modules by
> with-imported-modules+config.
> * gnu/system/shadow.scm (account-shepherd-service): Ditto.
> * gnu/system/linux-initrd.scm (raw-initrd): Ditto.
> * gnu/services/base.scm (default-serial-port): Ditto,
> (file-system-shepherd-service): ditto,
> (udev-shepherd-service): ditto.
> * gnu/services.scm (activation-script): Ditto.
> * gnu/machine/ssh.scm (machine-check-initrd-modules): Ditto.

[...]

> diff --git a/guix/self.scm b/guix/self.scm
> index e1350a7403..82bb55f8e7 100644
> --- a/guix/self.scm
> +++ b/guix/self.scm
> @@ -33,6 +33,8 @@
>    #:use-module (srfi srfi-35)
>    #:use-module (ice-9 match)
>    #:export (make-config.scm
> +            not-config?
> +            with-imported-modules+config
>              whole-package                     ;for internal use in 'guix pull'
>              compiled-guix
>              guix-derivation))
> @@ -1063,6 +1065,24 @@ Info manual."
>                 ;; made relative to a nonexistent anonymous module.
>                 #:splice? #t))
>  
> +(define not-config?
> +  ;; Select (guix …) and (gnu …) modules, except (guix config).
> +  (match-lambda
> +    (('guix 'config) #f)
> +    (('guix rest ...) #t)
> +    (('gnu rest ...) #t)
> +    (rest #f)))
> +
> +(define-syntax-rule (with-imported-modules+config modules exp ...)
> +  "Import the closure of MODULES and evaluate EXP within this context.  If the
> +(guix config) module is part of the closure, it is not selected.  This module
> +is always replaced by a mocked-one, created by MAKE-CONFIG.SCM pocedure."
> +  (with-imported-modules `(,@(source-module-closure
> +                              modules
> +                              #:select? not-config?)
> +                           ((guix config) => ,(make-config.scm)))
> +    exp ...))

Two remarks: I feel that this is not the best place for it, and I think
we should add (guix config) if and only if it’s actually part of the
closure.

For the name I’m tempted by a simpler but less descriptive option.

That would give us:

  (define-syntax-rule (with-imported-modules* modules exp ...)
    (let ((closure (source-module-closure modules #:select? not-config?)))
      (with-imported-modules (map (match-lambda
                                    (('guix 'config) …)
                                    (module module))
                                  closure)
        exp ...)))

WDYT?

As for the location… I think the reason things are like this is to avoid
having everything depend on (guix self), but maybe that’s OK after all?

If the zlib bindings were an external package, things would be easier
because we wouldn’t have to do this (guix config) dance.

More generally, we should look at all the uses of (guix config) and see
whether/how we can get rid of them.

But I digress… Thoughts?

Thank you,
Ludo’.