[bug#61363,2/2] self: Apply grafts to the outputs of the guix derivation.

Rather than having grafts apply to the derivation itself. This moves grafting
here to work like grafting for packages, where you can think of the grafted
outputs as a transformed variant of the ungrafted outputs.

I'm looking at this as it'll allow the Guix Data Service to compute the
derivations without grafts, and for these to be useful for substitutes
regardless of whether users are using grafts.

* guix/self.scm (compiled-guix, guix-derivation): Add a #:graft? keyword
argument, to control grafting when computing the guix derivation.
* build-aux/build-self.scm (build-program): Call guix-derivation with
 #:graft? (%graft?) to make the compute-guix-derivation script use or not use
grafts as desired.
---
 build-aux/build-self.scm |  4 ++-
 guix/self.scm            | 65 ++++++++++++++++++++++++++--------------
 2 files changed, 45 insertions(+), 24 deletions(-)

Hi,

Christopher Baines <mail@cbaines.net> skribis:

> Rather than having grafts apply to the derivation itself. This moves grafting
> here to work like grafting for packages, where you can think of the grafted
> outputs as a transformed variant of the ungrafted outputs.

Hmm.

> I'm looking at this as it'll allow the Guix Data Service to compute the
> derivations without grafts, and for these to be useful for substitutes
> regardless of whether users are using grafts.

How does it help exactly?  By disabling grafts in that context?

> +++ b/guix/self.scm
> @@ -752,7 +752,8 @@ (define* (compiled-guix source #:key
>                          (gzip (specification->package "gzip"))
>                          (bzip2 (specification->package "bzip2"))
>                          (xz (specification->package "xz"))
> -                        (guix (specification->package "guix")))
> +                        (guix (specification->package "guix"))
> +                        (graft? #t))
>    "Return a file-like object that contains a compiled Guix."
>    (define guile-avahi
>      (specification->package "guile-avahi"))
> @@ -802,6 +803,12 @@ (define dependencies
>                        guile-json guile-semver guile-ssh guile-sqlite3
>                        guile-lib guile-zlib guile-lzlib guile-zstd)))
>  
> +  (define packages
> +    (cons* gzip
> +           bzip2
> +           xz
> +           dependencies))
> +

[...]

> +         (let ((obj (built-modules (lambda (node)
> +                                     (list (node-source node)
> +                                           (node-compiled node))))))
> +           (if graft?
> +               (explicit-grafting obj packages)
> +               obj)))

There are two things I’m not comfortable with:

  1. Having <explicit-grafting> in (guix packages); it looks misplaced.

  2. More importantly, manually listing packages that might require
     grafting looks like a slippery slope (“oops! we’re not getting the
     GnuTLS graft for that CVE, too bad”).

I designed and implemented several variants to try and delay grafting.
One of them consisted in carrying graft information in gexps:

  https://git.savannah.gnu.org/cgit/guix.git/log?h=wip-gexp-grafts

It’s kinda similar to what you’re proposing in that graft information is
carried as far as possible.  The main difference is that it’s automated.

Hmm needs more thought.

Ludo’.

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

>> I'm looking at this as it'll allow the Guix Data Service to compute the
>> derivations without grafts, and for these to be useful for substitutes
>> regardless of whether users are using grafts.
>
> How does it help exactly?  By disabling grafts in that context?

So the Guix Data Service is somewhat built on the assumption that it's
cheap to compute derivations, at least with grafts disabled. That's
always been the case for packages, but for channel instance derivations
it's not reliably the case, since currently disabling grafts doesn't
apply to the whole process, and even if it did, the derivations you'd
get out wouldn't be that useful (since you can't transform the outputs
from those derivations to the outputs you'd get if using grafts).

With these changes, it's always relatively cheap to compute the channel
instance derivations, and it's always possible to compute the
derivations for any system without needing to be able to perform builds
for that system.

You can see this in how the data service has processed Guix before and
after these patches.

This is the channel instances before:

  https://data.qa.guix.gnu.org/revision/a582d863465990642d331bc05bf073f47fb80908/channel-instances

and this is after:

  https://data.qa.guix.gnu.org/revision/9cfbb22b556d28a0af345824ae5b3e00eb3f4a15/channel-instances

Given data.qa.guix.gnu.org is running on an x86_64-linux system, that
and i686-linux isn't generally a problem, but I'm guessing it only
managed to compute the powerpc64le-linux and aarch64-linux derivations
because it was able to substitute the necessary store items. For other
system computing the derivations would have failed.

I believe this change will also mean that the build farms will go from
performing the grafting for these builds, to being able to not do so, in
line with how builds for packages are handled. This isn't a big thing,
but I think it makes sense.

>> +++ b/guix/self.scm
>> @@ -752,7 +752,8 @@ (define* (compiled-guix source #:key
>>                          (gzip (specification->package "gzip"))
>>                          (bzip2 (specification->package "bzip2"))
>>                          (xz (specification->package "xz"))
>> -                        (guix (specification->package "guix")))
>> +                        (guix (specification->package "guix"))
>> +                        (graft? #t))
>>    "Return a file-like object that contains a compiled Guix."
>>    (define guile-avahi
>>      (specification->package "guile-avahi"))
>> @@ -802,6 +803,12 @@ (define dependencies
>>                        guile-json guile-semver guile-ssh guile-sqlite3
>>                        guile-lib guile-zlib guile-lzlib guile-zstd)))
>>  
>> +  (define packages
>> +    (cons* gzip
>> +           bzip2
>> +           xz
>> +           dependencies))
>> +
>
> [...]
>
>> +         (let ((obj (built-modules (lambda (node)
>> +                                     (list (node-source node)
>> +                                           (node-compiled node))))))
>> +           (if graft?
>> +               (explicit-grafting obj packages)
>> +               obj)))
>
> There are two things I’m not comfortable with:
>
>   1. Having <explicit-grafting> in (guix packages); it looks misplaced.

I didn't put it there at first, but I think it makes sense since
grafting is currently specific to packages, as is this additional code.

>   2. More importantly, manually listing packages that might require
>      grafting looks like a slippery slope (“oops! we’re not getting the
>      GnuTLS graft for that CVE, too bad”).
>
> I designed and implemented several variants to try and delay grafting.
> One of them consisted in carrying graft information in gexps:
>
>   https://git.savannah.gnu.org/cgit/guix.git/log?h=wip-gexp-grafts
>
> It’s kinda similar to what you’re proposing in that graft information is
> carried as far as possible.  The main difference is that it’s automated.

That's interesting, I think that making grafting not specific to
packages, and something where the replacement is handled at a lower
level (e.g. gexps) would be an alternative way to handle this.

Given that this approach works though, maybe the explicit-grafting
functionality could just sit and be used inside of (guix self). Given
that module is very explicit about what packages are used, it should be
possible to arrange the code so it's very hard to miss a package out,
which should address your concern about manually listing packages (maybe
specification->package can be tweaked so that it's possible to get all
the packages, and that can be the list considered for grafting).

I don't know of any other places where this approach would be useful, so
while it would be nice to have a more general grafting mechanism
eventually, I'd also like to be able to make these changes to channel
instance grafts sooner rather than later.

Christopher Baines <mail@cbaines.net> writes:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>>   2. More importantly, manually listing packages that might require
>>      grafting looks like a slippery slope (“oops! we’re not getting the
>>      GnuTLS graft for that CVE, too bad”).
>>
>> I designed and implemented several variants to try and delay grafting.
>> One of them consisted in carrying graft information in gexps:
>>
>>   https://git.savannah.gnu.org/cgit/guix.git/log?h=wip-gexp-grafts
>>
>> It’s kinda similar to what you’re proposing in that graft information is
>> carried as far as possible.  The main difference is that it’s automated.
>
> That's interesting, I think that making grafting not specific to
> packages, and something where the replacement is handled at a lower
> level (e.g. gexps) would be an alternative way to handle this.
>
> Given that this approach works though, maybe the explicit-grafting
> functionality could just sit and be used inside of (guix self). Given
> that module is very explicit about what packages are used, it should be
> possible to arrange the code so it's very hard to miss a package out,
> which should address your concern about manually listing packages (maybe
> specification->package can be tweaked so that it's possible to get all
> the packages, and that can be the list considered for grafting).
>
> I don't know of any other places where this approach would be useful, so
> while it would be nice to have a more general grafting mechanism
> eventually, I'd also like to be able to make these changes to channel
> instance grafts sooner rather than later.

I've sent a v2 series which changes along the above lines. The explicit
grafting stuff just sits in (guix self), and (guix self) more
rigeriously uses it's own definition of specification->package, which
should provide some protection against missing packages out. Obviously
it's not quite as rigerous as moving the grafting functionality in to
gexps, but hopefully it's rigerous enough for now.

Christopher Baines <mail@cbaines.net> writes:

> [[PGP Signed Part:Undecided]]
>
> Christopher Baines <mail@cbaines.net> writes:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>>   2. More importantly, manually listing packages that might require
>>>      grafting looks like a slippery slope (“oops! we’re not getting the
>>>      GnuTLS graft for that CVE, too bad”).
>>>
>>> I designed and implemented several variants to try and delay grafting.
>>> One of them consisted in carrying graft information in gexps:
>>>
>>>   https://git.savannah.gnu.org/cgit/guix.git/log?h=wip-gexp-grafts
>>>
>>> It’s kinda similar to what you’re proposing in that graft information is
>>> carried as far as possible.  The main difference is that it’s automated.
>>
>> That's interesting, I think that making grafting not specific to
>> packages, and something where the replacement is handled at a lower
>> level (e.g. gexps) would be an alternative way to handle this.
>>
>> Given that this approach works though, maybe the explicit-grafting
>> functionality could just sit and be used inside of (guix self). Given
>> that module is very explicit about what packages are used, it should be
>> possible to arrange the code so it's very hard to miss a package out,
>> which should address your concern about manually listing packages (maybe
>> specification->package can be tweaked so that it's possible to get all
>> the packages, and that can be the list considered for grafting).
>>
>> I don't know of any other places where this approach would be useful, so
>> while it would be nice to have a more general grafting mechanism
>> eventually, I'd also like to be able to make these changes to channel
>> instance grafts sooner rather than later.
>
> I've sent a v2 series which changes along the above lines. The explicit
> grafting stuff just sits in (guix self), and (guix self) more
> rigeriously uses it's own definition of specification->package, which
> should provide some protection against missing packages out. Obviously
> it's not quite as rigerous as moving the grafting functionality in to
> gexps, but hopefully it's rigerous enough for now.

This has stalled a bit, but it would be good to try and get things
merged. I've gone ahead and pushed the first two patches in the series I
last sent, these just make minor changes to prepare for the functional
change here. I've also resent that patch as as v3.