Re-enable external build tests with C++ modules#7202
Re-enable external build tests with C++ modules#7202LSUDOKO wants to merge 16 commits 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.
|
Can one of the admins verify this patch? |
|
@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:
The Problem: Proposed Solution:
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 |
|
@LSUDOKO What should we do with this PR? Are you planning to move this forward? |
yes sir i am trying to fix it |
hkaiser
left a comment
There was a problem hiding this comment.
Why did you remove the export statements form all the symbols?
@hkaiser |
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 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 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 |
|
@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. |
|
@LSUDOKO Ok, the CIs don't seem to mind removing the Also, you seem to mix up the two different types of Also, our CIs do run gcc 14 and up, and things still compile without removing the |
Okay sir ,i will do this |
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.
- 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
hkaiser
left a comment
There was a problem hiding this comment.
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 |
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/buildwere disabled whenever C++ modules were enabled:That prevented CI from checking whether downstream projects depending on HPX can be built with modules enabled.
What changed
Why this fixes the issue
The external test project already contains module-aware setup:
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.