Skip to content
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

Update HDF5 and add option to compile Fortran2003 interface #31781

Merged
merged 4 commits into from Nov 25, 2017

Conversation

robertodr
Copy link
Contributor

@robertodr robertodr commented Nov 17, 2017

Motivation for this change

The HDF5 package is updated from 1.8.18 to 1.10.1, as recommended on the HDF5 group website.
In addition, the possibility to compile the Fortran2003 interface is exposed via the fortran2003 option for the 1.8.19 version of the package.

Things done
  • Add a hdf5_1_8, updated from 1.8.18 to 1.8.19, to still provide the older version of HDF5.
  • Add a fortran2003 option to hdf5_1_8. This allows compiling the Fortran2003 bindings, by passing --enable-fortran2003 to the configure script. This option was removed from 1.10.1, for which --enable-fortran suffices to get the newer bindings compiled.
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@@ -14,17 +15,20 @@
# (--enable-unsupported could be used to force the build)
assert !cpp || mpi == null;

# Need a Fortran compiler for Fortran2003 bindings
assert fortran2003 -> gfortran == null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be != null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops! Yes, it should.

@@ -45,6 +49,7 @@ stdenv.mkDerivation rec {
configureFlags = []
++ optional cpp "--enable-cxx"
++ optional (gfortran != null) "--enable-fortran"
++ optional fortran2003 "--enable-fortran2003"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just throw --enable-fortran2003 in with --enable-fortran?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users might want to use the Fortran90 bindings instead of the Fortran2003 ones.

@lsix
Copy link
Member

lsix commented Nov 18, 2017

The doc states that:

Due to the requirements of some of the new features, the format of a 1.10.x HDF5 file is likely to be different from that of a 1.8.x HDF5 file. This means that tools and applications built to read 1.10.x files will be able to read a 1.8.x file, but tools built to read 1.8.x files may not be able to read a 1.10.x file. 

I am fine with updating to 1.10 branch as default, but I think it would be preferable to keep the 1.8 branch around since it is still widely used (and default in many distributions).

@robertodr
Copy link
Contributor Author

@lsix that's a good point, but I am not sure how to accomplish it...

@lsix
Copy link
Member

lsix commented Nov 18, 2017

@robertodr just copy the old version of the pkgs/tools/misc/hdf5/default.nix file as pkgs/tools/misc/hdf5/1_8.nix and add something like:

  hdf5_1_8 = callPackage ../tools/misc/hdf5/1.8.nix {
    gfortran = null;
    szip = null;
    mpi = null;
  };

in pkgs/top-level/all-packages.nix file next the the hdf5 definition.

This is how I would do it. Users should then use the override mechanism to use the 1.8 branch on their systems.

@robertodr
Copy link
Contributor Author

Thanks for the explanation @lsix!

@FRidh
Copy link
Member

FRidh commented Nov 21, 2017

Last time I checked, this bump broke several packages.

The following packages seem to build:

hdf5-cpp
hdf5-fortran
hdf5-mpi
netcdf
netcdf-mpi
netcdfcxx4
netcdffortran
octaveFull
python36Packages.h5py
python36Packages.h5py-mpi
python36Packages.netcdf4
python36Packages.odo
python36Packages.pandas
python36Packages.tables

@FRidh
Copy link
Member

FRidh commented Nov 21, 2017

Can you squash commits per expression.

@FRidh FRidh merged commit 88eea69 into NixOS:master Nov 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants