Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add experimental CMake module support #108

Merged
merged 3 commits into from
Jul 31, 2023
Merged

Conversation

tcbrindle
Copy link
Owner

If CMake is configured with FLUX_BUILD_MODULE=On (default is Off), then we'll attempt to build a module using add_module_library() from Victor Zverovich's modules.cmake.

We'll also attempt to build a small test program to verify that the module can be correctly imported.

Everything seems to work fine with Clang 16. MSVC generates the module and test program correctly, but seems to have trouble with flux::ref(), which rather rules out doing anything practical with modules right now.

With GCC 13, the Ninja generator currently runs into problems* building the module. With a Makefile generator, building the module succeeds, but the import fails complaining it can't find the generated module. I don't know if this is caused by a GCC bug or by modules.cmake.

If CMake is configured with FLUX_BUILD_MODULE=On (default is Off), then we'll attempt to build a module using add_module_library() defined in modules.cmake.

We'll also attempt to build a small test program to verify that the module can be correctly imported.

Everything seems to work fine with Clang 16.

With GCC 13, the Ninja generator currently runs into problems* building the module. With a Makefile generator, building the module succeeds, but the import fails complaining it can't find the generated module. I don't know if this is caused by a GCC bug or by modules.cmake.

MSVC is not yet tested.

* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105467
MSVC doesn't like the way we're #include-ing flux.hpp within the module purview, although it seems to work correctly, so we'll silence the warning.
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (141f271) 97.67% compared to head (1864f09) 97.67%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #108   +/-   ##
=======================================
  Coverage   97.67%   97.67%           
=======================================
  Files          66       66           
  Lines        2236     2236           
=======================================
  Hits         2184     2184           
  Misses         52       52           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tcbrindle tcbrindle merged commit 90e01d6 into main Jul 31, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant