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

cmake exported target uses undefined hdf5::hdf5 imported target

Open jschueller opened this issue 6 years ago • 17 comments

on mingw (win32), compiling netcdf with cmake, with a cmake-compiled hdf5 using its exported target (module mode, not config mode), results in an installed netcdf-targets.cmake that contains the hdf5::hdf5 target (and hdf5::hdf5_hl) in the INTERFACE_LINK_LIBRARIES properties.

But then, when using netcdf in module mode (with find_package(netCDF)) these hdf5 targets are undefined (the hdf5 cmake config is not included), and cmake is not happy:

-- Configuring done
CMake Error at CMake/vtkModule.cmake:2968 (add_library):
  Target "IONetCDF" links to target "hdf5::hdf5_hl-shared" but the target was not found

We first encountered this bug in vtk: https://gitlab.kitware.com/vtk/vtk/issues/17637

A workaround is to provide manually hdf5 paths when configuring netcdf, doing so results in actual library paths being written to the exported targets file instead of hdf5 targets.

jschueller avatar Jul 22 '19 06:07 jschueller

Well, netCDFConfig.cmake should do a find_package(HDF5) and may provide hints to where it found it at build-time via the HDF5_DIR and/or HDF5_ROOT variables.

mathstuf avatar Jul 22 '19 12:07 mathstuf

The netCDFConfig.cmake file is generated automatically; let me take a look to see if I can find out how it is properly modified/updated.

WardF avatar Jul 23 '19 22:07 WardF

@WardF it is configured from https://github.com/Unidata/netcdf-c/blob/master/netCDFConfig.cmake.in

jschueller avatar Jul 24 '19 06:07 jschueller

Coming back to revisit 4.7.1 issues in the lead-up to the release. @jschueller thanks for the pointer.

WardF avatar Aug 15 '19 21:08 WardF

Note that a similar issue is with missing a dependency on MPI as well. This commit: https://github.com/Unidata/netcdf-c/commit/bc2a3babf9cbe80fa7ae3dec5d7622594a7ae51e introduces a call to find_package(MPI) on MSVC but surely this should be done for all platforms that use mpi as ncdispatch.h includes mpi.h if HDF5_PARALLEL or USE_PNETCDF is defined.

jsharpe avatar Dec 18 '19 16:12 jsharpe

I think the issue here is that cmake's current FIndHDF5.cmake file doesn't define any imported targets (the hdf5::hdf5 target) but hdf5 build with cmake defines a cmake config file that does provide this target. See https://gitlab.kitware.com/cmake/cmake/issues/18560 for the upstream discussions regarding this.

So I suspect the best route to fixing this is to ignore the hdf5 provided config files by passing NO_MODULE to the find_package(HDF5) calls?

jsharpe avatar Dec 19 '19 10:12 jsharpe

VTK has a patched FindHDF5 that contains imported targets. It's been working pretty well there, but every time I breathe on it, new bugs crop up :) . I'll resurrect my MR to put it in over the holidays.

mathstuf avatar Dec 19 '19 18:12 mathstuf

@mathstuf Its actually in the context of VTK and the superbuild that I was seeing issues with this. I've had to define -DHDF5_NO_FIND_PACKAGE_CONFIG_FILE:BOOL=ON on dependencies of hdf5 to get it to work correctly for the netcdf build but then I have to manually define HDF5_HL_LIBRARIES and HDF5_C_LIBRARY_hdf5_hl to get it to pass the configuration step in paraview / VTK.

jsharpe avatar Dec 19 '19 18:12 jsharpe

That's probably with VTK 8.2 / ParaView 5.6 and older (where the build system choked on imported targets). Now with VTK 8.90 / ParaView 5.7 and newer, imported targets are heavily preferred (non-targets are usually warned about if it comes to the attention of the new build system infrastructure).

mathstuf avatar Dec 19 '19 19:12 mathstuf

hdf5 logic is complicated https://github.com/Unidata/netcdf-c/issues/877

jschueller avatar Apr 24 '20 13:04 jschueller

@jschueller Indeed.

WardF avatar Apr 28 '20 15:04 WardF

Addressing this now before it slips under the radar again.

WardF avatar Apr 28 '20 15:04 WardF

hi @WardF with your patch now I get just "hdf5_hl-shared" instead of the previous "hdf5::hdf5_hl-shared", but it's not right yet ;)

jschueller avatar Apr 29 '20 07:04 jschueller

Yes, the logic for HDF5 is tricky. Thanks for checking in, I was going to ask, once I had something that looked like a fix. I imagine I'll need to pattern something off of the hdf5 logic found in the netcdf-c top-level CMakeLists.txt file. It's good to have something to do :).

WardF avatar Apr 29 '20 20:04 WardF

Is there progress on this? I see that a netcdf-c cmake built library has a pkgconfig file that references libraries that don't exist (Libs.private: -lhdf5_hl-shared -lhdf5-shared)

Name: netCDF
Description: NetCDF Client Library for C
URL: https://www.unidata.ucar.edu/netcdf
Version: 4.9.2
Libs: -L${libdir} -lnetcdf
Libs.private: -lhdf5_hl-shared -lhdf5-shared -lm -lz -lzstd -lbz2 -lcurl -lpnetcdf -lxml2
Cflags: -I${includedir}

Chrismarsh avatar Feb 23 '24 17:02 Chrismarsh

Wow this has really lingered. My sincere, embarrassed apologies. @K20shores this feels like something that might fit in with the work we're doing over at #2847?

WardF avatar Feb 26 '24 23:02 WardF

@WardF, yes, I think the updates we've added may fix this. I'll have to check

K20shores avatar Feb 27 '24 17:02 K20shores