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

HDF5 version number mismatch #1112

Closed
visr opened this issue Jun 3, 2020 · 18 comments
Closed

HDF5 version number mismatch #1112

visr opened this issue Jun 3, 2020 · 18 comments
Labels
HDF5 5️⃣ Builders and issues related to HDF5

Comments

@visr
Copy link
Contributor

visr commented Jun 3, 2020

I tried the new NetCDF_jll from #1090 in JuliaGeo/NCDatasets.jl#88.

It seems to mostly work, but NetCDF is giving a big error complaining Warning! ***HDF5 library version mismatched error***. Details are in the linked NCDatasets issue, but the key line is: Headers are 1.10.5, library is 1.10.4.

@Alexander-Barth already mentioned that it seems to be related to the fact that the headers are taken from the ARM build:

# Use headers from the ARM build, with the hope that they'll be fine
.

In #262 @musm already remarked:

Ok well we have a weird situation in that case, since the linux and macos versions are not the same as the windows versions

And indeed, Windows does not have this issue. Is there a reason that the versions are not the same? I'd be fine with either as long as they are equal.

@giordano
Copy link
Member

giordano commented Jun 3, 2020

No idea what's wrong with the header files in the armv7l build, but the share/RELEASE.txt file in https://github.com/JuliaPackaging/Yggdrasil/releases/download/HDF5-arm-linux-gnueabihf-v1.10.5/hdf5-arm-linux-gnueabihf-v1.10.5.tar.gz does mention 1.10.5 😕

@visr
Copy link
Contributor Author

visr commented Jun 3, 2020

Hmm yeah the error says the headers are 1.10.5 whilst the library is 1.10.4. So the ARM headers are probably fine. I just don't understand where the 1.10.4 stuff comes from.

@giordano giordano added the HDF5 5️⃣ Builders and issues related to HDF5 label Jun 4, 2020
@Alexander-Barth
Copy link
Contributor

@visr The compiled libraries from h5py are apparently still at the version 1.10.4.

What about to create our own HDF5 libraries compiled natively (without cross-compilation) using e.g. Tavis CI or GitHub ?

Or alternatively, and as a start, can we just compile Linux 64-bit (glibc) natively in build_tarballs.jl instead of extracting the library from h5py-2.10.0-cp27-cp27m-manylinux1_x86_64.whl ?

@giordano
Copy link
Member

giordano commented Jun 8, 2020

What about to create our own HDF5 libraries compiled natively (without cross-compilation) using e.g. Tavis CI or GitHub ?

The problem is compatibility, the risk it to cut out systems that are currently working with the binaries we're providing, or maybe not, it really depends on how it's compiled. In BinaryBuilder we purposely use a very old toolchain in order to be compatible with as many systems as possible

@visr
Copy link
Contributor Author

visr commented Jun 12, 2020

Perhaps we should set the environment variable HDF5_DISABLE_VERSION_CHECK after all? It's not pretty, but the tests seem to run just fine. Should check where to set it though, not sure if NCDatasets' __init__ is too late.

JuliaGeo/NCDatasets.jl@b4cbe9f

@Alexander-Barth
Copy link
Contributor

I think it is a bit dangerous to disable the HDF5 version check. Could it not result in data corruption? It would favor to have a consistent HDF5_jll package where all libraries are from the same version (or at least headers and libraries version match). As a first step, what about to compile the Linux version of HDF5 (x84_64 and possibly i686) from source in H/HDF5/build_tarballs.jl ?

I had also some luck with cross-compiling HDF5 from Linux to Windows (with wine) and Linux x84_64 to aarch64 (with qemu-aarch64) by setting CMAKE_CROSSCOMPILING_EMULATOR. wine and qemu are easily installable in alpine Linux.

@visr
Copy link
Contributor Author

visr commented Jul 11, 2020

I'd also favor a proper solution, but I fear it will take a while before that would be ready, whilst this at least allows us to drop conda.

Could it not result in data corruption?

That tests with HDF5_DISABLE_VERSION_CHECK are passing is a good signal, but probably not enough, so I looked into this a bit. The warning is about mixing 1.10.4 and 1.10.5, so only a patch difference. Looking onto the HDF forums, I see multiple people complaining about a warning in this case, as the convention is that these should be compatible, but HDF5 doesn't guarantee this [1]. Between 1.10.4 and 1.10.5, according to this ABI tracker [2], it should be safe however.

[1] https://forum.hdfgroup.org/t/c-c-abi-stability-and-binary-compatibility-between-patch-versions/5312
[2] https://abi-laboratory.pro/?view=timeline&l=hdf5

@Alexander-Barth
Copy link
Contributor

Alexander-Barth commented Jul 16, 2020

In fact, the header files are (unfortunately) also system dependent. I just compared the header files for aarch64-linux-gnu and x86_64-linux-gnu, and I see a lot of changes:

diff HDF5.v1.12.0.aarch64-linux-gnu/include HDF5.v1.12.0.x86_64-linux-gnu/include/ | wc -l
582

Some differences might be harmless, but I cannot tell for sure that they are not used in other libraries like NetCDF.
Currently, we just copied over the header files for ARM 1.10.5 for other platforms and other versions of the HDF5 library (1.10.4 as you know). I think we should avoid that.

For the Linux builds (on different architectures including powerpc64le (#1009) we can use qemu) and for Windows 64-bit we can use wine to cross-compile. Would you consider a PR for this change? (here is my current build script https://gist.github.com/Alexander-Barth/362485088fe112bd305c75313eba15c1)

However, for Windows 32-bit, Mac OS and FreeBSD this does not work (wine 32-bit is not available in Alpine Linux, neither is darling https://www.darlinghq.org/, which requires a kernel module. To me it is unlikely that it will work any way with an unprivileged container) .

As I mentioned before, I think we would start to make these binaries (Windows 32-bit and Mac OS) ourselves (with native compilation) instead of extracting them from h5py. This way, we can save the matching header files. I agree that we should use an old toolchain as mentioned by @giordano to be compatible with as many systems as possible. I would suppose that this is also possible for native compilation.

If this is the route that will be taken, I hope that there are some volunteers to test binaries on different platforms (I can test Linux on x86_64 and ARMv7l).

@visr
Copy link
Contributor Author

visr commented Aug 4, 2020

@Alexander-Barth maybe you could turn your gist into a PR? That will probably get more attention. I'm afraid I cannot add much to the pros and cons of particular compilation strategies for HDF5, I'm just hoping to get rid of the Conda.jl dependency in the somewhat near future. I can do more testing on Windows if you want, and since the Clima folks seem interested in this as well, perhaps they can test on their systems as well.

@musm
Copy link
Contributor

musm commented Aug 25, 2020

Can we not just use https://anaconda.org/conda-forge/hdf5/files for osx and linux, that way those version number's are sync'd

@visr
Copy link
Contributor Author

visr commented Aug 25, 2020

Yeah I don't see why not. Note that the version mismatch that gives this issue is between the version of the headers that are copied from the ARM build are not in sync with other builds. It would be good to move to HDF5 1.12 if all of these can be upgraded.

In your link I see they have 1.12, there is also a mingw build of 1.12: https://packages.msys2.org/base/mingw-w64-hdf5
And the latest NetCDF release supports HDF5 1.12: https://www.unidata.ucar.edu/blogs/news/entry/netcdf-4-7-4.

@Alexander-Barth
Copy link
Contributor

@musm, yes I think this is also a very good idea. I would propose to use them for osx and windows. For Linux, we can compile natively (or use anaconda too). HDF5 requires libcurl, zlib and openssl (among others). This is what I see in info/index.json:

{
  "arch": "x86_64",
  "build": "nompi_hc457bb4_101",
  "build_number": 101,
  "depends": [
    "libcurl >=7.71.1,<8.0a0",
    "libcxx >=10.0.1",
    "libgfortran >=4.0.0,<5.0.0.a0",
    "openssl >=1.1.1g,<1.1.2a",
    "zlib >=1.2.11,<1.3.0a0"
  ],
  "license": "LicenseRef-HDF5",
  "license_family": "BSD",
  "name": "hdf5",
  "platform": "osx",
  "subdir": "osx-64",
  "timestamp": 1596840779159,
  "version": "1.12.0"
}

My understanding is that we have to use the same version of these libraries (libcurl, zlib, openssl...) in Yggdrasil. Is this correct?

@musm
Copy link
Contributor

musm commented Nov 15, 2020

Anyone want to test https://github.com/musm/HDF5_jll.jl ? I updated the binaries to v1.12.0. They use conda-forge primarily (osx and linux) and we have to use mysys2 for Windows.

@musm
Copy link
Contributor

musm commented Nov 15, 2020

In the old script it used the same header files for all the targets, which seems wrong. I updated the script to copy over the headers for each target individually.

@musm
Copy link
Contributor

musm commented Nov 15, 2020

The only issue is that I had to ditch linux i686 and armv7, there's really nothing I can do for these platforms at this point.

@giordano
Copy link
Member

In the old script it used the same header files for all the targets, which seems wrong.

Usually the header files are platform-independent, are you saying that this isn't true for HDF5?

@musm
Copy link
Contributor

musm commented Nov 15, 2020

Because the versions of the tarballs weren't necessarily synced up it looks like.

visr added a commit to visr/Yggdrasil that referenced this issue Nov 17, 2020
giordano pushed a commit that referenced this issue Nov 18, 2020
* [NetCDF] rebuild against new HDF5 1.12.0 build

To hopefully help with #1112.

* try adding libcurl dependencies like other recipes

* fix adding Zlib dep twice

* declare JLL compatible with HDF5 1.12

Difference explained here:
github.com//pull/2125#discussion_r525584326

* using Pkg
@visr
Copy link
Contributor Author

visr commented Feb 16, 2021

Fixed by #2125.

@visr visr closed this as completed Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HDF5 5️⃣ Builders and issues related to HDF5
Projects
None yet
Development

No branches or pull requests

4 participants