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 support for CMake 3.28 C++ module features #3679

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

jcar87
Copy link
Contributor

@jcar87 jcar87 commented Oct 12, 2023

CMake 3.28 (rc1 announcement) adds support for C++ modules that is no longer experimental (so no need to add the experimental flags).

This PR adds support for this using the new features, when the build is configured with FMT_MODULE set to ON and the CMake version is 3.28 or higher.

Tested both building fmt, and then calling cmake install and using the fmt module from a different project. This works with:

  • Linux, Clang 17, ninja 1.11.1
  • Linux, gcc 13.1, ninja 1.11.1 (note that I'm using a patched version of gcc with dependency scanning support, and additional CMake variables since CMake expects gcc 14 for this, but gcc14 ICEs when building fmt as a module, I've reported this here)
  • Windows, msvc 19.37
    • Building fmt itself works with the Visual Studio generator or Ninja
    • Using the imported targets with find_package(fmt) works with Ninja, but not with the Visual Studio generator - reported this here.

Hopefully this is OK. For CMake versions earlier than 3.28, this should behave as it was previously.

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

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

Thanks for the PR but the part of CMake you are changing is deprecated and will be removed. Please add this to add_module_library instead.

@jcar87
Copy link
Contributor Author

jcar87 commented Oct 15, 2023

Hi @vitaut - thanks for the review.
The changes in this PR are mostly in add_module_library() - the only change in enable_module() is that I've added a conditional to skip the logic altogether when CMake is >= 3.28, as I can see that enable_module() is still called, and what it aims to do is already handled by CMake+ninja or by MSBuild.

However if this is not the best way, I'm happy to move the conditional out of enable_module() and into the two places where it is still called?

@vitaut
Copy link
Contributor

vitaut commented Oct 15, 2023

Sorry, I just realized that it's not super clear from the code and my comment but what I meant is that the source of truth of add_module_library is in https://github.com/vitaut/modules. Could you submit the change there to prevent it being overwritten in this repo next time we sync the two?

@vitaut vitaut merged commit cbb18c2 into fmtlib:master Nov 13, 2023
40 checks passed
@vitaut
Copy link
Contributor

vitaut commented Nov 13, 2023

I merged it for now but as commented earlier it will be overwritten unless also added to the main modules repo. Thanks!

@vitaut
Copy link
Contributor

vitaut commented Nov 14, 2023

Revered because of incompatibility with CMake 3.16: #3714.

@jcar87
Copy link
Contributor Author

jcar87 commented Nov 17, 2023

Apologies @vitaut - I will make the contribution in the main modules repo and make sure there's no regression with older CMake.

@vitaut
Copy link
Contributor

vitaut commented Nov 18, 2023

No worries and thanks for working on this.

happymonkey1 pushed a commit to happymonkey1/fmt that referenced this pull request Apr 7, 2024
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.

2 participants