adding an integration test to show fetch_content can work

[K20shores]

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

[LecrisUT]

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

[LecrisUT]

Preferably these tests are run as part of ctest using ctest --build-and-test with a FETCHCONTENT_SOURCE_DIR_<uppercaseName> option. That way you are checking the current version of the project is working with fetchcontent.

For an example: https://github.com/LecrisUT/CMake-Template/blob/a4f73dbd53d8cb98e6325296c2905b2ed40dcaa3/test/CMakeLists.txt#L114-L131

[K20shores]

Oh neat! I'll give that a try tomorrow!

[K20shores]

@LecrisUT I actually don't really understand what that test is doing and how it works. The stuff I added builds a small example project. From what I understand of what you sent, it seems like it would only build the netcdf project but not actually link to and use it. I feel like the integration test is a complete test. What am I missing here?

[LecrisUT]

The first link there is the general framework for an arbitrary (regression) test that runs a cmake project as if it's used by a user from tge ground up. In the template it's ueed for packaging, example and regular regression test.

For an example of a packaging test, see this CMakeLists.txt. There is a bit of fluff to allow testing in the build tree and outside, but if you look closely it's very minimal, FetchContent + link libraries + simple smoke test. See the parent folder for how the CMakeLists project is registered as a test.

In the framework link I've sent before, notice the --test-command ${CMAKE_CTEST_COMMAND}. That instructs that the test does ctest (after configure -> build) of the cmake project being pointed to. That way you xan be as thorough as you want, e.g. checking that project version matches library, checking features are enabled in the library, etc. This gives more navigable tests since it's just a ctest output run from a ctest test.

Compared to your approach, it's more or less identical, but this adds the ability to be called within the ctest itself, which will make it much easier to keep track of active cmake changes that break the test (it uses the current source as the "git commit" to FetchContent). Your current approach only tests main at any given point.

Basically what I suggest here is to move the execution outside of the dedicated Github Workflow and inside the project's test itself.

---

Sorry for being too wordy in my explanations most of the times, hope it doesn't put you off

[K20shores]

No worries! I like detailed explanations and yours makes sense to me. Thanks for the input!

[LecrisUT]

Not a fan of these approaches, but I understand the need for them right now. If hdf5 is fetch-contentable, this could be simplified greatly.

Anyway, LD_LIBRARY_PATH should be unnecessary if running from build directory and the dependencies are installed with RPATH. Also, for mac it's DYLD_LIBRARY_PATH

[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

[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.

[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.

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

[LecrisUT]

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?

[WardF]

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.