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

fmt v11 and c++20 modules support #10

Merged
merged 37 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
5bc0f0f
Remove enable_modules build config variable, use cxx.features.modules…
kamrann Jul 18, 2024
a836c69
Include bmis target type when setting export poptions
kamrann Jul 18, 2024
1f14622
Automatically enable FMT_ATTACH_TO_GLOBAL_MODULE when building as mod…
kamrann Jul 18, 2024
f808761
Replace import std; in smoke tests with #includes, pending implementi…
kamrann Jul 18, 2024
0757158
Add package configuration variable to toggle use of import std.
kamrann Jul 22, 2024
a9ee48b
Add config option for a modules-only mode.
kamrann Jul 22, 2024
b2e4c3b
Point upstream to master branch of upstream fmt - this has merged in …
kamrann Aug 26, 2024
c271d77
Add new package for hosting upstream tests.
kamrann Aug 30, 2024
110906e
Add .vs/ folder to gitignore
kamrann Aug 30, 2024
ab0cc5e
Align the test package version number to the main package.
kamrann Aug 30, 2024
79827a3
Adjusted fmt-tests buildfiles to attempt to fix issues with include p…
kamrann Aug 31, 2024
b5ecdbf
Marked all executable targets in fmt-tests as tests.
kamrann Aug 31, 2024
6007797
Tweak to ranges-test to match upstream CMake setup.
kamrann Sep 2, 2024
6042313
Symc to latest upstream
kamrann Sep 2, 2024
4841d77
Refactor smoke test buildfile to avoid conditional target dependencies.
kamrann Sep 2, 2024
014a3d8
Experiment with package specific CI configurations.
kamrann Sep 2, 2024
8055762
Force reproces=true on MSVC modules builds to work around compiler bug.
kamrann Sep 2, 2024
1130bb6
Fix for incorrect variable prefix.
kamrann Sep 2, 2024
c553ca8
Constrain modules CI build configs to latest.
kamrann Sep 2, 2024
495ab27
Experiment constraining to clang.
kamrann Sep 2, 2024
c328c6c
Switch from latest to experimental
kamrann Sep 2, 2024
b3b1dcc
Add msvc to CI package configs
kamrann Sep 2, 2024
82e02d5
Revert CI configs to latest
kamrann Sep 2, 2024
dc8fb4b
Change smoke test to use template parameter for format parse context,…
kamrann Sep 2, 2024
105613f
Update upstream to grab latest fixes.
kamrann Sep 4, 2024
f3a93ca
Merge branch 'pr/v11-modules' into tests-prototyping
kamrann Sep 4, 2024
780ba03
Upstream submodule update.
kamrann Sep 4, 2024
682c69b
Upstream update.
kamrann Sep 4, 2024
2922bb4
Upstream sync.
kamrann Sep 4, 2024
3d7398a
Upstream sync
kamrann Sep 4, 2024
c5c5ab5
Upstream sync.
kamrann Sep 7, 2024
05855de
Disable modules builds on CI.
kamrann Sep 7, 2024
8841258
Switch upstream branch bag to latest tagged release (11.0.2)
kamrann Sep 10, 2024
7c56c1c
Add some CI build exclusions.
kamrann Sep 10, 2024
6b5d114
Update package readme with latest information on modules compatibility.
kamrann Sep 10, 2024
8f3e148
Add comment to manifest re CI modules configs.
kamrann Sep 10, 2024
bffc9c4
Cleanup of fmt-tests buildfile organization and comments.
kamrann Sep 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@
fmt/fmt/include symlink=dir
fmt/fmt/src symlink=dir
fmt/doc symlink=dir
fmt-tests/basics/test symlink=dir
fmt-tests/test-main/test symlink=dir
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
.bdep/
.vs/

# Local default options files.
#
Expand Down
25 changes: 25 additions & 0 deletions fmt-tests/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Compiler/linker output.
#
*.d
*.t
*.i
*.i.*
*.ii
*.ii.*
*.o
*.obj
*.gcm
*.pcm
*.ifc
*.so
*.dylib
*.dll
*.a
*.lib
*.exp
*.pdb
*.ilk
*.exe
*.exe.dlls/
*.exe.manifest
*.pc
5 changes: 5 additions & 0 deletions fmt-tests/basics/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
basics

# Testscript output directory (can be symlink).
#
test-basics
45 changes: 45 additions & 0 deletions fmt-tests/basics/buildfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
include ../test-main/

#test_names =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these comments still useful?

# args-test \
# assert-test \
# base-test
gtest_test_names = args-test assert-test base-test chrono-test color-test compile-fp-test compile-test format-test gtest-extra-test noexception-test os-test ostream-test printf-test std-test unicode-test xchar-test
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this list have been formed from globing the .cc files? If not consider adding a comment about where to look to update this list (in upstream/tests/CMakeFiles.txt)


# header-only-test <- header only mode seemingly not supported by this build2 package?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes it was not supported, I basically had the same reasons as explained in the packaging guide although that wouldnt prevent support of header-only mode later if the demand is there. So far nobody needed it.

# scan-test <- some linker issues
# posix-mock-test <- needs posix-mock.h, and has some msvc/runtime conditional logic going on

standalone_test_names = enforce-checks-test # some way to pair these? -DFMT_ENFORCE_COMPILE_STRING

for test_name : $gtest_test_names
{
./: exe{$test_name} : test/cxx{$test_name}
exe{$test_name}: ../test-main/liba{test-main}:
{
bin.whole = true
}
}

./: exe{ranges-test} : test/cxx{ranges-test ranges-odr-test}
exe{ranges-test}: ../test-main/liba{test-main}:
{
bin.whole = true
}

# linker issues.
#./: exe{scan-test} : test/cxx{scan-test} ../test-main/liba{test-main}

# @todo: cmake adds this test conditionally on NOT ( msvc AND fmt-shared )
# in build2, i guess it's a bit different in that we can potentially build both shared and static variants within a single config?
# also not clear how fmt_shared/fmt_header_only is being configured, looks like the package doesn't do anything there. using defaults only?
# if $cxx. != msvc
#./: exe{format-impl-test} : test/cxx{format-impl-test header-only-test} ../test-main/liba{test-main}


import fmt = fmt%lib{fmt}

for test_name : $standalone_test_names
{
./: exe{$test_name} : test/cxx{$test_name} $fmt
}
1 change: 1 addition & 0 deletions fmt-tests/basics/test
4 changes: 4 additions & 0 deletions fmt-tests/build/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/config.build
/root/
/bootstrap/
build/
6 changes: 6 additions & 0 deletions fmt-tests/build/bootstrap.build
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
project = fmt-tests

using version
using config
using test
using dist
19 changes: 19 additions & 0 deletions fmt-tests/build/root.build
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Uncomment to suppress warnings coming from external libraries.
#
#cxx.internal.scope = current

cxx.std = latest

using cxx

hxx{*}: extension = h
mxx{*}: extension = cc
cxx{*}: extension = cc

# All executables are tests
#
exe{*}: test = true

# The test target for cross-testing (running tests under Wine, etc).
#
test.target = $cxx.target
1 change: 1 addition & 0 deletions fmt-tests/buildfile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
./: {*/ -build/} manifest
13 changes: 13 additions & 0 deletions fmt-tests/manifest
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
: 1
name: fmt-tests
version: 11.0.1-a.0.z
project: fmt
summary: Tests package for fmt upstream tests
license: MIT
url: https://github.com/fmtlib/fmt/

depends: * build2 >= 0.17.0
depends: * bpkg >= 0.17.0

depends: gtest ^1.11.0
depends: gmock ^1.11.0
14 changes: 14 additions & 0 deletions fmt-tests/test-main/buildfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

libs =
import libs += fmt%lib{fmt}
import libs += gtest%lib{gtest} gmock%lib{gmock}

liba{test-main}: test/cxx{test-main gtest-extra util} test/hxx{gtest-extra mock-allocator test-assert util} $libs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would defining it as libu would have helped usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I'm yet to understand the intended usage of libu*, despite having read through the docs information on them many times. When/why do you choose to use it?

I also found this: Note also that if you plan to link any of your unit tests in the whole archive mode, then you will also need to exclude the source file containing the primary executable's main() from the utility library
which suggests that if used here then test-main library could not contain text-main.cc. Having said that, the test-main library target is essentially reflecting upstream CMake and it's arguably not even worth having?


# Export options.
#
liba{test-main}:
{
cxx.export.poptions += "-I$src_base/test"
cxx.export.libs = $libs
}
1 change: 1 addition & 0 deletions fmt-tests/test-main/test
13 changes: 11 additions & 2 deletions fmt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,18 @@ See [`{fmt}` documentation](https://fmt.dev/) for usage and details.
Note: This is the source code for the build2 package of the `{fmt}` C++ library,
the actual library sources snapshot can be found in the `./upstream/` submodule.

## Configuration Options:
## Configuration Options

- `config.fmt.enable_modules` : Set to `true` to build and provide the `fmt` C++ modules. If the compiler and C++ version don't support C++ modules (which is C++ > 20), this will result in a compilation failure. Set to `true` if `$cxx.features.modules == true` which means the configuration is set to enable C++ modules, `false` otherwise.
### Experimental C++20 modules support

Modules support is WIP, both in the `build2` package and also in `fmt` upstream. Latest versions of MSVC or Clang are recommended, and the most up-to-date version of the package with regards to modules compatibility can be used via git and the [modules branch](https://github.com/build2-packaging/fmt/tree/modules). For example, in project `repositories.manifest`:
```
:
role: prerequisite
location: https://github.com/build2-packaging/fmt.git#modules
```

Enable with `config.cxx.features.modules=true`. When enabled, by default dual mode is used meaning that the library can be consumed either through `import` or via `#include`. To enable this safely, all entities are attached to the global module (extern "C++"). See `modules_only` option for the alternative.
- `config.fmt.enable_import_std` : Set to `true` to consume the standard library as a module when building the `fmt` module. Support dependent on compiler and std lib. Defaults to `false`.
- `config.fmt.modules_only` : Set to `true` to enable modules-only mode for the package. In this mode, `fmt` entities are fully encapsulated in the `fmt` module meaning `#include`-based consumption cannot be mixed, and the package will not export headers. Defaults to `false`.

5 changes: 2 additions & 3 deletions fmt/build/root.build
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,5 @@ test.target = $cxx.target
##############################
# Project-specific options:

# Set to true to build and provide the `fmt` C++ module.
config [bool] config.fmt.enable_modules ?= $cxx.features.modules # $($cxx.features.modules == true)

config [bool] config.fmt.enable_import_std ?= false
config [bool] config.fmt.modules_only ?= false
42 changes: 30 additions & 12 deletions fmt/fmt/buildfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,26 @@ imp_libs = # Implementation dependencies.

lib{fmt}: include/hxx{**} hxx{version} $imp_libs $int_libs

lib{fmt}: src/mxx{fmt} : include = ($config.fmt.enable_modules) # `fmt` C++ module only
lib{fmt}: src/cxx{** -fmt} : include = (!$config.fmt.enable_modules) # no modules only
# Automatically build as module if the feature is enabled
build_fmt_module = $cxx.features.modules
allow_header_usage = ($build_fmt_module == false || $config.fmt.modules_only == false)

if($build_fmt_module && $allow_header_usage)
info "Building fmt with dual module/header mode enabled"

# Workaround for MSVC bug: https://developercommunity.visualstudio.com/t/Separate-preprocessing-with-P-fails-wit/10707183
if($build_fmt_module && $cxx.class == 'msvc')
cc.reprocess = true

lib{fmt}: src/mxx{fmt} : include = ($build_fmt_module) # `fmt` C++ module only
lib{fmt}: src/cxx{** -fmt} : include = (!$build_fmt_module) # no modules only

# Meta data for users
lib{fmt}:
{
export.metadata = 1 fmt
fmt.is_module = [bool] $config.fmt.enable_modules
fmt.has_header = [bool] $allow_header_usage
fmt.has_module = [bool] $build_fmt_module
}

# Include the generated version header into the distribution (so that we don't
Expand All @@ -31,22 +43,28 @@ hxx{version} : in{version} $src_root/manifest
# Build options.
#
cxx.poptions =+ "-I$src_base/include" "-I$out_root" "-I$src_root"
objs{*}: cxx.poptions += -DFMT_LIB_EXPORT
{objs bmis}{*}: cxx.poptions += -DFMT_LIB_EXPORT

if($config.fmt.enable_modules)
cxx.poptions =+ -DFMT_MODULE=ON
# If building fmt module, also enable attaching to the global module in order to allow concurrent #include and import.
if($build_fmt_module)
{
cxx.poptions =+ -DFMT_MODULE
if($allow_header_usage)
# Support mixing consuming as both modules and headers within a single build
cxx.poptions =+ -DFMT_ATTACH_TO_GLOBAL_MODULE
if($config.fmt.enable_import_std)
cxx.poptions =+ -DFMT_IMPORT_STD
}

# Export options.
#
lib{fmt}:
lib{fmt}: cxx.export.libs = $int_libs
if($allow_header_usage) # Note: exported poptions only relevant for header consumption
{
cxx.export.poptions = "-I$src_base/include" "-I$out_root" "-I$src_root"
cxx.export.libs = $int_libs
lib{fmt}: cxx.export.poptions = "-I$src_base/include" "-I$out_root" "-I$src_root"
libs{fmt}: cxx.export.poptions += -DFMT_SHARED
}

libs{fmt}: cxx.export.poptions += -DFMT_SHARED


# For pre-releases use the complete version to make sure they cannot be used
# in place of another pre-release or the final version. See the version module
# for details on the version.* variable values.
Expand Down
12 changes: 12 additions & 0 deletions fmt/manifest
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ url: https://github.com/fmtlib/fmt/
package-url: https://github.com/build2-packaging/fmt/
package-email: [email protected]

tests: fmt-tests == $

depends: * build2 >= 0.17.0
depends: * bpkg >= 0.17.0

builds: default
builds: -freebsd ; fmt tests failing, fixed on upstream master, pending removal next package release after 11.0.2
build-exclude: linux_debian_12-clang_17 ; clang-17 bug with libstdc++ std::tuple (https://github.com/llvm/llvm-project/issues/61415)
build-exclude: **/x86_64-w64-mingw32 ; unknown error building installed tests 'error: unable to stat path D:\a\msys64\mingw64\lib\x86_64-w64-mingw32\14.1.0\pkgconfig\: the device is not ready'

# Modules support still not sufficient to enable on CI
#modules-builds: latest : &( +clang-18+ +msvc ) ; Modules builds only supported for latest Clang and MSVC
#modules-build-config: config.cxx.features.modules=true ; Enable c++20 modules
#modules-only-builds: latest : &( +clang-18+ +msvc ) ; Modules builds only supported for latest Clang and MSVC
#modules-only-build-config: config.cxx.features.modules=true config.fmt.modules_only=true ; Enable c++20 modules and disable header usage
14 changes: 9 additions & 5 deletions fmt/tests/basics/buildfile
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import! [metadata, rule_hint=cxx.link] libs = fmt%lib{fmt}

./ : exe{driver} : {cxx}{driver} hxx{tests.inl} $libs testscript{**}
./ :
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that line still necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently not ;)
I may have thought it more clear since the following two are both conditional, but indeed seems superfluous.


if $($libs: fmt.is_module)
{
./ : exe{driver-modules} : {cxx}{driver-modules} hxx{tests.inl} $libs testscript{**}
}
./ : exe{driver} : include = $($libs: fmt.has_header)
./ : exe{driver-modules} : include = $($libs: fmt.has_module)

exe{driver} : {cxx}{driver} hxx{tests.inl} $libs testscript{**}

# For purposes of verifying that fmt headers are not made available for include when in modules-only mode
cxx.poptions =+ "-DFMT_BUILD2_HAS_HEADER=($($libs: fmt.has_header) ? 1 : 0)"

exe{driver-modules} : {cxx}{driver-modules} hxx{tests.inl} $libs testscript{**}
12 changes: 9 additions & 3 deletions fmt/tests/basics/driver-modules.cxx
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@

// Verify that fmt headers are not available if config.fmt.modules_only is true
#if __has_include(<fmt/version.h>) != FMT_BUILD2_HAS_HEADER
#error fmt headers should be available for include iff config.fmt.modules_only == false
#endif

#include <cassert>
import std;
#include <string>
#include <chrono>
#include <vector>

import fmt;

#include "tests.inl"


1 change: 1 addition & 0 deletions fmt/tests/basics/driver.cxx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include <cassert>
#include <string>
#include <chrono>
#include <vector>

Expand Down
3 changes: 2 additions & 1 deletion fmt/tests/basics/tests.inl
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ struct date {

template <>
struct fmt::formatter<date> {
constexpr auto parse(format_parse_context& ctx) const { return ctx.begin(); }
template <typename ParseContext>
constexpr auto parse(ParseContext& ctx) const { return ctx.begin(); }

template <typename FormatContext>
constexpr auto format(const date& d, FormatContext& ctx) const {
Expand Down
2 changes: 2 additions & 0 deletions packages.manifest
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
: 1
location: fmt/
:
location: fmt-tests/
11 changes: 3 additions & 8 deletions repositories.manifest
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
: 1
summary: fmt project repository

#:
#role: prerequisite
#location: https://pkg.cppget.org/1/stable
#trust: ...

#:
#role: prerequisite
#location: https://git.build2.org/hello/libhello.git
:
role: prerequisite
location: https://pkg.cppget.org/1/stable