Repository navigation
build: make cloudini_lib embeddable with add_subdirectory / FetchContent - #142
Merged
Merged
Conversation
Embedded in another project, cloudini_lib leaked into its parent. Each item was
reproduced with the new consumer project in test/subproject (now run in CI):
- find_package(zstd CONFIG) clashed with zstd targets the parent had already
defined ('some (but not all) targets in this export set were already
defined'). zstd and lz4 lookups now reuse existing targets.
- In a ROS parent, the inherited ament_cmake result turned the embedded library
into a SHARED ament package with its own ament_package().
- CMAKE_BUILD_TYPE=Release was forced into the parent's cache; include(CTest)
defined BUILD_TESTING there; tests, tools, benchmarks, PCL and install rules
were on by default; data_path.hpp was written to the parent's
CMAKE_BINARY_DIR/include.
- No cloudini::cloudini_lib target existed in-tree, only after install.
When not top-level, only the static library is built. New options
CLOUDINI_WITH_PCL and CLOUDINI_INSTALL (default: ON when top-level), and the
cloudini::cloudini_lib alias is always defined. Top-level behaviour is
unchanged (standalone, installed package and ament builds verified).
Also: cloudini_lib uses std::thread but did not link Threads::Threads, so a
static consumer failed with 'undefined reference to pthread_create' on older
glibc (conda sysroot). Linked PUBLIC and declared in the package config.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01711BZcEtp3KsoF9uZHNFME
CMake project (was still 1.2.4), both package.xml files, the conda recipe and the changelogs. The recipe's sha256 can only be refreshed once the 1.3.1 tag exists; it is marked as stale. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01711BZcEtp3KsoF9uZHNFME
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes
cloudini_libsafe to embed withadd_subdirectory()/FetchContent. Found while embedding it in pj_bridge, which currently needs ~45 lines of workarounds; against this branch that block shrinks to:(verified locally: pj_bridge builds against this branch with exactly that, 303 tests pass, static link, nothing of Cloudini installed, parent build type untouched).
What leaked into the parent, and the fix
Each item was reproduced first with the new consumer project
cloudini_lib/test/subproject, which embeds the library like a parent would and fails at configure time if something leaks. It now runs in the Ubuntu CI job.find_package(zstd CONFIG)fails with "Some (but not all) targets in this export set were already defined" when the parent already definedzstd::libzstd_shared(e.g. from a Find module)ament_cmake_FOUNDturned the embedded library into a SHARED ament package with its ownament_package()CMAKE_BUILD_TYPE=Releaseforced into the parent's cache;include(CTest)definedBUILD_TESTINGthereCLOUDINI_WITH_PCLandCLOUDINI_INSTALLdata_path.hppwritten to the parent's${CMAKE_BINARY_DIR}/include, colliding with a parent file of the same name${CMAKE_CURRENT_BINARY_DIR}/includecloudini::cloudini_libonly existed after installTop-level detection uses
CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIRbecausePROJECT_IS_TOP_LEVELneeds CMake 3.21 and the project supports 3.16.Also fixed: missing Threads dependency
PointcloudEncoderusesstd::thread, butcloudini_libdid not linkThreads::Threads. A static consumer on an older glibc (conda sysroot) failed withundefined reference to pthread_create— the new consumer test caught this in a RoboStack environment. Now linked PUBLIC, withfind_dependency(Threads)in the package config andament_export_dependencies(... Threads).Top-level behaviour is unchanged — verified
cmake --installproduces the same package files, and afind_package(cloudini_lib)consumer links against the installed static librarylibcloudini_lib.soand exports its targetsVersion 1.3.1
The version is set to 1.3.1 everywhere it is declared:
project(cloudini_lib VERSION ...)(it was still 1.2.4 in the 1.3.0 release, sofind_package(cloudini_lib 1.3)failed against a 1.3.0 install), bothpackage.xmlfiles,conda/recipe.yaml, and a 1.3.1 section in both changelogs. This branch also contains a merge ofmain(#141).After tagging
1.3.1: refreshsha256inconda/recipe.yaml— it is still the 1.2.4 tarball's hash and is marked STALE (CI is unaffected, it builds the recipe from the checkout).cloudini_foxglove/package.json(1.2.2) is versioned separately and was left alone.Not changed
The ament install still exports
cloudini_lib::cloudini_libwhile the standalone install exportscloudini::cloudini_lib; unifying them would breakcloudini_ros, so it is not part of this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01711BZcEtp3KsoF9uZHNFME