Fix C++ modules BMI installation and re-enable external build tests#7206
Fix C++ modules BMI installation and re-enable external build tests#7206arpittkhandelwal wants to merge 1 commit intoTheHPXProject:masterfrom
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull request overview
This PR updates HPX’s CMake module support to rely on CMake’s native C++20 modules/BMI handling (requiring CMake ≥ 3.29) and re-enables the external dependent-project build tests when HPX_WITH_CXX_MODULES=ON.
Changes:
- Removed manual compiler flag injection for module BMI output (
-fmodule-output=...) and prebuilt module search paths (-fprebuilt-module-path=...) inHPX_CXXModules.cmake. - Re-enabled
tests/unit/buildexternal build tests for module-enabled builds by removing the earlyreturn()guard.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/unit/build/CMakeLists.txt |
Re-enables external build tests when modules are enabled. |
cmake/HPX_CXXModules.cmake |
Drops manual module BMI flag management to let modern CMake manage BMI generation/consumption. |
549eca4 to
70290e8
Compare
70290e8 to
c27ae7d
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates HPX’s CMake C++20 modules support to rely on CMake’s native module/BMI handling (CMake ≥ 3.29) and re-enables downstream external-build CI coverage when HPX_WITH_CXX_MODULES=ON.
Changes:
- Re-enabled external build tests by removing the
HPX_WITH_CXX_MODULESearly-return intests/unit/build/CMakeLists.txt. - Simplified module producer configuration to stop overriding CMake’s BMI/output handling, leaning on native
FILE_SET ... TYPE CXX_MODULES/CXX_MODULES_BMI. - Updated cmake-format command metadata to match the updated
hpx_configure_module_producersignature.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/unit/build/CMakeLists.txt | Removes the guard that skipped external build tests when modules are enabled. |
| libs/CMakeLists.txt | Updates module producer invocation to match the simplified macro API. |
| cmake/HPX_CXXModules.cmake | Removes manual module output/prebuilt-path flags and relies on CMake-native scanning/export behavior. |
| .cmake-format.py | Adjusts formatting metadata for the updated CMake function signature. |
c27ae7d to
3738eb9
Compare
3738eb9 to
7a3c5a2
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates HPX’s CMake support for C++20 modules to rely on CMake’s native FILE_SET CXX_MODULES/BMI handling (now that HPX_WITH_CXX_MODULES requires newer CMake), and re-enables external build tests that were previously disabled for modules-enabled builds.
Changes:
- Remove manual compiler flag injection for module BMI output/consumption from
HPX_CXXModules.cmakeand simplify the producer macro API. - Re-enable
tests/unit/buildexternal build tests when C++ modules are enabled, with a pkg-config build tweak to force header fallback where needed. - Make module-consumer setup more robust across build-tree vs installed-target naming by checking for
HPXInternal::targets.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/build/CMakeLists.txt |
Re-enables external build tests with modules enabled; adjusts pkg-config build flags to disable module imports for that test path. |
libs/CMakeLists.txt |
Updates module producer setup to match the simplified hpx_configure_module_producer API. |
cmake/HPX_SetupTarget.cmake |
Adds support for consuming either build-tree or HPXInternal::-namespaced module interface targets when configuring module scanning. |
cmake/HPX_CXXModules.cmake |
Removes manual -fmodule-output/-fprebuilt-module-path flag management, relying on native CMake module handling. |
.cmake-format.py |
Updates cmake-format parser config for the updated hpx_configure_module_producer signature. |
Comments suppressed due to low confidence (1)
cmake/HPX_CXXModules.cmake:91
- The doc comment for
hpx_configure_module_consumersays it "sets necessary consumer compiler flags for clang and gcc", but the function now only propagatesCXX_SCAN_FOR_MODULESand adds Clang linker flags (-fuse-ld=lld,--error-limit=0). Please update the comment to reflect the current behavior (or reintroduce the described compiler-flag propagation if still required).
# hpx_configure_module_consumer(<consumer> <producer>])
#
# * propagates module-related properties from producer interface target
# * sets necessary consumer compiler flags for clang and gcc
function(hpx_configure_module_consumer consumer producer)
|
@arpittkhandelwal The changes to the build system seem not to have broken anything - good. However, they have notfixed anything either :/ Also, I think the copilot comments bear some truths with them. |
7a3c5a2 to
7b8741f
Compare
7b8741f to
7e8f2a6
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates HPX’s CMake infrastructure for C++20 modules to rely on CMake’s native BMI/module handling, and re-enables external build tests when HPX_WITH_CXX_MODULES=ON so downstream find_package(HPX) consumers are exercised again.
Changes:
- Remove manual compiler flag injection for module BMI output/consumption and rely on CMake’s native module scanning/FILE_SET behavior.
- Re-enable external build tests for module-enabled builds and adjust pkg-config external test flags.
- Improve module-consumer setup logic to handle both in-tree and exported
HPXInternal::module interface targets.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/build/CMakeLists.txt |
Re-enables external build tests with modules and adjusts pkg-config CXX flags. |
libs/CMakeLists.txt |
Updates module producer configuration call to match the simplified producer macro. |
examples/hello_world_component/CMakeLists.txt |
Adds module-aware linking/scanning for the external hello-world example. |
cmake/HPX_SetupTarget.cmake |
Makes module consumer configuration robust to HPXInternal:: exported target names. |
cmake/HPX_CXXModules.cmake |
Removes manual BMI path flags and simplifies producer/consumer module configuration. |
.cmake-format.py |
Updates formatting metadata for the changed CMake macro signature. |
|
@arpittkhandelwal I like your solution to the issue a lot. Thanks. However, while some of the external target now build, some others do not build yet. Please have a look at #7202, which applies other changes that may be helpful in resolving the issue. Could you please tend to the cmake-format issue reported? Also, please either accept the copilot changes or mark them as being irrelevant. |
7e8f2a6 to
cf503da
Compare
|
@hkaiser sir I found the root cause of the HPX.Core not found CI failures. We had a conflict between our manual compiler flags and CMake 3.28+'s native module tracking. CMake automatically generates BMIs to CMakeFiles/.dir/ via FILE_SET CXX_MODULES, but our manual -fmodule-output flags were forcing Clang to write to a completely different directory. This caused a path mismatch when consumers tried to import HPX.Core. Fix: I completely removed the manual -fmodule-output and -fprebuilt-module-path flags from HPX_CXXModules.cmake. We now rely 100% on CMake's native FILE_SET CXX_MODULES and INTERFACE_CXX_SCAN_FOR_MODULES ON to automatically map dependencies and paths for both internal and external builds. |
|
@arpittkhandelwal Wow, this seems to have done the trick! Thank you so much! I think we can go ahead and merge this PR now. |
hkaiser
left a comment
There was a problem hiding this comment.
Otherwise, LGTM! Thanks!
2977701 to
41305ae
Compare
|
@hkaiser Fixed both I removed the redundant CMake version guard in HPX_GeneratePackage.cmake (since modules already require CMake 3.28+) and corrected the misleading comment in HPX_CollectStdHeaders.cmake to accurately describe the STANDARD_LIBRARY_HEADERS filtering. |
41305ae to
48cc927
Compare
48cc927 to
6ffc411
Compare
|
The failing ARM cross-compiling CIs point towards a missing CMake version check (that CI currently uses CMake V3.22.1). We need to update that docker image (unrelated to this PR), but should also make sure we see a proper error messages if CMake doesn't support C++ modules yet and those are enabled. For this PR here we should make sure that C++ module CMake constructs are being used only if |
6ffc411 to
fa700da
Compare
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
fa700da to
6ffc411
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
cmake/HPX_GeneratePackage.cmake:35
CXX_MODULES_DIRECTORYis only being passed to the build-treeexport()calls. The correspondinginstall(EXPORT ...)steps forHPXInternalTargetsandHPXTargetsdon't passCXX_MODULES_DIRECTORY, which can result in the installed package missing the per-target C++ module metadata directory needed for downstreamfind_package(HPX)consumption with modules enabled. Consider forwarding the same${_cxx_modules_directory_arg}to bothinstall(EXPORT ...)calls as well (guarded byHPX_WITH_CXX_MODULES).
# Export HPXInternalTargets in the build directory. Use the EXPORT signature so
# CMake also generates the per-target C++ module metadata files. Note:
# CXX_MODULES_DIRECTORY requires CMake 3.28+, which is already the minimum
# required when HPX_WITH_CXX_MODULES is enabled.
export(
TARGETS ${HPX_EXPORT_INTERNAL_TARGETS}
NAMESPACE HPXInternal::
FILE "${CMAKE_CURRENT_BINARY_DIR}/lib/cmake/${HPX_PACKAGE_NAME}/HPXInternalTargets.cmake"
CXX_MODULES_DIRECTORY cxx-modules
)
# Export HPXInternalTargets in the install directory
install(
EXPORT HPXInternalTargets
6ffc411 to
c6bd3f7
Compare
I've updated both the logic and the comments to correctly enforce and state CMake 3.28+ as the minimum. I also guarded the CXX_MODULES_DIRECTORY construct in export() so it is exclusively used when HPX_WITH_CXX_MODULES=ON. |
- Resolved Linux configuration mismatches (GNU extensions) by propagating CMAKE_CXX_EXTENSIONS and HPX_CXX_STANDARD to external tests. - Fixed Windows installation crashes by removing CXX_MODULES_DIRECTORY from install(EXPORT) and adding MSVC guards for Clang linker flags. - Improved module metadata propagation by explicitly linking HPXInternal module targets in external examples. - Added CXX_STANDARD support to hpx_setup_target and applied it to hello_world_component. - Skipped pkg-config tests when modules are enabled due to lack of BMI discovery support. - Fixed CMake quoting and logic bugs in standard header collection. - Applied project-wide cmake-format and resolved CI linting failures.
e9a7c6c to
6d869bb
Compare
Summary
This PR standardizes the export of C++ module BMI files to external projects and officially re-enables the external build tests when
HPX_WITH_CXX_MODULES=ON.Fixes #7197
Closes #7202
Problem
Previously, external build tests were disabled (#7190) because HPX lacked the CMake infrastructure to correctly export and install the Binary Module Interface (BMI) generation paths downstream. Without these paths, any external project trying to
importHPX modules structurally failed.Root Cause
CMake 3.28 natively supports C++20 module dependency tracking via the
FILE_SET CXX_MODULESfeature, generating proper module interfaces. However, the custom macroshpx_configure_module_producerandhpx_configure_module_consumerwere overriding this by manually injecting-fmodule-output=and-fprebuilt-module-path=compiler flags. This hardcoded manipulation fundamentally conflicts with modern CMake's internal module tracking and installation processes, corrupting the exported target configuration (HPXTargets.cmake).What changed
Since
HPX_WITH_CXX_MODULESnow enforcesCMAKE_VERSION >= 3.29, we can entirely rely on native functionality:-fmodule-output=flags fromHPX_CXXModules.cmake.-fprebuilt-module-path=flags fromHPX_CXXModules.cmake(safely retainedfuse-ld=lldspecifically for Clang).return()check block fromtests/unit/build/CMakeLists.txtthat bypassed downstream tests.Why this fixes the issue
The native CMake target definitions now assume full control over BMI file sets, successfully resolving paths dynamically. When an external project consumes HPX via
find_package(HPX)through the CI pipeline, CMake inherently handles the BMI dependencies.