Profile hooks ignore system and target

  • Done
  • quality assurance status badge
Details
3 participants
  • Jan Nieuwenhuizen
  • Ludovic Courtès
  • Mathieu Othacehe
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
important
L
L
Ludovic Courtès wrote on 10 May 2020 22:38
(address . bug-guix@gnu.org)
87a72f8qzi.fsf@inria.fr
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:

Toggle snippet (17 lines)
$ 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

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

Toggle snippet (7 lines)
$ ./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

Actually all the profile hooks refer to the native derivation.

To be continued…

Ludo’.
L
L
Ludovic Courtès wrote on 11 May 2020 22:15
control message for bug #41182
(address . control@debbugs.gnu.org)
87tv0mfcsy.fsf@gnu.org
severity 41182 important
quit
L
L
Ludovic Courtès wrote on 11 May 2020 23:13
Re: bug#41182: Profile hooks ignore system and target
(address . 41182@debbugs.gnu.org)
87k11idvkz.fsf@gnu.org
Hi,

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

Toggle quote (2 lines)
> 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’.
Toggle diff (80 lines)
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)))
M
M
Mathieu Othacehe wrote on 12 May 2020 10:35
(name . Ludovic Courtès)(address . ludo@gnu.org)
87ftc5czyx.fsf@gmail.com
Hey Ludo,

Toggle quote (7 lines)
> 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:

Toggle snippet (5 lines)
(target -> (if (eq? target 'current)
(%current-target-system)
target))

to

Toggle snippet (5 lines)
(target (if (eq? target 'current)
(current-target-system)
(return target)))

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
J
J
Jan Nieuwenhuizen wrote on 13 May 2020 11:45
(name . Ludovic Courtès)(address . ludo@gnu.org)
875zd0malo.fsf@gnu.org
Ludovic Courtès writes:

Hello!

Toggle quote (6 lines)
> 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!

Toggle quote (15 lines)
> 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:

Toggle snippet (17 lines)
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)))

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
M
M
Mathieu Othacehe wrote on 13 May 2020 12:37
(name . Jan Nieuwenhuizen)(address . janneke@gnu.org)
87tv0k86ih.fsf@gmail.com
Hello Jan,

Toggle quote (7 lines)
> 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
L
L
Ludovic Courtès wrote on 14 May 2020 14:24
(address . 41182@debbugs.gnu.org)
87pnb6vh5l.fsf@gnu.org
Hi,

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

Toggle quote (8 lines)
> 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:

Toggle snippet (23 lines)
$ 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>)

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’.
L
L
Ludovic Courtès wrote on 14 May 2020 17:23
(address . 41182-done@debbugs.gnu.org)
871rnmv8up.fsf@gnu.org
Alright, fixed!

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

Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 41182@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 41182
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch