netcdf-c icon indicating copy to clipboard operation
netcdf-c copied to clipboard

adding an integration test to show fetch_content can work

Open K20shores opened this issue 11 months ago • 5 comments

Adds an integration test to show that netcdf can use cmake's fetch_content

Not included in mingw and cygwin builds; could not figure out how to get all of the packages to be found properly and it was purely a thing related to mingw and cygwin, not netcdf

K20shores avatar Mar 15 '24 19:03 K20shores

Note: the project is technically fetch-contentable, but it does not play well with other projects due to option name clash, missing defaults based on IS_PROJECT_TOP_LEVEL.

👍 to have this in the CI, but I encourage to put it as a ctest instead

#2895, as you've seen, adds NETCDF_ as a prefix to address the concern of conflicting variables

K20shores avatar Mar 18 '24 22:03 K20shores

#2895, as you've seen, adds NETCDF_ as a prefix to address the concern of conflicting variables

Yep just saw that. Do you prefer to have this finished first, since it has minimal changes and then fix the default options based on IS_PROJECT_TOP_LEVEL in a separate PR? There would be other things like target aliasing related to this so probably can group those together.

LecrisUT avatar Mar 18 '24 22:03 LecrisUT

#2895, as you've seen, adds NETCDF_ as a prefix to address the concern of conflicting variables

Yep just saw that. Do you prefer to have this finished first, since it has minimal changes and then fix the default options based on IS_PROJECT_TOP_LEVEL in a separate PR? There would be other things like target aliasing related to this so probably can group those together.

Yes, that's the goal. But I've got to figure out these pesky windows actions. Cygwin fails because HDF5 is installed with autotools on Cygwin so there aren't any pkg-config or cmake config files to use to find hdf5. I'd have to write some logic by hand to do all that and frankly I'm lazy. Mingw is failing because netcdf is having trouble linking with the curl libs for some reason

K20shores avatar Mar 19 '24 18:03 K20shores

I did not check the CI in detail, but the CI should mimic the user's experience, i.e. how would a user install HDF5 (preferably without compiling by hand). From what I know they would be using vcpkg, but I am not sure if that is for the user to use as well, or only for the dependencies. The other tool I've heard of is chocolatey which is a package manager afaiu.

Most preferably though, since it's a required dependency importing it as FetchContent could solve any compatibility issues, although it can hide build issues when the user uses find_package and uses an untested version.

An intermediate solution, I would suggest for now is to convert the CI to run the default cmake configure/build/install on hdf5. If we have issues converting that, the user would have even more issues as well.


Sorry I just read the CI for cygwin and it uses a pre-defined cygwin/cygwin-install-action@v2 action, so it seems to be an issue with cygwin packaging?

LecrisUT avatar Mar 19 '24 18:03 LecrisUT

It is also not necessarily the case that we need this to work for cygwin out-of-the-box. If we have it working for *nix and Visual-Studio based builds, that may be a good enough start point. Cygwin fetch_content() is not important enough to hold up the release altogether.

WardF avatar Mar 21 '24 17:03 WardF