From mboxrd@z Thu Jan 1 00:00:00 1970 From: Timothy Sample Subject: Re: Graft hooks Date: Tue, 21 Aug 2018 11:42:21 -0400 Message-ID: <87k1ojafqa.fsf@ngyro.com> References: <87k1ovbc0t.fsf@ngyro.com> <87y3d1h05e.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:60906) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fs8nh-0006Tw-Oh for guix-devel@gnu.org; Tue, 21 Aug 2018 11:42:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fs8nb-0003FT-63 for guix-devel@gnu.org; Tue, 21 Aug 2018 11:42:32 -0400 List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Ludovic =?utf-8?Q?Court=C3=A8s?= Cc: guix-devel@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable ludo@gnu.org (Ludovic Court=C3=A8s) writes: > Hello Timothy, > > Timothy Sample skribis: > >> The basic idea would be to add a field (or use a property) to the >> package record. Let=E2=80=99s call it =E2=80=9Cgraft-hook=E2=80=9D. 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=E2=80=99t looked = at all >> the details yet, because maybe this is a bad idea and I shouldn=E2=80=99= 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=E2=80=99d really li= ke to > do it for GDB as discussed in .) > > Package properties would be the right way to make it extensible, but > there are complications (notably we=E2=80=99d need to use gexps, but build > systems don=E2=80=99t 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=E2=80=99s a draft patch (it=E2=80=99s 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=E2=80=99t know how well it generalizes, though. For GDB, we might be able use a heuristic like whether there is a =E2=80=9Cdebug=E2=80=9D 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 =E2=80=9Cracket-build-system=E2=80=9D, 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=E2=80=99s =E2=80=9Craco setup=E2= =80=9D command to recompile the files and fix the checksums. Unfortunately, it also updates timestamps. I=E2=80=99m pretty sure our Racket package is not reproducible at the moment, so I didn=E2=80=99t worry about it too much. T= he 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 =E2=80=9Craco setup=E2=80=9D). --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=diff.patch Content-Description: Patch 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 @@ (($ (? 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)) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > Thanks, > Ludo=E2=80=99. --=-=-=--