eclipse-zenoh / zenoh-c

C API for Zenoh
http://zenoh.io
Other
82 stars 57 forks source link

CMake: Fix relative paths in generated PackageConfig #788

Closed alerei closed 2 weeks ago

alerei commented 1 month ago

Building and installing zenoh-c with

-DCMAKE_INSTALL_PREFIX=/usr/local -DCMAKE_INSTALL_INCLUDEDIR=include/x86_64-linux-gnu -DCMAKE_INSTALL_LIBDIR=lib/x86_64-linux-gnu -DCMAKE_INSTALL_BINDIR=bin/x86_64-linux-gnu

leads to the following error when building a dependent project that uses find_package(zenohc 1.0.0 QUIET GLOBAL):

CMake Error in CMakeLists.txt:
  Imported target "__zenohc_shared" includes non-existent path

    "/usr/local/lib/include/x86_64-linux-gnu"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.

~Calculating absolute paths from _IMPORT_PREFIX does not work as intended in that case as it makes assumptions about the relative location of zenohcConfig.cmake.~

~I propose getting rid of _IMPORT_PREFIX entirely and rely on CMAKE_INSTALL_FULL_<dir> instead as described in https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html~

~Note that this requires introducing a new variable named ZENOHC_INSTALL_FULL_DYLIBDIR to account for the differences in the Win32 and Linux build.~

github-actions[bot] commented 1 month ago

PR missing one of the required labels: {'enhancement', 'internal', 'new feature', 'documentation', 'bug', 'breaking-change', 'dependencies'}

github-actions[bot] commented 1 month ago

PR missing one of the required labels: {'dependencies', 'internal', 'new feature', 'breaking-change', 'bug', 'enhancement', 'documentation'}

DenisBiryukov91 commented 1 month ago

@alerei As far as I understand, this would make package non-relocatable, i.e it will no longer be possible to cross-compile it on host and install to target's sys root, since absolute paths will be different for host and target.

alerei commented 1 month ago

@DenisBiryukov91 Right, thanks for pointing that out as a use-case. I'll come up with another solution to the original issue and update this PR.

alerei commented 2 weeks ago

Revisiting the CMake docs the correct approach seems to be setting PATH_VARS in configure_package_config_file. As a result, paths are built using CMake's internal PACKAGE_PREFIX_DIR, which corresponds to what _IMPORT_PREFIX was before.

This should build the correct relative paths, keeping the installed files relocatable.

DenisBiryukov91 commented 2 weeks ago

@alerei LGTM, could you sign eclipse ECA to make corresponding ci check pass ?

alerei commented 2 weeks ago

@DenisBiryukov91 Done