[bug#61949] [PATCH] pack: Move common build code to (guix build pack).

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

Hi Maxim and all,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> The rationale is to reduce the number of derivations built per pack to ideally
> one, to minimize storage requirements.  The number of derivations had gone up
> with 68380db4 ("pack: Extract populate-profile-root from
> self-contained-tarball/builder.") as a side effect to improving code reuse.
>
> * guix/scripts/pack.scm (guix): Add commentary comment.
> (populate-profile-root, self-contained-tarball/builder): Extract to...
> * guix/build/pack.scm (populate-profile-root): ... this, and...
> (build-self-contained-tarball): ... that, adjusting for use on the build side.
> (assert-utf8-locale): New procedure.
> (self-contained-tarball, debian-archive, rpm-archive): Adjust accordingly.

Sorry for dropping the ball!

> Changes in v2:
> - Drop '!' from populate-profile-root!
> - Move top commentary comment below define-module block
> - Move expressions after definitions for Guile 2.0 compatibility
> - Remove #:target and #:archiver from build-self-contained-tarball

I believe the only outstanding issue for me is the ability to run tests
without relying on an external store.

>  (define* (self-contained-tarball name profile
>                                   #:key target
>                                   (profile-name "guix-profile")
> @@ -367,16 +240,40 @@ (define* (self-contained-tarball name profile
>      (warning (G_ "entry point not supported in the '~a' format~%")
>               'tarball))
>  
> +  (define database
> +    (and localstatedir?
> +         (file-append (store-database (list profile))
> +                      "/db/db.sqlite")))
> +
>    (gexp->derivation (string-append name ".tar"
>                                     (compressor-extension compressor))
> -    (self-contained-tarball/builder profile
> -                                    #:profile-name profile-name
> -                                    #:target target
> -                                    #:localstatedir? localstatedir?
> -                                    #:deduplicate? deduplicate?
> -                                    #:symlinks symlinks
> -                                    #:compressor compressor
> -                                    #:archiver archiver)))
> +    (with-extensions (list guile-gcrypt)
> +      (with-imported-modules `(((guix config) => ,(make-config.scm))

You can remove (guix config) here since it’s unused.

> +                               ,@(source-module-closure
> +                                  `((guix build pack)
> +                                    (guix build utils))
> +                                  #:select? not-config?))

I figured that, to allow tests to run without an external store, we need
to change it like this:

  (gexp->derivation (string-append name ".tar"
                                   (compressor-extension compressor))
    (with-extensions (if deduplicate? (list guile-gcrypt) '())
      (with-imported-modules (let ((lst (source-module-closure
                                         `((guix build pack)
                                           (guix build utils))
                                         #:select? not-config?)))
                               (if deduplicate?
                                   lst
                                   (delete '(guix store deduplication) lst)))
        #~(begin
            (use-modules (guix build pack)
                         (guix build utils))
            …))))

Quite some boilerplate given that #:deduplicate? is always #f in practice.
So maybe it’s not worth the trouble, after all.

I’ll let you rebase and adjust as you see fit, but it LGTM!

Thanks,
Ludo’.