bug#65225: ‘guix shell --system=ALIEN’ builds for both systems

bug#65225: ‘guix shell --system=ALIEN’ builds for both systems

[Tobias Geerinckx-Rice via Bug reports for GNU Guix]

Hi,

ekaitz noticed this when building a mes-derived package that fails on 
x86_64.  Here's a reproducer that uses upstream Guix:

$ guix shell --system=riscv64-linux drawterm --no-grafts --rebuild-cache 
--dry-run | grep drv$
   /gnu/store/4c02fgswkbldys93w4vgj0gwax2ly4bh-profile.drv

$ guix build --dry-run 
/gnu/store/4c02fgswkbldys93w4vgj0gwax2ly4bh-profile.drv | grep drawterm
   
/gnu/store/mkxyhjq117wdalc0gbz4468blqxih1kn-drawterm-20210628-1.c97fe46
   
/gnu/store/h8dk35aw2n8rj1hki6dkdrknfly26vq8-drawterm-20210628-1.c97fe46

/gnu/store/fl4iimlcdnlkarjm8m6z3392wss6b8yr-drawterm-20210628-1.c97fe46.drv
→ 
/gnu/store/mkxyhjq117wdalc0gbz4468blqxih1kn-drawterm-20210628-1.c97fe46
is riscv64-linux.

/gnu/store/9a1pji59hzacrmpb65nk3pp3m01niqf3-drawterm-20210628-1.c97fe46.drv
→ 
/gnu/store/h8dk35aw2n8rj1hki6dkdrknfly26vq8-drawterm-20210628-1.c97fe46
is x86_64-linux.

Eventually the ‘correct’ riscv64 drawterm ends up in the shell, but Guix 
should never have built the x86_64 version.

Kind regards,

T G-R

Sent from a Web browser.  Excuse or enjoy my brevity.

bug#65225: [PATCH] environment: Build the profile for the requested system.

[Tobias Geerinckx-Rice via Bug reports for GNU Guix]

Previously, ‘--system=’ did not affect profile hooks, meaning that all
packages would be built for both the host and requested systems.

* guix/scripts/environment.scm (guix-environment*): Parameterize
%current-system to match the requested ‘--system=’.

Reported by ekaitz in #guix.
---
 guix/scripts/environment.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 9712389842..27f7e53549 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -1146,7 +1146,8 @@ (define (guix-environment* opts)
           (warning (G_ "no packages specified; creating an empty environment~%")))
 
         ;; Use the bootstrap Guile when requested.
-        (parameterize ((%graft? (assoc-ref opts 'graft?))
+        (parameterize ((%current-system system)
+                       (%graft? (assoc-ref opts 'graft?))
                        (%guile-for-build
                         (and store-needed?
                              (package-derivation

base-commit: 9e71d4fd6b3893ae87cb079b57d7a8fe6e9e7914
-- 
2.41.0

bug#65225: [PATCH] environment: Build the profile for the requested system.

[Josselin Poiret via Bug reports for GNU Guix]

[-- Attachment #1: Type: text/plain, Size: 5051 bytes --]

Hi Tobias,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

> Previously, ‘--system=’ did not affect profile hooks, meaning that all
> packages would be built for both the host and requested systems.
>
> * guix/scripts/environment.scm (guix-environment*): Parameterize
> %current-system to match the requested ‘--system=’.
>
> Reported by ekaitz in #guix.
> ---
>  guix/scripts/environment.scm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
> index 9712389842..27f7e53549 100644
> --- a/guix/scripts/environment.scm
> +++ b/guix/scripts/environment.scm
> @@ -1146,7 +1146,8 @@ (define (guix-environment* opts)
>            (warning (G_ "no packages specified; creating an empty environment~%")))
>  
>          ;; Use the bootstrap Guile when requested.
> -        (parameterize ((%graft? (assoc-ref opts 'graft?))
> +        (parameterize ((%current-system system)
> +                       (%graft? (assoc-ref opts 'graft?))
>                         (%guile-for-build
>                          (and store-needed?
>                               (package-derivation
>
> base-commit: 9e71d4fd6b3893ae87cb079b57d7a8fe6e9e7914
> -- 
> 2.41.0

So, I've looked into this deeper because this fix didn't seem satisfying
to me: it suggests that the implementation of profile-derivation itself
is wrong, and I wanted to fix it instead.  Here's what I uncovered:

%current-system (applies mutatis mutandis to %current-target-system)
is a Guile parameter.  That means that it is accessed through a function
call, and its values really depends on where that function call occurs.
Now, this interacts with the store monad in a cumbersome way: monadic
values in this case are functions (lambda (store) ...) returning two
values, the actual output and the store.  These functions are run only
at the run-with-store call.

Now, there are two non-equivalent ways of getting the current system in
a monadic context.  You can either do

(mlet ((system -> (%current-system))
  ...)

or

(mlet ((system (current-system))
  ...)

The former directly evaluates (%current-system), while the latter only
evaluates (%current-system) when the monadic value is run!

What does this mean for our case here?  Well, the problem lies with how
the hooks are lowered: they use (gexp->derivation ...) without the
optional #:system keyword.  This looks up the current system at call
time with the mlet -> construct, so everything should be okay, right?
Well, the hooks are run through a mapm/accumulate-builds call, which
puts everything in a monadic value, effectively delaying the look-up
until monadic run time.

--8<---------------cut here---------------start------------->8---
(mlet* %store-monad (...
                     (extras (if (null? (manifest-entries manifest))
                                 (return '())
                                 (mapm/accumulate-builds (lambda (hook)
                                                           (hook manifest))
                                                         hooks))))
                     ...)
--8<---------------cut here---------------end--------------->8---

At this point, I thought: “Well, I could just parameterize
%current-system inside the (lambda (hook) ...), and all would be well,
right?  Well, it didn't seem to work and I was pretty confused by it.  I
tested a bit and noticed that actually, contrary to what was intended
(there even is a comment in gexp-derivation about it), gexp-derivation
looks up the system at monadic run time!  It looks like this:

--8<---------------cut here---------------start------------->8---
(mlet* %store-monad ( ;; The following binding forces '%current-system' and
                     ;; '%current-target-system' to be looked up at >>=
                     ;; time.
                     (graft?    (set-grafting graft?))

                     (system -> (or system (%current-system)))
                     (target -> (if (eq? target 'current)
                                    (%current-target-system)
                                    target))
                     ...)
  ...)
--8<---------------cut here---------------end--------------->8---

Well, the issue here is that such an mlet starts by translating the
graft? binding into a >>= call, which ends up putting the rest of the
translation into a function call that will *not* be called until the
monadic value is run.  That means that the system and target bindings
afterwards are *not* looked up at call time but at monadic run time!
And sure enough, moving the (graft? ...) binding after the (target ->
...) one does solve this!

IMO, this is way too complicated to keep in mind at all times, and there
are bugs lurking under the surface absolutely everywhere, waiting for a
corner case to be uncovered.

I'll send a new patch once I've fixed and tested this further.

Best,
-- 
Josselin Poiret

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 682 bytes --]

bug#65225: [PATCH] environment: Build the profile for the requested system.

[Josselin Poiret via Bug reports for GNU Guix]

[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]

Hi everyone,

Actually, let me add a big erratum, since it appears I misread the
intention of gexp->derivation (and realized 2 minutes after writing the
previous email).

Josselin Poiret <dev@jpoiret.xyz> writes:
> --8<---------------cut here---------------start------------->8---
> (mlet* %store-monad ( ;; The following binding forces '%current-system' and
>                      ;; '%current-target-system' to be looked up at >>=
>                      ;; time.
>                      (graft?    (set-grafting graft?))
>
>                      (system -> (or system (%current-system)))
>                      (target -> (if (eq? target 'current)
>                                     (%current-target-system)
>                                     target))
>                      ...)
>   ...)
> --8<---------------cut here---------------end--------------->8---
>
> Well, the issue here is that such an mlet starts by translating the
> graft? binding into a >>= call, which ends up putting the rest of the
> translation into a function call that will *not* be called until the
> monadic value is run.  That means that the system and target bindings
> afterwards are *not* looked up at call time but at monadic run time!

This is actually what the comment above hints at, I misunderstood its
meaning.  It seems that this piece of code used to be (before 2015)

--8<---------------cut here---------------start------------->8---
(mlet* %store-monad ( ;; The following binding forces '%current-system' and
                     ;; '%current-target-system' to be looked up at >>=
                     ;; time.
                     (unused    (return #f)

                     (system -> (or system (%current-system)))
                     (target -> (if (eq? target 'current)
                                    (%current-target-system)
                                    target))
                     ...)
  ...)
--8<---------------cut here---------------end--------------->8---

probably at a time when (current-system) didn't exist.  In turn, this
means that gexp->derivation intentionally delays getting the current
system to monadic run time.  Thus, we probably need to pass an optional
#:system argument to the hooks that they can forward to
gexp->derivation to fix this “properly”

> IMO, this is way too complicated to keep in mind at all times, and there
> are bugs lurking under the surface absolutely everywhere, waiting for a
> corner case to be uncovered.

My comment still stands.

Best,
-- 
Josselin Poiret

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 682 bytes --]

bug#65225: [PATCH] environment: Build the profile for the requested system.

[Ludovic Courtès]

Hi,

Josselin Poiret <dev@jpoiret.xyz> skribis:

> IMO, this is way too complicated to keep in mind at all times, and there
> are bugs lurking under the surface absolutely everywhere, waiting for a
> corner case to be uncovered.

Great writeup!  Yeah, that’s the sad situation of mixing parameters and
monads; it’s a longstanding issue and as you write, there have been
bugs.  The solution to these has been primarily to avoid relying on
default values and pass explicit values.  The other trick is what
‘lower-gexp’ does, with the comment you quoted, or what
‘gexp->derivation’ does.

The good news is that the monadic interface is kept internal
and not really exposed to users, who should stick to file-like objects
as much as possible.

Back to profile hooks: it seems that passing #:system #f to
‘gexp->derivation’ in each of these hooks would solve the problem, no?

Alternatively, each hook could take ‘system’ as a second argument.

Willing to give it a shot?

Thanks,
Ludo’.

[bug#66640] [PATCH 0/2] Build profile hooks for the right system

[Ludovic Courtès]

Hello Guix,

This fixes <https://issues.guix.gnu.org/65225>, ensuring that profile
hooks are built for the right system.  It does so by passing each
profile hook the targeted system.

Thoughts?

Thanks,
Ludo'.

Ludovic Courtès (2):
  packages: Add ‘system’ parameter for ‘set-guile-for-build’.
  profiles: Hooks honor the #:system parameter of ‘profile-derivation’.

 gnu/bootloader.scm |  5 +++--
 guix/channels.scm  |  3 ++-
 guix/packages.scm  |  7 ++++---
 guix/profiles.scm  | 49 ++++++++++++++++++++++++++++++----------------
 tests/profiles.scm | 24 ++++++++++++++++++++++-
 5 files changed, 64 insertions(+), 24 deletions(-)


base-commit: 8d6b3dd0b863ccada887da8cd347727dd04cb456
-- 
2.41.0

[bug#66640] [PATCH 1/2] packages: Add ‘system’ parameter for ‘set-guile-for-build’.

[Ludovic Courtès]

* guix/packages.scm (set-guile-for-build): Add ‘system’ parameter.
---
 guix/packages.scm | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/guix/packages.scm b/guix/packages.scm
index f70fad695e..e2e82692ad 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -2022,11 +2022,12 @@ (define* (package-output store package
 ;;; Monadic interface.
 ;;;
 
-(define (set-guile-for-build guile)
+(define* (set-guile-for-build guile #:optional system)
   "This monadic procedure changes the Guile currently used to run the build
-code of derivations to GUILE, a package object."
+code of derivations to GUILE, a package object, compiled for SYSTEM."
   (lambda (store)
-    (let ((guile (package-derivation store guile)))
+    (let ((guile (package-derivation store guile
+                                     (or system (%current-system)))))
       (values (%guile-for-build guile) store))))
 
 (define* (package-file package
-- 
2.41.0

[bug#66640] [PATCH 2/2] profiles: Hooks honor the #:system parameter of ‘profile-derivation’.

[Ludovic Courtès]

Fixes <https://issues.guix.gnu.org/65225>.

* guix/profiles.scm (info-dir-file, package-cache-file)
(info-dir-file, ghc-package-cache-file, ca-certificate-bundle)
(emacs-subdirs, gdk-pixbuf-loaders-cache-file, glib-schemas)
(gtk-icon-themes, gtk-im-modules, linux-module-database)
(xdg-desktop-database, xdg-mime-database, fonts-dir-file)
(manual-database, manual-database/optional): Add optional #:system
parameter and pass it to ‘gexp->derivation’.
(profile-derivation): Pass HOOK a second parameter, SYSTEM.
* gnu/bootloader.scm (efi-bootloader-profile)[efi-bootloader-profile-hook]:
Add optional #:system parameter and pass it to ‘gexp->derivation’.
* guix/channels.scm (package-cache-file): Likewise.
* tests/profiles.scm ("profile-derivation, #:system, and hooks"): New
test.

Reported-by: Tobias Geerinckx-Rice <me@tobias.gr>
---
 gnu/bootloader.scm |  5 +++--
 guix/channels.scm  |  3 ++-
 guix/profiles.scm  | 49 ++++++++++++++++++++++++++++++----------------
 tests/profiles.scm | 24 ++++++++++++++++++++++-
 4 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/gnu/bootloader.scm b/gnu/bootloader.scm
index 2c36d8c6cf..ba06de7618 100644
--- a/gnu/bootloader.scm
+++ b/gnu/bootloader.scm
@@ -2,7 +2,7 @@
 ;;; Copyright © 2017 David Craven <david@craven.ch>
 ;;; Copyright © 2017, 2020, 2022 Mathieu Othacehe <othacehe@gnu.org>
 ;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
-;;; Copyright © 2019, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2019, 2021, 2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2022 Josselin Poiret <dev@jpoiret.xyz>
 ;;; Copyright © 2022 Reza Alizadeh Majd <r.majd@pantherx.org>
@@ -335,7 +335,7 @@ (define (efi-bootloader-profile packages files hooks)
 local-file, etc., or package contents produced with file-append.
 
 HOOKS lists additional hook functions to modify the profile."
-  (define (efi-bootloader-profile-hook manifest)
+  (define* (efi-bootloader-profile-hook manifest #:optional system)
     (define build
         (with-imported-modules '((guix build utils))
           #~(begin
@@ -383,6 +383,7 @@ (define (efi-bootloader-profile packages files hooks)
 
     (gexp->derivation "efi-bootloader-profile"
                       build
+                      #:system system
                       #:local-build? #t
                       #:substitutable? #f
                       #:properties
diff --git a/guix/channels.scm b/guix/channels.scm
index 681adafc6c..f01903642d 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -926,7 +926,7 @@ (define* (channel-instances->manifest instances #:key system)
                        (entries ->  (map instance->entry instances derivations)))
     (return (manifest entries))))
 
-(define (package-cache-file manifest)
+(define* (package-cache-file manifest #:optional system)
   "Build a package cache file for the instance in MANIFEST.  This is meant to
 be used as a profile hook."
   ;; Note: Emit a profile in format version 3, which was introduced in 2017
@@ -961,6 +961,7 @@ (define (package-cache-file manifest)
 
     (gexp->derivation-in-inferior "guix-package-cache" build
                                   profile
+                                  #:system system
 
                                   ;; If the Guix in PROFILE is too old and
                                   ;; lacks 'guix repl', don't build the cache
diff --git a/guix/profiles.scm b/guix/profiles.scm
index fea766879d..5d2fb8dc64 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -993,7 +993,7 @@ (define* (manifest-lookup-package manifest name #:optional version)
   (anym %store-monad
         entry-lookup-package (manifest-entries manifest)))
 
-(define (info-dir-file manifest)
+(define* (info-dir-file manifest #:optional system)
   "Return a derivation that builds the 'dir' file for all the entries of
 MANIFEST."
   (define texinfo                                 ;lazy reference
@@ -1051,13 +1051,14 @@ (define (info-dir-file manifest)
                                    '#$(manifest-inputs manifest)))))))
 
   (gexp->derivation "info-dir" build
+                    #:system system
                     #:local-build? #t
                     #:substitutable? #f
                     #:properties
                     `((type . profile-hook)
                       (hook . info-dir))))
 
-(define (ghc-package-cache-file manifest)
+(define* (ghc-package-cache-file manifest #:optional system)
   "Return a derivation that builds the GHC 'package.cache' file for all the
 entries of MANIFEST, or #f if MANIFEST does not have any GHC packages."
   (define ghc                                     ;lazy reference
@@ -1108,6 +1109,7 @@ (define (ghc-package-cache-file manifest)
     (if (any (cut string-prefix? "ghc" <>)
              (map manifest-entry-name (manifest-entries manifest)))
         (gexp->derivation "ghc-package-cache" build
+                          #:system system
                           #:local-build? #t
                           #:substitutable? #f
                           #:properties
@@ -1115,7 +1117,7 @@ (define (ghc-package-cache-file manifest)
                             (hook . ghc-package-cache)))
         (return #f))))
 
-(define (ca-certificate-bundle manifest)
+(define* (ca-certificate-bundle manifest #:optional system)
   "Return a derivation that builds a single-file bundle containing the CA
 certificates in the /etc/ssl/certs sub-directories of the packages in
 MANIFEST.  Single-file bundles are required by programs such as Git and Lynx."
@@ -1179,13 +1181,14 @@ (define (ca-certificate-bundle manifest)
                #t))))))
 
   (gexp->derivation "ca-certificate-bundle" build
+                    #:system system
                     #:local-build? #t
                     #:substitutable? #f
                     #:properties
                     `((type . profile-hook)
                       (hook . ca-certificate-bundle))))
 
-(define (emacs-subdirs manifest)
+(define* (emacs-subdirs manifest #:optional system)
   (define build
     (with-imported-modules (source-module-closure
                             '((guix build profiles)
@@ -1219,13 +1222,14 @@ (define (emacs-subdirs manifest)
                   (newline port)
                   #t)))))))
   (gexp->derivation "emacs-subdirs" build
+                    #:system system
                     #:local-build? #t
                     #:substitutable? #f
                     #:properties
                     `((type . profile-hook)
                       (hook . emacs-subdirs))))
 
-(define (gdk-pixbuf-loaders-cache-file manifest)
+(define* (gdk-pixbuf-loaders-cache-file manifest #:optional system)
   "Return a derivation that produces a loaders cache file for every gdk-pixbuf
 loaders discovered in MANIFEST."
   (define gdk-pixbuf                    ;lazy reference
@@ -1264,6 +1268,7 @@ (define (gdk-pixbuf-loaders-cache-file manifest)
 
     (if gdk-pixbuf
         (gexp->derivation "gdk-pixbuf-loaders-cache-file" build
+                          #:system system
                           #:local-build? #t
                           #:substitutable? #f
                           #:properties
@@ -1271,7 +1276,7 @@ (define (gdk-pixbuf-loaders-cache-file manifest)
                             (hook . gdk-pixbuf-loaders-cache-file)))
         (return #f))))
 
-(define (glib-schemas manifest)
+(define* (glib-schemas manifest #:optional system)
   "Return a derivation that unions all schemas from manifest entries and
 creates the Glib 'gschemas.compiled' file."
   (define glib  ; lazy reference
@@ -1318,6 +1323,7 @@ (define (glib-schemas manifest)
     ;; Don't run the hook when there's nothing to do.
     (if %glib
         (gexp->derivation "glib-schemas" build
+                          #:system system
                           #:local-build? #t
                           #:substitutable? #f
                           #:properties
@@ -1325,7 +1331,7 @@ (define (glib-schemas manifest)
                             (hook . glib-schemas)))
         (return #f))))
 
-(define (gtk-icon-themes manifest)
+(define* (gtk-icon-themes manifest #:optional system)
   "Return a derivation that unions all icon themes from manifest entries and
 creates the GTK+ 'icon-theme.cache' file for each theme."
   (define gtk+  ; lazy reference
@@ -1377,6 +1383,7 @@ (define (gtk-icon-themes manifest)
     ;; Don't run the hook when there's nothing to do.
     (if %gtk+
         (gexp->derivation "gtk-icon-themes" build
+                          #:system system
                           #:local-build? #t
                           #:substitutable? #f
                           #:properties
@@ -1384,7 +1391,7 @@ (define (gtk-icon-themes manifest)
                             (hook . gtk-icon-themes)))
         (return #f))))
 
-(define (gtk-im-modules manifest)
+(define* (gtk-im-modules manifest #:optional system)
   "Return a derivation that builds the cache files for input method modules
 for both major versions of GTK+."
 
@@ -1454,6 +1461,7 @@ (define (gtk-im-modules manifest)
                            #t))))
       (if (or gtk+ gtk+-2)
           (gexp->derivation "gtk-im-modules" gexp
+                            #:system system
                             #:local-build? #t
                             #:substitutable? #f
                             #:properties
@@ -1461,7 +1469,7 @@ (define (gtk-im-modules manifest)
                               (hook . gtk-im-modules)))
           (return #f)))))
 
-(define (linux-module-database manifest)
+(define* (linux-module-database manifest #:optional system)
   "Return a derivation that unites all the kernel modules of the manifest
 and creates the dependency graph of all these kernel modules.
 
@@ -1511,13 +1519,14 @@ (define (linux-module-database manifest)
                 (_ (error "Specified Linux kernel and Linux kernel modules
 are not all of the same version"))))))))
   (gexp->derivation "linux-module-database" build
+                    #:system system
                     #:local-build? #t
                     #:substitutable? #f
                     #:properties
                     `((type . profile-hook)
                       (hook . linux-module-database))))
 
-(define (xdg-desktop-database manifest)
+(define* (xdg-desktop-database manifest #:optional system)
   "Return a derivation that builds the @file{mimeinfo.cache} database from
 desktop files.  It's used to query what applications can handle a given
 MIME type."
@@ -1551,6 +1560,7 @@ (define (xdg-desktop-database manifest)
     ;; Don't run the hook when 'glib' is not referenced.
     (if glib
         (gexp->derivation "xdg-desktop-database" build
+                          #:system system
                           #:local-build? #t
                           #:substitutable? #f
                           #:properties
@@ -1558,7 +1568,7 @@ (define (xdg-desktop-database manifest)
                             (hook . xdg-desktop-database)))
         (return #f))))
 
-(define (xdg-mime-database manifest)
+(define* (xdg-mime-database manifest #:optional system)
   "Return a derivation that builds the @file{mime.cache} database from manifest
 entries.  It's used to query the MIME type of a given file."
   (define shared-mime-info  ; lazy reference
@@ -1605,6 +1615,7 @@ (define (xdg-mime-database manifest)
     ;; Don't run the hook when there are no GLib based applications.
     (if glib
         (gexp->derivation "xdg-mime-database" build
+                          #:system system
                           #:local-build? #t
                           #:substitutable? #f
                           #:properties
@@ -1615,7 +1626,7 @@ (define (xdg-mime-database manifest)
 ;; Several font packages may install font files into same directory, so
 ;; fonts.dir and fonts.scale file should be generated here, instead of in
 ;; packages.
-(define (fonts-dir-file manifest)
+(define* (fonts-dir-file manifest #:optional system)
   "Return a derivation that builds the @file{fonts.dir} and @file{fonts.scale}
 files for the fonts of the @var{manifest} entries."
   (define mkfontscale
@@ -1676,6 +1687,7 @@ (define (fonts-dir-file manifest)
                             directories)))))))
 
   (gexp->derivation "fonts-dir" build
+                    #:system system
                     #:modules '((guix build utils)
                                 (guix build union)
                                 (srfi srfi-26))
@@ -1685,7 +1697,7 @@ (define (fonts-dir-file manifest)
                     `((type . profile-hook)
                       (hook . fonts-dir))))
 
-(define (manual-database manifest)
+(define* (manual-database manifest #:optional system)
   "Return a derivation that builds the manual page database (\"mandb\") for
 the entries in MANIFEST."
   (define gdbm-ffi
@@ -1761,23 +1773,24 @@ (define (manual-database manifest)
               (force-output))))))
 
   (gexp->derivation "manual-database" build
+                    #:system system
                     #:substitutable? #f
                     #:local-build? #t
                     #:properties
                     `((type . profile-hook)
                       (hook . manual-database))))
 
-(define (manual-database/optional manifest)
+(define* (manual-database/optional manifest #:optional system)
   "Return a derivation to build the manual database of MANIFEST, but only if
 MANIFEST contains the \"man-db\" package.  Otherwise, return #f."
   ;; Building the man database (for "man -k") is expensive and rarely used.
   ;; Build it only if the profile also contains "man-db".
   (mlet %store-monad ((man-db (manifest-lookup-package manifest "man-db")))
     (if man-db
-        (manual-database manifest)
+        (manual-database manifest system)
         (return #f))))
 
-(define (texlive-font-maps manifest)
+(define* (texlive-font-maps manifest #:optional system)
   "Return a derivation that builds the TeX Live font maps for the entries in
 MANIFEST."
   (define entry->texlive-input
@@ -1898,6 +1911,7 @@ (define (texlive-font-maps manifest)
     ;; incomplete modular TeX Live installations to generate errors.
     (if (any texlive-scripts-entry? (manifest-entries manifest))
         (gexp->derivation "texlive-font-maps" build
+                          #:system system
                           #:substitutable? #f
                           #:local-build? #t
                           #:properties
@@ -1977,7 +1991,8 @@ (define* (profile-derivation manifest
                        (extras (if (null? (manifest-entries manifest))
                                    (return '())
                                    (mapm/accumulate-builds (lambda (hook)
-                                                             (hook manifest))
+                                                             (hook manifest
+                                                                   system))
                                                            hooks))))
     (define extra-inputs
       (filter-map (lambda (drv)
diff --git a/tests/profiles.scm b/tests/profiles.scm
index 9ad03f2b24..9c419ada93 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2013-2022 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2014 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -382,6 +382,28 @@ (define glibc
        (_          (built-derivations (list drv))))
     (return (file-exists? (string-append bindir "/guile")))))
 
+(test-assertm "profile-derivation, #:system, and hooks"
+  ;; Make sure all the profile hooks are built for the system specified with
+  ;; #:system, even if that does not match (%current-system).
+  ;; See <https://issues.guix.gnu.org/65225>.
+  (mlet* %store-monad
+      ((system -> (if (string=? (%current-system) "riscv64-linux")
+                      "x86_64-linux"
+                      "riscv64-linux"))
+       (entry -> (package->manifest-entry packages:coreutils))
+       (_        (set-guile-for-build (default-guile) system))
+       (drv      (profile-derivation (manifest (list entry))
+                                     #:system system))
+       (refs     (references* (derivation-file-name drv))))
+    (return (and (string=? (derivation-system drv) system)
+                 (pair? refs)
+                 (every (lambda (ref)
+                          (or (not (string-suffix? ".drv" ref))
+                              (let ((drv (read-derivation-from-file ref)))
+                                (string=? (derivation-system drv)
+                                          system))))
+                        refs)))))
+
 (test-assertm "profile-derivation relative symlinks, one entry"
   (mlet* %store-monad
       ((entry ->   (package->manifest-entry %bootstrap-guile))
-- 
2.41.0

bug#65225: ‘guix shell --system=ALIEN’ builds for both systems

[Maxim Cournoyer]

Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> Hello Guix,
>
> This fixes <https://issues.guix.gnu.org/65225>, ensuring that profile
> hooks are built for the right system.  It does so by passing each
> profile hook the targeted system.
>
> Thoughts?
>
> Thanks,
> Ludo'.
>
> Ludovic Courtès (2):
>   packages: Add ‘system’ parameter for ‘set-guile-for-build’.
>   profiles: Hooks honor the #:system parameter of ‘profile-derivation’.
>
>  gnu/bootloader.scm |  5 +++--
>  guix/channels.scm  |  3 ++-
>  guix/packages.scm  |  7 ++++---
>  guix/profiles.scm  | 49 ++++++++++++++++++++++++++++++----------------
>  tests/profiles.scm | 24 ++++++++++++++++++++++-
>  5 files changed, 64 insertions(+), 24 deletions(-)

I see the cover letter, but not the patches :-).  Where did they go?

-- 
Thanks,
Maxim

bug#65225: ‘guix shell --system=ALIEN’ builds for both systems

[Simon Tournier]

Hi Maxim,

On Fri, 20 Oct 2023 at 18:46, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> I see the cover letter, but not the patches :-).  Where did they go?

They are in #66640 [1].  The cover letter had been CC:
65225@debbugs.gnu.org, I guess.

1: https://issues.guix.gnu.org/issue/66640

Cheers,
simon

bug#65225: ‘guix shell --system=ALIEN’ builds for both systems

[Maxim Cournoyer]

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi Maxim,
>
> On Fri, 20 Oct 2023 at 18:46, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>> I see the cover letter, but not the patches :-).  Where did they go?
>
> They are in #66640 [1].  The cover letter had been CC:
> 65225@debbugs.gnu.org, I guess.

Thanks!

-- 
Maxim

bug#65225: [bug#66640] [PATCH 0/2] Build profile hooks for the right system

[Simon Tournier]

Hi Ludo,

A naive question about the default value.


On Thu, 19 Oct 2023 at 16:48, Ludovic Courtès <ludo@gnu.org> wrote:

>   packages: Add ‘system’ parameter for ‘set-guile-for-build’.

The change reads,

--8<---------------cut here---------------start------------->8---
+(define* (set-guile-for-build guile #:optional system)
[...]
+    (let ((guile (package-derivation store guile
+                                     (or system (%current-system)))))
--8<---------------cut here---------------end--------------->8---

>   profiles: Hooks honor the #:system parameter of ‘profile-derivation’.

The change reads for all hooks,

--8<---------------cut here---------------start------------->8---
+  (define* (efi-bootloader-profile-hook manifest #:optional system)
[...]
     (gexp->derivation "efi-bootloader-profile"
                       build
+                      #:system system
--8<---------------cut here---------------end--------------->8---

Do I understand correctly when I consider that
’(efi-bootloader-profile-hook manifest)’ pass #false as ’system’ and
then it is ’set-guile-for-build’ that sets ’(%current-system)’?

Somehow, my question is about the readability of default values.

Why not,

    #:system (or system (%current-system))

for all the hooks?


Cheers,
simon

[bug#66640] [PATCH 0/2] Build profile hooks for the right system

[Ludovic Courtès]

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> Do I understand correctly when I consider that
> ’(efi-bootloader-profile-hook manifest)’ pass #false as ’system’ and
> then it is ’set-guile-for-build’ that sets ’(%current-system)’?

Yes.

> Somehow, my question is about the readability of default values.
>
> Why not,
>
>     #:system (or system (%current-system))
>
> for all the hooks?

Because that value would be captured at call time rather than at
monadic-bind time.  See Josselin’s excellent explanation of this trap:

  https://issues.guix.gnu.org/65225#4-lineno34

HTH!

Ludo’.

[bug#66640] [PATCH 0/2] Build profile hooks for the right system

[Simon Tournier]

Hi,

On Mon, 23 Oct 2023 at 21:53, Ludovic Courtès <ludo@gnu.org> wrote:

> Because that value would be captured at call time rather than at
> monadic-bind time.  See Josselin’s excellent explanation of this trap:
>
>   https://issues.guix.gnu.org/65225#4-lineno34

Yeah, all clear.  Thank you.


Before the patch:

--8<---------------cut here---------------start------------->8---
$ guix shell --system=riscv64-linux sterm --no-grafts --rebuild-cache --dry-run 2>&1 | grep profile
  /gnu/store/0l02iwcq3bzh38qykg70ygy5cf5c2hml-profile.drv

$ guix repl
GNU Guile 3.0.9
Copyright (C) 1995-2023 Free Software Foundation, Inc.

Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
This program is free software, and you are welcome to redistribute it
under certain conditions; type `,show c' for details.

Enter `,help' for help.
scheme@(guix-user)> ,run-in-store (references* "/gnu/store/0l02iwcq3bzh38qykg70ygy5cf5c2hml-profile.drv")
$1 = ("/gnu/store/4zg87dbcf7mcr82jlbfsbqqffn1miaml-ca-certificate-bundle.drv"
 "/gnu/store/5myp281kr169dag03plk5wy0s1jmkk40-emacs-subdirs.drv"
 "/gnu/store/7s9j9ns9mmfg0vg8wpjvjj0x8rrq992f-info-dir.drv"
 "/gnu/store/8q36aw38kr2879ll9zrn3cg7w5xpxmjd-module-import"
 "/gnu/store/bvnibpf0kfk2w4vh069q97mk98q3pwbz-profile-builder"
 "/gnu/store/fw22lk3vnal8477lpbi0f5wfgh8i1w6f-fonts-dir.drv"
 "/gnu/store/j0vmqz44kmsp8jhpxw8wa4jq60rjfgaj-module-import-compiled.drv"
 "/gnu/store/s38y5afcy2bwzsspqsz2n4riiqy77gr4-sterm-20200306.drv"
 "/gnu/store/x6qzhxy1wilipnlygi508l9yvz66dlvs-glibc-utf8-locales-2.35.drv"
 "/gnu/store/zraigp7miin3vzr5dcbr4i9rvds0i07r-guile-3.0.9.drv")
scheme@(guix-user)> $1
;;; <stdin>:4:0: warning: possibly unbound variable `$1'
$2 = ("/gnu/store/4zg87dbcf7mcr82jlbfsbqqffn1miaml-ca-certificate-bundle.drv" "/gnu/store/5myp281kr169dag03plk5wy0s1jmkk40-emacs-subdirs.drv" "/gnu/store/7s9j9ns9mmfg0vg8wpjvjj0x8rrq992f-info-dir.drv" "/gnu/store/8q36aw38kr2879ll9zrn3cg7w5xpxmjd-module-import" "/gnu/store/bvnibpf0kfk2w4vh069q97mk98q3pwbz-profile-builder" "/gnu/store/fw22lk3vnal8477lpbi0f5wfgh8i1w6f-fonts-dir.drv" "/gnu/store/j0vmqz44kmsp8jhpxw8wa4jq60rjfgaj-module-import-compiled.drv" "/gnu/store/s38y5afcy2bwzsspqsz2n4riiqy77gr4-sterm-20200306.drv" "/gnu/store/x6qzhxy1wilipnlygi508l9yvz66dlvs-glibc-utf8-locales-2.35.drv" "/gnu/store/zraigp7miin3vzr5dcbr4i9rvds0i07r-guile-3.0.9.drv")
scheme@(guix-user)> ,pp (map (lambda (drv) (list drv (or (not (string-suffix? ".drv" drv)) (derivation-system (read-derivation-from-file drv))))) $1)
$3 = (("/gnu/store/4zg87dbcf7mcr82jlbfsbqqffn1miaml-ca-certificate-bundle.drv"
  "x86_64-linux")
 ("/gnu/store/5myp281kr169dag03plk5wy0s1jmkk40-emacs-subdirs.drv"
  "x86_64-linux")
 ("/gnu/store/7s9j9ns9mmfg0vg8wpjvjj0x8rrq992f-info-dir.drv"
  "x86_64-linux")
 ("/gnu/store/8q36aw38kr2879ll9zrn3cg7w5xpxmjd-module-import"
  #t)
 ("/gnu/store/bvnibpf0kfk2w4vh069q97mk98q3pwbz-profile-builder"
  #t)
 ("/gnu/store/fw22lk3vnal8477lpbi0f5wfgh8i1w6f-fonts-dir.drv"
  "x86_64-linux")
 ("/gnu/store/j0vmqz44kmsp8jhpxw8wa4jq60rjfgaj-module-import-compiled.drv"
  "riscv64-linux")
 ("/gnu/store/s38y5afcy2bwzsspqsz2n4riiqy77gr4-sterm-20200306.drv"
  "riscv64-linux")
 ("/gnu/store/x6qzhxy1wilipnlygi508l9yvz66dlvs-glibc-utf8-locales-2.35.drv"
  "riscv64-linux")
 ("/gnu/store/zraigp7miin3vzr5dcbr4i9rvds0i07r-guile-3.0.9.drv"
  "x86_64-linux"))
--8<---------------cut here---------------end--------------->8---

After the patch:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix shell --system=riscv64-linux sterm --no-grafts --rebuild-cache --dry-run 2>&1 | grep profile
  /gnu/store/1nnji4fr8mgpsal0rcnzbdjdm4p04yk3-profile.drv

$ guix repl
GNU Guile 3.0.9
Copyright (C) 1995-2023 Free Software Foundation, Inc.

Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
This program is free software, and you are welcome to redistribute it
under certain conditions; type `,show c' for details.

Enter `,help' for help.
scheme@(guix-user)> ,run-in-store (references* "/gnu/store/1nnji4fr8mgpsal0rcnzbdjdm4p04yk3-profile.drv")
$1 = ("/gnu/store/2ff9j170rckkr2zs0l6sf4bbkx7fk5vc-profile-builder"
 "/gnu/store/5mi4ij2vf5cxhsbh52n04h48mvc6z6r8-module-import-compiled.drv"
 "/gnu/store/5sbcg8siv5jzzbdy4m7bfravpxg95j7h-emacs-subdirs.drv"
 "/gnu/store/7rz0kd6bigj3gdlp9l4z1v91sxy43bib-fonts-dir.drv"
 "/gnu/store/8l03g4sg271c0wc2axhpn689r6faa0ij-info-dir.drv"
 "/gnu/store/94ad504y18rg0d1kgznlj0ib1l56ljxl-ca-certificate-bundle.drv"
 "/gnu/store/w314gb2gc9z6jd9ng7gpn88zvhhsax09-sterm-20200306.drv"
 "/gnu/store/x6qzhxy1wilipnlygi508l9yvz66dlvs-glibc-utf8-locales-2.35.drv"
 "/gnu/store/y545dx7df92al3yz1a9swnf0lhjg9igi-module-import"
 "/gnu/store/zb3vrbwv8qx9430n839ljbalnik8019g-guile-3.0.9.drv")
scheme@(guix-user)> $1
;;; <stdin>:2:0: warning: possibly unbound variable `$1'
$2 = ("/gnu/store/2ff9j170rckkr2zs0l6sf4bbkx7fk5vc-profile-builder" "/gnu/store/5mi4ij2vf5cxhsbh52n04h48mvc6z6r8-module-import-compiled.drv" "/gnu/store/5sbcg8siv5jzzbdy4m7bfravpxg95j7h-emacs-subdirs.drv" "/gnu/store/7rz0kd6bigj3gdlp9l4z1v91sxy43bib-fonts-dir.drv" "/gnu/store/8l03g4sg271c0wc2axhpn689r6faa0ij-info-dir.drv" "/gnu/store/94ad504y18rg0d1kgznlj0ib1l56ljxl-ca-certificate-bundle.drv" "/gnu/store/w314gb2gc9z6jd9ng7gpn88zvhhsax09-sterm-20200306.drv" "/gnu/store/x6qzhxy1wilipnlygi508l9yvz66dlvs-glibc-utf8-locales-2.35.drv" "/gnu/store/y545dx7df92al3yz1a9swnf0lhjg9igi-module-import" "/gnu/store/zb3vrbwv8qx9430n839ljbalnik8019g-guile-3.0.9.drv")
scheme@(guix-user)> ,pp (map (lambda (drv) (list drv (or (not (string-suffix? ".drv" drv)) (derivation-system (read-derivation-from-file drv))))) $1)
$3 = (("/gnu/store/2ff9j170rckkr2zs0l6sf4bbkx7fk5vc-profile-builder"
  #t)
 ("/gnu/store/5mi4ij2vf5cxhsbh52n04h48mvc6z6r8-module-import-compiled.drv"
  "riscv64-linux")
 ("/gnu/store/5sbcg8siv5jzzbdy4m7bfravpxg95j7h-emacs-subdirs.drv"
  "riscv64-linux")
 ("/gnu/store/7rz0kd6bigj3gdlp9l4z1v91sxy43bib-fonts-dir.drv"
  "riscv64-linux")
 ("/gnu/store/8l03g4sg271c0wc2axhpn689r6faa0ij-info-dir.drv"
  "riscv64-linux")
 ("/gnu/store/94ad504y18rg0d1kgznlj0ib1l56ljxl-ca-certificate-bundle.drv"
  "riscv64-linux")
 ("/gnu/store/w314gb2gc9z6jd9ng7gpn88zvhhsax09-sterm-20200306.drv"
  "riscv64-linux")
 ("/gnu/store/x6qzhxy1wilipnlygi508l9yvz66dlvs-glibc-utf8-locales-2.35.drv"
  "riscv64-linux")
 ("/gnu/store/y545dx7df92al3yz1a9swnf0lhjg9igi-module-import"
  #t)
 ("/gnu/store/zb3vrbwv8qx9430n839ljbalnik8019g-guile-3.0.9.drv"
  "x86_64-linux"))
--8<---------------cut here---------------end--------------->8---

Therefore, it seems fixing the issue.  All the x86_64-linux above are
turned as riscv64-linux.  However, still a naive question:

Why ’guile’ is not?  Is it expected?

If not, does it mean that some ’set-guile-for-build’ is missing?

Cheers,
simon

bug#66640: [PATCH 0/2] Build profile hooks for the right system

[Ludovic Courtès]

Hi,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

>  ("/gnu/store/zb3vrbwv8qx9430n839ljbalnik8019g-guile-3.0.9.drv"
>   "x86_64-linux"))
>
> Therefore, it seems fixing the issue.  All the x86_64-linux above are
> turned as riscv64-linux.  However, still a naive question:
>
> Why ’guile’ is not?  Is it expected?

Oh, that’s another bug, this one in ‘guix shell’ (missing argument for
‘package-derivation’).

I pushed this:

  344e39c928 profiles: Hooks honor the #:system parameter of ‘profile-derivation’.
  9d4b720e1f packages: Add ‘system’ parameter for ‘set-guile-for-build’.
  b3ec2a0d37 environment: Honor ‘-s’ for guile-for-build.

Thanks for reviewing and reporting the issue!

Ludo’.

bug#65225: ‘guix shell --system=ALIEN’ builds for both systems

[Tobias Geerinckx-Rice via Bug reports for GNU Guix]

Oh, OK.

Parts of scripts/shell.scm (and perhaps environment.scm) assume that

$ guix build --system={x86_64,riscv64}-linux foo

is equivalent to

$ guix build --system=riscv64-linux foo

It's not.  This includes and causes things like commit 
9c513303156b418567b9d2cde9f8df66190051ac.  There's a lot of fast & loose 
assoc-reffing going on here that makes me nervous.

Anyway, I'll get me to a PC and then fix the code that's injecting a 
spurious extra ‘(system . (%current-system))’ that's causing these 
double builds.  Simple™.

Kind regards,

T G-R

Sent from a Web browser.  Excuse or enjoy my brevity.

bug#65225: ‘guix shell --system=ALIEN’ builds for both systems

[Tobias Geerinckx-Rice via Bug reports for GNU Guix]

Oh OK.

> the code that's injecting a spurious extra ‘(system . 
> (%current-system))’ that's causing these double builds

is not actually the problem.  Instead, the profile hooks are 
unconditionally built for the host's (%current-system).  Forcing them to 
match ‘--system=’ does the trick.  Simple™!

Kind regards,

T G-R

Sent from a Web browser.  Excuse or enjoy my brevity.