eic / EICrecon

EIC Reconstruction - JANA based
https://eic.github.io/EICrecon
GNU Lesser General Public License v3.0
5 stars 24 forks source link

fix: prepend target include directories #1519

Open wdconinc opened 1 week ago

wdconinc commented 1 week ago

Briefly, what does this PR introduce?

Instead of appending target_include_directories, this PR prepend them. That ensures that compiles with custom software dependencies are not finding /opt/local/include headers first.

Now we get (note /home/wdconinc/git/acts/install/include before /opt/local/include): [ 14%] Building CXX object src/algorithms/tracking/CMakeFiles/algorithms_tracking_library.dir/AmbiguitySolver.cc.o cd /home/wdconinc/git/EICrecon/build/src/algorithms/tracking && /usr/bin/clang++ -DActs_VERSION_MAJOR=32 -DBOOST_SPIRIT_USE_PHOENIX_V3 -DDD4HEP_USE_XERCESC -DFMT_SHARED -DHAVE_PODIO -DJANA2_HAVE_PODIO=1 -DJANA2_HAVE_ROOT=1 -DPODIO_ENABLE_RNTUPLE=0 -DSPDLOG_COMPILED_LIB -DSPDLOG_FMT_EXTERNAL -DSPDLOG_SHARED_LIB -DUSE_ONNX -Dalgorithms_tracking_library_EXPORTS -I/home/wdconinc/git/EICrecon/src/algorithms/tracking -I/home/wdconinc/git/EICrecon/build/include -I/home/wdconinc/git/EICrecon/src -isystem /home/wdconinc/git/acts/install/include -isystem /opt/local/include -isystem /opt/local/include/eigen3 -isystem /opt/local/include/root -isystem /opt/software/linux-debian12-x86_64_v2/gcc-12.2.0/xerces-c-3.2.5-5j6nth35rmn64gt3xwnsbqhsexfyjxy2/include -O3 -DNDEBUG -std=c++20 -fPIC -MD -MT src/algorithms/tracking/CMakeFiles/algorithms_tracking_library.dir/AmbiguitySolver.cc.o -MF CMakeFiles/algorithms_tracking_library.dir/AmbiguitySolver.cc.o.d -o CMakeFiles/algorithms_tracking_library.dir/AmbiguitySolver.cc.o -c /home/wdconinc/git/EICrecon/src/algorithms/tracking/AmbiguitySolver.cc (after cmake -Bbuild -S. --fresh -DActs_ROOT=$HOME/git/acts/install/lib/cmake/)

Previously, this picked up the headers in /opt/local/include but still tried to link to the Acts_ROOT libraries, leading to linking failures (and, worse, runtime issues).

Also:

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

Does this PR introduce breaking changes? What changes might users need to make to their code?

No.

Does this PR change default behavior?

No.

wdconinc commented 1 week ago

This isn't a good solution, but it works for Acts because it expicitly adds the include directories which previously weren't added at all (maybe that's the actual bug). I think what would be more functional is to prepend versions which are hinted on command line, but append versions which are found without hints.

github-actions[bot] commented 1 week ago

Capybara summary for PR 1519