mbox series

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

Message ID cover.1697726601.git.ludo@gnu.org
Headers show
Series Build profile hooks for the right system | expand

Message

Ludovic Courtès Oct. 19, 2023, 2:48 p.m. UTC 
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

Comments

Simon Tournier Oct. 23, 2023, 10:36 a.m. UTC | #1 
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
Ludovic Courtès Oct. 23, 2023, 7:53 p.m. UTC | #2 
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’.
Simon Tournier Oct. 24, 2023, 6:34 p.m. UTC | #3 
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
Ludovic Courtès Oct. 27, 2023, 11:36 p.m. UTC | #4 
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’.