[bug#47989] channels: Add a #:system argument to channel-instances->manifest.

This allows computing a manifest for a specific system. Previously this was
possible, but only through changing %current-system, which caused the
derivation to be computed using that system as well (so computing a derivation
for aarch64-linux on x86_64-linux would require running aarch64-linux code).

This new argument adds the possibility of computing derivations for non-native
systems, without having to run non-native code.

I'm looking at this as it will enable the Guix Data Service to compute channel
instance derivations without relying on QEMU emulation for non-native
systems (it should be faster as well).

* guix/channels.scm (build-from-source): Add #:system argument and pass to
build.
(build-channel-instance): Add system argument and pass to build-from-source.
(channel-instance-derivations): Add #:system argument and pass to
build-channel-instance, also rename system to current-system-value.
(channel-instances->manifest): Add #:system argument and pass to
channel-instance-derivations.
---
 guix/channels.scm | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

Hi,

Christopher Baines <mail@cbaines.net> skribis:

> This allows computing a manifest for a specific system. Previously this was
> possible, but only through changing %current-system, which caused the
> derivation to be computed using that system as well (so computing a derivation
> for aarch64-linux on x86_64-linux would require running aarch64-linux code).

I remember discussing it, but I wonder if I was confused.

I think you can always do the equivalent of (say):

  guix time-machine -- build -s armhf-linux hello -d

… where Guix itself is built natively but it then computes a derivation
for a different architecture.

The equivalent code would be roughly:

  (let ((inferior (inferior-for-channels …)))
    (inferior-package-derivation store
                                 (car (lookup-inferior-packages inferior "hello"))
                                 "armhf-linux"))

Does that make sense?

(You may also want to turn off grafts in the inferior.)

Ludo’.

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

> Christopher Baines <mail@cbaines.net> skribis:
>
>> This allows computing a manifest for a specific system. Previously this was
>> possible, but only through changing %current-system, which caused the
>> derivation to be computed using that system as well (so computing a derivation
>> for aarch64-linux on x86_64-linux would require running aarch64-linux code).
>
> I remember discussing it, but I wonder if I was confused.
>
> I think you can always do the equivalent of (say):
>
>   guix time-machine -- build -s armhf-linux hello -d
>
> … where Guix itself is built natively but it then computes a derivation
> for a different architecture.
>
> The equivalent code would be roughly:
>
>   (let ((inferior (inferior-for-channels …)))
>     (inferior-package-derivation store
>                                  (car (lookup-inferior-packages inferior "hello"))
>                                  "armhf-linux"))
>
> Does that make sense?

Not really, this is just about manifests for channel instances, so
nothing to do with package derivations as far as I'm aware.

Hi Christopher,

Christopher Baines <mail@cbaines.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Christopher Baines <mail@cbaines.net> skribis:
>>
>>> This allows computing a manifest for a specific system. Previously this was
>>> possible, but only through changing %current-system, which caused the
>>> derivation to be computed using that system as well (so computing a derivation
>>> for aarch64-linux on x86_64-linux would require running aarch64-linux code).
>>
>> I remember discussing it, but I wonder if I was confused.
>>
>> I think you can always do the equivalent of (say):
>>
>>   guix time-machine -- build -s armhf-linux hello -d
>>
>> … where Guix itself is built natively but it then computes a derivation
>> for a different architecture.
>>
>> The equivalent code would be roughly:
>>
>>   (let ((inferior (inferior-for-channels …)))
>>     (inferior-package-derivation store
>>                                  (car (lookup-inferior-packages inferior "hello"))
>>                                  "armhf-linux"))
>>
>> Does that make sense?
>
> Not really,

:-)

> this is just about manifests for channel instances, so nothing to do
> with package derivations as far as I'm aware.

I re-read your message and must have misunderstood.  It’s the derivation
of channel instances that you want for a given system, right?  (What’s
the use case though?)  In that case something along the lines of the
patch makes perfect sense.

Thanks,
Ludo’.

Christopher Baines <mail@cbaines.net> skribis:

> This allows computing a manifest for a specific system. Previously this was
> possible, but only through changing %current-system, which caused the
> derivation to be computed using that system as well (so computing a derivation
> for aarch64-linux on x86_64-linux would require running aarch64-linux code).
>
> This new argument adds the possibility of computing derivations for non-native
> systems, without having to run non-native code.
>
> I'm looking at this as it will enable the Guix Data Service to compute channel
> instance derivations without relying on QEMU emulation for non-native
> systems (it should be faster as well).
>
> * guix/channels.scm (build-from-source): Add #:system argument and pass to
> build.
> (build-channel-instance): Add system argument and pass to build-from-source.
> (channel-instance-derivations): Add #:system argument and pass to
> build-channel-instance, also rename system to current-system-value.
> (channel-instances->manifest): Add #:system argument and pass to
> channel-instance-derivations.

[...]

>  (define* (build-from-source instance
> -                            #:key core verbose? (dependencies '()))
> +                            #:key core verbose? (dependencies '()) system)
>    "Return a derivation to build Guix from INSTANCE, using the self-build
>  script contained therein.  When CORE is true, build package modules under
>  SOURCE using CORE, an instance of Guix."

Please mention SYSTEM in the docstring.

> +(define* (channel-instance-derivations instances #:key system)
>    "Return the list of derivations to build INSTANCES, in the same order as
>  INSTANCES."
>    (define core-instance
> @@ -757,14 +759,15 @@ INSTANCES."
>      (resolve-dependencies instances))
>  
>    (define (instance->derivation instance)
> -    (mlet %store-monad ((system (current-system)))
> +    (mlet %store-monad ((current-system-value (current-system)))
>        (mcached (if (eq? instance core-instance)
> -                   (build-channel-instance instance)
> +                   (build-channel-instance instance system)
>                     (mlet %store-monad ((core (instance->derivation core-instance))
>                                         (deps (mapm %store-monad instance->derivation
>                                                     (edges instance))))
> -                     (build-channel-instance instance core deps)))
> +                     (build-channel-instance instance system core deps)))
>                 instance
> +               current-system-value
>                 system)))

Here, there should not be any additional key to ‘mcached’ since there’s
only one system being targeted.

Instead, it should look something like this:

  (define (instance->derivation core-instance)
    (mlet %store-monad ((system (if system (return system) (current-system))))
      (mcached …         ;pass ‘system’ to callees
               instance
               system))) ;unchanged

HTH,
Ludo’.

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

> Christopher Baines <mail@cbaines.net> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Christopher Baines <mail@cbaines.net> skribis:
>>>
>>>> This allows computing a manifest for a specific system. Previously this was
>>>> possible, but only through changing %current-system, which caused the
>>>> derivation to be computed using that system as well (so computing a derivation
>>>> for aarch64-linux on x86_64-linux would require running aarch64-linux code).
>>>
>>> I remember discussing it, but I wonder if I was confused.
>>>
>>> I think you can always do the equivalent of (say):
>>>
>>>   guix time-machine -- build -s armhf-linux hello -d
>>>
>>> … where Guix itself is built natively but it then computes a derivation
>>> for a different architecture.
>>>
>>> The equivalent code would be roughly:
>>>
>>>   (let ((inferior (inferior-for-channels …)))
>>>     (inferior-package-derivation store
>>>                                  (car (lookup-inferior-packages inferior "hello"))
>>>                                  "armhf-linux"))
>>>
>>> Does that make sense?
>>
>> Not really,
>
> :-)
>
>> this is just about manifests for channel instances, so nothing to do
>> with package derivations as far as I'm aware.
>
> I re-read your message and must have misunderstood.  It’s the derivation
> of channel instances that you want for a given system, right?  (What’s
> the use case though?)  In that case something along the lines of the
> patch makes perfect sense.

Yep, the Guix Data Service currently uses channel-instances->manifest to
compute the channel instance derivations (which show up here for example
[1]. Currently it computes the derivations for different systems by
setting %current-system, but this has the side effect of also running
the Guile code for computing the derivation with Guile for that system.

1: https://data.guix.gnu.org/revision/afec2784174058fdd85d9698e1fa748c45bfa8ee/channel-instances

That effectively only works if QEMU binfmt support is available for
those other systems. It would be faster just to use the native Guile,
and this would also avoid substitute availability problems (I had to
disable armhf-linux emulation on the data.guix.gnu.org machine when the
substitute availability from ci.guix.gnu.org got worse recently as too
much time was being spent just building armhf-linux things).

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

> Christopher Baines <mail@cbaines.net> skribis:
>
>> This allows computing a manifest for a specific system. Previously this was
>> possible, but only through changing %current-system, which caused the
>> derivation to be computed using that system as well (so computing a derivation
>> for aarch64-linux on x86_64-linux would require running aarch64-linux code).
>>
>> This new argument adds the possibility of computing derivations for non-native
>> systems, without having to run non-native code.
>>
>> I'm looking at this as it will enable the Guix Data Service to compute channel
>> instance derivations without relying on QEMU emulation for non-native
>> systems (it should be faster as well).
>>
>> * guix/channels.scm (build-from-source): Add #:system argument and pass to
>> build.
>> (build-channel-instance): Add system argument and pass to build-from-source.
>> (channel-instance-derivations): Add #:system argument and pass to
>> build-channel-instance, also rename system to current-system-value.
>> (channel-instances->manifest): Add #:system argument and pass to
>> channel-instance-derivations.
>
> [...]
>
>>  (define* (build-from-source instance
>> -                            #:key core verbose? (dependencies '()))
>> +                            #:key core verbose? (dependencies '()) system)
>>    "Return a derivation to build Guix from INSTANCE, using the self-build
>>  script contained therein.  When CORE is true, build package modules under
>>  SOURCE using CORE, an instance of Guix."
>
> Please mention SYSTEM in the docstring.
>
>> +(define* (channel-instance-derivations instances #:key system)
>>    "Return the list of derivations to build INSTANCES, in the same order as
>>  INSTANCES."
>>    (define core-instance
>> @@ -757,14 +759,15 @@ INSTANCES."
>>      (resolve-dependencies instances))
>>
>>    (define (instance->derivation instance)
>> -    (mlet %store-monad ((system (current-system)))
>> +    (mlet %store-monad ((current-system-value (current-system)))
>>        (mcached (if (eq? instance core-instance)
>> -                   (build-channel-instance instance)
>> +                   (build-channel-instance instance system)
>>                     (mlet %store-monad ((core (instance->derivation core-instance))
>>                                         (deps (mapm %store-monad instance->derivation
>>                                                     (edges instance))))
>> -                     (build-channel-instance instance core deps)))
>> +                     (build-channel-instance instance system core deps)))
>>                 instance
>> +               current-system-value
>>                 system)))
>
> Here, there should not be any additional key to ‘mcached’ since there’s
> only one system being targeted.
>
> Instead, it should look something like this:
>
>   (define (instance->derivation core-instance)
>     (mlet %store-monad ((system (if system (return system) (current-system))))
>       (mcached …         ;pass ‘system’ to callees
>                instance
>                system))) ;unchanged

Sure, I've sent an updated patch that updates docstrings and makes the
above changes.

Hi,

Christopher Baines <mail@cbaines.net> skribis:

> Yep, the Guix Data Service currently uses channel-instances->manifest to
> compute the channel instance derivations (which show up here for example
> [1]. Currently it computes the derivations for different systems by
> setting %current-system, but this has the side effect of also running
> the Guile code for computing the derivation with Guile for that system.
>
> 1: https://data.guix.gnu.org/revision/afec2784174058fdd85d9698e1fa748c45bfa8ee/channel-instances

I see, that’s because you explicitly want to channel derivations for
several systems in this case.  Got it!

Ludo’.