Add cmake function(add_fmt_module) to enable support of C++ fmt modules #4039

ClausKlein · 2024-07-01T09:04:26Z

Add workarounds to test installed FILE_SET CXX_MODULES

Add C++23 module test if possible

see too https://discourse.cmake.org/t/advice-on-c-20-modules-boost/10641

vitaut

Thanks for the PR. As a general comment, unit tests are supposed to be run on the source, not on the installed artefacts and I don't think we should be changing that.

ClausKlein · 2024-07-02T17:03:53Z

Thanks for the PR. As a general comment, unit tests are supposed to be run on the source, not on the installed artefacts and I don't think we should be changing that.

In this case, it is not possible. Consider treating it as an RFC.

The concept used is from Boost, and the reasons for implementing it this way are discussed. See C++20 modules and Boost.

The current CMakeLists.txt files are partly too complex and not obvious. In my opinion, they need to be refactored.

ClausKlein · 2024-07-03T20:51:51Z

Now, the old CI test works without installation, but the fmt c++ module is not usable:

1/1 Test #22: find-package-test ................***Failed    6.65 sec
Internal cmake changing into directory: /Users/clausklein/Workspace/cpp/fmt/build/test/find-package-test
======== CMake output     ======
FMT_MODULE: TRUE
FMT_ROOT is: /Users/clausklein/Workspace/cpp/fmt
Configuring done (0.0s)
Generating done (0.0s)
Build files have been written to: /Users/clausklein/Workspace/cpp/fmt/build/test/find-package-test
======== End CMake output ======
Change Dir: '/Users/clausklein/Workspace/cpp/fmt/build/test/find-package-test'

Run Clean Command: /usr/local/bin/ninja clean
[1/1] Cleaning all built files...
Cleaning... 28 files.

Run Build Command(s): /usr/local/bin/ninja
[1/16] Scanning /Users/clausklein/Workspace/cpp/fmt/test/find-package-test/main.cc for CXX dependencies
[2/16] Scanning /Users/clausklein/Workspace/cpp/fmt/test/find-package-test/main.cc for CXX dependencies
[3/16] Generating CXX dyndep file CMakeFiles/library-test.dir/CXX.dd
[4/16] Scanning /Users/clausklein/Workspace/cpp/fmt/test/find-package-test/main.cc for CXX dependencies
[5/16] Generating CXX dyndep file CMakeFiles/header-only-test.dir/CXX.dd
[6/16] Scanning /Users/clausklein/Workspace/cpp/fmt/src/fmt.cc for CXX dependencies
[7/16] Generating CXX dyndep file CMakeFiles/fmt-module.dir/CXX.dd
[8/16] Generating CXX dyndep file CMakeFiles/module-test.dir/CXX.dd
[9/16] Building CXX object CMakeFiles/library-test.dir/main.cc.o
[10/16] Linking CXX executable library-test
[11/16] Building CXX object CMakeFiles/module-test.dir/main.cc.o
[12/16] Building CXX object CMakeFiles/header-only-test.dir/main.cc.o
[13/16] Linking CXX executable header-only-test
[14/16] Building CXX object CMakeFiles/fmt-module.dir/Users/clausklein/Workspace/cpp/fmt/src/fmt.cc.o
[15/16] Linking CXX static library libfmt-module.a
[16/16] Linking CXX executable module-test
FAILED: module-test 
: && /usr/local/opt/llvm/bin/clang++ -O3 -DNDEBUG -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/usr/local/opt/llvm/lib/c++    -stdlib=libc++ CMakeFiles/module-test.dir/main.cc.o -o module-test  libfmt-module.a && :
Undefined symbols for architecture x86_64:
  "fmt::v10::vprint(fmt::v10::basic_string_view<char>, fmt::v10::basic_format_args<fmt::v10::context>)", referenced from:
      _main in main.cc.o
ld: symbol(s) not found for architecture x86_64
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.



0% tests passed, 1 tests failed out of 1

Total Test time (real) =   6.65 sec

The following tests FAILED:
	 22 - find-package-test (Failed)
Errors while running CTest
bash-5.2$

ClausKlein

do you have any idea?

support/cmake/fmt-config.cmake.in

vitaut · 2024-07-04T15:35:47Z

do you have any idea?

I'm not sure what causing the error, the vprint overload in question is defined here:

fmt/include/fmt/format-inl.h

Line 1707 in 232c6bc

FMT_FUNC void vprint(string_view fmt, format_args args) {

BTW what version of clang do you use? On my system (AppleClang 15.0.0.15000309) it doesn't even get to this point failing with

CMake Error in CMakeLists.txt:
  The target named "fmt" has C++ sources that may use modules, but the
  compiler does not provide a way to discover the import graph dependencies.
  See the cmake-cxxmodules(7) manual for details.  Use the
  CMAKE_CXX_SCAN_FOR_MODULES variable to enable or disable scanning.

test/find-package-test/main.cc

test/find-package-test/CMakeLists.txt

vitaut · 2024-07-05T16:29:18Z

test/CMakeLists.txt

@@ -106,6 +110,7 @@ add_fmt_test(enforce-checks-test)
 target_compile_definitions(enforce-checks-test PRIVATE
                           -DFMT_ENFORCE_COMPILE_STRING)

+# FIXME: never reached? CK


What is it about?

with FMT_MODULE=1 there is no working unit test:

bash-5.2$ ninja -C build test ninja: Entering directory `build' [0/1] Running tests... Test project /Users/clausklein/Workspace/cpp/fmt/build No tests were found!!! bash-5.2$

vitaut · 2024-07-05T16:32:14Z

support/cmake/fmt-config.cmake.in

@@ -5,3 +5,31 @@ if (NOT TARGET fmt::fmt)
 endif ()

 check_required_components(fmt)
+
+function(add_fmt_module NAME)


I don't think we should be duplicating the logic to create the fmt module.

The it will not work to use the installed fmt module:

[6/7] /usr/local/opt/llvm/bin/clang++ -DFMT_MODULE -isystem /usr/local/include -std=c++20 -std=c++23 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk -stdlib=libc++ -MD -MT CMakeFiles/module-test.dir/main.cc.o -MF CMakeFiles/module-test.dir/main.cc.o.d @CMakeFiles/module-test.dir/main.cc.o.modmap -o CMakeFiles/module-test.dir/main.cc.o -c /Users/clausklein/Workspace/cpp/fmt/test/find-package-test/main.cc FAILED: CMakeFiles/module-test.dir/main.cc.o /usr/local/opt/llvm/bin/clang++ -DFMT_MODULE -isystem /usr/local/include -std=c++20 -std=c++23 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk -stdlib=libc++ -MD -MT CMakeFiles/module-test.dir/main.cc.o -MF CMakeFiles/module-test.dir/main.cc.o.d @CMakeFiles/module-test.dir/main.cc.o.modmap -o CMakeFiles/module-test.dir/main.cc.o -c /Users/clausklein/Workspace/cpp/fmt/test/find-package-test/main.cc error: C++23 was disabled in PCH file but is currently enabled error: module file CMakeFiles/fmt__fmt@synth_8c418311d24f.dir/ad882f9916e2.bmi cannot be loaded due to a configuration mismatch with the current compilation [-Wmodule-file-config-mismatch] /Users/clausklein/Workspace/cpp/fmt/test/find-package-test/main.cc:3:2: error: expected unqualified-id 3 | #else | ^ 3 errors generated. ninja: build stopped: subcommand failed. bash-5.2$

For the installed case, find_package should take care of the necessary configuration. If the installed targets are somehow not usable directly we should fix that and not try recreating them here.

This is not possible!

If you compile the fmt library with one compiler and c++ standard if you install it.
No other compiler or compiler options can be used for the installed c++ module!

Read more about the problems on consuming-installed-c-20-modules

Please try the example from https://github.com/ClausKlein/cxx-modules/blob/master/cmake/readme.md

That works with the boost concept.

Sure but this doesn't mean that we have to duplicate the build config in the test. It means that the test should check if the installed package is compatible, otherwise you are testing the wrong thing.

It means that the test should check if the installed package is compatible, otherwise you are testing the wrong thing.

Do you have an idea how to check this?
If compilation or linking fails, it does not have a compatible setting...; and then?

Add workarounds to test installed FILE_SET CXX_MODULES Add C++23 module test if possible

Disable module test on MSVC CI Disable module test on MINGW CI

But the module tests does not work this way!

Install ninja on modern OS for CI Use Ninja generator on CI Build on macos-14 only current std c++20 and c++23 Prevent build of not working tests - Note: most tests have compile errors with c++20 and FMT_MODULE set.

ClausKlein · 2024-07-09T17:31:13Z

The CMake files need to be refactored.

The CI needs to be modernised!

That needs to much work for me.

vitaut requested changes Jul 2, 2024

View reviewed changes

ClausKlein marked this pull request as draft July 2, 2024 17:17

ClausKlein requested a review from vitaut July 2, 2024 17:17

ClausKlein commented Jul 3, 2024

View reviewed changes

support/cmake/fmt-config.cmake.in Outdated Show resolved Hide resolved

support/cmake/fmt-config.cmake.in Outdated Show resolved Hide resolved

ClausKlein commented Jul 4, 2024

View reviewed changes

test/find-package-test/main.cc Show resolved Hide resolved

ClausKlein commented Jul 4, 2024

View reviewed changes

test/find-package-test/CMakeLists.txt Outdated Show resolved Hide resolved

vitaut reviewed Jul 5, 2024

View reviewed changes

ClausKlein requested a review from vitaut July 5, 2024 20:26

ClausKlein and others added 9 commits July 9, 2024 18:02

Add cmake function(add_fmt_module)

f6c753c

Add workarounds to test installed FILE_SET CXX_MODULES Add C++23 module test if possible

Fix FMT_PEDANTIC tests on CI

8675252

Disable module test on MSVC CI Disable module test on MINGW CI

Respect review comment

7c8f11c

But the module tests does not work this way!

Make it works with installed fmt package

9c6b3d2

Do not use add_fmt_module(fmt-module)

3aebb40

Back to my working example

0384ef8

Rebased with master

eee89fd

Do not used add_fmt_module()

36c2bd5

Modernize CI to test with clang-18

9025543

Install ninja on modern OS for CI Use Ninja generator on CI Build on macos-14 only current std c++20 and c++23 Prevent build of not working tests - Note: most tests have compile errors with c++20 and FMT_MODULE set.

ClausKlein closed this Jul 9, 2024

Add cmake function(add_fmt_module) to enable support of C++ fmt modules #4039

Add cmake function(add_fmt_module) to enable support of C++ fmt modules #4039

Uh oh!

Conversation

ClausKlein commented Jul 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vitaut left a comment

Choose a reason for hiding this comment

Uh oh!

ClausKlein commented Jul 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ClausKlein commented Jul 3, 2024

Uh oh!

ClausKlein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vitaut commented Jul 4, 2024

Uh oh!

Uh oh!

Uh oh!

vitaut Jul 5, 2024

Choose a reason for hiding this comment

Uh oh!

ClausKlein Jul 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vitaut Jul 5, 2024

Choose a reason for hiding this comment

Uh oh!

ClausKlein Jul 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vitaut Jul 7, 2024

Choose a reason for hiding this comment

Uh oh!

ClausKlein Jul 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

This is not possible!

Uh oh!

vitaut Jul 8, 2024

Choose a reason for hiding this comment

Uh oh!

ClausKlein Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ClausKlein commented Jul 9, 2024

The CMake files need to be refactored.

The CI needs to be modernised!

That needs to much work for me.

Uh oh!

Uh oh!

ClausKlein commented Jul 1, 2024 •

edited

Loading

ClausKlein commented Jul 2, 2024 •

edited

Loading

ClausKlein Jul 5, 2024 •

edited

Loading

ClausKlein Jul 5, 2024 •

edited

Loading

ClausKlein Jul 7, 2024 •

edited

Loading

ClausKlein Jul 8, 2024 •

edited

Loading