search

eic / EICrecon

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

cmake: use CONFIG mode for onnxruntime #1414

Closed veprbl closed 4 months ago

veprbl commented 4 months ago

This prevents EICrecon from picking up Findonnxruntime.cmake from Acts. The actual onnxruntime only ever provided the config, to my best knowledge.

veprbl commented 4 months ago

Ha, how did you figure out this one?

The easy way, of course. It fails my builds like so:

CMake Warning at CMakeLists.txt:205 (message):
  Acts has not been built with the requested C++ standard 20.
-- Found OnnxRuntime library at /lib/libonnxruntime.dylib
-- Found OnnxRuntime includes at /include (API version: 16)
CMake Error at /lib/cmake/Acts/Modules/Findonnxruntime.cmake:36 (string):
  string sub-command REPLACE requires at least four arguments.
Call Stack (most recent call first):
  CMakeLists.txt:225 (find_package)
CMake Warning (dev) at /share/cmake-3.27/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
  The package name passed to `find_package_handle_standard_args`
  (OnnxRuntime) does not match the name of the calling package (onnxruntime).
  This can lead to problems in calling code that expects `find_package`
  result variables (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  /lib/cmake/Acts/Modules/Findonnxruntime.cmake:43 (find_package_handle_standard_args)
  CMakeLists.txt:225 (find_package)
wdconinc commented 4 months ago

Should this be considered a bug elsewhere? Acts or spack?

veprbl commented 4 months ago

Should this be considered a bug elsewhere? Acts or spack?

It depends on your viewpoint. Perhaps, it's not very wise of Acts to ship a module that requires a _acts_onnxruntime_version variable to be set. But also, I don't expect CMake modules to work flawlessly in the first place.