refactor(structure): fix install export paths (WIP: layout/namespace plan)#700
Merged
Merged
Conversation
added 2 commits
May 31, 2026 01:35
The header install step referenced directories that do not exist in the
tree: interfaces/ and implementations/{thread_pool,typed_thread_pool,
lockfree}/include/. Those dangling install(DIRECTORY ...) entries left
the install/export surface inconsistent and undermined
find_package(thread_system) (issue #696).
Replace them with rules that target the real layout:
- install the canonical public API from include/ recursively, now also
matching *.hpp and *.tpp (template implementations) so headers like the
typed_thread_pool .tpp files are exported, not just .h;
- install the legacy forwarding-stub headers that still exist
(core/base/include, core/sync/include, utilities/include) flat into the
include prefix so old flat #include names keep resolving during the
EPIC #683 deprecation window, each guarded by EXISTS.
Refs #696
In-tree tests and examples link against the targets thread_pool and typed_thread_pool, but create_thread_system_targets() only aliased thread_base, utilities and interfaces to the unified thread_system library. A clean configure with tests enabled therefore failed with "target thread_pool not found". Add additive ALIAS targets for thread_pool and typed_thread_pool so the in-tree target graph matches the installed export, which already exposes thread_system::thread_pool and thread_system::typed_thread_pool through thread_system-config.cmake.in. Refs #696
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.
Closes #696
Status: DRAFT — build NOT verified locally
The ecosystem currently has a vcpkg-hash infrastructure failure (common_system v0.2.0)
that breaks local and CI C++ builds, and no CMake toolchain was available in the working
environment, so the changes here are CMake configure-graph fixes that could not be
compile- or configure-verified. This PR is opened as a draft for human review and to land
the audit-blocking install/export consistency fixes; the larger layout and namespace work
is described as a concrete plan below.
Root cause found
Two concrete, audit-blocking inconsistencies in the build/install configuration, both in
the "broken install paths" family from #696:
cmake/thread_system_install.cmakereferenced non-existent directories.install_thread_system_headers()installed frominterfaces/andimplementations/{thread_pool,typed_thread_pool,lockfree}/include/, none of which existin the tree. The canonical public API actually lives entirely under
include/kcenon/thread/...(129 headers), with thin forwarding-stub headers undercore/base/include/,core/sync/include/, andutilities/include/(verified to bedeprecation shims that
#include <kcenon/thread/...>, per the EPIC [EPIC] thread_system: absorb legacy directories into standard layout #683 window). Thedangling
install(DIRECTORY ...)entries corrupted the install/export surface thatfind_package(thread_system)depends on.cmake/thread_system_targets.cmakewas missing two alias targets. In-tree tests andexamples link against
thread_poolandtyped_thread_pool, butcreate_thread_system_targets()only aliasedthread_base,utilities, andinterfacesto the unifiedthread_systemlibrary. A clean configure with testsenabled fails with
target "thread_pool" not found. The installed package alreadypromises
thread_system::thread_pool/thread_system::typed_thread_poolviathread_system-config.cmake.in, so the in-tree graph was out of sync with the exportedone.
Done (in this PR)
cmake/thread_system_install.cmake— remove the danglinginterfaces/andimplementations/.../include/install rules; install the real public API frominclude/recursively (now also matching*.hppand*.tpp, so templateimplementations such as the typed_thread_pool
.tppfiles are exported, not only.h);install the still-present legacy forwarding stubs
(
core/base/include,core/sync/include,utilities/include) flat into the includeprefix so old flat
#include "thread_base.h"style usage keeps resolving during theEPIC [EPIC] thread_system: absorb legacy directories into standard layout #683 window, each guarded by
EXISTS.cmake/thread_system_targets.cmake— add additivethread_poolandtyped_thread_poolALIAS targets tothread_system, so the in-tree target graph matchesthe installed export.
Both changes were checked for CMake syntax (balanced
function/foreach/if, balancedparentheses, no duplicated directives) but not compile/configure-verified (see Status).
Plan (deferred — needs build verification)
A. Stale build-only include dirs (same broken-path family)
~13 files under
tests/unit/*andexamples/*still add${CMAKE_CURRENT_SOURCE_DIR}/../../implementations/<x>/includetotarget_include_directories(...). These point at directories that no longer exist. Acompiler silently ignores a missing
-Ipath, so they do not break the build, but they aredead/misleading and should be removed once a configure run confirms each test/example still
finds its headers via the
thread_systemtarget'sinclude/interface.Files:
tests/unit/{thread_pool_test,typed_thread_pool_test,core_test,interfaces_test,thread_base_test,platform_test,diagnostics_test}/CMakeLists.txt,examples/{composition_example,service_registry_sample,integration_example,multi_process_monitoring_integration}/CMakeLists.txt.B. Layout unification (Problem 2: src/ vs top-level core/ + utilities/)
The tree mixes three source roots:
include/kcenon/thread/...(canonical headers),src/...(compiled.cpp, globbed bycreate_thread_system_targets()), and the legacystub roots
core/{base,sync}/include+utilities/include. Pick one convention and applyrepo-wide:
include/+src/as the single source of truth, then delete thecore/andutilities/stub trees (and the install rules added here) at the end of theEPIC [EPIC] thread_system: absorb legacy directories into standard layout #683 deprecation window; or
core/+utilities/are meant to be first-class, move the canonical headers thereand update the
file(GLOB_RECURSE)roots,target_include_directories, andinstall(DIRECTORY ...)accordingly.Either path requires a configure+build to confirm no header is dropped.
C. Namespace migration to kcenon::thread (Problem 3)
This appears largely complete already:
kcenon::threadis used in ~177 locations and thereare currently 0 uses of the legacy
utility_module::/thread_module::/thread_pool_module::namespaces ininclude/,src/,core/,utilities/. Remainingwork: (1) confirm no legacy namespace aliases remain in headers, (2) decide whether to drop
the legacy CMake target aliases (
thread_base,thread_pool,typed_thread_pool,utilities,interfaces) and forwarding-stub headers at the end of the deprecationwindow, and (3) align the install export NAMESPACE — the export currently uses
thread_system::while the C++ namespace and the module target usekcenon::. Unifyingthese is additive-then-removable but should be its own verified PR.
Risks
environment). Reasoning is from the configure graph and the on-disk layout only.
expected them under a subdir, the destination may need adjustment (none observed in-tree).
in this PR.