From: Josselin Poiret via Bug reports for GNU Guix <bug-guix@gnu.org>
To: Tobias Geerinckx-Rice <me@tobias.gr>, 65225@debbugs.gnu.org
Cc: "Tobias Geerinckx-Rice" <me@tobias.gr>,
"Simon Tournier" <zimon.toutoune@gmail.com>,
"Mathieu Othacehe" <othacehe@gnu.org>,
"Ludovic Courtès" <ludo@gnu.org>,
"Christopher Baines" <mail@cbaines.net>,
"Ricardo Wurmus" <rekado@elephly.net>
Subject: bug#65225: [PATCH] environment: Build the profile for the requested system.
Date: Sat, 12 Aug 2023 12:33:45 +0200 [thread overview]
Message-ID: <871qg8o8hi.fsf@jpoiret.xyz> (raw)
In-Reply-To: <2f29e4cbc33a82509a5980bc5ddd5f8ae53cf113.1691280000.git.me@tobias.gr>
[-- 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 --]
next prev parent reply other threads:[~2023-08-12 10:34 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-11 11:10 bug#65225: ‘guix shell --system=ALIEN’ builds for both systems Tobias Geerinckx-Rice via Bug reports for GNU Guix
2023-08-06 0:00 ` bug#65225: [PATCH] environment: Build the profile for the requested system Tobias Geerinckx-Rice via Bug reports for GNU Guix
2023-08-12 10:33 ` Josselin Poiret via Bug reports for GNU Guix [this message]
2023-08-12 10:42 ` Josselin Poiret via Bug reports for GNU Guix
2023-08-12 20:51 ` Ludovic Courtès
2023-10-19 14:48 ` [bug#66640] [PATCH 0/2] Build profile hooks for the right system Ludovic Courtès
2023-10-19 14:53 ` [bug#66640] [PATCH 1/2] packages: Add ‘system’ parameter for ‘set-guile-for-build’ Ludovic Courtès
2023-10-19 14:53 ` [bug#66640] [PATCH 2/2] profiles: Hooks honor the #:system parameter of ‘profile-derivation’ Ludovic Courtès
2023-10-20 22:46 ` bug#65225: ‘guix shell --system=ALIEN’ builds for both systems Maxim Cournoyer
2023-10-23 10:16 ` Simon Tournier
2023-10-23 14:29 ` Maxim Cournoyer
2023-10-23 10:36 ` bug#65225: [bug#66640] [PATCH 0/2] Build profile hooks for the right system Simon Tournier
2023-10-23 19:53 ` Ludovic Courtès
2023-10-24 18:34 ` Simon Tournier
2023-10-27 23:36 ` bug#66640: " Ludovic Courtès
2023-08-11 11:43 ` bug#65225: ‘guix shell --system=ALIEN’ builds for both systems Tobias Geerinckx-Rice via Bug reports for GNU Guix
2023-08-11 15:58 ` Tobias Geerinckx-Rice via Bug reports for GNU Guix
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871qg8o8hi.fsf@jpoiret.xyz \
--to=bug-guix@gnu.org \
--cc=65225@debbugs.gnu.org \
--cc=dev@jpoiret.xyz \
--cc=ludo@gnu.org \
--cc=mail@cbaines.net \
--cc=me@tobias.gr \
--cc=othacehe@gnu.org \
--cc=rekado@elephly.net \
--cc=zimon.toutoune@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.