[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#65225: [PATCH] environment: Build the profile for the requested syst
From: |
Josselin Poiret |
Subject: |
bug#65225: [PATCH] environment: Build the profile for the requested system. |
Date: |
Sat, 12 Aug 2023 12:33:45 +0200 |
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
signature.asc
Description: PGP signature