GNU bug report logs - #35666
[PATCH 0/2] Build a thread-safe hdf5 library

Previous Next

Package: guix-patches;

Reported by: Ludovic Courtès <ludo <at> gnu.org>

Date: Fri, 10 May 2019 09:57:01 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Fri, 10 May 2019 09:57:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Ludovic Courtès <ludo <at> gnu.org>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Fri, 10 May 2019 09:57:01 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-patches <at> gnu.org
Cc: Eric Bavier <bavier <at> cray.com>, Ricardo Wurmus <rekado <at> elephly.net>,
 Ludovic Courtès <ludo <at> gnu.org>,
 Paul Garlick <pgarlick <at> tourbillion-technology.com>
Subject: [PATCH 0/2] Build a thread-safe hdf5 library
Date: Fri, 10 May 2019 11:56:30 +0200
Hello!

A colleague of mine noticed that our ‘hdf5’ library wasn’t
thread-safe.  Turns out there’s an option to make it thread-safe (oh!),
it’s turned off by default (oh?!), and when you pass it ‘configure’
invites you to turn off no less than C++, Fortran, and the high-level
interface (d’oh!).

It also tells you that, if you insist, you can go ahead and pass
‘--enable-unsupported’, but you’re on your own.

We found that Debian chose to pass ‘--enable-unsupported’, and indeed
that seems to be saner than providing a variant that does very little,
but does it in a thread-safe way.

Thoughts?

Thanks,
Ludo’.

Ludovic Courtès (2):
  gnu: hdf5: Build a thread-safe library.
  gnu: hdf5: Add dependency on Perl.

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

-- 
2.21.0





Information forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Fri, 10 May 2019 10:07:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: 35666 <at> debbugs.gnu.org
Cc: Eric Bavier <bavier <at> cray.com>, Ricardo Wurmus <rekado <at> elephly.net>,
 Ludovic Courtès <ludovic.courtes <at> inria.fr>,
 Paul Garlick <pgarlick <at> tourbillion-technology.com>
Subject: [PATCH 2/2] gnu: hdf5: Add dependency on Perl.
Date: Fri, 10 May 2019 12:05:46 +0200
From: Ludovic Courtès <ludovic.courtes <at> inria.fr>

* gnu/packages/maths.scm (hdf5)[native-inputs]: Add PERL.
---
 gnu/packages/maths.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index 2c3889ece2..7ea94d1060 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -774,7 +774,8 @@ incompatible with HDF5.")
     (inputs
      `(("zlib" ,zlib)))
     (native-inputs
-     `(("gfortran" ,gfortran)))
+     `(("gfortran" ,gfortran)
+       ("perl" ,perl)))                 ;part of the test machinery needs Perl
     (outputs '("out"       ; core library
                "fortran")) ; fortran interface
     (arguments
-- 
2.21.0





Information forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Fri, 10 May 2019 10:07:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: 35666 <at> debbugs.gnu.org
Cc: Eric Bavier <bavier <at> cray.com>, Ricardo Wurmus <rekado <at> elephly.net>,
 Ludovic Courtès <ludovic.courtes <at> inria.fr>,
 Paul Garlick <pgarlick <at> tourbillion-technology.com>
Subject: [PATCH 1/2] gnu: hdf5: Build a thread-safe library.
Date: Fri, 10 May 2019 12:05:45 +0200
From: Ludovic Courtès <ludovic.courtes <at> inria.fr>

* gnu/packages/maths.scm (hdf5)[arguments]: Pass "--enable-threadsafe
--with-pthread --enable-unsupported".
---
 gnu/packages/maths.scm | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index d59028599f..2c3889ece2 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -781,7 +781,17 @@ incompatible with HDF5.")
      `(;; Some of the users, notably Flann, need the C++ interface.
        #:configure-flags '("--enable-cxx"
                            "--enable-fortran"
-                           "--enable-fortran2003")
+                           "--enable-fortran2003"
+
+                           ;; Build a thread-safe library.  Unfortunately,
+                           ;; 'configure' invites you to either turn off C++,
+                           ;; Fortran, and the high-level interface (HL), or
+                           ;; to pass '--enable-unsupported'.  Debian
+                           ;; packagers chose to pass '--enable-unsupported'
+                           ;; and we follow their lead here.
+                           "--enable-threadsafe"
+                           "--with-pthread"
+                           "--enable-unsupported")
        ;; Use -fPIC to allow the R bindings to link with the static libraries
        #:make-flags (list "CFLAGS=-fPIC"
                           "CXXFLAGS=-fPIC")
-- 
2.21.0





Information forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Fri, 10 May 2019 11:54:02 GMT) Full text and rfc822 format available.

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Eric Bavier <bavier <at> cray.com>,
 Paul Garlick <pgarlick <at> tourbillion-technology.com>, guix-patches <at> gnu.org
Subject: Re: [PATCH 0/2] Build a thread-safe hdf5 library
Date: Fri, 10 May 2019 13:52:42 +0200
Ludovic Courtès <ludo <at> gnu.org> writes:

> A colleague of mine noticed that our ‘hdf5’ library wasn’t
> thread-safe.  Turns out there’s an option to make it thread-safe (oh!),
> it’s turned off by default (oh?!), and when you pass it ‘configure’
> invites you to turn off no less than C++, Fortran, and the high-level
> interface (d’oh!).

Oh!

> It also tells you that, if you insist, you can go ahead and pass
> ‘--enable-unsupported’, but you’re on your own.
>
> We found that Debian chose to pass ‘--enable-unsupported’, and indeed
> that seems to be saner than providing a variant that does very little,
> but does it in a thread-safe way.

What other effects does “--enable-unsupported” have?  I see that in
Fedora “--enable-threadsafe” was removed in 2008 because it’s
“incompatible with --enable-cxx and --enable-fortran”.

Instead they seem to be building different flavours: one with
--enable-fortran, another with --enable-cxx, yet another with MPI and
--enable-parallel.

Do we have contact to the hdf5 developers to ask what the implications
of “enable-unsupported” are?

--
Ricardo





Information forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Fri, 10 May 2019 13:08:01 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludovic.courtes <at> inria.fr>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: bavier <at> cray.com, 35666 <at> debbugs.gnu.org, pgarlick <at> tourbillion-technology.com
Subject: Re: [bug#35666] [PATCH 0/2] Build a thread-safe hdf5 library
Date: Fri, 10 May 2019 15:07:29 +0200
Hi!

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:

[…]

>> It also tells you that, if you insist, you can go ahead and pass
>> ‘--enable-unsupported’, but you’re on your own.
>>
>> We found that Debian chose to pass ‘--enable-unsupported’, and indeed
>> that seems to be saner than providing a variant that does very little,
>> but does it in a thread-safe way.
>
> What other effects does “--enable-unsupported” have?  I see that in
> Fedora “--enable-threadsafe” was removed in 2008 because it’s
> “incompatible with --enable-cxx and --enable-fortran”.

“--enable-unsupported” allows you to force a build that combines C++,
Fortran, and thread-safety.  If you don’t pass that flag, you have to
choose between thread-safety and C++/Fortran¹.  A tough choice!

> Instead they seem to be building different flavours: one with
> --enable-fortran, another with --enable-cxx, yet another with MPI and
> --enable-parallel.

Problem is, my colleagues have code that expects both C++ and
thread-safety (as crazy as it might seem).  They were using the Debian
package until now and hadn’t realized about this.

> Do we have contact to the hdf5 developers to ask what the implications
> of “enable-unsupported” are?

I think it’s a warranty-void kind of flag: by passing it, the user
asserts they understand they’re using a configuration not “officially
supported” by the HDF Group, meaning that if it’s buggy, we’re on our
own.

Thoughts?

Ludo’.

¹ You would think it’s an April fool’s day prank, but it’s not!  We’re
  in May, at least in my timezone.




Information forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Fri, 10 May 2019 15:10:01 GMT) Full text and rfc822 format available.

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

From: Eric Bavier <bavier <at> cray.com>
To: Ludovic Courtès <ludovic.courtes <at> inria.fr>, Ricardo
 Wurmus <rekado <at> elephly.net>
Cc: "35666 <at> debbugs.gnu.org" <35666 <at> debbugs.gnu.org>,
 "pgarlick <at> tourbillion-technology.com" <pgarlick <at> tourbillion-technology.com>
Subject: Re: [bug#35666] [PATCH 0/2] Build a thread-safe hdf5 library
Date: Fri, 10 May 2019 15:09:25 +0000
I think this should be fine, though I've not heard of anyone who has relied on this feature.  The "unsupported" part here is that the posix lock used for thread-safety is not hoisted into the higher-level API calls.  So if your colleague is using the C++ interface and expecting thread-safety, they are out of luck.  So the disclaimer is that only the low-level C interface gains thread-safety, and the rest are no better.

Eric Bavier, Scientific Libraries, Cray Inc.

________________________________________
From: Ludovic Courtès <ludovic.courtes <at> inria.fr>
Sent: Friday, May 10, 2019 08:07
To: Ricardo Wurmus
Cc: Eric Bavier; pgarlick <at> tourbillion-technology.com; 35666 <at> debbugs.gnu.org
Subject: Re: [bug#35666] [PATCH 0/2] Build a thread-safe hdf5 library

Hi!

Ricardo Wurmus <rekado <at> elephly.net> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:

[…]

>> It also tells you that, if you insist, you can go ahead and pass
>> ‘--enable-unsupported’, but you’re on your own.
>>
>> We found that Debian chose to pass ‘--enable-unsupported’, and indeed
>> that seems to be saner than providing a variant that does very little,
>> but does it in a thread-safe way.
>
> What other effects does “--enable-unsupported” have?  I see that in
> Fedora “--enable-threadsafe” was removed in 2008 because it’s
> “incompatible with --enable-cxx and --enable-fortran”.

“--enable-unsupported” allows you to force a build that combines C++,
Fortran, and thread-safety.  If you don’t pass that flag, you have to
choose between thread-safety and C++/Fortran¹.  A tough choice!

> Instead they seem to be building different flavours: one with
> --enable-fortran, another with --enable-cxx, yet another with MPI and
> --enable-parallel.

Problem is, my colleagues have code that expects both C++ and
thread-safety (as crazy as it might seem).  They were using the Debian
package until now and hadn’t realized about this.

> Do we have contact to the hdf5 developers to ask what the implications
> of “enable-unsupported” are?

I think it’s a warranty-void kind of flag: by passing it, the user
asserts they understand they’re using a configuration not “officially
supported” by the HDF Group, meaning that if it’s buggy, we’re on our
own.

Thoughts?

Ludo’.

¹ You would think it’s an April fool’s day prank, but it’s not!  We’re
  in May, at least in my timezone.




Information forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Fri, 10 May 2019 15:29:01 GMT) Full text and rfc822 format available.

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

From: Paul Garlick <pgarlick <at> tourbillion-technology.com>
To: Ludovic Courtès <ludovic.courtes <at> inria.fr>, Ricardo
 Wurmus <rekado <at> elephly.net>
Cc: bavier <at> cray.com, 35666 <at> debbugs.gnu.org
Subject: Re: [bug#35666] [PATCH 0/2] Build a thread-safe hdf5 library
Date: Fri, 10 May 2019 16:27:52 +0100
Hi Ludo and Ricardo,

> Thoughts?
> 

I notice in the Debian rules file [1] that the "--enable-threadsafe"
flag is listed as  one of the SERIAL_FLAGS.  In the Guix case the
package hdf5-parallel-openmpi inherits its configure flags from the
hdf5 (serial) package.

Will we also have to explicitly delete the "--enable-threadsafe" flag
from the hdf5-parallel-openmpi definition?

Best regards,

Paul.

[1] https://sources.debian.org/src/hdf5/1.10.4+repack-10/debian/rules/





Information forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Tue, 14 May 2019 07:29:03 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludovic.courtes <at> inria.fr>
To: Eric Bavier <bavier <at> cray.com>
Cc: Ricardo Wurmus <rekado <at> elephly.net>,
 "35666 <at> debbugs.gnu.org" <35666 <at> debbugs.gnu.org>,
 "pgarlick <at> tourbillion-technology.com" <pgarlick <at> tourbillion-technology.com>
Subject: Re: [bug#35666] [PATCH 0/2] Build a thread-safe hdf5 library
Date: Tue, 14 May 2019 09:28:11 +0200
Hi Eric,

Eric Bavier <bavier <at> cray.com> skribis:

> I think this should be fine, though I've not heard of anyone who has
> relied on this feature.  The "unsupported" part here is that the posix
> lock used for thread-safety is not hoisted into the higher-level API
> calls.  So if your colleague is using the C++ interface and expecting
> thread-safety, they are out of luck.  So the disclaimer is that only
> the low-level C interface gains thread-safety, and the rest are no
> better.

I’m not sure I understand.  Do you mean that, just because you use the
C++ API instead of the C API, the library is not thread-safe?

They do see crashes vanish when using the library compiled with
‘--enable-threadsafe’, and reliably so.

Thanks,
Ludo’.




Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Tue, 14 May 2019 10:22:02 GMT) Full text and rfc822 format available.

Notification sent to Ludovic Courtès <ludo <at> gnu.org>:
bug acknowledged by developer. (Tue, 14 May 2019 10:22:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Paul Garlick <pgarlick <at> tourbillion-technology.com>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, bavier <at> cray.com,
 35666-done <at> debbugs.gnu.org
Subject: Re: [bug#35666] [PATCH 0/2] Build a thread-safe hdf5 library
Date: Tue, 14 May 2019 12:21:08 +0200
Hi Paul,

Paul Garlick <pgarlick <at> tourbillion-technology.com> skribis:

> I notice in the Debian rules file [1] that the "--enable-threadsafe"
> flag is listed as  one of the SERIAL_FLAGS.  In the Guix case the
> package hdf5-parallel-openmpi inherits its configure flags from the
> hdf5 (serial) package.
>
> Will we also have to explicitly delete the "--enable-threadsafe" flag
> from the hdf5-parallel-openmpi definition?

Good point, I think so.

I did that and pushed it as 549d15712fdc1f58ce0dd11117eb79535ec19f2c.

Let me know if anything is amiss!

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Tue, 14 May 2019 12:04:01 GMT) Full text and rfc822 format available.

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Ludovic Courtès <ludovic.courtes <at> inria.fr>
Cc: bavier <at> cray.com, 35666 <at> debbugs.gnu.org, pgarlick <at> tourbillion-technology.com
Subject: Re: [bug#35666] [PATCH 0/2] Build a thread-safe hdf5 library
Date: Tue, 14 May 2019 14:02:41 +0200
Ludovic Courtès <ludovic.courtes <at> inria.fr> writes:

>> Do we have contact to the hdf5 developers to ask what the implications
>> of “enable-unsupported” are?
>
> I think it’s a warranty-void kind of flag: by passing it, the user
> asserts they understand they’re using a configuration not “officially
> supported” by the HDF Group, meaning that if it’s buggy, we’re on our
> own.

I don’t object to adding the option.  It sounds like you confirmed that
it fixes serious problems in practise, so I think it’s a good idea to
enable it.

--
Ricardo





Information forwarded to guix-patches <at> gnu.org:
bug#35666; Package guix-patches. (Tue, 14 May 2019 14:41:02 GMT) Full text and rfc822 format available.

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

From: Eric Bavier <bavier <at> cray.com>
To: Ludovic Courtès <ludovic.courtes <at> inria.fr>
Cc: Ricardo Wurmus <rekado <at> elephly.net>,
 "35666 <at> debbugs.gnu.org" <35666 <at> debbugs.gnu.org>,
 "pgarlick <at> tourbillion-technology.com" <pgarlick <at> tourbillion-technology.com>
Subject: Re: [bug#35666] [PATCH 0/2] Build a thread-safe hdf5 library
Date: Tue, 14 May 2019 14:40:35 +0000
> I’m not sure I understand.  Do you mean that, just because you use the
> C++ API instead of the C API, the library is not thread-safe?

The thread-safety of the C++ interface itself is not guaranteed/"supported".

> They do see crashes vanish when using the library compiled with
> ‘--enable-threadsafe’, and reliably so.

Great.  I'm not familiar enough with the C++ interface code to say which areas might cause problems in a threaded context.  It's likely that whatever problems they were seeing before was not at the interface layer but deeper.

Eric Bavier, Scientific Libraries, Cray Inc.

________________________________________
From: Ludovic Courtès <ludovic.courtes <at> inria.fr>
Sent: Tuesday, May 14, 2019 02:28
To: Eric Bavier
Cc: Ricardo Wurmus; 35666 <at> debbugs.gnu.org; pgarlick <at> tourbillion-technology.com
Subject: Re: [bug#35666] [PATCH 0/2] Build a thread-safe hdf5 library

Hi Eric,

Eric Bavier <bavier <at> cray.com> skribis:

> I think this should be fine, though I've not heard of anyone who has
> relied on this feature.  The "unsupported" part here is that the posix
> lock used for thread-safety is not hoisted into the higher-level API
> calls.  So if your colleague is using the C++ interface and expecting
> thread-safety, they are out of luck.  So the disclaimer is that only
> the low-level C interface gains thread-safety, and the rest are no
> better.

I’m not sure I understand.  Do you mean that, just because you use the
C++ API instead of the C API, the library is not thread-safe?

They do see crashes vanish when using the library compiled with
‘--enable-threadsafe’, and reliably so.

Thanks,
Ludo’.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 12 Jun 2019 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 291 days ago.

Previous Next


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