[bug#67822] [PATCH] gnu: maths: petsc: Reduce closure size.

[bug#67822] [PATCH] gnu: maths: petsc: Reduce closure size.

[Lars Bilke]

Reduces closure size by around 350 MB.

May break CMake-based projects which use the following script:

https://github.com/jedbrown/cmake-modules/blob/master/FindPETSc.cmake

Use pkg-config based finding instead. See
https://github.com/jedbrown/cmake-modules/blob/master/README.md.

Change-Id: I2e6900747b2118546f0a39ceb109b3f2f90e6949
---
 gnu/packages/maths.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index 0f4d29b40f..7f3e80efa4 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -3484,6 +3484,8 @@ (define-public petsc
                           '("configure.log" "make.log" "gmake.log"
                             "test.log" "error.log" "RDict.db"
                             "PETScBuildInternal.cmake"
+                            "petscvariables"
+                            "configure-hash"
                             ;; Once installed, should uninstall with Guix
                             "uninstall.py")))))
           (add-after 'install 'move-examples

base-commit: ac61e9705fb8c450c6cd0c1731fbb1b909c1f944
-- 
2.43.0

[bug#67822] [PATCH v2] gnu: maths: petsc: Reduce closure size.

[Lars Bilke]

Reduces closure size by around 350 MB by removing refernces to build
dependencies (e.g. gcc).

The v2 patch does not delete petscvariables but only removes some store
references. This fixes downstream packages such as e.g. dealii-openmpi.

Change-Id: I2e6900747b2118546f0a39ceb109b3f2f90e6949
---
 gnu/packages/maths.scm | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index 0f4d29b40f..8e8e380336 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"))

base-commit: ac61e9705fb8c450c6cd0c1731fbb1b909c1f944
-- 
2.43.0

[bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.

[Lars Bilke]

Reduces closure size by around 350 MB by removing refernces to build dependencies (e.g. gcc).

Fixed a regex from v2.

Change-Id: I2e6900747b2118546f0a39ceb109b3f2f90e6949
---
 gnu/packages/maths.scm | 12 ++++++++++++
 1 file changed, 12 insertions(+)

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

base-commit: ac61e9705fb8c450c6cd0c1731fbb1b909c1f944
-- 
2.43.0

[bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.

[Ludovic Courtès]

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

[bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.

[Lars Bilke]

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

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

[-- Attachment #2: S/MIME digital signature --]
[-- Type: application/pkcs7-signature, Size: 5435 bytes --]

[bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.

[Efraim Flashner]

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

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.


-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

[bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.

[Ludovic Courtès]

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

[bug#67822] [PATCH v4] gnu: maths: petsc: Reduce closure size.

[Lars Bilke]

Reduces closure size by around 350 MB by removing refernces to build
dependencies (e.g. gcc).

New ion v4:

Used proposed :graph: syntax.
Also removed gfortran reference.

Change-Id: I2e6900747b2118546f0a39ceb109b3f2f90e6949
---
 gnu/packages/maths.scm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index adc7beb655..7dc93ce8ee 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -3484,8 +3484,21 @@ (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")
+                (("([[:graph:]]+)/bin/gcc") "gcc")
+                (("([[:graph:]]+)/bin/g\\+\\+") "g++")
+                (("([[:graph:]]+)/bin/make") "make")
+                (("([[:graph:]]+)/bin/diff") "diff")
+                (("([[:graph:]]+)/bin/sed") "sed")
+                (("([[:graph:]]+)/bin/gfortran") "gfortran")
+                ))))
           (add-after 'install 'move-examples
             (lambda* (#:key outputs #:allow-other-keys)
               (let* ((out (assoc-ref outputs "out"))

base-commit: a1a645337a76cf5fb55d20ee694fcb6a52fcf11b
-- 
2.43.0

[bug#67822] [PATCH v4] gnu: maths: petsc: Reduce closure size.

[Lars Bilke]

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

Dear Ludo,

may I kindly ask for another review here? I have implemented the suggestions in v4 and would like to benefit from the size reduction as I am preparing some container-based computations on several HPC clusters which do not have Guix installed ( yet ;-) ).

Thanks a lot!
Lars

On 9 Jan 2024, at 19:15, Lars Bilke wrote:

> Reduces closure size by around 350 MB by removing refernces to build
> dependencies (e.g. gcc).
>
> New ion v4:
>
> Used proposed :graph: syntax.
> Also removed gfortran reference.
>
> Change-Id: I2e6900747b2118546f0a39ceb109b3f2f90e6949
> ---
>  gnu/packages/maths.scm | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
> index adc7beb655..7dc93ce8ee 100644
> --- a/gnu/packages/maths.scm
> +++ b/gnu/packages/maths.scm
> @@ -3484,8 +3484,21 @@ (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")
> +                (("([[:graph:]]+)/bin/gcc") "gcc")
> +                (("([[:graph:]]+)/bin/g\\+\\+") "g++")
> +                (("([[:graph:]]+)/bin/make") "make")
> +                (("([[:graph:]]+)/bin/diff") "diff")
> +                (("([[:graph:]]+)/bin/sed") "sed")
> +                (("([[:graph:]]+)/bin/gfortran") "gfortran")
> +                ))))
>            (add-after 'install 'move-examples
>              (lambda* (#:key outputs #:allow-other-keys)
>                (let* ((out (assoc-ref outputs "out"))
>
> base-commit: a1a645337a76cf5fb55d20ee694fcb6a52fcf11b
> -- 
> 2.43.0

[-- Attachment #2: S/MIME digital signature --]
[-- Type: application/pkcs7-signature, Size: 5435 bytes --]

bug#67822: [PATCH v4] gnu: maths: petsc: Reduce closure size.

[Ludovic Courtès]

Hi,

Lars Bilke <lars.bilke@ufz.de> skribis:

> Reduces closure size by around 350 MB by removing refernces to build
> dependencies (e.g. gcc).
>
> New ion v4:
>
> Used proposed :graph: syntax.
> Also removed gfortran reference.
>
> Change-Id: I2e6900747b2118546f0a39ceb109b3f2f90e6949

[...]

> +                (("([[:graph:]]+)/bin/sed") "sed")
> +                (("([[:graph:]]+)/bin/gfortran") "gfortran")
> +                ))))

I move those lonely parens to the previous line, tweaked the commit log,
and applied it.  Thank you!

Ludo’.