Namespace installed headers and runtime libs to allow depthai/depthai_v3 co-installation - #1946
Namespace installed headers and runtime libs to allow depthai/depthai_v3 co-installation#1946bjsowa wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Similar changes apply to |
Purpose
The main purpose of this PR is to fix an issue which prevents installing
ros-jazzy-depthai-v3alongsideros-jazzy-depthaipackages. Currently, both packages install their headers into/opt/ros/jazzy/include/depthai, which causes a collision and prevents both packages from being installed at the same time.To avoid such collisions,
ament_cmakedocumentation recommends namespacing the installed headers under the package name, e.g.include/depthai_v3/depthaiinstead ofinclude/depthai(see ament_cmake documentation).The package also installs third-party dependencies (xtensor, xtl, libnop, XLink) via
FetchContent, which also install their headers to the same unnamespacedinclude/path and a dynamic library under a generic name (libdynamic_calibration.so) tolib/which could also collide with other variants of the same package.Specification
CMAKE_INSTALL_INCLUDEDIRis now set once, globally, toinclude/${PROJECT_NAME}near the top ofCMakeLists.txt, before dependencies are processed. This namespaces bothdepthai's own installed headers and the vendoredFetchContentdependencies (xtensor, xtl, libnop, XLink), which all install headers via the plainCMAKE_INSTALL_INCLUDEDIRvariable.includepath inside the$<INSTALL_INTERFACE:...>generator expression of their ownINTERFACE_INCLUDE_DIRECTORIEStarget property (instead of referencing${CMAKE_INSTALL_INCLUDEDIR}), so their exported CMake targets would otherwise still advertise the old, unnamespaced path to consumers. A new_depthai_namespace_install_interface_includedir()macro incmake/depthaiDependencies.cmakepatches this property in-place forxtl,xtensor,libnop, andXLinkPublicright after they're made available viaFetchContent.target_include_directories(${TARGET_CORE_NAME} ...)now references${CMAKE_INSTALL_INCLUDEDIR}directly instead of a hardcodedincludepath, staying in sync automatically.libdynamic_calibration.so(used whenDEPTHAI_DYNAMIC_CALIBRATION_SUPPORTis enabled) is now installed tolib/${PROJECT_NAME}/instead of directly underlib/.INSTALL_RPATHfor the core target is extended with$ORIGIN/${PROJECT_NAME}(and@loader_path/${PROJECT_NAME}on macOS) so it's still found at runtime. Windows DLL install location is left unchanged (must stay next to the loading binary).libdynamic_calibration.sounderlib/${PROJECT_NAME}/reintroduces a name collision it was meant to avoid, but at a different resolution layer (ld/ld.sosearch order) instead of the filesystem: on Linux, some distros (e.g. aros-*-depthaisystem package) may already ship an unrelated, incompatiblelibdynamic_calibration.soonLD_LIBRARY_PATH. Two additional fixes close this gap:${TARGET_CORE_NAME}is now linked with-Wl,--disable-new-dtags, forcing the classicDT_RPATHtag (searched beforeLD_LIBRARY_PATHat runtime) instead of the defaultDT_RUNPATH(searched after it), so the correct, namespaced copy is always preferred at runtime regardless of environment.${TARGET_CORE_NAME}also gets anINTERFACE-Wl,-rpath-link,<dir>link option (pointing at theFetchContentsource dir at build time, and atlib/${PROJECT_NAME}at install time via$<BUILD_INTERFACE:>/$<INSTALL_INTERFACE:>). Sincedynamic_calibration_importedis aPRIVATEdependency, downstream consumers (e.g.depthai_bridge) otherwise have no path hint for it, andld's link-time search order for transitiveNEEDEDsymbols checksLD_LIBRARY_PATHbefore the dependency's ownRPATH/RUNPATH(the reverse of the runtime order) - without this, consumers could fail to link with "undefined reference" errors against a wrong/olderlibdynamic_calibration.sofound viaLD_LIBRARY_PATH.lib/cmake/libnop,lib/cmake/XLink,share/cmake/xtensor,share/cmake/xtl, andshare/pkgconfig/{xtensor,xtl}.pc- are still installed unnamespaced by their owninstall(EXPORT)/install(FILES)rules. Only their header install locations and exportedINTERFACE_INCLUDE_DIRECTORIESare namespaced here (see Dependencies & Potential Impact below).Old install layout:
New install layout:
Dependencies & Potential Impact
#includestatement changes for consumers -#include <depthai/...>keeps working unchanged,lib/cmake/libnop,lib/cmake/XLink,share/cmake/xtensor,share/cmake/xtl, andshare/pkgconfig/{xtensor,xtl}.pcremain unnamespaced (see Specification) since we can't change vendored third-party install rules without patching sources fetched fresh each configure.dynamic_calibration.soruntime location; Windows DLL handling is unchanged.--disable-new-dtags/-rpath-linkfixes only apply onUNIX(Linux and macOS); Windows resolves DLLs differently (viaPATHand the runtime-dependency-copying macro already in place) and is unaffected.depthaipackage (e.g.depthai_bridge) automatically pick up the-rpath-linkfix throughdepthai::core'sINTERFACE_LINK_OPTIONS- no changes needed on their end.Deployment Plan
None / not applicable - this is a build/install-layout change only, no runtime service impact. A clean rebuild of
depthai_v3(and any package consuming it, e.g.depthai_bridge_v3,depthai_ros_driver) is recommended after pulling this change to avoid stale install artifacts from the previous unnamespaced layout.Testing & Validation
depthai_v3and confirmedinstall/depthai_v3/include/only contains the namespaceddepthai_v3/subfolder (previously also had unnamespacednop/,XLink/,xtensor/,xtensor.hpp,xtl/).depthai_v3alongsidedepthai(V2 version) in the same workspace usingcolcon build --merge-installand confirmed both packages install successfully with no collisions.depthai_v3Targets.cmake(andlibnop's/XLink's own independently-exported configs) correctly referenceinclude/depthai_v3inINTERFACE_INCLUDE_DIRECTORIES.depthai_bridge_v3anddepthai_ros_driver_v3(downstream consumers viafind_package(depthai_v3 CONFIG)) from scratch against the new layout - build and link successfully with no changes needed on the consumer side.libdynamic_calibration.soinstalls tolib/depthai_v3/andlddresolves it correctly via the extendedRPATH(no "not found" entries).AI Usage
Assisted-by: Claude Sonnet 5.0
Submitted code was reviewed by a human: YES
The author is taking the responsibility for the contribution: YES