unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#67822] [PATCH] gnu: maths: petsc: Reduce closure size.
@ 2023-12-14 12:56 Lars Bilke
  2023-12-14 14:53 ` [bug#67822] [PATCH v2] " Lars Bilke
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Lars Bilke @ 2023-12-14 12:56 UTC (permalink / raw)
  To: 67822; +Cc: Lars Bilke, Andreas Enge, Efraim Flashner, Eric Bavier

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





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#67822] [PATCH v2] gnu: maths: petsc: Reduce closure size.
  2023-12-14 12:56 [bug#67822] [PATCH] gnu: maths: petsc: Reduce closure size Lars Bilke
@ 2023-12-14 14:53 ` Lars Bilke
  2023-12-15  8:55 ` [bug#67822] [PATCH v3] " Lars Bilke
  2024-01-09 18:15 ` [bug#67822] [PATCH v4] " Lars Bilke
  2 siblings, 0 replies; 10+ messages in thread
From: Lars Bilke @ 2023-12-14 14:53 UTC (permalink / raw)
  To: 67822; +Cc: Lars Bilke, Andreas Enge, Efraim Flashner, Eric Bavier

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





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.
  2023-12-14 12:56 [bug#67822] [PATCH] gnu: maths: petsc: Reduce closure size Lars Bilke
  2023-12-14 14:53 ` [bug#67822] [PATCH v2] " Lars Bilke
@ 2023-12-15  8:55 ` Lars Bilke
  2024-01-05 11:08   ` Ludovic Courtès
  2024-01-09 18:15 ` [bug#67822] [PATCH v4] " Lars Bilke
  2 siblings, 1 reply; 10+ messages in thread
From: Lars Bilke @ 2023-12-15  8:55 UTC (permalink / raw)
  To: 67822; +Cc: Lars Bilke, Andreas Enge, Efraim Flashner, Eric Bavier

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





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.
  2023-12-15  8:55 ` [bug#67822] [PATCH v3] " Lars Bilke
@ 2024-01-05 11:08   ` Ludovic Courtès
  2024-01-05 11:52     ` Lars Bilke
  0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2024-01-05 11:08 UTC (permalink / raw)
  To: Lars Bilke; +Cc: 67822, Andreas Enge, Efraim Flashner, Eric Bavier

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




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.
  2024-01-05 11:08   ` Ludovic Courtès
@ 2024-01-05 11:52     ` Lars Bilke
  2024-01-07  9:09       ` Efraim Flashner
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Bilke @ 2024-01-05 11:52 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 67822, Andreas Enge, Efraim Flashner, Eric Bavier

[-- 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 --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.
  2024-01-05 11:52     ` Lars Bilke
@ 2024-01-07  9:09       ` Efraim Flashner
  2024-01-08 17:20         ` Ludovic Courtès
  0 siblings, 1 reply; 10+ messages in thread
From: Efraim Flashner @ 2024-01-07  9:09 UTC (permalink / raw)
  To: Lars Bilke; +Cc: 67822, Ludovic Courtès, Andreas Enge, Eric Bavier

[-- 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 --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#67822] [PATCH v3] gnu: maths: petsc: Reduce closure size.
  2024-01-07  9:09       ` Efraim Flashner
@ 2024-01-08 17:20         ` Ludovic Courtès
  0 siblings, 0 replies; 10+ messages in thread
From: Ludovic Courtès @ 2024-01-08 17:20 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: 67822, Andreas Enge, Lars Bilke, Eric Bavier

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




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#67822] [PATCH v4] gnu: maths: petsc: Reduce closure size.
  2023-12-14 12:56 [bug#67822] [PATCH] gnu: maths: petsc: Reduce closure size Lars Bilke
  2023-12-14 14:53 ` [bug#67822] [PATCH v2] " Lars Bilke
  2023-12-15  8:55 ` [bug#67822] [PATCH v3] " Lars Bilke
@ 2024-01-09 18:15 ` Lars Bilke
  2024-02-27  7:49   ` Lars Bilke
  2024-02-27  9:50   ` bug#67822: " Ludovic Courtès
  2 siblings, 2 replies; 10+ messages in thread
From: Lars Bilke @ 2024-01-09 18:15 UTC (permalink / raw)
  To: 67822; +Cc: Lars Bilke, Andreas Enge, Efraim Flashner, Eric Bavier

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





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [bug#67822] [PATCH v4] gnu: maths: petsc: Reduce closure size.
  2024-01-09 18:15 ` [bug#67822] [PATCH v4] " Lars Bilke
@ 2024-02-27  7:49   ` Lars Bilke
  2024-02-27  9:50   ` bug#67822: " Ludovic Courtès
  1 sibling, 0 replies; 10+ messages in thread
From: Lars Bilke @ 2024-02-27  7:49 UTC (permalink / raw)
  To: ludo; +Cc: 67822

[-- 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 --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#67822: [PATCH v4] gnu: maths: petsc: Reduce closure size.
  2024-01-09 18:15 ` [bug#67822] [PATCH v4] " Lars Bilke
  2024-02-27  7:49   ` Lars Bilke
@ 2024-02-27  9:50   ` Ludovic Courtès
  1 sibling, 0 replies; 10+ messages in thread
From: Ludovic Courtès @ 2024-02-27  9:50 UTC (permalink / raw)
  To: Lars Bilke; +Cc: 67822-done, Andreas Enge, Efraim Flashner, Eric Bavier

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




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-02-27  9:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-14 12:56 [bug#67822] [PATCH] gnu: maths: petsc: Reduce closure size Lars Bilke
2023-12-14 14:53 ` [bug#67822] [PATCH v2] " Lars Bilke
2023-12-15  8:55 ` [bug#67822] [PATCH v3] " Lars Bilke
2024-01-05 11:08   ` Ludovic Courtès
2024-01-05 11:52     ` Lars Bilke
2024-01-07  9:09       ` Efraim Flashner
2024-01-08 17:20         ` Ludovic Courtès
2024-01-09 18:15 ` [bug#67822] [PATCH v4] " Lars Bilke
2024-02-27  7:49   ` Lars Bilke
2024-02-27  9:50   ` bug#67822: " Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).