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

Remove now unused cmake #2890

Merged
merged 5 commits into from
Mar 19, 2024
Merged

Remove now unused cmake #2890

merged 5 commits into from
Mar 19, 2024

Conversation

K20shores
Copy link
Contributor

@K20shores K20shores commented Mar 15, 2024

With #2847 and the work done by @ZedThree, I believe we can now

When I make and install netcdf on a mac and linux, I don't see the hdf5::hdf5-shared libraries included in <install-prefix>/lib/pkgconfig/netcdf.pc and <install-prefix>/lib/cmake/netCDF/netCDFConfig.cmake only includes a call to find_dependency(HDF5) (as configured).

My bet is that #2847 with its updates to finding and using hdf5, and updated hdf5 and cmake versions fixed this.

Related is a spack PR that should now be redundant, along with the filter code that I removed

@Chrismarsh
Copy link

Great to see!
Further work was done on this in this Spack PR.
Will these changes be back-ported or will they be only for upcoming netcdf releases?

@WardF
Copy link
Member

WardF commented Mar 18, 2024

This looks good, but isn't quite there. We need to sort out the subsequent lack of spacing in libnetcdf.settings, and the output of nc-config --libs --static, e.g.:

Extra libraries:	-lHDF5::HDF5;-lhdf5::hdf5_hl;-lm;-lz;-lblosc;-lzstd;-lbz2;-lsz;-lcurl;-lxml2

and

wfisher@shetland:~/Desktop/tmp/tmp/netcdf-c/build$ ./nc-config --static --libs
-L/usr/local/lib -lnetcdf -lHDF5::HDF5;-lhdf5::hdf5_hl;-lm;-lz;-lblosc;-lzstd;-lbz2;-lsz;-lcurl;-lxml2

Copy link
Member

@WardF WardF left a comment

Choose a reason for hiding this comment

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

This looks good but leaves undesired formatting in nc-config --libs --static. We can't assume the end user who is invoking nc-config will be working within a cmake-focused ecosystem.

@WardF
Copy link
Member

WardF commented Mar 18, 2024

Great to see! Further work was done on this in this Spack PR. Will these changes be back-ported or will they be only for upcoming netcdf releases?

They'll only be in upcoming releases; I'd love to backport but we are resource constrained, unfortunately. Hopefully we'll have it out shortly! That's the goal at least.

@K20shores
Copy link
Contributor Author

This looks good, but isn't quite there. We need to sort out the subsequent lack of spacing in libnetcdf.settings, and the output of nc-config --libs --static, e.g.:

Extra libraries:	-lHDF5::HDF5;-lhdf5::hdf5_hl;-lm;-lz;-lblosc;-lzstd;-lbz2;-lsz;-lcurl;-lxml2

and

wfisher@shetland:~/Desktop/tmp/tmp/netcdf-c/build$ ./nc-config --static --libs
-L/usr/local/lib -lnetcdf -lHDF5::HDF5;-lhdf5::hdf5_hl;-lm;-lz;-lblosc;-lzstd;-lbz2;-lsz;-lcurl;-lxml2

@WardF fixed. I replaced a thing that replaced colons with spaces that I had removed, assuming it wasn't useful

WardF added 2 commits March 19, 2024 11:19
o Added --build-system to report whether netcdf was configured using cmake or autotools.
o For cmake-based installs, added an argument which will allow non-cmake projects to get traditionally formated dependancy syntax, --libs-ac-syntax.
WardF
WardF previously approved these changes Mar 19, 2024
@WardF WardF merged commit e36b111 into Unidata:main Mar 19, 2024
103 checks passed
@K20shores K20shores deleted the h5_shared branch March 22, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants