bug#41182: Profile hooks ignore system and target

bug#41182: Profile hooks ignore system and target

[Ludovic Courtès]

Hello!

As janneke found (and maybe Mathieu also), the profile hooks called by
‘profile-derivation’ for some reason end up using a different system and
target, as can be seen in this example:

--8<---------------cut here---------------start------------->8---
$ guix describe
Generacio 141	May 10 2020 11:41:44	(nuna)
  guix 279569c
    repository URL: https://git.savannah.gnu.org/git/guix.git
    branch: master
    commit: 279569ca8251b9ae3f96e4486e4614e340d0fe4f
$ guix pack --target=arm-linux-gnueabihf --no-grafts idutils -d
/gnu/store/rjbs4a5l1vi3winbf71xz3d0hhmdwnjj-tarball-pack.tar.gz.drv
$ guix gc -R /gnu/store/rjbs4a5l1vi3winbf71xz3d0hhmdwnjj-tarball-pack.tar.gz.drv | grep idutils-4.6.drv
/gnu/store/kfv7bwzapb3lfirdpyjh5zcrrpld90ni-idutils-4.6.drv
/gnu/store/1y5rjcvs6giag414wg4ngz7cp4mxy76v-idutils-4.6.drv
$ guix build idutils --no-grafts -d
/gnu/store/kfv7bwzapb3lfirdpyjh5zcrrpld90ni-idutils-4.6.drv
$ guix build idutils --no-grafts -d --target=arm-linux-gnueabihf
/gnu/store/1y5rjcvs6giag414wg4ngz7cp4mxy76v-idutils-4.6.drv
--8<---------------cut here---------------end--------------->8---

We should be seeing only the cross-compile derivation.  Where does the
native derivation come from?

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix graph --path -t derivation /gnu/store/rjbs4a5l1vi3winbf71xz3d0hhmdwnjj-tarball-pack.tar.gz.drv /gnu/store/kfv7bwzapb3lfirdpyjh5zcrrpld90ni-idutils-4.6.drv
/gnu/store/rjbs4a5l1vi3winbf71xz3d0hhmdwnjj-tarball-pack.tar.gz.drv
/gnu/store/hxp1gqbmmq4hjwnb8m1amp15k0ax455m-profile.drv
/gnu/store/3180cdca49sl6xhn9prx9xcwv20jlpdl-fonts-dir.drv
/gnu/store/kfv7bwzapb3lfirdpyjh5zcrrpld90ni-idutils-4.6.drv
--8<---------------cut here---------------end--------------->8---

Actually all the profile hooks refer to the native derivation.

To be continued…

Ludo’.

bug#41182: Profile hooks ignore system and target

[Ludovic Courtès]

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

Hi,

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

> Actually all the profile hooks refer to the native derivation.

I’ve looked at it and this problem is surprisingly tricky to address.

I’ve tried to address it in an API-compatible way, which meant setting
the ‘%current-system’ and ‘%current-target-system’ parameters around the
hook calls, but that is ugly, hard to get right (dynamic binding and
monadic code really don’t go together well :-/), and actually raises
another issue (‘mapm/accumulate-builds’ appears to ignore the initial
dynamic bindings for these two parameters).  Hacky patch attached to
illustrate.

So I’m very much tempted to instead require each hook to take ‘system’
and ‘#:target’ arguments and pass them to ‘gexp->derivation’.  It’ll
break the API, in case someone out there has custom profile hooks
(unlikely given that it’s not really documented), but I’d say that’s OK.

Thoughts?

Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3875 bytes --]

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 59ef5d078b..4f90e9e41d 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -3013,9 +3013,10 @@ memoized as a function of '%current-system'."
                                                '/memoized))))
          #'(begin
              (define memoized
-               (mlambda (system) exp))
+               (mlambda (system target) exp))
              (define-syntax identifier
-               (identifier-syntax (memoized (%current-system))))))))))
+               (identifier-syntax (memoized (%current-system)
+                                            (%current-target-system))))))))))
 
 (define/system-dependent linux-libre-headers-boot0
   ;; Note: this is wrapped in a thunk to nicely handle circular dependencies
diff --git a/guix/profiles.scm b/guix/profiles.scm
index 25ff146bdf..58d7e0e450 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1580,6 +1580,18 @@ This is one of the things to do for the result to be relocatable.
 
 When TARGET is true, it must be a GNU triplet, and the packages in MANIFEST
 are cross-built for TARGET."
+  (define (call-with-system+target system target thunk)
+    (mlet* %store-monad ((system0 (set-current-system system))
+                         (target0 (set-current-target (pk 'set-target target)))
+                         (result  (thunk)))
+      (mbegin %store-monad
+        (set-current-system system0)
+        (set-current-target target0)
+        (return result))))
+
+  (define-syntax-rule (with-system+target system target exp)
+    (call-with-system+target system target (lambda () exp)))
+
   (mlet* %store-monad ((system (if system
                                    (return system)
                                    (current-system)))
@@ -1592,9 +1604,12 @@ are cross-built for TARGET."
                                                          #:target target)))
                        (extras (if (null? (manifest-entries manifest))
                                    (return '())
-                                   (mapm/accumulate-builds (lambda (hook)
-                                                             (hook manifest))
-                                                           hooks))))
+                                   (with-system+target
+                                    system
+                                    target
+                                    (mapm/accumulate-builds (lambda (hook)
+                                                         (hook manifest))
+                                          hooks)))))
     (define inputs
       (append (filter-map (lambda (drv)
                             (and (derivation? drv)
@@ -1689,6 +1704,8 @@ are cross-built for TARGET."
   (match profile
     (($ <profile> name manifest hooks
                   locales? allow-collisions? relative-symlinks?)
+     (pk 'prof-c system target (%current-target-system))
+     ;; (display-backtrace (make-stack #t) (current-error-port) #f 80)
      (profile-derivation manifest
                          #:name name
                          #:hooks hooks
diff --git a/guix/store.scm b/guix/store.scm
index 6c7c07fd2d..92158bd658 100644
--- a/guix/store.scm
+++ b/guix/store.scm
@@ -1899,7 +1899,10 @@ coalesce them into a single call."
     (values (map/accumulate-builds store
                                    (lambda (obj)
                                      (run-with-store store
-                                       (mproc obj)))
+                                       (mproc obj)
+                                       ;; #:system (%current-system)
+                                       ;; #:target (%current-target-system)
+                                       ))
                                    lst)
             store)))
 

bug#41182: Profile hooks ignore system and target

[Mathieu Othacehe]

Hey Ludo,

> So I’m very much tempted to instead require each hook to take ‘system’
> and ‘#:target’ arguments and pass them to ‘gexp->derivation’.  It’ll
> break the API, in case someone out there has custom profile hooks
> (unlikely given that it’s not really documented), but I’d say that’s OK.
>
> Thoughts?

What seems strange to me is that gexp->derivation has target set to
'current by default, so it should use the defined target. Now, that I
look at it, it's using "%current-target-system".

Would it make any difference to switch:

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

to

--8<---------------cut here---------------start------------->8---
(target (if (eq? target 'current)
            (current-target-system)
            (return target)))
--8<---------------cut here---------------end--------------->8---

like for lower-object, gexp->file and gexp->script?

Regarding breaking the profile hooks API, it's fine by me.

Thanks for investigating this,

Mathieu

bug#41182: Profile hooks ignore system and target

[Jan Nieuwenhuizen]

Ludovic Courtès writes:

Hello!

> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> Actually all the profile hooks refer to the native derivation.
>
> I’ve looked at it and this problem is surprisingly tricky to address.

Thank you so much for looking into this!

> I’ve tried to address it in an API-compatible way, which meant setting
> the ‘%current-system’ and ‘%current-target-system’ parameters around the
> hook calls, but that is ugly, hard to get right (dynamic binding and
> monadic code really don’t go together well :-/), and actually raises
> another issue (‘mapm/accumulate-builds’ appears to ignore the initial
> dynamic bindings for these two parameters).  Hacky patch attached to
> illustrate.
>
> So I’m very much tempted to instead require each hook to take ‘system’
> and ‘#:target’ arguments and pass them to ‘gexp->derivation’.  It’ll
> break the API, in case someone out there has custom profile hooks
> (unlikely given that it’s not really documented), but I’d say that’s OK.
>
> Thoughts?

I'm all for breaking the API if that gets us further.  How about doing
that on the wip-hurd-vm branch, complete building the vm-image with
services and when we have found this (and possible next steps) are
useful fixes, commit to core-updates?

It would be useful to get substitutes for such a change, though --
testing this took all night.

Other than that I can offer my observations trying to build a kind of
minimal Hurd system using

    fb120a69a8 services: hurd: Use activation-service, hurd-etc-service.

both

    ./pre-inst-env guix system build --target=i586-pc-gnu gnu/system/examples/bare-hurd.tmpl
    ./pre-inst-env guix system vm-image --target=i586-pc-gnu gnu/system/examples/bare-hurd.tmpl

still want to build a native Hurd (in my case for x86_64).

So, I tried applying what your patch seems to be suggesting:

--8<---------------cut here---------------start------------->8---
diff --git a/guix/store.scm b/guix/store.scm
index 92158bd658..b27ad0fab3 100644
--- a/guix/store.scm
+++ b/guix/store.scm
@@ -1900,8 +1900,8 @@ coalesce them into a single call."
                                    (lambda (obj)
                                      (run-with-store store
                                        (mproc obj)
-                                       ;; #:system (%current-system)
-                                       ;; #:target (%current-target-system)
+                                       #:system (%current-system)
+                                       #:target (%current-target-system)
                                        ))
                                    lst)
             store)))
--8<---------------cut here---------------end--------------->8---

and with that "system build" succeeds (after a while) but "system vm-image" says

    guix system: error: gnu/packages/glib.scm:404:2: gobject-introspection@1.62.0: build system `meson' does not support cross builds

This could be mostly harmless...still building a full (non-tiny/minimal)
qemu or grub maybe?

Greetings
janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

bug#41182: Profile hooks ignore system and target

[Mathieu Othacehe]

Hello Jan,

> and with that "system build" succeeds (after a while) but "system vm-image" says
>
>     guix system: error: gnu/packages/glib.scm:404:2: gobject-introspection@1.62.0: build system `meson' does not support cross builds
>
> This could be mostly harmless...still building a full (non-tiny/minimal)
> qemu or grub maybe?

I remember fixing a very similar issue with
d4ddf22d54f9374715c651aaebda2315e9f89272. The issue was cross-building
QEMU does not work, because it's dragging packages built with
meson-build-system.

Turns out we want to use a native build of QEMU here, so I replaced
#$qemu by #+qemu. Now there must be a similar issue somewhere. I'm
building your branch to see if I can figure it out.

Thanks for sharing your progress!

Mathieu

bug#41182: Profile hooks ignore system and target

[Ludovic Courtès]

Hi,

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

> I’ve tried to address it in an API-compatible way, which meant setting
> the ‘%current-system’ and ‘%current-target-system’ parameters around the
> hook calls, but that is ugly, hard to get right (dynamic binding and
> monadic code really don’t go together well :-/), and actually raises
> another issue (‘mapm/accumulate-builds’ appears to ignore the initial
> dynamic bindings for these two parameters).  Hacky patch attached to
> illustrate.

I was able to boil this second sub-problem down to a simple case:

--8<---------------cut here---------------start------------->8---
$ cat /tmp/t.scm
(use-modules (guix)
             (guix grafts)
             (gnu packages idutils))

(define target
  (getenv "REAL_TARGET"))

(%graft? #f)

(with-store s
  (parameterize ((%current-target-system (getenv "TARGET")))
    (pk (if target
            (package-cross-derivation s idutils target)
            (package-derivation s idutils)))))
$ REAL_TARGET=arm-linux-gnueabihf ./pre-inst-env guile /tmp/t.scm 

;;; (#<derivation /gnu/store/1y5rjcvs6giag414wg4ngz7cp4mxy76v-idutils-4.6.drv => /gnu/store/6kq4ick0jljrfjnhw0v2yghr8nalhrqi-idutils-4.6 7f0867c0de10>)
$ TARGET=arm-linux-gnueabihf REAL_TARGET=arm-linux-gnueabihf ./pre-inst-env guile /tmp/t.scm 

;;; (#<derivation /gnu/store/4k4nqr1rpm07ypq9inhvsghrqma5yacy-idutils-4.6.drv => /gnu/store/v4rgm5yhyx5ir3622hhxcaz3a10flcyr-idutils-4.6 7f7a4b1010a0>)
--8<---------------cut here---------------end--------------->8---

IOW, the initial value of ‘%current-target-system’ leads us to pick
ld-wrapper -> guile -> bdw-gc -> libatomic-ops in the second case, which
is wrong and due to this conditional in libgc’s inputs:

   (propagated-inputs
    (if (%current-target-system)
        ;; The build system refuses to check for compiler intrinsics when
        ;; cross-compiling, and demands using libatomic-ops instead.
        `(("libatomic-ops" ,libatomic-ops))
        '()))

As it turns out, ‘guix pack’ and ‘guix system’ are the only programs
that set ‘%current-target-system’ at the top level, via
‘run-with-store’.

Ludo’.

bug#41182: Profile hooks ignore system and target

[Ludovic Courtès]

Alright, fixed!

  80963744a2 store: 'mapm/accumulate-builds' preserves '%current-target-system'.
  f52fbf7094 packages: Ensure bags are insensitive to '%current-target-system'.

Ludo’.