| Message ID | 6a6413853ce16af65e143bd727715be6dd7d6048.1702630557.git.lars.bilke@ufz.de |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#67822,v3] gnu: maths: petsc: Reduce closure size. | expand |
Hi Lars, Lars Bilke <lars.bilke@ufz.de> skribis: > Reduces closure size by around 350 MB by removing refernces to build dependencies (e.g. gcc). Woow, nice! > + (add-after 'clean-install 'clear-reference-to-compiler > + (lambda* (#:key inputs outputs #:allow-other-keys) > + ;; Do not retain a reference to GCC and other build only inputs. > + (let ((out (assoc-ref outputs "out"))) > + (substitute* (string-append out "/lib/petsc/conf/petscvariables") > + (("/gnu/store/.*/bin/gcc") "gcc") > + (("/gnu/store/.*/bin/g\\+\\+") "g++") > + (("/gnu/store/.*/bin/make") "make") > + (("/gnu/store/.*/bin/diff") "diff") > + (("/gnu/store/.*/bin/sed") "sed") Can we instead patch the thing that creates ‘petscvariables’ in the first place? The reason I’m suggesting it is because in general we avoid hardcoding /gnu/store in substitution patterns because it’s possible to configure Guix with a different store directory. Thanks, and apologies for the delay! Ludo’.
Hi Ludo, On 5 Jan 2024, at 12:08, Ludovic Courtès wrote: > Can we instead patch the thing that creates ‘petscvariables’ in the > first place? > > The reason I’m suggesting it is because in general we avoid hardcoding > /gnu/store in substitution patterns because it’s possible to configure > Guix with a different store directory. Thanks for your feedback! In v1 of this patch I removed the 'petscvariables'-file completely but this broke dependent packages as well as not-yet packaged projects which use the file for finding the PETSc library and configuriung their build system. Is there a possibility to replace the hard-coded /gnu/store with a variable which evaluates to the current store directory? Sincerely, Lars
On Fri, Jan 05, 2024 at 12:52:02PM +0100, Lars Bilke wrote: > Hi Ludo, > > On 5 Jan 2024, at 12:08, Ludovic Courtès wrote: > > > Can we instead patch the thing that creates ‘petscvariables’ in the > > first place? > > > > The reason I’m suggesting it is because in general we avoid hardcoding > > /gnu/store in substitution patterns because it’s possible to configure > > Guix with a different store directory. > > Thanks for your feedback! > > In v1 of this patch I removed the 'petscvariables'-file completely but this broke dependent packages as well as not-yet packaged projects which use the file for finding the PETSc library and configuriung their build system. > > Is there a possibility to replace the hard-coded /gnu/store with a variable which evaluates to the current store directory? There's %store-directory in (guix build utils). In fact, it looks like git might have some code that you can borrow.
Hi, Efraim Flashner <efraim@flashner.co.il> skribis: > On Fri, Jan 05, 2024 at 12:52:02PM +0100, Lars Bilke wrote: >> Hi Ludo, >> >> On 5 Jan 2024, at 12:08, Ludovic Courtès wrote: >> >> > Can we instead patch the thing that creates ‘petscvariables’ in the >> > first place? >> > >> > The reason I’m suggesting it is because in general we avoid hardcoding >> > /gnu/store in substitution patterns because it’s possible to configure >> > Guix with a different store directory. >> >> Thanks for your feedback! >> >> In v1 of this patch I removed the 'petscvariables'-file completely but this broke dependent packages as well as not-yet packaged projects which use the file for finding the PETSc library and configuriung their build system. >> >> Is there a possibility to replace the hard-coded /gnu/store with a variable which evaluates to the current store directory? > > There's %store-directory in (guix build utils). In fact, it looks like > git might have some code that you can borrow. Yes. However, I think we should use literal strings for patterns in ‘substitute*’. That is, I would avoid: (substitute* … (((string-append (%store-directory) "/bin/whatever")) …)) in favor of, say: (substitute* … (("([[:graph:]]+)/bin/whatever") …)) This is to make things easier to understand, 100% correct (in theory we should use ‘regexp-quote’ when turning strings into regexps), and to leave room for how ‘substitute*’ is implemented. Thanks, Ludo’.
diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm index 0f4d29b40f..4b2643e535 100644 --- a/gnu/packages/maths.scm +++ b/gnu/packages/maths.scm @@ -3484,8 +3484,20 @@ (define-public petsc '("configure.log" "make.log" "gmake.log" "test.log" "error.log" "RDict.db" "PETScBuildInternal.cmake" + "configure-hash" ;; Once installed, should uninstall with Guix "uninstall.py"))))) + (add-after 'clean-install 'clear-reference-to-compiler + (lambda* (#:key inputs outputs #:allow-other-keys) + ;; Do not retain a reference to GCC and other build only inputs. + (let ((out (assoc-ref outputs "out"))) + (substitute* (string-append out "/lib/petsc/conf/petscvariables") + (("/gnu/store/.*/bin/gcc") "gcc") + (("/gnu/store/.*/bin/g\\+\\+") "g++") + (("/gnu/store/.*/bin/make") "make") + (("/gnu/store/.*/bin/diff") "diff") + (("/gnu/store/.*/bin/sed") "sed") + )))) (add-after 'install 'move-examples (lambda* (#:key outputs #:allow-other-keys) (let* ((out (assoc-ref outputs "out"))