New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
alembic: use threadsafe hdf5 #30682
alembic: use threadsafe hdf5 #30682
Conversation
|
So the issue about |
d72b991
to
0b59ffd
Compare
pkgs/top-level/all-packages.nix
Outdated
alembic = callPackage ../development/libraries/alembic {}; | ||
alembic = callPackage ../development/libraries/alembic { | ||
hdf5 = hdf5-threadsafe; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since hdf5-threadsafe
is in all-packages.nix why not just pass that in directly instead of overriding hdf5? This makes things more explicit for alembic and easier to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean to directly use hdf5-threadsafe
inside alembic
? Yes, that's a solution. I'll update the PR with this.
0b59ffd
to
04df8c6
Compare
Any update? I'm using this (2 lines) changes in production since October without any issue, can we merge? |
@@ -14,7 +14,7 @@ stdenv.mkDerivation rec | |||
|
|||
outputs = [ "bin" "dev" "out" "lib" ]; | |||
|
|||
buildInputs = [ unzip cmake openexr hdf5 ]; | |||
buildInputs = [ unzip cmake openexr hdf5-threadsafe ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please shift unzip and cmake to nativeBuildInputs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Compiled with thread safe support and without the High Level library which is incompatible with thread safety.
04df8c6
to
2e4c22b
Compare
Thanks @guibou ! |
Motivation for this change
Allowing
alembic
(and perhaps other libraries) to loadhdf5
encoded files in a concurrent manner. To achieve this, this change:hdf5
in a separate packagehdf5-threadsafe
.alembic
packageDiscussion
By default,
hdf5
is not thread-safe hence--enable-threadsafe
is mandatory to loadalembic
file (usinghdf5
) in a concurrent context.However this change is not benign for all
hdf5
users: the High Level (hl
) library included inhdf5
does not officially support thread safety. The recommended way is to--disable-hl
. However this may not be acceptable for some users of hdf5 (actually 21 derivation in nixpkgs). So I do not propose this for the default hdf5 derivation.Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)