Skip to content

Re-enable external build tests with C++ modules#7202

Open
LSUDOKO wants to merge 16 commits intoTheHPXProject:masterfrom
LSUDOKO:fix/7197-reenable-modules-external-build-tests
Open

Re-enable external build tests with C++ modules#7202
LSUDOKO wants to merge 16 commits intoTheHPXProject:masterfrom
LSUDOKO:fix/7197-reenable-modules-external-build-tests

Conversation

@LSUDOKO
Copy link
Copy Markdown
Contributor

@LSUDOKO LSUDOKO commented Apr 19, 2026

Summary

This PR re-enables the external build tests when HPX_WITH_CXX_MODULES=ON.

Closes #7197.

Problem

As part of #7190, the external build tests in tests/unit/build were disabled whenever C++ modules were enabled:

if(HPX_WITH_CXX_MODULES)
  return()
endif()

That prevented CI from checking whether downstream projects depending on HPX can be built with modules enabled.

What changed

  • removed the temporary HPX_WITH_CXX_MODULES early return from tests/unit/build/CMakeLists.txt

Why this fixes the issue

The external test project already contains module-aware setup:

  • it checks HPX_WITH_CXX_MODULES
  • it enables CMAKE_CXX_SCAN_FOR_MODULES when needed

The Linux C++ modules workflow already builds the top-level tests target, and tests.unit.build is part of tests.unit on non-Windows builds. Removing the
temporary guard therefore re-enables the intended CI coverage for external dependent-project builds with modules enabled.

@LSUDOKO LSUDOKO requested a review from hkaiser as a code owner April 19, 2026 08:48
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 19, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric Results
Duplication 0

View in Codacy

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.

@StellarBot
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Apr 19, 2026

@LSUDOKO if it was that easy, we wouldn't have disabled the tests. The problem is that our CMake build system doesn't support the proper installation of the BMI files yet. FOr this reason the external builds fail: https://github.com/TheHPXProject/hpx/actions/runs/24625178967/job/72002820159?pr=7202

@arpittkhandelwal
Copy link
Copy Markdown
Contributor

@LSUDOKO if it was that easy, we wouldn't have disabled the tests. The problem is that our CMake build system doesn't support the proper installation of the BMI files yet. FOr this reason the external builds fail: https://github.com/TheHPXProject/hpx/actions/runs/24625178967/job/72002820159?pr=7202

SirI have been looking into the root cause for issue #7197 and why the CI fails when we re-enable the external build tests here.

As you noted, the issue is that our CMake build system doesn't properly support the installation and export of the BMI files for downstream projects yet. I believe I have identified why this is happening and would like your thoughts on a proposed fix.

Currently, HPX is in a transitional state regarding C++ modules:

  1. In libs/CMakeLists.txt, we are correctly utilizing CMake 3.28's native support via FILE_SET CXX_MODULES and install(TARGETS ... CXX_MODULES_BMI ...).
  2. However, in cmake/HPX_CXXModules.cmake, we are still using custom macros (hpx_configure_module_producer and hpx_configure_module_consumer) that manually inject compiler flags like -fmodule-output= and -fprebuilt-module-path=.

The Problem:
By manually overriding these module flags, we conflict with CMake's internal module tracking. When CMake handles a FILE_SET CXX_MODULES natively, it needs to manage the BMI paths itself in order to correctly generate the installation payload and correctly write out the exported targets (HPXTargets.cmake) for downstream consumers.

Proposed Solution:
Since we already enforce CMAKE_VERSION >= 3.29 for C++ modules, we can rely entirely on CMake's native module handling. We should:

  1. Strip out the manual -fmodule-output= and -fprebuilt-module-path= compiler flags from hpx_configure_module_producer and hpx_configure_module_consumer.
  2. Let CMake natively manage the compilation, installation, and downstream export of the CXX_MODULES_BMI.
  3. Keep the guard removed in tests/unit/build/CMakeLists.txt so that CI verifies the behavior.

Sir, do you think this is the right approach to fully resolve the BMI export issue? If this sounds good to you, I can go ahead and implement this fix.

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Apr 20, 2026

@LSUDOKO if it was that easy, we wouldn't have disabled the tests. The problem is that our CMake build system doesn't support the proper installation of the BMI files yet. FOr this reason the external builds fail: https://github.com/TheHPXProject/hpx/actions/runs/24625178967/job/72002820159?pr=7202

SirI have been looking into the root cause for issue #7197 and why the CI fails when we re-enable the external build tests here.

As you noted, the issue is that our CMake build system doesn't properly support the installation and export of the BMI files for downstream projects yet. I believe I have identified why this is happening and would like your thoughts on a proposed fix.

Currently, HPX is in a transitional state regarding C++ modules:

  1. In libs/CMakeLists.txt, we are correctly utilizing CMake 3.28's native support via FILE_SET CXX_MODULES and install(TARGETS ... CXX_MODULES_BMI ...).
  2. However, in cmake/HPX_CXXModules.cmake, we are still using custom macros (hpx_configure_module_producer and hpx_configure_module_consumer) that manually inject compiler flags like -fmodule-output= and -fprebuilt-module-path=.

The Problem: By manually overriding these module flags, we conflict with CMake's internal module tracking. When CMake handles a FILE_SET CXX_MODULES natively, it needs to manage the BMI paths itself in order to correctly generate the installation payload and correctly write out the exported targets (HPXTargets.cmake) for downstream consumers.

Proposed Solution: Since we already enforce CMAKE_VERSION >= 3.29 for C++ modules, we can rely entirely on CMake's native module handling. We should:

  1. Strip out the manual -fmodule-output= and -fprebuilt-module-path= compiler flags from hpx_configure_module_producer and hpx_configure_module_consumer.
  2. Let CMake natively manage the compilation, installation, and downstream export of the CXX_MODULES_BMI.
  3. Keep the guard removed in tests/unit/build/CMakeLists.txt so that CI verifies the behavior.

Sir, do you think this is the right approach to fully resolve the BMI export issue? If this sounds good to you, I can go ahead and implement this fix.

I'm not sure what's causing the existing setup to fail. You may be right in any case. The only thing we can do is to try things.

@arpittkhandelwal
Copy link
Copy Markdown
Contributor

@LSUDOKO if it was that easy, we wouldn't have disabled the tests. The problem is that our CMake build system doesn't support the proper installation of the BMI files yet. FOr this reason the external builds fail: https://github.com/TheHPXProject/hpx/actions/runs/24625178967/job/72002820159?pr=7202

SirI have been looking into the root cause for issue #7197 and why the CI fails when we re-enable the external build tests here.
As you noted, the issue is that our CMake build system doesn't properly support the installation and export of the BMI files for downstream projects yet. I believe I have identified why this is happening and would like your thoughts on a proposed fix.
Currently, HPX is in a transitional state regarding C++ modules:

  1. In libs/CMakeLists.txt, we are correctly utilizing CMake 3.28's native support via FILE_SET CXX_MODULES and install(TARGETS ... CXX_MODULES_BMI ...).
  2. However, in cmake/HPX_CXXModules.cmake, we are still using custom macros (hpx_configure_module_producer and hpx_configure_module_consumer) that manually inject compiler flags like -fmodule-output= and -fprebuilt-module-path=.

The Problem: By manually overriding these module flags, we conflict with CMake's internal module tracking. When CMake handles a FILE_SET CXX_MODULES natively, it needs to manage the BMI paths itself in order to correctly generate the installation payload and correctly write out the exported targets (HPXTargets.cmake) for downstream consumers.
Proposed Solution: Since we already enforce CMAKE_VERSION >= 3.29 for C++ modules, we can rely entirely on CMake's native module handling. We should:

  1. Strip out the manual -fmodule-output= and -fprebuilt-module-path= compiler flags from hpx_configure_module_producer and hpx_configure_module_consumer.
  2. Let CMake natively manage the compilation, installation, and downstream export of the CXX_MODULES_BMI.
  3. Keep the guard removed in tests/unit/build/CMakeLists.txt so that CI verifies the behavior.

Sir, do you think this is the right approach to fully resolve the BMI export issue? If this sounds good to you, I can go ahead and implement this fix.

I'm not sure what's causing the existing setup to fail. You may be right in any case. The only thing we can do is to try things.

Thanks for your input! I’ve implemented the proposed approach and opened a PR to address the BMI export issue by relying on CMake’s native module handling. #7206

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Apr 24, 2026

@LSUDOKO What should we do with this PR? Are you planning to move this forward?

@LSUDOKO
Copy link
Copy Markdown
Contributor Author

LSUDOKO commented Apr 24, 2026

@LSUDOKO What should we do with this PR? Are you planning to move this forward?

yes sir i am trying to fix it

Copy link
Copy Markdown
Contributor

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the export statements form all the symbols?

Comment thread cmake/templates/hpx.ixx.in Outdated
Comment thread cmake/templates/hpx.ixx.in
Comment thread cmake/HPX_CollectStdHeaders.cmake Outdated
@LSUDOKO
Copy link
Copy Markdown
Contributor Author

LSUDOKO commented Apr 25, 2026

Why did you remove the export statements form all the symbols?

@hkaiser
i actually only removed the export macros from template specializations, not from all symbols.
when the headers are included inside that extern "C++" block in the module, the compiler rejects using the export keyword specifically on specializations (it triggers an 'unbraced
export-declaration' error). Since C++20 modules automatically export specializations as long as the primary template is exported, removing the macro from them was necessary to get
the core module to compile without losing any visibility

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Apr 25, 2026

Why did you remove the export statements form all the symbols?

@hkaiser i actually only removed the export macros from template specializations, not from all symbols. when the headers are included inside that extern "C++" block in the module, the compiler rejects using the export keyword specifically on specializations (it triggers an 'unbraced export-declaration' error). Since C++20 modules automatically export specializations as long as the primary template is exported, removing the macro from them was necessary to get the core module to compile without losing any visibility

Why did it compile before in that case (and it still compiles on master)? I agree that full specializations are exported automatically if the main template is exported. Those already were not marked in the HPX code base. I'm not sure about partial specializations, however.

@LSUDOKO
Copy link
Copy Markdown
Contributor Author

LSUDOKO commented Apr 25, 2026

Why did it compile before in that case (and it still compiles on master)? I agree that full specializations are exported automatically if the main template is exported. Those already were not marked in the HPX code base. I'm not sure about partial specializations, however.

the reason it seems to work on master right now is mostly down to the macros. In a regular build, those macros expand to visibility attributes (like
attribute((visibility("default")))), which the compiler is fine with on specializations. But once we turn modules ON, the macro turns into a literal export keyword

newer compilers (like GCC 14 or Clang 16) are now much stricter. They throw that 'unbraced export-declaration' error because the export keyword isn't allowed to be nested inside
the extern "C++" block that we restored in the module interface

regarding partial specializations I was curious about that too, so I checked the standard. In C++20 ([module.interface]/6), it's confirmed that any specialization (partial or
full) is automatically exported if its primary template is. I even tested it with a small external project using the partial specializations in hpx_core, and they were visible and
worked perfectly. So, removing the macros is the only way to satisfy the new compiler rules without actually breaking the interface

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Apr 25, 2026

@LSUDOKO The purpose of this PR should be to fix the dependent target build tests that currently fail as the C+ modules are not been found (neither from the internal build directories nor from an installed version). Everything else doesn't need any changes, I assume as everything in HPX builds just fine (i.g., the core libraries, the tests, and the examples). The problem is in the CMake build system that doesn't correctly propagate C++ module settings to external builds.

@hkaiser
Copy link
Copy Markdown
Contributor

hkaiser commented Apr 26, 2026

@LSUDOKO Ok, the CIs don't seem to mind removing the export keywords from the partial specializations. Good. The original issue has not been solved by this, however.

Also, you seem to mix up the two different types of EXPORT macros we use. There is HPX_EXPORT and HPX_CORE_EXPORT, which indeed expand to the visibility macros needed for shared library symbol visibility. There are also HPX_CXX_EXPORT and HPX_CXX_CORE_EXPORT, which are responsible for generating the export keywords for the C++ module build system.

Also, our CIs do run gcc 14 and up, and things still compile without removing the export keywords on master. I think the compilation problems you're seeing are caused by something else. For this reason I'd like to separate the removal of the export macros into a separate PR to avoid watering down this particular PR here. Could you do that, please?

@LSUDOKO
Copy link
Copy Markdown
Contributor Author

LSUDOKO commented Apr 26, 2026

@LSUDOKO Ok, the CIs don't seem to mind removing the export keywords from the partial specializations. Good. The original issue has not been solved by this, however.

Also, you seem to mix up the two different types of EXPORT macros we use. There is HPX_EXPORT and HPX_CORE_EXPORT, which indeed expand to the visibility macros needed for shared library symbol visibility. There are also HPX_CXX_EXPORT and HPX_CXX_CORE_EXPORT, which are responsible for generating the export keywords for the C++ module build system.

Also, our CIs do run gcc 14 and up, and things still compile without removing the export keywords on master. I think the compilation problems you're seeing are caused by something else. For this reason I'd like to separate the removal of the export macros into a separate PR to avoid watering down this particular PR here. Could you do that, please?

Okay sir ,i will do this

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 27, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric Results
Duplication 0

View in Codacy

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.

Comment thread cmake/HPX_CollectStdHeaders.cmake Outdated
LSUDOKO added 2 commits April 27, 2026 18:02
- Removed manual -fmodule-output flags to let CMake manage BMIs natively.
- Re-enabled external build tests in tests/unit/build.
- Updated HPX_CollectStdHeaders.cmake to automatically include POSIX headers in GMF.
- Restored extern "C++" block in hpx.ixx.in for ABI compatibility.
- Disabled legacy pkg-config tests for module builds.

Closes TheHPXProject#7197
Copy link
Copy Markdown
Contributor

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have removed large parts of the external build targets. Was that intended?

Comment thread cmake/HPX_CollectStdHeaders.cmake Outdated
@LSUDOKO
Copy link
Copy Markdown
Contributor Author

LSUDOKO commented Apr 27, 2026

You have removed large parts of the external build targets. Was that intended?

sorry about the removed targets! That was a complete mistake on my part while I was trying to fix the pseudo-target
registrationI accidentally simplified the loop too much. I've already restored the full set of tests in the latest
commit
as for link.h and dlfcn.h, I added them because the Linux CI was failing with: error: conflicting declaration of
'struct link_map' in module 'HPX.Core'. Even though they are extern "C", the compiler seemed to complain because
they were being included within the module purview (via dll_dlopen.hpp) putting them in the Global Module Fragment
(via std_headers.hpp) was the only way I found to stop that conflict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix external builds with C++ modules enabled

4 participants