GNU bug report logs - #67822
[PATCH] gnu: maths: petsc: Reduce closure size.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67822 in the body.
You can then email your comments to 67822 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Lars Bilke <lars.bilke <at> ufz.de>
To: guix-patches <at> gnu.org
Cc: Lars Bilke <lars.bilke <at> ufz.de>
Subject: [PATCH] gnu: maths: petsc: Reduce closure size.
Date: Thu, 14 Dec 2023 13:56:08 +0100

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

Message #8 received at 67822 <at> debbugs.gnu.org (full text, mbox):

From: Lars Bilke <lars.bilke <at> ufz.de>
To: 67822 <at> debbugs.gnu.org
Cc: Lars Bilke <lars.bilke <at> ufz.de>
Subject: [PATCH v2] gnu: maths: petsc: Reduce closure size.
Date: Thu, 14 Dec 2023 15:53:07 +0100

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

Message #11 received at 67822 <at> debbugs.gnu.org (full text, mbox):

From: Lars Bilke <lars.bilke <at> ufz.de>
To: 67822 <at> debbugs.gnu.org
Cc: Lars Bilke <lars.bilke <at> ufz.de>
Subject: [PATCH v3] gnu: maths: petsc: Reduce closure size.
Date: Fri, 15 Dec 2023 09:55:57 +0100

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

Message #14 received at 67822 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lars Bilke <lars.bilke <at> ufz.de>
Cc: 67822 <at> debbugs.gnu.org, Andreas Enge <andreas <at> enge.fr>,
 Efraim Flashner <efraim <at> flashner.co.il>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.
Date: Fri, 05 Jan 2024 12:08:40 +0100

Hi Lars,

Lars Bilke <lars.bilke <at> 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’.

Message #17 received at 67822 <at> debbugs.gnu.org (full text, mbox):

From: Lars Bilke <lars.bilke <at> ufz.de>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 67822 <at> debbugs.gnu.org, Andreas Enge <andreas <at> enge.fr>,
 Efraim Flashner <efraim <at> flashner.co.il>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.
Date: Fri, 05 Jan 2024 12:52:02 +0100

[Message part 1 (text/plain, inline)]

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

[smime.p7s (application/pkcs7-signature, attachment)]

Message #20 received at 67822 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Lars Bilke <lars.bilke <at> ufz.de>
Cc: 67822 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>,
 Andreas Enge <andreas <at> enge.fr>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.
Date: Sun, 7 Jan 2024 11:09:51 +0200

[Message part 1 (text/plain, inline)]

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 <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #23 received at 67822 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 67822 <at> debbugs.gnu.org, Andreas Enge <andreas <at> enge.fr>,
 Lars Bilke <lars.bilke <at> ufz.de>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.
Date: Mon, 08 Jan 2024 18:20:07 +0100

Hi,

Efraim Flashner <efraim <at> 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’.

Message #26 received at 67822 <at> debbugs.gnu.org (full text, mbox):

From: Lars Bilke <lars.bilke <at> ufz.de>
To: 67822 <at> debbugs.gnu.org
Cc: Lars Bilke <lars.bilke <at> ufz.de>
Subject: [PATCH v4] gnu: maths: petsc: Reduce closure size.
Date: Tue, 09 Jan 2024 19:15:04 +0100

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

Message #29 received at 67822 <at> debbugs.gnu.org (full text, mbox):

From: Lars Bilke <lars.bilke <at> ufz.de>
To: ludo <at> gnu.org
Cc: 67822 <at> debbugs.gnu.org
Subject: Re: [PATCH v4] gnu: maths: petsc: Reduce closure size.
Date: Tue, 27 Feb 2024 08:49:56 +0100

[Message part 1 (text/plain, inline)]

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

[smime.p7s (application/pkcs7-signature, attachment)]

Message #34 received at 67822-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lars Bilke <lars.bilke <at> ufz.de>
Cc: 67822-done <at> debbugs.gnu.org, Andreas Enge <andreas <at> enge.fr>,
 Efraim Flashner <efraim <at> flashner.co.il>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#67822] [PATCH v4] gnu: maths: petsc: Reduce closure size.
Date: Tue, 27 Feb 2024 10:50:33 +0100

Hi,

Lars Bilke <lars.bilke <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.