Graft hooks

Graft hooks

[Timothy Sample]

Hi Guix,

I just submitted a patch for <https://bugs.gnu.org/30680>, but now I’m
wondering if there isn’t a more general way to solve the problem.

The bug has to do with grafting and checksums.  I know three bugs that
follow this theme: the one above (Racket), <https://bugs.gnu.org/19973>
(GDB), and <https://bugs.gnu.org/25752> (Go).  The basic problem is that
these packages store checksums of files during build time.  If they get
updated due to grafting, the files change, but the checksums do not.
The checksums become invalid, which causes other problems like trying to
update files in the store or asserting that debugging information is
invalid.

The patch I submitted makes Racket assume that files in the store are
good.  It patches Racket to skip checksum validation if it is checking a
file in the store.  A similar approach could be taken for GDB and Go.

It occurs to me that if we had some way to run package-specific code
during grafting we could solve problems like this easily and without
patching software that is not broken.

The basic idea would be to add a field (or use a property) to the
package record.  Let’s call it “graft-hook”.  It would be Scheme code
that gets run after grafting takes place, giving us a chance to patch
special things like checksums.  The hook would be passed the list of
files that were been modified during grafting.  Then, in the Racket
package for example, I could write a graft-hook that updates the SHA-1
hash of each of the modified source files.

Since grafting is done at the derivation level, the hook code would have
to be propagated down from the package level.  I haven’t looked at all
the details yet, because maybe this is a bad idea and I shouldn’t waste
my time!  :)  My first impression is that it is not too tricky.

Are these problems too specialized to deserve a general mechanism like
this?  Let me know what you think!


-- Tim

Re: Graft hooks

[Christopher Lemmer Webber]

Timothy Sample writes:

> Hi Guix,
>
> I just submitted a patch for <https://bugs.gnu.org/30680>, but now I’m
> wondering if there isn’t a more general way to solve the problem.
>
> The bug has to do with grafting and checksums.  I know three bugs that
> follow this theme: the one above (Racket), <https://bugs.gnu.org/19973>
> (GDB), and <https://bugs.gnu.org/25752> (Go).  The basic problem is that
> these packages store checksums of files during build time.  If they get
> updated due to grafting, the files change, but the checksums do not.
> The checksums become invalid, which causes other problems like trying to
> update files in the store or asserting that debugging information is
> invalid.
>
> The patch I submitted makes Racket assume that files in the store are
> good.  It patches Racket to skip checksum validation if it is checking a
> file in the store.  A similar approach could be taken for GDB and Go.
>
> It occurs to me that if we had some way to run package-specific code
> during grafting we could solve problems like this easily and without
> patching software that is not broken.
>
> The basic idea would be to add a field (or use a property) to the
> package record.  Let’s call it “graft-hook”.  It would be Scheme code
> that gets run after grafting takes place, giving us a chance to patch
> special things like checksums.  The hook would be passed the list of
> files that were been modified during grafting.  Then, in the Racket
> package for example, I could write a graft-hook that updates the SHA-1
> hash of each of the modified source files.

This seems like a really good approach to me and seems also much nicer /
safer in the long run than the solution in #30680 since it wouldn't just
patch out the package in question's checks, it would correct them.  That
seems very good indeed to me.

> Since grafting is done at the derivation level, the hook code would have
> to be propagated down from the package level.  I haven’t looked at all
> the details yet, because maybe this is a bad idea and I shouldn’t waste
> my time!  :)  My first impression is that it is not too tricky.
>
> Are these problems too specialized to deserve a general mechanism like
> this?  Let me know what you think!
>
>
> -- Tim

As said, it seems good to me.  But I would be interested in what Mark
would think, since he is mostly responsible for the grafts design.

Re: Graft hooks

[Gábor Boskovits]

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

Christopher Lemmer Webber <cwebber@dustycloud.org> ezt írta (időpont: 2018.
aug. 13., H, 2:28):

> Timothy Sample writes:
>
> > Hi Guix,
> >
> > I just submitted a patch for <https://bugs.gnu.org/30680>, but now I’m
> > wondering if there isn’t a more general way to solve the problem.
> >
> > The bug has to do with grafting and checksums.  I know three bugs that
> > follow this theme: the one above (Racket), <https://bugs.gnu.org/19973>
> > (GDB), and <https://bugs.gnu.org/25752> (Go).  The basic problem is that
> > these packages store checksums of files during build time.  If they get
> > updated due to grafting, the files change, but the checksums do not.
> > The checksums become invalid, which causes other problems like trying to
> > update files in the store or asserting that debugging information is
> > invalid.
> >
> > The patch I submitted makes Racket assume that files in the store are
> > good.  It patches Racket to skip checksum validation if it is checking a
> > file in the store.  A similar approach could be taken for GDB and Go.
> >
> > It occurs to me that if we had some way to run package-specific code
> > during grafting we could solve problems like this easily and without
> > patching software that is not broken.
> >
> > The basic idea would be to add a field (or use a property) to the
> > package record.  Let’s call it “graft-hook”.  It would be Scheme code
> > that gets run after grafting takes place, giving us a chance to patch
> > special things like checksums.  The hook would be passed the list of
> > files that were been modified during grafting.  Then, in the Racket
> > package for example, I could write a graft-hook that updates the SHA-1
> > hash of each of the modified source files.
>
>
+1
I think this would be a good design choice.
We also gain back the security that the original check provides.


> This seems like a really good approach to me and seems also much nicer /
> safer in the long run than the solution in #30680 since it wouldn't just
> patch out the package in question's checks, it would correct them.  That
> seems very good indeed to me.
>
> > Since grafting is done at the derivation level, the hook code would have
> > to be propagated down from the package level.  I haven’t looked at all
> > the details yet, because maybe this is a bad idea and I shouldn’t waste
> > my time!  :)  My first impression is that it is not too tricky.
> >
> > Are these problems too specialized to deserve a general mechanism like
> > this?  Let me know what you think!
> >
> >
> > -- Tim
>
> As said, it seems good to me.  But I would be interested in what Mark
> would think, since he is mostly responsible for the grafts design.
>
>

[-- Attachment #2: Type: text/html, Size: 3672 bytes --]

Re: Graft hooks

[Ludovic Courtès]

Hello Timothy,

Timothy Sample <samplet@ngyro.com> skribis:

> The basic idea would be to add a field (or use a property) to the
> package record.  Let’s call it “graft-hook”.  It would be Scheme code
> that gets run after grafting takes place, giving us a chance to patch
> special things like checksums.  The hook would be passed the list of
> files that were been modified during grafting.  Then, in the Racket
> package for example, I could write a graft-hook that updates the SHA-1
> hash of each of the modified source files.
>
> Since grafting is done at the derivation level, the hook code would have
> to be propagated down from the package level.  I haven’t looked at all
> the details yet, because maybe this is a bad idea and I shouldn’t waste
> my time!  :)  My first impression is that it is not too tricky.
>
> Are these problems too specialized to deserve a general mechanism like
> this?  Let me know what you think!

I agree that this would be the right thing to do!  (I’d really like to
do it for GDB as discussed in <https://bugs.gnu.org/19973>.)

Package properties would be the right way to make it extensible, but
there are complications (notably we’d need to use gexps, but build
systems don’t use gexps yet.)

So as a first step, would like to try and implement the checksum update
for Racket directly in (guix build grafts)?  The implementation would
need to be clearly separate from the generic grafting code (like profile
hooks in (guix profiles)) such that we can easily add similar hooks and
move them to a separate file later.

WDYT?

Thanks,
Ludo’.

Re: Graft hooks

[Timothy Sample]

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

ludo@gnu.org (Ludovic Courtès) writes:

> Hello Timothy,
>
> Timothy Sample <samplet@ngyro.com> skribis:
>
>> The basic idea would be to add a field (or use a property) to the
>> package record.  Let’s call it “graft-hook”.  It would be Scheme code
>> that gets run after grafting takes place, giving us a chance to patch
>> special things like checksums.  The hook would be passed the list of
>> files that were been modified during grafting.  Then, in the Racket
>> package for example, I could write a graft-hook that updates the SHA-1
>> hash of each of the modified source files.
>>
>> Since grafting is done at the derivation level, the hook code would have
>> to be propagated down from the package level.  I haven’t looked at all
>> the details yet, because maybe this is a bad idea and I shouldn’t waste
>> my time!  :)  My first impression is that it is not too tricky.
>>
>> Are these problems too specialized to deserve a general mechanism like
>> this?  Let me know what you think!
>
> I agree that this would be the right thing to do!  (I’d really like to
> do it for GDB as discussed in <https://bugs.gnu.org/19973>.)
>
> Package properties would be the right way to make it extensible, but
> there are complications (notably we’d need to use gexps, but build
> systems don’t use gexps yet.)

But soon, right?  ;)

> So as a first step, would like to try and implement the checksum update
> for Racket directly in (guix build grafts)?  The implementation would
> need to be clearly separate from the generic grafting code (like profile
> hooks in (guix profiles)) such that we can easily add similar hooks and
> move them to a separate file later.
>
> WDYT?

I just looked at (guix profiles) and it was a good jumping-off point.
Thanks for the hint!

Here’s a draft patch (it’s mercifully small).  I have a few questions
about it, but if it looks like the right approach, I will clean it up
and submit it.

Basically, it checks if we are grafting Racket, and then adds some code
to the build expression to run the hook.

This approach works well for Racket, and it has the benefit of not
changing the graft build expression except when necessary, which
prevents spurious rebuilds.  I don’t know how well it generalizes,
though.  For GDB, we might be able use a heuristic like whether there is
a “debug” output.  For Go, it gets tricky because we would have to check
if Go was used to build part of the output (I guess we could check the
dependency graph, but that sounds like it could be slow).  Similarly, if
we ever have a “racket-build-system”, we will need the same thing for
Racket.  Maybe these are problems for the future, since they are much
easier to solve if we can have graft hooks at the package or
build-system level.

Also, is there a preference for patching the files using Guile or using
an external tool?  This patch uses Racket’s “raco setup” command to
recompile the files and fix the checksums.  Unfortunately, it also
updates timestamps.  I’m pretty sure our Racket package is not
reproducible at the moment, so I didn’t worry about it too much.  The
timestamps could be patched out, though.  The reason I shied away from
writing my own code is that Racket also hashes all the dependencies for
a bytecode file.  This means that the custom code would have to traverse
the Racket dependency graph to get the checksums right.  It is not too
hard to do so, but it would be a couple hundred lines of code (compared
to the five or so it took to invoke “raco setup”).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch --]
[-- Type: text/x-patch, Size: 2649 bytes --]

diff --git a/guix/grafts.scm b/guix/grafts.scm
index d6b0e93e8..88a99312d 100644
--- a/guix/grafts.scm
+++ b/guix/grafts.scm
@@ -75,6 +75,36 @@
     (($ <graft> (? string? item))
      item)))
 
+(define (fix-racket-checksums store drv system)
+  (define racket-drv
+    (let ((package-derivation (module-ref (resolve-interface '(guix packages))
+                                          'package-derivation))
+          (racket (module-ref (resolve-interface '(gnu packages scheme))
+                              'racket)))
+      (package-derivation store racket system #:graft? #f)))
+
+  (define hook-exp
+    `(lambda (input output mapping)
+       (let ((raco (string-append output "/bin/raco")))
+         ;; Setting PLT_COMPILED_FILE_CHECK to "exists" tells Racket to
+         ;; ignore timestamps when checking if a compiled file is valid.
+         ;; Without it, Racket attempts a complete rebuild of
+         ;; everything.
+         (setenv "PLT_COMPILED_FILE_CHECK" "exists")
+         ;; All of the --no-* flags below keep Racket from making
+         ;; unecessary and unhelpful changes (like rewriting scripts and
+         ;; reverting their shebangs in the process).
+         (invoke raco "setup" "--no-launcher" "--no-install"
+                 "--no-post-install" "--no-info-domain" "--no-docs"))))
+
+  (if (string=? (derivation-file-name drv)
+                (derivation-file-name racket-drv))
+      hook-exp
+      #f))
+
+(define %graft-hooks
+  (list fix-racket-checksums))
+
 (define* (graft-derivation/shallow store drv grafts
                                    #:key
                                    (name (derivation-name drv))
@@ -104,6 +134,11 @@ are not recursively applied to dependencies of DRV."
                   (assoc-ref (derivation-outputs drv) output))))
          outputs))
 
+  (define hook-exps
+    (filter-map (lambda (hook)
+                  (hook store drv system))
+                %graft-hooks))
+
   (define build
     `(begin
        (use-modules (guix build graft)
@@ -120,7 +155,10 @@ are not recursively applied to dependencies of DRV."
          (for-each (lambda (input output)
                      (format #t "grafting '~a' -> '~a'...~%" input output)
                      (force-output)
-                     (rewrite-directory input output mapping))
+                     (rewrite-directory input output mapping)
+                     ,@(map (lambda (exp)
+                              `(,exp input output mapping))
+                            hook-exps))
                    (match old-outputs
                      (((names . files) ...)
                       files))

[-- Attachment #3: Type: text/plain, Size: 27 bytes --]



> Thanks,
> Ludo’.

Re: Graft hooks

[Ludovic Courtès]

Hello Timothy,

Timothy Sample <samplet@ngyro.com> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Hello Timothy,
>>
>> Timothy Sample <samplet@ngyro.com> skribis:
>>
>>> The basic idea would be to add a field (or use a property) to the
>>> package record.  Let’s call it “graft-hook”.  It would be Scheme code
>>> that gets run after grafting takes place, giving us a chance to patch
>>> special things like checksums.  The hook would be passed the list of
>>> files that were been modified during grafting.  Then, in the Racket
>>> package for example, I could write a graft-hook that updates the SHA-1
>>> hash of each of the modified source files.
>>>
>>> Since grafting is done at the derivation level, the hook code would have
>>> to be propagated down from the package level.  I haven’t looked at all
>>> the details yet, because maybe this is a bad idea and I shouldn’t waste
>>> my time!  :)  My first impression is that it is not too tricky.
>>>
>>> Are these problems too specialized to deserve a general mechanism like
>>> this?  Let me know what you think!
>>
>> I agree that this would be the right thing to do!  (I’d really like to
>> do it for GDB as discussed in <https://bugs.gnu.org/19973>.)
>>
>> Package properties would be the right way to make it extensible, but
>> there are complications (notably we’d need to use gexps, but build
>> systems don’t use gexps yet.)
>
> But soon, right?  ;)

Well, it’s complicated.  :-)

Also, I realized that some things, like the .gnu_debuglink and build-id
hooks, don’t really fit in any package; they’re transverse.

> Here’s a draft patch (it’s mercifully small).  I have a few questions
> about it, but if it looks like the right approach, I will clean it up
> and submit it.
>
> Basically, it checks if we are grafting Racket, and then adds some code
> to the build expression to run the hook.

OK.  In theory, should it be just for Racket, or should it also be for
Racket libraries (we don’t have any currently AFAIK)?

> Also, is there a preference for patching the files using Guile or using
> an external tool?  This patch uses Racket’s “raco setup” command to
> recompile the files and fix the checksums.  Unfortunately, it also
> updates timestamps.  I’m pretty sure our Racket package is not
> reproducible at the moment, so I didn’t worry about it too much.  The
> timestamps could be patched out, though.  The reason I shied away from
> writing my own code is that Racket also hashes all the dependencies for
> a bytecode file.  This means that the custom code would have to traverse
> the Racket dependency graph to get the checksums right.  It is not too
> hard to do so, but it would be a couple hundred lines of code (compared
> to the five or so it took to invoke “raco setup”).

Regarding whether or not to write our own code: let’s do whichever is
more convenient.  In this case, using ‘raco setup’ looks like the right
thing to do, given that raco is available in the build environment
anyway (see below); for .gnu_debuglink, I found it nicer (and more fun
:-)) to write a Guile module.

Regarding timestamps: I guess there’s no problem since timestamps are
reset in the store.

Some comments:

> diff --git a/guix/grafts.scm b/guix/grafts.scm
> index d6b0e93e8..88a99312d 100644
> --- a/guix/grafts.scm
> +++ b/guix/grafts.scm
> @@ -75,6 +75,36 @@
>      (($ <graft> (? string? item))
>       item)))
>  
> +(define (fix-racket-checksums store drv system)
> +  (define racket-drv
> +    (let ((package-derivation (module-ref (resolve-interface '(guix packages))
> +                                          'package-derivation))
> +          (racket (module-ref (resolve-interface '(gnu packages scheme))
> +                              'racket)))
> +      (package-derivation store racket system #:graft? #f)))
> +
> +  (define hook-exp
> +    `(lambda (input output mapping)
> +       (let ((raco (string-append output "/bin/raco")))
> +         ;; Setting PLT_COMPILED_FILE_CHECK to "exists" tells Racket to
> +         ;; ignore timestamps when checking if a compiled file is valid.
> +         ;; Without it, Racket attempts a complete rebuild of
> +         ;; everything.
> +         (setenv "PLT_COMPILED_FILE_CHECK" "exists")
> +         ;; All of the --no-* flags below keep Racket from making
> +         ;; unecessary and unhelpful changes (like rewriting scripts and
> +         ;; reverting their shebangs in the process).
> +         (invoke raco "setup" "--no-launcher" "--no-install"
> +                 "--no-post-install" "--no-info-domain" "--no-docs"))))

Since this is used when grafting Racket, I would suggest moving this
graft to the “build side” entirely, similar to what I did in
<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19973#25>.  Probably
you’d just add a single procedure to (guix build graft) and add it to
%graft-hooks.

That procedure could be the same as what you have above, except that
it’d run OUT/bin/raco, if it exists, and do nothing if OUT/bin/raco does
not exist.

WDYT?

Thanks,
Ludo’.

Re: Graft hooks

[Timothy Sample]

Hi Ludo,

ludo@gnu.org (Ludovic Courtès) writes:

> Hello Timothy,
>
> Timothy Sample <samplet@ngyro.com> skribis:
>
>> ludo@gnu.org (Ludovic Courtès) writes:
>>
>>> Hello Timothy,
>>>
>>> Timothy Sample <samplet@ngyro.com> skribis:
>>>
>>>> The basic idea would be to add a field (or use a property) to the
>>>> package record.  Let’s call it “graft-hook”.  It would be Scheme code
>>>> that gets run after grafting takes place, giving us a chance to patch
>>>> special things like checksums.  The hook would be passed the list of
>>>> files that were been modified during grafting.  Then, in the Racket
>>>> package for example, I could write a graft-hook that updates the SHA-1
>>>> hash of each of the modified source files.
>>>>
>>>> Since grafting is done at the derivation level, the hook code would have
>>>> to be propagated down from the package level.  I haven’t looked at all
>>>> the details yet, because maybe this is a bad idea and I shouldn’t waste
>>>> my time!  :)  My first impression is that it is not too tricky.
>>>>
>>>> Are these problems too specialized to deserve a general mechanism like
>>>> this?  Let me know what you think!
>>>
>>> I agree that this would be the right thing to do!  (I’d really like to
>>> do it for GDB as discussed in <https://bugs.gnu.org/19973>.)
>>>
>>> Package properties would be the right way to make it extensible, but
>>> there are complications (notably we’d need to use gexps, but build
>>> systems don’t use gexps yet.)
>>
>> But soon, right?  ;)
>
> Well, it’s complicated.  :-)
>
> Also, I realized that some things, like the .gnu_debuglink and build-id
> hooks, don’t really fit in any package; they’re transverse.

You’re right.  Packages are the wrong place.  What about build systems?
Maybe it makes sense (theoretically) to define graft hooks in build
systems, and then modify them (if necessary) using the “arguments” field
of a package.  Isn’t it the GNU or Go build systems that are ultimately
responsible for these checksums?  Shouldn’t it be their job to tell the
grafting code how to fix them?

>> Here’s a draft patch (it’s mercifully small).  I have a few questions
>> about it, but if it looks like the right approach, I will clean it up
>> and submit it.
>>
>> Basically, it checks if we are grafting Racket, and then adds some code
>> to the build expression to run the hook.
>
> OK.  In theory, should it be just for Racket, or should it also be for
> Racket libraries (we don’t have any currently AFAIK)?

I could imagine attaching the hook to our (theoretical) Racket build
system.  ;)  In the meantime, we can check if “share/racket” exists in
the output and run the code then.  Of course, if it were a Racket
library (and not Racket itself), I wouldn’t be able to use “raco setup”,
because I wouldn’t be able to find it.

>> Also, is there a preference for patching the files using Guile or using
>> an external tool?  This patch uses Racket’s “raco setup” command to
>> recompile the files and fix the checksums.  Unfortunately, it also
>> updates timestamps.  I’m pretty sure our Racket package is not
>> reproducible at the moment, so I didn’t worry about it too much.  The
>> timestamps could be patched out, though.  The reason I shied away from
>> writing my own code is that Racket also hashes all the dependencies for
>> a bytecode file.  This means that the custom code would have to traverse
>> the Racket dependency graph to get the checksums right.  It is not too
>> hard to do so, but it would be a couple hundred lines of code (compared
>> to the five or so it took to invoke “raco setup”).
>
> Regarding whether or not to write our own code: let’s do whichever is
> more convenient.  In this case, using ‘raco setup’ looks like the right
> thing to do, given that raco is available in the build environment
> anyway (see below); for .gnu_debuglink, I found it nicer (and more fun
> :-)) to write a Guile module.
>
> Regarding timestamps: I guess there’s no problem since timestamps are
> reset in the store.

Whoops!  I didn’t mean the file metadata, I meant in the bytecode files
themselves.  Also, I only saw the changes in diffoscope and assumed they
were timestamps.  I looked more closely and realized they were more
checksums that I didn’t notice before (it should have been obvious
because they are 20 bytes...).  They are supposed to be updated.
Nothing to see here.  :)

> Some comments:
>
>> diff --git a/guix/grafts.scm b/guix/grafts.scm
>> index d6b0e93e8..88a99312d 100644
>> --- a/guix/grafts.scm
>> +++ b/guix/grafts.scm
>> @@ -75,6 +75,36 @@
>>      (($ <graft> (? string? item))
>>       item)))
>>  
>> +(define (fix-racket-checksums store drv system)
>> +  (define racket-drv
>> +    (let ((package-derivation (module-ref (resolve-interface '(guix packages))
>> +                                          'package-derivation))
>> +          (racket (module-ref (resolve-interface '(gnu packages scheme))
>> +                              'racket)))
>> +      (package-derivation store racket system #:graft? #f)))
>> +
>> +  (define hook-exp
>> +    `(lambda (input output mapping)
>> +       (let ((raco (string-append output "/bin/raco")))
>> +         ;; Setting PLT_COMPILED_FILE_CHECK to "exists" tells Racket to
>> +         ;; ignore timestamps when checking if a compiled file is valid.
>> +         ;; Without it, Racket attempts a complete rebuild of
>> +         ;; everything.
>> +         (setenv "PLT_COMPILED_FILE_CHECK" "exists")
>> +         ;; All of the --no-* flags below keep Racket from making
>> +         ;; unecessary and unhelpful changes (like rewriting scripts and
>> +         ;; reverting their shebangs in the process).
>> +         (invoke raco "setup" "--no-launcher" "--no-install"
>> +                 "--no-post-install" "--no-info-domain" "--no-docs"))))
>
> Since this is used when grafting Racket, I would suggest moving this
> graft to the “build side” entirely, similar to what I did in
> <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19973#25>.  Probably
> you’d just add a single procedure to (guix build graft) and add it to
> %graft-hooks.
>
> That procedure could be the same as what you have above, except that
> it’d run OUT/bin/raco, if it exists, and do nothing if OUT/bin/raco does
> not exist.
>
> WDYT?

This makes sense.

Following my note above, I think I will try and finish my update code in
Guile, and then use the existence of “share/racket” as the heuristic to
determine if it should run.

Maybe I will package a Racket library, too, so I can test it.

> Thanks,
> Ludo’.

Thanks for the feedback!


-- Tim

Re: Graft hooks

[Christopher Lemmer Webber]

Ludovic Courtès writes:

>
>> Here’s a draft patch (it’s mercifully small).  I have a few questions
>> about it, but if it looks like the right approach, I will clean it up
>> and submit it.
>>
>> Basically, it checks if we are grafting Racket, and then adds some code
>> to the build expression to run the hook.
>
> OK.  In theory, should it be just for Racket, or should it also be for
> Racket libraries (we don’t have any currently AFAIK)?

That's something I'd like to change soon :)

Re: Graft hooks

[Mark H Weaver]

Hi Ludovic,

ludo@gnu.org (Ludovic Courtès) writes:

> Since this is used when grafting Racket, I would suggest moving this
> graft to the “build side” entirely, similar to what I did in
> <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19973#25>.  Probably
> you’d just add a single procedure to (guix build graft) and add it to
> %graft-hooks.
>
> That procedure could be the same as what you have above, except that
> it’d run OUT/bin/raco, if it exists, and do nothing if OUT/bin/raco does
> not exist.
>
> WDYT?

I think it would be quite unfortunate if _every_ graft had to run
_every_ graft hook, or if every graft hook had to be defined in
(guix grafts) and/or (guix build graft).

It's reasonable to have a few global graft hooks, e.g. for handling
debugging information, but I would greatly prefer for Guix to also have
a mechanism allowing individual packages or build systems to introduce
graft hooks without modifying (guix grafts) or (guix build graft), and
for such a mechanism to be used for Racket and its libraries.

Having said this, I haven't looked at this issue carefully, so perhaps
I'm midjudging the difficulty of adding such a mechanism.

What do you think?

      Mark

Re: Graft hooks

[Gábor Boskovits]

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

Mark H Weaver <mhw@netris.org> ezt írta (időpont: 2018. aug. 23., Cs, 9:18):

> Hi Ludovic,
>
> ludo@gnu.org (Ludovic Courtès) writes:
>
> > Since this is used when grafting Racket, I would suggest moving this
> > graft to the “build side” entirely, similar to what I did in
> > <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19973#25>.  Probably
> > you’d just add a single procedure to (guix build graft) and add it to
> > %graft-hooks.
> >
> > That procedure could be the same as what you have above, except that
> > it’d run OUT/bin/raco, if it exists, and do nothing if OUT/bin/raco does
> > not exist.
> >
> > WDYT?
>
> I think it would be quite unfortunate if _every_ graft had to run
> _every_ graft hook, or if every graft hook had to be defined in
> (guix grafts) and/or (guix build graft).
>
> It's reasonable to have a few global graft hooks, e.g. for handling
> debugging information, but I would greatly prefer for Guix to also have
> a mechanism allowing individual packages or build systems to introduce
> graft hooks without modifying (guix grafts) or (guix build graft), and
> for such a mechanism to be used for Racket and its libraries.
>
> Having said this, I haven't looked at this issue carefully, so perhaps
> I'm midjudging the difficulty of adding such a mechanism.
>
> What do you think?
>
>       Mark
>
>
I think there are actually three different levels of problems solved by
graft hooks.
One level is global, like the debugging symbols, and I'm fine with it in
(guix grafts).
Another level is language/build system/ecosystem specific, like the Racket
or the
java manifest issue, so I believe it would be nice to be able to use build
system
code to address problems at this level. I can also believe that there are
problems
relating to this on the individual package level, where the solution that
Mark mentioned
comes into play. WDYT?

Reflecting on this a Racket build system seems to be a good idea.

g_bor

[-- Attachment #2: Type: text/html, Size: 2642 bytes --]

Re: Graft hooks

[Ludovic Courtès]

Hello Timothy,

Timothy Sample <samplet@ngyro.com> skribis:

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

[...]

>>>> I agree that this would be the right thing to do!  (I’d really like to
>>>> do it for GDB as discussed in <https://bugs.gnu.org/19973>.)
>>>>
>>>> Package properties would be the right way to make it extensible, but
>>>> there are complications (notably we’d need to use gexps, but build
>>>> systems don’t use gexps yet.)
>>>
>>> But soon, right?  ;)
>>
>> Well, it’s complicated.  :-)
>>
>> Also, I realized that some things, like the .gnu_debuglink and build-id
>> hooks, don’t really fit in any package; they’re transverse.
>
> You’re right.  Packages are the wrong place.  What about build systems?
> Maybe it makes sense (theoretically) to define graft hooks in build
> systems, and then modify them (if necessary) using the “arguments” field
> of a package.  Isn’t it the GNU or Go build systems that are ultimately
> responsible for these checksums?  Shouldn’t it be their job to tell the
> grafting code how to fix them?

It’s not completely clearcut.  For instance, the GCC toolchain can
produce .gnu_debuglink and build-IDs, but the GCC toolchain and
‘gnu-build-system’ are not the only one doing so: ‘guile-build-system’,
‘go-build-system’, etc. could do that as well.  So .gnu_debuglink and
build-IDs are an example of something that doesn’t “belong” to any
single package or build system, I think.

[...]

>> Regarding timestamps: I guess there’s no problem since timestamps are
>> reset in the store.
>
> Whoops!  I didn’t mean the file metadata, I meant in the bytecode files
> themselves.  Also, I only saw the changes in diffoscope and assumed they
> were timestamps.  I looked more closely and realized they were more
> checksums that I didn’t notice before (it should have been obvious
> because they are 20 bytes...).  They are supposed to be updated.
> Nothing to see here.  :)

OK.  :-)

> Following my note above, I think I will try and finish my update code in
> Guile, and then use the existence of “share/racket” as the heuristic to
> determine if it should run.
>
> Maybe I will package a Racket library, too, so I can test it.

Great, thank you!

Ludo’.

Re: Graft hooks

[Ludovic Courtès]

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Since this is used when grafting Racket, I would suggest moving this
>> graft to the “build side” entirely, similar to what I did in
>> <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19973#25>.  Probably
>> you’d just add a single procedure to (guix build graft) and add it to
>> %graft-hooks.
>>
>> That procedure could be the same as what you have above, except that
>> it’d run OUT/bin/raco, if it exists, and do nothing if OUT/bin/raco does
>> not exist.
>>
>> WDYT?
>
> I think it would be quite unfortunate if _every_ graft had to run
> _every_ graft hook, or if every graft hook had to be defined in
> (guix grafts) and/or (guix build graft).
>
> It's reasonable to have a few global graft hooks, e.g. for handling
> debugging information, but I would greatly prefer for Guix to also have
> a mechanism allowing individual packages or build systems to introduce
> graft hooks without modifying (guix grafts) or (guix build graft), and
> for such a mechanism to be used for Racket and its libraries.

Sure, like I wrote, a more extensible solution along the lines of what
Timothy and you suggest sounds better long-term.  It just happens to be
harder to implement today (in particular until ‘wip-build-systems-gexp’
is merged), and not entirely clear how this could work, as discussed
with Timothy.

That’s why I’m proposing the simple approach for now.  I think we can
revisit the issue if/when we have graft hooks tied to a particular
package or build system.

Note that the .gnu_debuglink hook in <https://bugs.gnu.org/19973> runs
only when there’s a “debug” output, and the Racket hook that Timothy
proposes would only run when Racket is available.  So you get extra
run-time overhead only if there’s something to do.

HTH,
Ludo’.

Re: Graft hooks

[Mark H Weaver]

Hi Ludovic,

ludo@gnu.org (Ludovic Courtès) writes:

> Mark H Weaver <mhw@netris.org> skribis:
>
>> I think it would be quite unfortunate if _every_ graft had to run
>> _every_ graft hook, or if every graft hook had to be defined in
>> (guix grafts) and/or (guix build graft).
>>
>> It's reasonable to have a few global graft hooks, e.g. for handling
>> debugging information, but I would greatly prefer for Guix to also have
>> a mechanism allowing individual packages or build systems to introduce
>> graft hooks without modifying (guix grafts) or (guix build graft), and
>> for such a mechanism to be used for Racket and its libraries.
>
> Sure, like I wrote, a more extensible solution along the lines of what
> Timothy and you suggest sounds better long-term.  It just happens to be
> harder to implement today (in particular until ‘wip-build-systems-gexp’
> is merged), and not entirely clear how this could work, as discussed
> with Timothy.

That's fine.  This solution is fine for now, as long as the number of
graft hooks is fairly small.  Maybe we can revisit this after
'wip-build-systems-gexp' is merged.

      Thanks,
        Mark