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

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

[Christopher Baines]

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(-)

diff --git a/guix/channels.scm b/guix/channels.scm
index c40fc0c507..70a09e74ff 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -657,7 +657,7 @@ that unconditionally resumes the continuation."
               store))))
 
 (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."
@@ -700,20 +700,22 @@ SOURCE using CORE, an instance of Guix."
           (with-trivial-build-handler
            (build source
                   #:verbose? verbose? #:version commit
+                  #:system system
                   #:channel-metadata (channel-instance->sexp instance)
                   #:pull-version %pull-version))))
 
       ;; Build a set of modules that extend Guix using the standard method.
       (standard-module-derivation name source core dependencies)))
 
-(define* (build-channel-instance instance
+(define* (build-channel-instance instance system
                                  #:optional core (dependencies '()))
   "Return, as a monadic value, the derivation for INSTANCE, a channel
 instance.  DEPENDENCIES is a list of extensions providing Guile modules that
 INSTANCE depends on."
   (build-from-source instance
                      #:core core
-                     #:dependencies dependencies))
+                     #:dependencies dependencies
+                     #:system system))
 
 (define (resolve-dependencies instances)
   "Return a procedure that, given one of the elements of INSTANCES, returns
@@ -743,7 +745,7 @@ list of instances it depends on."
   (lambda (instance)
     (vhash-foldq* cons '() instance edges)))
 
-(define (channel-instance-derivations instances)
+(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)))
 
   (unless core-instance
@@ -865,7 +868,7 @@ derivation."
                     intro))))))
             '()))))
 
-(define (channel-instances->manifest instances)
+(define* (channel-instances->manifest instances #:key system)
   "Return a profile manifest with entries for all of INSTANCES, a list of
 channel instances."
   (define (instance->entry instance drv)
@@ -883,7 +886,8 @@ channel instances."
         (properties
          `((source ,(channel-instance->sexp instance)))))))
 
-  (mlet* %store-monad ((derivations (channel-instance-derivations instances))
+  (mlet* %store-monad ((derivations (channel-instance-derivations instances
+                                                                  #:system system))
                        (entries ->  (map instance->entry instances derivations)))
     (return (manifest entries))))
 
-- 
2.30.1

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

[Ludovic Courtès]

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’.

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

[Christopher Baines]

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


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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

[Ludovic Courtès]

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’.

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

[Ludovic Courtès]

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’.

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

[Christopher Baines]

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


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).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

[Christopher Baines]

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 | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/guix/channels.scm b/guix/channels.scm
index c40fc0c507..476d62e1f4 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -657,10 +657,11 @@ that unconditionally resumes the continuation."
               store))))
 
 (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."
+SOURCE using CORE, an instance of Guix.  By default, build for the current
+system, or SYSTEM if specified."
   (define name
     (symbol->string
      (channel-name (channel-instance-channel instance))))
@@ -700,20 +701,22 @@ SOURCE using CORE, an instance of Guix."
           (with-trivial-build-handler
            (build source
                   #:verbose? verbose? #:version commit
+                  #:system system
                   #:channel-metadata (channel-instance->sexp instance)
                   #:pull-version %pull-version))))
 
       ;; Build a set of modules that extend Guix using the standard method.
       (standard-module-derivation name source core dependencies)))
 
-(define* (build-channel-instance instance
+(define* (build-channel-instance instance system
                                  #:optional core (dependencies '()))
   "Return, as a monadic value, the derivation for INSTANCE, a channel
-instance.  DEPENDENCIES is a list of extensions providing Guile modules that
-INSTANCE depends on."
+instance, for SYSTEM.  DEPENDENCIES is a list of extensions providing Guile
+modules that INSTANCE depends on."
   (build-from-source instance
                      #:core core
-                     #:dependencies dependencies))
+                     #:dependencies dependencies
+                     #:system system))
 
 (define (resolve-dependencies instances)
   "Return a procedure that, given one of the elements of INSTANCES, returns
@@ -743,9 +746,9 @@ list of instances it depends on."
   (lambda (instance)
     (vhash-foldq* cons '() instance edges)))
 
-(define (channel-instance-derivations instances)
+(define* (channel-instance-derivations instances #:key system)
   "Return the list of derivations to build INSTANCES, in the same order as
-INSTANCES."
+INSTANCES.  Build for the current system by default, or SYSTEM if specified."
   (define core-instance
     ;; The 'guix' channel is treated specially: it's an implicit dependency of
     ;; all the other channels.
@@ -757,13 +760,13 @@ INSTANCES."
     (resolve-dependencies instances))
 
   (define (instance->derivation instance)
-    (mlet %store-monad ((system (current-system)))
+    (mlet %store-monad ((system (if system (return system) (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
                system)))
 
@@ -865,9 +868,10 @@ derivation."
                     intro))))))
             '()))))
 
-(define (channel-instances->manifest instances)
+(define* (channel-instances->manifest instances #:key system)
   "Return a profile manifest with entries for all of INSTANCES, a list of
-channel instances."
+channel instances.  By default, build for the current system, or SYSTEM if
+specified."
   (define (instance->entry instance drv)
     (let ((commit  (channel-instance-commit instance))
           (channel (channel-instance-channel instance)))
@@ -883,7 +887,8 @@ channel instances."
         (properties
          `((source ,(channel-instance->sexp instance)))))))
 
-  (mlet* %store-monad ((derivations (channel-instance-derivations instances))
+  (mlet* %store-monad ((derivations (channel-instance-derivations instances
+                                                                  #:system system))
                        (entries ->  (map instance->entry instances derivations)))
     (return (manifest entries))))
 
-- 
2.30.1

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

[Christopher Baines]

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


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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

[Ludovic Courtès]

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’.

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

[Ludovic Courtès]

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.

LGTM!

(Please double-check that ‘make as-derivation’ or ‘guix pull --url=$PWD …’
work, in case we overlooked something.)

Thank you,
Ludo’.

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

[Christopher Baines]

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


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.
>
> LGTM!
>
> (Please double-check that ‘make as-derivation’ or ‘guix pull --url=$PWD …’
> work, in case we overlooked something.)

Great, I've pushed this as 34985fb6ae7deffd40443766f5408649a0cbbff2 now.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]