Subject: Re: [bug#38612] Pass system and target arguments to gexp->file.

From: Ludovic Courtès <[email-masked]>
Date: Thu, 26 Dec 2019 19:04:30 +0100

Hello,

Mathieu Othacehe <[email-masked]> skribis:

> From a05baf4f4328ce2ca6da6860f6e596cd7559a08a Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <[email-masked]>
> Date: Tue, 24 Dec 2019 18:24:37 +0100
> Subject: [PATCH 1/2] system: operating-system-boot-parameters-file: Fix
>  cross-compilation.
>
> * gnu/system.scm (operating-system-boot-parameters-file): Add system and
> target arguments and pass them to gexp->file call,
> (operating-system-directory-base-entries): pass current system and target to
> operating-system-boot-parameters-file procedure.
> ---
>  gnu/system.scm | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/gnu/system.scm b/gnu/system.scm
> index abdbb081e6..e7af7e7b47 100644
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -470,7 +470,10 @@ value of the SYSTEM-SERVICE-TYPE service."
>    (let ((locale (operating-system-locale-directory os)))
>      (mlet %store-monad ((kernel -> (operating-system-kernel os))
>                          (initrd -> (operating-system-initrd-file os))
> -                        (params    (operating-system-boot-parameters-file os)))
> +                        (params    (operating-system-boot-parameters-file
> +                                    os
> +                                    #:system (%current-system)
> +                                    #:target (%current-target-system))))

In general, in monadic code, we should refer to (current-system) and
(current-target-system), not to the SRFI-39 parameters.

> -(define* (operating-system-boot-parameters-file os
> -                                                #:key system-kernel-arguments?)
> +(define* (operating-system-boot-parameters-file
> +          os
> +          #:key
> +          system-kernel-arguments?
> +          system
> +          target)
>     "Return a file that describes the boot parameters of OS.  The primary use of
>  this file is the reconstruction of GRUB menu entries for old configurations.
>  
> @@ -1085,7 +1092,9 @@ being stored into the \"parameters\" file)."
>                       (device
>                        #$(device->sexp (boot-parameters-store-device params)))
>                       (mount-point #$(boot-parameters-store-mount-point params))))
> -                 #:set-load-path? #f)))
> +                 #:set-load-path? #f
> +                 #:system system
> +                 #:target target)))

By default, ‘gexp->file’ now uses the current system and target, so this
change shouldn’t be necessary if you just want to use those.  Am I
missing something?

The general guideline is that it would be good if only primitives
(monadic procedures in (guix gexp) as well as gexp compilers) would have
an explicit #:system and #:target parameter.

> From 0ce67afc4f33074e20824751c22ba01cf6a3e184 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <[email-masked]>
> Date: Wed, 25 Dec 2019 09:49:53 +0100
> Subject: [PATCH 2/2] services: Fix cross-compilation.
>
> * gnu/services.scm (system-derivation): Pass current system and target at bind
> time to lower-object,
> (compute-boot-script): also pass current system and target at bind time to
> gexp->file.

[...]

>  (define (system-derivation mentries mextensions)
>    "Return as a monadic value the derivation of the 'system' directory
>  containing the given entries."
> -  (mlet %store-monad ((entries    mentries)
> +  (mlet %store-monad ((system (current-system))
> +                      (target (current-target-system))
> +                      (entries    mentries)
>                        (extensions (sequence %store-monad mextensions)))

Please alight the RHS of ‘mlet’ bindings.  :-)

>      (lower-object
>       (file-union "system"
> -                 (append entries (concatenate extensions))))))
> +                 (append entries (concatenate extensions)))
> +     system
> +     #:target target)))

I guess this is needed here because ‘lower-object’ has #:target default
to #f.

> +  (mlet %store-monad ((system (current-system))
> +                      (target (current-target-system)))
> +    (gexp->file "boot"
> +                ;; Clean up and activate the system, then spawn shepherd.
> +                #~(begin #$@(reverse gexps))
> +                #:system system
> +                #:target target)))

This one is unnecessary now that ‘gexp->file’ honors the current system
and target, right?

Thanks,
Ludo’.