Conversation
HPTT hardcoded `EXPORT hpttTargets` in its install(TARGETS) call. CMake
refuses to generate an export whose link interface names a target that is
not itself installed and exported, so a parent project that embeds HPTT
via add_subdirectory(), links hptt_static into a library of its own, and
then exports that library hits
install(EXPORT "parentTargets" ...) includes target "parentlib" which
requires target "hptt_static" that is not in any export set.
with no way to resolve it from HPTT's side. The workaround is for the
parent to set HPTT_INSTALL=OFF and write its own
`install(TARGETS hptt_static EXPORT <its own set>)`, which duplicates
HPTT's install rule in the consumer, hardcodes the un-namespaced target
name, and has to be revisited whenever HPTT's artifact set changes.
Add HPTT_EXPORT_SET (default `hpttTargets`, so standalone installs are
unchanged) and pass it to install(TARGETS). Emit hpttTargets.cmake and
hpttConfig.cmake only when HPTT still owns its export set: once the
targets belong to a parent's set they ship as part of the parent's
package, and a second package here would re-declare targets the parent
already exports.
Verified on Windows: a parent that links hptt_static PUBLIC into a static
library and exports it now configures, builds and installs; a downstream
project consuming that parent through find_package resolves
Parent::hptt_static transitively and its transposes match the reference.
The installed tree contains no lib/cmake/hptt. The default configuration
and HPTT_INSTALL=OFF both behave exactly as before.
Co-Authored-By: Claude <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: beb2310377
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| EXPORT ${HPTT_EXPORT_SET} | ||
| ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
| LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
| RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}) |
There was a problem hiding this comment.
Preserve component membership for embedded installs
When a parent installs its export set as a named component (for example, Development), cmake --install --component Development installs the parent export file but omits HPTT's libraries and headers because these rules remain in the default Unspecified component. The resulting package fails during find_package(parent) because its imported Parent::hptt_static references a missing library. This follows the install command documentation: component-specific installation executes only rules associated with that component, while rules without COMPONENT belong to Unspecified; expose a component setting and apply it to both the target and header rules.
Useful? React with 👍 / 👎.
| # installs those targets as part of its own package, and generating a | ||
| # second package here would ship an hpttTargets.cmake that re-declares | ||
| # targets the parent already exports. | ||
| if(HPTT_EXPORT_SET STREQUAL "hpttTargets") |
There was a problem hiding this comment.
Distinguish export-set ownership from its name
When a parent project's own export set is also named hpttTargets, this value-based ownership check incorrectly treats it as HPTT's standalone set. HPTT consequently installs a second package whose hpttTargets.cmake contains every target the parent added to that set under the hptt:: namespace; that leaks parent targets and can make find_package(hptt) fail on parent-specific dependencies that hpttConfig.cmake does not discover. Track standalone ownership independently of the export-set string rather than reserving an otherwise valid parent set name implicitly.
Useful? React with 👍 / 👎.
Based on #5 — targets
msvc-build, notmain. Independent of #6; the two only share a base.What actually fails, and what doesn't
HPTT_INSTALL=ON(default), parentinstall(EXPORT parentTargets)HPTT_INSTALL=OFF, STATIC parent,PUBLICorPRIVATElinkHPTT_INSTALL=OFF, STATIC parent,PUBLIC $<BUILD_INTERFACE:...>HPTT_INSTALL=OFF, SHARED parent,PRIVATElinkHPTT_INSTALL=OFF, SHARED parent,PUBLIClinkThe
HPTT_INSTALL=OFFfailures are not really bugs: a static library that links a static dependency genuinely needs that dependency at the consumer's link step, so it has to be installed.HPTT_INSTALL=OFF+ export is a contradiction, and this PR does not change that.So what is this for
Bundling. Taking the working default path, a parent that links
hptt_staticand exports its own target gets an install tree containing bothlib/cmake/hpttandlib/cmake/parent, and itsparentTargets.cmakereferenceshptt::hptt_static— so the parent's own Config file mustfind_dependency(hptt), and consumers see HPTT as a separate package. I verified that a consumer fails without thatfind_dependency.Cytnx explicitly does not want that shape. From
CytnxBKNDCMakeLists.cmake:202-209:To get that today they set
HPTT_INSTALL=OFFand hand-writeinstall(TARGETS hptt_static EXPORT cytnx_targets ARCHIVE DESTINATION ...). That works, but it reimplements HPTT's install rule in the consumer, hardcodes the un-namespaced target namehptt_static, and pins the artifact kind toARCHIVE— enabling the shared library would silently not be covered.HPTT_EXPORT_SETgives that same outcome as a supported option: HPTT'sinstall(TARGETS)joins the parent's set, and HPTT skips emittinghpttTargets.cmake/hpttConfig.cmakebecause those targets now ship inside the parent's package.Why not just pick a common conventional export-set name instead
Because a name alone cannot merge two sets, and when it does merge them the result is wrong. I tested it.
Export sets are identified by their exact name within a project, so renaming
hpttTargetsto anything else is still a different set from the parent's — nothing changes. The only way a name helps is if the parent adopts HPTT's name verbatim. That does merge the sets, and it configures and installs cleanly, but bothinstall(EXPORT)calls then emit the entire merged set under their own namespace and filename:HPTT's package ends up exporting the parent's library as
hptt::parentlib. Every target is declared twice under two namespaces. It also inverts control: the parent must bend to whatever name HPTT hardcodes, and HPTT still ships a competing package.HPTT_EXPORT_SETis the same merge with the direction reversed — the parent names the set, owns the namespace and the output file, and HPTT stands down from emitting its own package. That last part is what prevents the cross-contamination above.Verification (Windows, cl 19.44 + Ninja)
hptt_staticPUBLICinto a static library and exporting it: configures, builds, installs.find_package(parent CONFIG)+Parent::parentlibresolvesParent::hptt_statictransitively, links, and its transposes match the reference exactly.lib/cmake/parentand nolib/cmake/hptt.hpttConfig.cmake+hpttTargets.cmake, andfind_package(hptt CONFIG)still works.HPTT_INSTALL=OFFstill configures, so Cytnx's current workaround keeps working unchanged. This is an option, not a migration.Linux and macOS are not built here; the change is pure CMake with no platform-conditional logic.
Reasonable to close if you would rather Cytnx keep its explicit
install(TARGETS ... EXPORT cytnx_targets), or ship HPTT as a separate installed package alongside. Nothing here is required for the MSVC work in #5.