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

curl linking problem for ISMRMD with system HDF5 on Ubuntu 22.04 #935

Open
Cate-Gascoigne opened this issue Jan 8, 2025 · 11 comments · May be fixed by #937
Open

curl linking problem for ISMRMD with system HDF5 on Ubuntu 22.04 #935

Cate-Gascoigne opened this issue Jan 8, 2025 · 11 comments · May be fixed by #937

Comments

@Cate-Gascoigne
Copy link

When choosing USE_SYSTEM_HDF5=ON, CMake finds the Ubuntu hdf5 libraries. However, we get the following error

cd /home/cate/devel/build/builds/ISMRMRD/build/utilities && /usr/local/bin/cmake -E cmake_link_script CMakeFiles/ismrmrd_generate_cartesian_shepp_logan.dir/link.txt --verbose=ON
/usr/bin/c++ -DBOOST_ERROR_CODE_HEADER_ONLY -DBOOST_SYSTEM_NO_DEPRECATED -Wall -std=c++11 -Werror -O3 -DNDEBUG -rdynamic CMakeFiles/ismrmrd_generate_cartesian_shepp_logan.dir/generate_cartesian_shepp_logan.cpp.o CMakeFiles/ismrmrd_generate_cartesian_shepp_logan.dir/ismrmrd_phantom.cpp.o -o ismrmrd_generate_cartesian_shepp_logan  -Wl,-rpath,/home/cate/devel/build/builds/ISMRMRD/build:/home/cate/anaconda3/lib:/usr/lib/x86_64-linux-gnu/hdf5/serial: ../libismrmrd.so.1.13.7 /home/cate/anaconda3/lib/libboost_program_options.so.1.82.0 -lfftw3 -lfftw3f /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so /usr/lib/x86_64-linux-gnu/libpugixml.so.1.12
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so: undefined reference to `curl_global_init@CURL_OPENSSL_4'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so: undefined reference to `curl_easy_perform@CURL_OPENSSL_4'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so: undefined reference to `curl_slist_free_all@CURL_OPENSSL_4'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so: undefined reference to `curl_easy_setopt@CURL_OPENSSL_4'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so: undefined reference to `curl_easy_init@CURL_OPENSSL_4'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so: undefined reference to `curl_slist_append@CURL_OPENSSL_4'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so: undefined reference to `curl_easy_cleanup@CURL_OPENSSL_4'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so: undefined reference to `curl_global_cleanup@CURL_OPENSSL_4'
collect2: error: ld returned 1 exit status

@KrisThielemans thinks this is because APT installs HDF5 with its hdf5-config.cmake, and CMake does not know it needs to link with curl.

Adding -lcurl to the above line resolves this, but this is not really a SIRF or even ISMRMRD problem.

@paskino
Copy link
Contributor

paskino commented Jan 30, 2025

Discussion with @KrisThielemans
Possible solutions:

  • we ask ubuntu to package the hdf5-config.cmake
  • we ask CMake to attempt to test for the curl linking
  • we take libhdf5 from conda
  • add a horrible workaround by adding the -l curl to this line. Requires to add a switch to say "use horrible workaround"
  • we default build HDF5

Decision is to set the USE_SYSTEM_HDF5=OFF as default behaviour and try to ask ubuntu and CMake if that can be fixed.

@KrisThielemans
Copy link
Member

@paskino I tried this on the 3.6.0 VM and there it works. I found this is because on that system h5cc is installed, and CMake's FindHDF5.cmake uses it to find compilation and linking problems. So, I think the easiest solution is to install h5cc and h5c++.

On debian/ubuntu, this can be done with

sudo apt install hd5-helpers

which we do for our VM/docker

${APT_GET_INSTALL} \
libfftw3-dev \
libhdf5-serial-dev \
hdf5-helpers \

Could you confirm that on the system where you had problems, h5cc was not installed, and that it gets fixed if you do? If so, I suggested we add a some lines to https://github.com/SyneRBI/SIRF-SuperBuild?tab=readme-ov-file#faqs, pointing to here, and close this issue.

PS: We do recommend already to our users to use that install script, https://github.com/SyneRBI/SIRF/wiki/SIRF-SuperBuild-Ubuntu#1-install-dependencies-via-apt

@KrisThielemans
Copy link
Member

I've created https://gitlab.kitware.com/cmake/cmake/-/issues/26654, but don't really expect a fix in CMake.

@KrisThielemans
Copy link
Member

Decision is to set the USE_SYSTEM_HDF5=OFF as default behaviour

we already do

option(USE_SYSTEM_HDF5 "Build using an external version of HDF5" OFF)

@paskino
Copy link
Contributor

paskino commented Feb 10, 2025

I tested the docker build on master:

docker build . --build-arg NUM_PARALLEL_BUILDS=1 --build-arg RUN_CTEST=0 --target=build --no-cache

hdf5-helpers seems installed however the build fails with the default USE_SYSTEM_HDF5=ON

@KrisThielemans
Copy link
Member

Does it fail with the above error? is h5cc on the image? What's the CMake log for ISMRMD? Ideally you add -DHDF5_FIND_DEBUG=ON.

@paskino
Copy link
Contributor

paskino commented Feb 11, 2025

That's what CMake finds. I think the problem is that HDF5 is installed both by conda and on ubuntu system if BUILD_CIL=ON (via the dependency on h5py). CMake does not manage to separate the 2.

CMake SuperBuild log

HDF5_C_COMPILER_EXECUTABLE       /opt/conda/bin/h5cc
HDF5_C_INCLUDE_DIR               /usr/include/hdf5/serial
HDF5_DIFF_EXECUTABLE             /opt/conda/bin/h5diff
HDF5_DIR                         HDF5_DIR-NOTFOUND
HDF5_IS_PARALLEL                 OFF
HDF5_SOURCE_DIR                  /opt/SIRF-SuperBuild/sources/HDF5
HDF5_TAG                         hdf5-1_10_1
HDF5_URL                         https://github.com/HDFGroup/hdf5/
HDF5_hdf5_LIBRARY_DEBUG          HDF5_hdf5_LIBRARY_DEBUG-NOTFOUND
HDF5_hdf5_LIBRARY_RELEASE        /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so

CMake ISMRMRD log

HDF5_C_COMPILER_EXECUTABLE       /opt/conda/bin/h5cc
HDF5_C_INCLUDE_DIR               /usr/include/hdf5/serial
HDF5_C_LIBRARIES                 /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so
HDF5_DIFF_EXECUTABLE             /opt/conda/bin/h5diff
HDF5_DIR                         HDF5_DIR-NOTFOUND
HDF5_FIND_DEBUG                  ON
HDF5_INCLUDE_DIRS                /usr/include/hdf5/serial
HDF5_IS_PARALLEL                 OFF
HDF5_LIBRARIES                   /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so
HDF5_hdf5_LIBRARY_DEBUG          HDF5_hdf5_LIBRARY_DEBUG-NOTFOUND
HDF5_hdf5_LIBRARY_RELEASE        /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so

@paskino
Copy link
Contributor

paskino commented Feb 11, 2025

I tried not to install the hdf5 packages below

libhdf5-serial-dev \
libboost-dev libboost-all-dev \
libfftw3-dev \
h5utils \
jq \
hdf5-tools \

However libarmadillo-dev depends on a whole lot of hdf5 libraries and ancillary hdf5 packages.

I suggest to move the installation of h5py for CIL after the build stage.

paskino added a commit to paskino/SIRF-SuperBuild that referenced this issue Feb 11, 2025
@KrisThielemans
Copy link
Member

KrisThielemans commented Feb 11, 2025

I did a test on my local machine with a new conda env with only h5py (conda create -n test h5py) and the following CMakeLists.txt:

cmake_minimum_required(VERSION 3.14.0)
PROJECT(TESTHDF5)
find_package(HDF5 COMPONENTS C)

Running with cmake -S . -B build/ -DHDF5_FIND_DEBUG=ON, I get

HDF5 C compiler wrapper is unable to compile a minimal HDF5 program.
-- Found HDF5: /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so 
<snip>

Root cause therefore: h5py depends on hdf5, which installs all include files and libraries as well as h5cc. However, trying to use h5cc leads to

 h5cc cmake_hdf5_test.c 
/home/sirfuser/mambaforge/envs/test/bin/h5cc: 1: eval: x86_64-conda-linux-gnu-cc: not found

Indeed, https://github.com/SyneRBI/SIRF/wiki/Building-SIRF-and-CIL-with-conda says to add cxx-compiler. Adding that to my conda env means that CMake finds the conda version of hdf5 and all is well.

Summary: when using conda, install both h5py and cxx-compiler.

@KrisThielemans
Copy link
Member

I suggest to move the installation of h5py for CIL after the build stage

While this might build, I think it's a failing strategy unfortunately. Reason: we build with system hdf5, and then install conda hdf5, so any python program will use the conda hdf5 shared library at run-time, while ISMRMRD, STIR and possibly ITK assume it will be the system hdf5. In my experience, this can easily lead to segfaults. (libgcc shared libraries are backwards compatible somehow, hdf5 shared libraries are not).

The only case where this might be safe is if we are NOT using the conda env where CIL is installed. I have no idea if this is the case, but it seems unlikely.

I see 3 solutions:

  • remove h5py requirement from CIL (why does it need it anyway?), even if only for our own builds. However, this is probably not a good strategy, as people might install h5py anyway (maybe sirf-exercises do already)
  • forget about most apt packages and use conda packages instead (see the wiki page, but it will need work for gadgetron).
  • continue mixing apt and conda, and pray that hdf5 (or other libraries) don't trip us up.

@paskino
Copy link
Contributor

paskino commented Feb 12, 2025

On the VM we use the system python, while on docker we install conda or better conda is installed by the base image to run jupyter.

I agree that it is dangerous, but I thought that once built, the libraries would find the libraries they had been linked to during the build. I might be wrong.

Several CIL requirements are needed for reading scanner data such as h5py and dxchange. We can debate whether this is a requirement or just a piece of software that makes the functionality more complete. For us here the important bits of CIL are in the optimisation framework which doesn't really depend on those packages.

However, this docker image is the base of the joined courses that we've been running, therefore we've kept the full list of dependencies.

If I remember correctly Gadgetron used to be tested only on Ubuntu, relying on dependencies available on apt.

It may not be a bad idea to use conda packages all the way through. Gadgetron has a list in their meta.yaml which can be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants