GNU bug report logs - #72071
[PATCH 0/2] Fix hdf5-parallel-openmpi

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 72071 in the body.
You can then email your comments to 72071 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: Luca Cirrottola <luca.cirrottola <at> inria.fr>
To: guix-patches <at> gnu.org
Cc: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Subject: [PATCH 0/2] Fix hdf5-parallel-openmpi
Date: Fri, 12 Jul 2024 11:40:59 +0200

Package hdf5-parallel-openmpi builds often fail with the error reported in https://issues.guix.gnu.org/68313 . The issue has been discussed in https://github.com/open-mpi/ompi/issues/11240 and https://github.com/HDFGroup/hdf5/issues/2196, and solved by https://github.com/HDFGroup/hdf5/pull/3421 that is included in HDF5 release 1.14.3. My proposed solution consists of two commits, the first for updating the HDF5 version in hdf5-parallel-openmpi to 1.14.3, the second for fixing a new test error due to the recipe modifying a file which is used as a test reference (my proposition is to remove this modification, but I don't know why it was introduced in the first place, so thanks for any comment about this).

Luca Cirrottola (2):
  Solve issue https://issues.guix.gnu.org/68313 by updating HDF5 version
    to 1.14.3 (containing the official fix
    https://github.com/HDFGroup/hdf5/pull/3421).
  Fix broken HDF5 test: Do not modify the file src/libhdf5.settings
    generated by the configure script, as it will be checked against a
    new version generated by the test code in
    test/test_check_version.sh.in.

 gnu/packages/maths.scm | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

-- 
2.34.1

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

From: Luca Cirrottola <luca.cirrottola <at> inria.fr>
To: 72071 <at> debbugs.gnu.org
Cc: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Subject: [PATCH 1/2] Solve issue https://issues.guix.gnu.org/68313 by updating
 HDF5 version to 1.14.3 (containing the official fix
 https://github.com/HDFGroup/hdf5/pull/3421).
Date: Fri, 12 Jul 2024 11:48:59 +0200

---
 gnu/packages/maths.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index 391c11891e..8116569bca 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -1814,7 +1814,7 @@ (define-public hdf5-1.10
 (define-public hdf5-1.14
   (package
     (inherit hdf5-1.8)
-    (version "1.14.0")
+    (version "1.14.3")
     (source
      (origin
        (method url-fetch)
@@ -1828,7 +1828,7 @@ (define-public hdf5-1.14
                                         (take (string-split version #\.) 2))
                                  "/src/hdf5-" version ".tar.bz2")))
        (sha256
-        (base32 "181bdh8hp7v9xqwcby3lknr92lxlicc2hqscba3f5nhf8lrr9rz4"))
+        (base32 "05zr11y3bivfwrbvzbky1q2gjf6r7n92cvvdnh5jilbmxljg49cl"))
        (patches (search-patches "hdf5-config-date.patch"))))))
 
 (define-public hdf5
@@ -2069,7 +2069,7 @@ (define-public hdf-eos5
     (license (license:non-copyleft home-page))))
 
 (define-public hdf5-parallel-openmpi
-  (package/inherit hdf5-1.10                      ;use the latest
+  (package/inherit hdf5-1.14                      ;use the latest
     (name "hdf5-parallel-openmpi")
     (inputs
      `(("mpi" ,openmpi)
-- 
2.34.1

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

From: Luca Cirrottola <luca.cirrottola <at> inria.fr>
To: 72071 <at> debbugs.gnu.org
Cc: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Subject: [PATCH 2/2] Fix broken HDF5 test: Do not modify the file
 src/libhdf5.settings generated by the configure script,
 as it will be checked against a new version generated by the test code in
 test/test_check_version.sh.in.
Date: Fri, 12 Jul 2024 11:49:00 +0200

---
 gnu/packages/maths.scm | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index 8116569bca..2f12deba3b 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -1729,19 +1729,6 @@ (define-public hdf5-1.8
                (("libhdf5hl_fortran_la_LDFLAGS =")
                 (string-append "libhdf5hl_fortran_la_LDFLAGS = -Wl,-rpath="
                                (assoc-ref outputs "fortran") "/lib")))))
-         (add-after 'configure 'patch-settings
-           (lambda _
-             ;; libhdf5.settings contains the full path of the
-             ;; compilers used, and its contents are included in
-             ;; libhdf5.so.  We truncate the hashes to avoid
-             ;; unnecessary store references to those compilers:
-             (substitute* "src/libhdf5.settings"
-              (("(/gnu/store/)([a-zA-Z0-9]*)" all prefix hash)
-               (string-append prefix (string-take hash 10) "..."))
-              ;; Don't record the build-time kernel version to make the
-              ;; settings file reproducible.
-              (("Uname information:.*")
-               "Uname information: Linux\n"))))
          (add-after 'install 'patch-references
            (lambda* (#:key inputs outputs #:allow-other-keys)
              (let ((bin (string-append (assoc-ref outputs "out") "/bin"))
-- 
2.34.1

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

From: Ludovic Courtès <ludovic.courtes <at> inria.fr>
To: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Cc: 72071 <at> debbugs.gnu.org
Subject: Re: [bug#72071] [PATCH 0/2] Fix hdf5-parallel-openmpi
Date: Fri, 19 Jul 2024 18:01:30 +0200

Hello Luca!

Luca Cirrottola <luca.cirrottola <at> inria.fr> skribis:

> Package hdf5-parallel-openmpi builds often fail with the error reported in https://issues.guix.gnu.org/68313 . The issue has been discussed in https://github.com/open-mpi/ompi/issues/11240 and https://github.com/HDFGroup/hdf5/issues/2196, and solved by https://github.com/HDFGroup/hdf5/pull/3421 that is included in HDF5 release 1.14.3. My proposed solution consists of two commits, the first for updating the HDF5 version in hdf5-parallel-openmpi to 1.14.3, the second for fixing a new test error due to the recipe modifying a file which is used as a test reference (my proposition is to remove this modification, but I don't know why it was introduced in the first place, so thanks for any comment about this).
>
> Luca Cirrottola (2):
>   Solve issue https://issues.guix.gnu.org/68313 by updating HDF5 version
>     to 1.14.3 (containing the official fix
>     https://github.com/HDFGroup/hdf5/pull/3421).

This first patch LGTM.

Do I get it right that there’s one test failure if we apply just this
one patch?

>   Fix broken HDF5 test: Do not modify the file src/libhdf5.settings
>     generated by the configure script, as it will be checked against a
>     new version generated by the test code in
>     test/test_check_version.sh.in.

Here, the ‘patch-settings’ phase was here (1) to ensure bit-for-bit
reproducible builds regardless of the kernel (the “Uname” change), and
(2) to avoid keeping references to the compiler (hdf5 would depend on
GCC & co., so its closure as reported by ‘guix size hdf5’ would be much
bigger than necessary).

I haven’t looked at ‘test_check_version.sh.in’, but can you think of a
way to preserve the changes ‘patch-settings’ makes while still allowing
that test to pass?

Thanks,
Ludo’.

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

From: Luca Cirrottola <luca.cirrottola <at> inria.fr>
To: Ludovic Courtès <ludovic.courtes <at> inria.fr>
Cc: 72071 <at> debbugs.gnu.org
Subject: Re: [bug#72071] [PATCH 0/2] Fix hdf5-parallel-openmpi
Date: Thu, 25 Jul 2024 12:08:45 +0200 (CEST)

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

Hello Ludo'!

Yes, after applying the first patch with the package version update, a failure appears in "test_check_version" (I attach the log for future reference). I don't know if this happens now because of an update in the HDF5 package, or because the old version failed before reaching this test.
Anyway, this test compares two versions of a file generated according to the template in src/libhdf5.settings.in, the first one generated in the "configure" phase, the second one in the "check" phase. Since the "patch-settings" phase modifies the first file, the test fails.
Thanks for the explanations for the "patch-settings" phase! I will come back in the next message with a replacement for the second patch which hopefully does not break the expected behaviour.

Thanks,
Luca

----- Mail original -----
> De: "Ludovic Courtès" <ludovic.courtes <at> inria.fr>
> À: "Luca Cirrottola" <luca.cirrottola <at> inria.fr>
> Cc: 72071 <at> debbugs.gnu.org
> Envoyé: Vendredi 19 Juillet 2024 18:01:30
> Objet: Re: [bug#72071] [PATCH 0/2] Fix hdf5-parallel-openmpi

> Hello Luca!
> 
> Luca Cirrottola <luca.cirrottola <at> inria.fr> skribis:
> 
>> Package hdf5-parallel-openmpi builds often fail with the error reported in
>> https://issues.guix.gnu.org/68313 . The issue has been discussed in
>> https://github.com/open-mpi/ompi/issues/11240 and
>> https://github.com/HDFGroup/hdf5/issues/2196, and solved by
>> https://github.com/HDFGroup/hdf5/pull/3421 that is included in HDF5 release
>> 1.14.3. My proposed solution consists of two commits, the first for updating
>> the HDF5 version in hdf5-parallel-openmpi to 1.14.3, the second for fixing a
>> new test error due to the recipe modifying a file which is used as a test
>> reference (my proposition is to remove this modification, but I don't know why
>> it was introduced in the first place, so thanks for any comment about this).
>>
>> Luca Cirrottola (2):
>>   Solve issue https://issues.guix.gnu.org/68313 by updating HDF5 version
>>     to 1.14.3 (containing the official fix
>>     https://github.com/HDFGroup/hdf5/pull/3421).
> 
> This first patch LGTM.
> 
> Do I get it right that there’s one test failure if we apply just this
> one patch?
> 
>>   Fix broken HDF5 test: Do not modify the file src/libhdf5.settings
>>     generated by the configure script, as it will be checked against a
>>     new version generated by the test code in
>>     test/test_check_version.sh.in.
> 
> Here, the ‘patch-settings’ phase was here (1) to ensure bit-for-bit
> reproducible builds regardless of the kernel (the “Uname” change), and
> (2) to avoid keeping references to the compiler (hdf5 would depend on
> GCC & co., so its closure as reported by ‘guix size hdf5’ would be much
> bigger than necessary).
> 
> I haven’t looked at ‘test_check_version.sh.in’, but can you think of a
> way to preserve the changes ‘patch-settings’ makes while still allowing
> that test to pass?
> 
> Thanks,
> Ludo’.

[ypb72f7f2xc13a3sjb5jwg45gsjw1g-hdf5-parallel-openmpi-1.14.3.drv.gz (application/gzip, attachment)]

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

From: Luca Cirrottola <luca.cirrottola <at> inria.fr>
To: 72071 <at> debbugs.gnu.org
Cc: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Subject: [PATCH 0/1] Split the patch-settings phase into copy-settings,
 patch-settings-for-build, restore-settings-for-check,
 patch-settings-for-install
Date: Thu, 25 Jul 2024 14:49:02 +0200

Let the test_check_version test success by backing-up the original
settings file (to use it as a test reference) and swap it with the
patched file. Swap back the two files before installation.

Luca Cirrottola (1):
  Backup original settings, so they can be restored before the check
    phase and patched again before the install phase (since the
    install-data target depends on this file).

 gnu/packages/maths.scm | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

-- 
2.34.1

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

From: Luca Cirrottola <luca.cirrottola <at> inria.fr>
To: 72071 <at> debbugs.gnu.org
Cc: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Subject: [PATCH 1/1] Backup original settings,
 so they can be restored before the check phase and patched again
 before the install phase (since the install-data target depends on this
 file).
Date: Thu, 25 Jul 2024 14:49:03 +0200

---
 gnu/packages/maths.scm | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index 8116569bca..6787f9bb49 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -1729,7 +1729,13 @@ (define-public hdf5-1.8
                (("libhdf5hl_fortran_la_LDFLAGS =")
                 (string-append "libhdf5hl_fortran_la_LDFLAGS = -Wl,-rpath="
                                (assoc-ref outputs "fortran") "/lib")))))
-         (add-after 'configure 'patch-settings
+         (add-after 'configure 'copy-settings
+           (lambda _
+             ;; Backup the original settings (because they are needed as a test
+             ;; reference during the "check" phase) before patching them in the
+             ;; "patch-settings-for-build" phase.
+             (copy-file "src/libhdf5.settings" "libhdf5_settings.orig")))
+         (add-before 'build 'patch-settings-for-build
            (lambda _
              ;; libhdf5.settings contains the full path of the
              ;; compilers used, and its contents are included in
@@ -1742,7 +1748,19 @@ (define-public hdf5-1.8
               ;; settings file reproducible.
               (("Uname information:.*")
                "Uname information: Linux\n"))))
-         (add-after 'install 'patch-references
+         (add-before 'check 'restore-settings-for-check
+           (lambda _
+             ;; Swap the file that has been patched in the
+             ;; "patch-settings-for-build" phase with the original one which
+             ;; has been saved in the "copy-settings" phase, so that it can be
+             ;; used as a test reference in the "check" phase.
+             (copy-file "src/libhdf5.settings" "libhdf5_settings.patched")
+             (rename-file "libhdf5_settings.orig" "src/libhdf5.settings")))
+         (add-before 'install 'patch-settings-for-install
+           (lambda _
+             ;; Restore the patched settings file before installation.
+             (rename-file "libhdf5_settings.patched" "src/libhdf5.settings")))
+        (add-after 'install 'patch-references
            (lambda* (#:key inputs outputs #:allow-other-keys)
              (let ((bin (string-append (assoc-ref outputs "out") "/bin"))
                    (zlib (assoc-ref inputs "zlib")))
-- 
2.34.1

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

From: Luca Cirrottola <luca.cirrottola <at> inria.fr>
To: 72071 <at> debbugs.gnu.org
Cc: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Subject: [PATCH 0/2] A clean series of patches
Date: Mon, 29 Jul 2024 19:36:02 +0200

Hello Ludo', following your suggestion, I have implemented a simpler
solution that just skips off the problematic test (without moving
around files). I send a new patch series with the full modifications
to be applied to the recipe, containing one commit for the package
version update, and one commit for skipping the test.

Let me know if it looks better (no pressure).

Thanks,
Luca

Luca Cirrottola (2):
  Solve issue https://issues.guix.gnu.org/68313 by updating HDF5 version
    to 1.14.3 (containing the official fix
    https://github.com/HDFGroup/hdf5/pull/3421).
  Skip test_check_version since the patch-settings phase needs to modify
    the test reference file.

 gnu/packages/maths.scm | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.34.1

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

From: Luca Cirrottola <luca.cirrottola <at> inria.fr>
To: 72071 <at> debbugs.gnu.org
Cc: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Subject: [PATCH 1/2] Solve issue https://issues.guix.gnu.org/68313 by updating
 HDF5 version to 1.14.3 (containing the official fix
 https://github.com/HDFGroup/hdf5/pull/3421).
Date: Mon, 29 Jul 2024 19:36:03 +0200

---
 gnu/packages/maths.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index 391c11891e..8116569bca 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -1814,7 +1814,7 @@ (define-public hdf5-1.10
 (define-public hdf5-1.14
   (package
     (inherit hdf5-1.8)
-    (version "1.14.0")
+    (version "1.14.3")
     (source
      (origin
        (method url-fetch)
@@ -1828,7 +1828,7 @@ (define-public hdf5-1.14
                                         (take (string-split version #\.) 2))
                                  "/src/hdf5-" version ".tar.bz2")))
        (sha256
-        (base32 "181bdh8hp7v9xqwcby3lknr92lxlicc2hqscba3f5nhf8lrr9rz4"))
+        (base32 "05zr11y3bivfwrbvzbky1q2gjf6r7n92cvvdnh5jilbmxljg49cl"))
        (patches (search-patches "hdf5-config-date.patch"))))))
 
 (define-public hdf5
@@ -2069,7 +2069,7 @@ (define-public hdf-eos5
     (license (license:non-copyleft home-page))))
 
 (define-public hdf5-parallel-openmpi
-  (package/inherit hdf5-1.10                      ;use the latest
+  (package/inherit hdf5-1.14                      ;use the latest
     (name "hdf5-parallel-openmpi")
     (inputs
      `(("mpi" ,openmpi)
-- 
2.34.1

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

From: Luca Cirrottola <luca.cirrottola <at> inria.fr>
To: 72071 <at> debbugs.gnu.org
Cc: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Subject: [PATCH 2/2] Skip test_check_version since the patch-settings phase
 needs to modify the test reference file.
Date: Mon, 29 Jul 2024 19:36:04 +0200

---
 gnu/packages/maths.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index 8116569bca..4e69fb1eb9 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -1728,7 +1728,12 @@ (define-public hdf5-1.8
              (substitute* "hl/fortran/src/Makefile.in"
                (("libhdf5hl_fortran_la_LDFLAGS =")
                 (string-append "libhdf5hl_fortran_la_LDFLAGS = -Wl,-rpath="
-                               (assoc-ref outputs "fortran") "/lib")))))
+                               (assoc-ref outputs "fortran") "/lib")))
+             ;; Skip test_check_version since the "patch-settings" phase
+             ;; will modify the test reference.
+             (substitute* "test/test_check_version.sh.in"
+              (("TESTING\\(\\).*" all)
+               (string-append all "\nSKIP; exit 0\n")))))
          (add-after 'configure 'patch-settings
            (lambda _
              ;; libhdf5.settings contains the full path of the
-- 
2.34.1

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

From: Ludovic Courtès <ludovic.courtes <at> inria.fr>
To: Luca Cirrottola <luca.cirrottola <at> inria.fr>
Cc: 68313-done <at> debbugs.gnu.org, 72071-done <at> debbugs.gnu.org
Subject: Re: [bug#72071] [PATCH 0/2] A clean series of patches
Date: Mon, 19 Aug 2024 12:37:27 +0200

Hi Luca,

Luca Cirrottola <luca.cirrottola <at> inria.fr> skribis:

> Hello Ludo', following your suggestion, I have implemented a simpler
> solution that just skips off the problematic test (without moving
> around files). I send a new patch series with the full modifications
> to be applied to the recipe, containing one commit for the package
> version update, and one commit for skipping the test.
>
> Let me know if it looks better (no pressure).
>
> Thanks,
> Luca
>
> Luca Cirrottola (2):
>   Solve issue https://issues.guix.gnu.org/68313 by updating HDF5 version
>     to 1.14.3 (containing the official fix
>     https://github.com/HDFGroup/hdf5/pull/3421).
>   Skip test_check_version since the patch-settings phase needs to modify
>     the test reference file.

I reorganized the changes a bit; in particular, since skipping the test
is necessary for 1.14.3 to build, I squashed the upgrade and the test
skip in the same commit.

Also, the test change would break hdf <at> 1.8, so I moved that to hdf <at> 1.14.

The result is:

  84c62cf166 * gnu: hdf5-parallel-openmpi: Update to 1.14.x.
  832f2bcb92 * gnu: hdf5 <at> 1.14: Update to 1.14.3.

Tested with:

  ./pre-inst-env guix build --no-grafts \
    hdf5 <at> 1.8 hdf5 <at> 1.14 hdf5-parallel-openmpi hdf5 <at> 1.10

Thanks!

Ludo’.

Previous Next

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