fix(profiler-hub): resolve export-set, and schema-API build failures#8610
Merged
dgaliffiAMD merged 12 commits intoJul 21, 2026
Conversation
install(EXPORT) failed because CMake propagates a STATIC library's PRIVATE link dependencies into its exported INTERFACE_LINK_LIBRARIES, and the existing $<BUILD_INTERFACE:fmt::fmt> guard did not stop CMake's export validation from resolving the vendored fmt alias target, which is never installed. Link the real dependencies plainly and explicitly override profiler-hub-static's INTERFACE_LINK_LIBRARIES after the fact: only the real find_package()-imported fmt::fmt is kept for install, everything else (vendored fmt, spdlog, the SQL schema helpers) is dropped since it is already baked into the static archive.
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
…port interface Only fmt was special-cased in the previous fix, so spdlog, nlohmann_json, and rocprofiler-sdk-rocpd were unconditionally dropped from profiler-hub-static's exported INTERFACE_LINK_LIBRARIES even when they are real, find_package()-resolvable targets a consumer could link against. Gate each on its own *_FOUND flag instead of hardcoding fmt as the sole exception.
adjordje-amd
added a commit
that referenced
this pull request
Jul 15, 2026
…rify CI Temporarily set ROCPROFSYS_PROFILER_HUB_GIT_TAG to users/adjordje-amd/profiler-hub-fix-fmt-export (PR #8610, issue #8609) so CI's tier-2 ExternalProject_Add fallback picks up the fmt export fix instead of develop, which still has the bug. This is verification-only. Must be reverted back to "develop" before this PR merges, once #8610 lands on develop.
…ckage find_package(nlohmann_json QUIET) had no version floor, so on RHEL 8 CI images it silently accepted the EPEL json-devel-3.6.1 package. That release predates json_fwd.hpp (added upstream in 3.9.0) and its install() rule never ships that header, so json_serializers.hpp's `#include <nlohmann/json_fwd.hpp>` fails to compile. Pin the same minimum version used by the FetchContent fallback, matching the existing fmt.cmake pattern, so incompatible system packages are rejected and the fallback fetch kicks in instead.
…ORT) install(EXPORT) validates a SHARED target's raw PRIVATE link libraries directly (not just INTERFACE_LINK_LIBRARIES, which stays unpopulated for private deps of a shared library), so $<BUILD_INTERFACE:> or an INTERFACE_LINK_LIBRARIES override applied only to profiler-hub-static never stopped profiler-hub (SHARED) from failing the same "fmt not in any export set" check when fmt was vendored via FetchContent. Reproduced only when profiler-hub is pulled in via add_subdirectory() alongside a full rocprofiler-systems configure with no system fmt >= 11.2.0 available (e.g. Debian 12 CI); a standalone profiler-hub configure never exposed this because system fmt happened to satisfy the version floor there. Link both profiler-hub and profiler-hub-static against $<BUILD_INTERFACE:profiler-hub-objects> instead of re-declaring fmt/spdlog/nlohmann_json/rocprofiler-sdk-rocpd PRIVATE on each target; the object library already carries those as PRIVATE requirements, so this satisfies the real build-time link while keeping every in-tree-only or conditionally-vendored dependency out of both targets' exported requirements. Restore profiler-hub-static's INTERFACE_LINK_LIBRARIES override for the find_package()-resolvable case so consumers of the installed static package still get real transitive dependencies.
…_LIBS rocprofiler-systems sets BUILD_SHARED_LIBS ON project-wide and add_subdirectory()s profiler-hub, so the CMake cache variable leaked into profiler-hub's FetchContent'd fmt, producing a SHARED libfmt.so.11. FMT_INSTALL is OFF, so nothing installs it, and no consumer install() rule captures it either, leaving rocprof-sys-instrument unable to resolve libfmt.so.11 at runtime after install. Back up and force BUILD_SHARED_LIBS OFF around FetchContent_MakeAvailable(fmt), matching the existing pattern used for spdlog, so fmt is always statically linked into profiler-hub irrespective of the embedding project's shared-lib default.
Some super-project build orchestrators (e.g. TheRock) intercept find_package() calls and reject any package not explicitly declared as a dependency of this subproject. TheRock has never declared nlohmann_json for rocprofiler-systems since profiler-hub is a brand-new transitive dependency it doesn't yet know about, causing a hard CMake configure error. Vendoring unconditionally avoids the dependency-declaration requirement entirely, matching the pattern used here for fmt and spdlog when no system package satisfies the version floor.
…target IMPORTED-ness, not _FOUND TheRock builds rocprofiler-systems as a subproject and redirects find_package() to its own super-project packages via a global CMake dependency provider. When profiler-hub's version-pinned find_package(fmt 11.2.0) doesn't match TheRock's pinned fmt, it falls back to FetchContent vendoring (in-tree, non-IMPORTED "fmt" target aliased as fmt::fmt), setting fmt_FOUND=FALSE. Later in the same directory scope, an unrelated, unversioned find_package(fmt) elsewhere succeeds against TheRock's real package and flips fmt_FOUND back to TRUE - but fmt::fmt still refers to the earlier vendored alias, since exported package configs guard target creation with if(NOT TARGET fmt::fmt). The _FOUND-gated logic then added $<INSTALL_INTERFACE:fmt::fmt> to profiler-hub-static's link interface, and install(EXPORT) correctly rejected it: the underlying "fmt" target is not IMPORTED and not in any export set. Replace the _FOUND checks with a check of the actual fmt/spdlog/nlohmann_json target's IMPORTED property (resolving through ALIASED_TARGET first), which reflects what will actually be referenced regardless of how many times find_package() ran or in what order.
rocprofiler-sdk-rocpd PR #5267 added a schema_version parameter to rocpd_sql_load_schema (and its callback typedef) between options and variables, growing the signature from 8 to 9 arguments. TheRock's CI provides this newer rocprofiler-sdk-rocpd, causing a build failure that only surfaces there since USE_SCHEMA_FROM_ROCPROFILER_SDK_ROCPD requires a real find_package hit, which no other tested environment has. Pass rocpd_version_triplet_t{0, 0, 0}, the documented sentinel for "use latest available schema" (see sql.cpp), matching the pattern rocprofiler-systems itself uses for the same API. Update load_schema_cb to match the callback typedef's new parameter.
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
adjordje-amd
changed the title
adjordje-amd
changed the title
…deps profiler-hub-static's exported INTERFACE_LINK_LIBRARIES can reference spdlog::spdlog and rocprofiler-sdk-rocpd::rocprofiler-sdk-rocpd (when real, IMPORTED targets), but profiler-hub-config.cmake.in only ever emitted find_dependency(fmt). A consumer's find_package(profiler-hub) + link against profiler-hub-static failed at generate time whenever those were system packages. Generalize the fmt-only find_dependency logic into a loop shared with the INTERFACE_LINK_LIBRARIES computation, and drop nlohmann_json from that interface entirely: it is a PRIVATE, header-only dependency never exposed in the installed public headers, so it needs neither a link entry nor a find_dependency() call.
…ma ABIs Exercises the version-conditional rocpd_sql_load_schema() call added in sqlite_backend_impl.hpp, confirming get_schema_query() still returns a non-empty schema string regardless of which rocprofiler-sdk-rocpd ABI (or the bundled schema fallback) is active in the build.
dgaliffiAMD
approved these changes
Jul 20, 2026
anujshuk-amd
pushed a commit
that referenced
this pull request
Jul 21, 2026
…8610) ## Motivation Started as a single fmt export-set bug (fixes #8609), found while building rocprofiler-systems PR #8607, the first change to drive an in-tree `add_subdirectory(profilers/profiler-hub)` build without a pre-installed profiler-hub package on the search path. Each subsequent verification pass (debian/rhel CI, TheRock's superbuild) exposed further build-portability bugs in profiler-hub's own CMake and one C++ API call site, all fixed here as they were found. ## Technical Details - `install(EXPORT "profiler-hub" ...)` rejected `profiler-hub-static`'s vendored fmt: CMake copies a STATIC target's PRIVATE deps into its own `INTERFACE_LINK_LIBRARIES`; fixed by explicitly overriding that property post-link to keep only genuinely `find_package()`-imported deps. - nlohmann_json's `find_package()` had no minimum version, silently accepting an ancient RHEL 8.10 EPEL package missing a header added upstream in a later release; then switched to always vendoring it via FetchContent instead of `find_package()` at all, since TheRock (ROCm's superbuild) rejects any `find_package()` call for a dependency not pre-declared for this subproject. - fmt built SHARED instead of its usual static default: `rocprofiler-systems` sets `BUILD_SHARED_LIBS ON` project-wide and `add_subdirectory()`s profiler-hub, leaking that cache variable into profiler-hub's FetchContent'd fmt with no guard; fixed by forcing `BUILD_SHARED_LIBS OFF` around the fetch, matching the existing pattern already used for spdlog. - `fmt`/`spdlog`/`nlohmann_json`'s install-interface logic was keyed off each package's `_FOUND` variable, which can flip true later in the same directory scope (e.g. TheRock redirects an unrelated, unversioned `find_package(fmt)` elsewhere to its own real package) without recreating the target if one already exists under that name - so a previously-vendored, non-`IMPORTED` target could get incorrectly exported. Fixed by checking the target's actual `IMPORTED` property instead. - `rocpd_sql_load_schema` call site was written against an older API: rocprofiler-sdk-rocpd PR #5267 added a `schema_version` parameter, growing the signature from 8 to 9 arguments. This path only compiles when `rocprofiler-sdk-rocpd` is genuinely found, which only happens in TheRock's build. Fixed by passing the documented "use latest schema" sentinel, matching the pattern rocprofiler-systems itself already uses for the same API. ## Test Plan Each fix verified against the real `dgaliffiamd/rocprofiler-systems:ci-rocm-6.4-debian-12` CI container (not host approximation) covering both the vendored-dependency and real-system-package paths, plus real GitHub Actions CI runs on PR #8607 (which pulls this branch) for the TheRock-specific and rocprofiler-sdk-rocpd-specific paths that can't be reproduced locally. ## Test Result - Vendored-fmt and real-system-fmt configure/build/`cmake --install` both succeed cleanly; installed `profiler-hub-targets.cmake` has no stray entries. - `ldd` on installed binaries resolves all dependencies with no "not found" entries. - Unit suite passes in every verified context. - TheRock's build now passes the CMake configure stage that previously failed on both the fmt export-set and nlohmann_json declaration issues. ## Checklist - [x] Scoped only to `profilers/profiler-hub/` - [x] Pre-commit hooks pass - [x] Tests pass in all verified contexts - [x] No unrelated changes
adjordje-amd
added a commit
that referenced
this pull request
Jul 21, 2026
…mit, not develop ROCPROFSYS_PROFILER_HUB_GIT_TAG tracked the develop branch, so the fallback sparse checkout silently picked up whatever was newest on develop instead of a known-good, reviewed state. Pin it to the merge commit for profiler-hub PR #8610, which fixed profiler-hub's CMake export-set and schema-API build failures. git clone --branch only accepts branch/tag names, not raw commit SHAs, so the fallback checkout is rewritten to git init + remote add + git fetch <ref> + checkout FETCH_HEAD, which works uniformly for branches, tags, or commits. The --filter=blob:none --depth 1 fetch filter and sparse-checkout of profilers/profiler-hub are unchanged, so the checkout stays as narrow as before.
cmcknigh
pushed a commit
that referenced
this pull request
Jul 22, 2026
…8610) ## Motivation Started as a single fmt export-set bug (fixes #8609), found while building rocprofiler-systems PR #8607, the first change to drive an in-tree `add_subdirectory(profilers/profiler-hub)` build without a pre-installed profiler-hub package on the search path. Each subsequent verification pass (debian/rhel CI, TheRock's superbuild) exposed further build-portability bugs in profiler-hub's own CMake and one C++ API call site, all fixed here as they were found. ## Technical Details - `install(EXPORT "profiler-hub" ...)` rejected `profiler-hub-static`'s vendored fmt: CMake copies a STATIC target's PRIVATE deps into its own `INTERFACE_LINK_LIBRARIES`; fixed by explicitly overriding that property post-link to keep only genuinely `find_package()`-imported deps. - nlohmann_json's `find_package()` had no minimum version, silently accepting an ancient RHEL 8.10 EPEL package missing a header added upstream in a later release; then switched to always vendoring it via FetchContent instead of `find_package()` at all, since TheRock (ROCm's superbuild) rejects any `find_package()` call for a dependency not pre-declared for this subproject. - fmt built SHARED instead of its usual static default: `rocprofiler-systems` sets `BUILD_SHARED_LIBS ON` project-wide and `add_subdirectory()`s profiler-hub, leaking that cache variable into profiler-hub's FetchContent'd fmt with no guard; fixed by forcing `BUILD_SHARED_LIBS OFF` around the fetch, matching the existing pattern already used for spdlog. - `fmt`/`spdlog`/`nlohmann_json`'s install-interface logic was keyed off each package's `_FOUND` variable, which can flip true later in the same directory scope (e.g. TheRock redirects an unrelated, unversioned `find_package(fmt)` elsewhere to its own real package) without recreating the target if one already exists under that name - so a previously-vendored, non-`IMPORTED` target could get incorrectly exported. Fixed by checking the target's actual `IMPORTED` property instead. - `rocpd_sql_load_schema` call site was written against an older API: rocprofiler-sdk-rocpd PR #5267 added a `schema_version` parameter, growing the signature from 8 to 9 arguments. This path only compiles when `rocprofiler-sdk-rocpd` is genuinely found, which only happens in TheRock's build. Fixed by passing the documented "use latest schema" sentinel, matching the pattern rocprofiler-systems itself already uses for the same API. ## Test Plan Each fix verified against the real `dgaliffiamd/rocprofiler-systems:ci-rocm-6.4-debian-12` CI container (not host approximation) covering both the vendored-dependency and real-system-package paths, plus real GitHub Actions CI runs on PR #8607 (which pulls this branch) for the TheRock-specific and rocprofiler-sdk-rocpd-specific paths that can't be reproduced locally. ## Test Result - Vendored-fmt and real-system-fmt configure/build/`cmake --install` both succeed cleanly; installed `profiler-hub-targets.cmake` has no stray entries. - `ldd` on installed binaries resolves all dependencies with no "not found" entries. - Unit suite passes in every verified context. - TheRock's build now passes the CMake configure stage that previously failed on both the fmt export-set and nlohmann_json declaration issues. ## Checklist - [x] Scoped only to `profilers/profiler-hub/` - [x] Pre-commit hooks pass - [x] Tests pass in all verified contexts - [x] No unrelated changes
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
3 participants
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.
Motivation
Started as a single fmt export-set bug (fixes #8609), found while building rocprofiler-systems PR #8607, the first change to drive an in-tree
add_subdirectory(profilers/profiler-hub)build without a pre-installed profiler-hub package on the search path. Each subsequent verification pass (debian/rhel CI, TheRock's superbuild) exposed further build-portability bugs in profiler-hub's own CMake and one C++ API call site, all fixed here as they were found.Technical Details
install(EXPORT "profiler-hub" ...)rejectedprofiler-hub-static's vendored fmt: CMake copies a STATIC target's PRIVATE deps into its ownINTERFACE_LINK_LIBRARIES; fixed by explicitly overriding that property post-link to keep only genuinelyfind_package()-imported deps.find_package()had no minimum version, silently accepting an ancient RHEL 8.10 EPEL package missing a header added upstream in a later release; then switched to always vendoring it via FetchContent instead offind_package()at all, since TheRock (ROCm's superbuild) rejects anyfind_package()call for a dependency not pre-declared for this subproject.rocprofiler-systemssetsBUILD_SHARED_LIBS ONproject-wide andadd_subdirectory()s profiler-hub, leaking that cache variable into profiler-hub's FetchContent'd fmt with no guard; fixed by forcingBUILD_SHARED_LIBS OFFaround the fetch, matching the existing pattern already used for spdlog.fmt/spdlog/nlohmann_json's install-interface logic was keyed off each package's_FOUNDvariable, which can flip true later in the same directory scope (e.g. TheRock redirects an unrelated, unversionedfind_package(fmt)elsewhere to its own real package) without recreating the target if one already exists under that name - so a previously-vendored, non-IMPORTEDtarget could get incorrectly exported. Fixed by checking the target's actualIMPORTEDproperty instead.rocpd_sql_load_schemacall site was written against an older API: rocprofiler-sdk-rocpd PR [rocprofiler-sdk] [rocpd] Update rocprofiler-sdk-rocpd api to support versioning #5267 added aschema_versionparameter, growing the signature from 8 to 9 arguments. This path only compiles whenrocprofiler-sdk-rocpdis genuinely found, which only happens in TheRock's build. Fixed by passing the documented "use latest schema" sentinel, matching the pattern rocprofiler-systems itself already uses for the same API.Test Plan
Each fix verified against the real
dgaliffiamd/rocprofiler-systems:ci-rocm-6.4-debian-12CI container (not host approximation) covering both the vendored-dependency and real-system-package paths, plus real GitHub Actions CI runs on PR #8607 (which pulls this branch) for the TheRock-specific and rocprofiler-sdk-rocpd-specific paths that can't be reproduced locally.Test Result
cmake --installboth succeed cleanly; installedprofiler-hub-targets.cmakehas no stray entries.lddon installed binaries resolves all dependencies with no "not found" entries.Checklist
profilers/profiler-hub/