search

georust / netcdf

High-level netCDF bindings for Rust
Apache License 2.0
81 stars 28 forks source link

Bump netcdf source #114

Closed weiznich closed 9 months ago

weiznich commented 9 months ago

This pulls in https://github.com/Unidata/netcdf-c/pull/2756 which fixes an issue with building netcdf-src in a directory containing a whitespace somewhere in the path.

lnicola commented 9 months ago

Looks like https://github.com/Unidata/netcdf-c/pull/2623 has indeed been merged.

weiznich commented 9 months ago

I'm not sure about the CI errors: Are they likely caused by this change or are they happening from time to time?

lnicola commented 9 months ago

No idea, it passed three weeks ago. But hdf5-sys hasn't changed in two years.

  /usr/bin/ld: cannot find -lhdf5_hl_debug: No such file or directory
  /usr/bin/ld: cannot find -lhdf5_debug: No such file or directory
magnusuMET commented 9 months ago

Unfortunately the fix in 2756 has not landed in an official release, which I would like to see before packaging it.

2623 has been merged and we could bump the source to the offical release (v4.92) instead of a personal branch.

CI is not happy because the hdf5 library was not found properly by cmake. We need to change the build script somewhat to get the names/paths on the expected format.

magnusuMET commented 9 months ago

Nothing wrong with CI, reran it just now

magnusuMET commented 9 months ago

Found the issue and made a PR in https://github.com/Unidata/netcdf-c/pull/2761

weiznich commented 9 months ago

Thanks for looking into this :+1:

Do you want to keep the PR open till the next netcdf release? Or is it better to close it?

Also: Would you be open to accept a backport of the linked fix to "private" fork that is based on the latest release?

magnusuMET commented 9 months ago

I would be willing to accept a backport with the fix applied to 4.9.2 since this seems to be an important issue in your case. Do specify a rev in the submodule (which I forgot on my fork...)

weiznich commented 9 months ago

I would be willing to accept a backport with the fix applied to 4.9.2 since this seems to be an important issue in your case. Do specify a rev in the submodule (which I forgot on my fork...)

I've updated the PR to reference a version of the netcdf source that is the v4.9.2 tag + the fix cherry-picked on top of it. I'm not sure if you are OK with accepting the forked repo or if you would like to have the ownership of the branch. In the later case you would need to pull in the branch into your fork.

magnusuMET commented 9 months ago

I am afraid the branch is not merged on top of 4.9.2 from Unidata, but instead from my fork. You need to rebase against rev 9328ba17cb53f13a63707547c94f4715243dafdf from https://github.com/Unidata/netcdf-c

weiznich commented 9 months ago

It seems like git submodule does not update the rev in the .gitmodules file automatically, so that what was pushed did not correspond with my local version. It should be fixed now.

magnusuMET commented 9 months ago

Thanks for the PR, this update should be included in 0.3.2 which was just pushed

weiznich commented 9 months ago

Thanks for the fast release :heart: