-
Notifications
You must be signed in to change notification settings - Fork 214
build: support meson build system #375
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
base: main
Are you sure you want to change the base?
Conversation
|
@microsoft-github-policy-service agree |
911dd00 to
3f00088
Compare
fcf4cdf to
94cae03
Compare
mingxwa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution! Adding Meson support is a great addition to make the library more accessible. However, I have some concerns regarding the maintenance of the build files and the CI configuration.
.github/workflows/meson.yml
Outdated
| env: ${{ matrix.mode.extra_envs }} | ||
| run: | | ||
| meson test -C build-${{ matrix.flavor }} --timeout-multiplier 5 --print-errorlogs | ||
| meson test --benchmark -C build-${{ matrix.flavor }} --timeout-multiplier 5 --print-errorlogs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Executing benchmarks in the CI pipeline is generally not necessary for verification. Please remove the benchmark execution steps and only run the unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other workflows runs benchmarks, so I included this.
done in ce70232
|
Thank you for the extensive review. I will work on the changes later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive Meson build system support to the project alongside the existing CMake setup. The implementation includes building the library, tests, benchmarks, and extracting examples from documentation, with support for C++20 modules and various compiler configurations.
Key changes:
- Complete Meson build configuration with feature parity to CMake (modules, tests, benchmarks, examples)
- Python automation scripts for test/benchmark discovery and documentation example extraction
- GitHub Actions CI workflow for testing Meson builds across multiple platforms and compilers
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
meson.build |
Main build configuration defining project metadata, compiler checks, dependencies, and library targets |
meson.options |
Build options for enabling/disabling tests, benchmarks, examples, modules, and freestanding support |
meson.format |
Code formatting configuration for Meson files |
tools/meson_extract_doctest.py |
Script to extract C++ code examples from markdown documentation |
tools/meson_autogen_tests.py |
Script to auto-generate test case lists from GTest JSON output |
tools/meson_autogen_doctest.py |
Script to auto-generate documentation example file lists |
tools/meson_autogen_benchmarks.py |
Script to auto-generate benchmark suite definitions |
tools/meson.build |
Build configuration to locate Python automation scripts |
tools/extract_example_code_from_docs.py |
Refactored script with type hints and improved structure for extracting code examples |
tests/meson.build |
Test suite configuration including standard, freestanding, and module tests |
tests/modules/meson.build |
C++20 modules-specific test configuration |
benchmarks/meson.build |
Benchmark suite configuration |
docs/meson.build |
Configuration for building and testing documentation examples |
docs/example.cpp.in |
Template file for generated example code |
subprojects/*.wrap |
Meson wrap files for external dependencies (googletest, google-benchmark, fmt) |
tests/proxy_reflection_tests.cpp |
Fixed test using more accurate type trait combination |
include/proxy/v4/proxy_fmt.h |
Added fmt v12+ compatibility check for wchar_t support |
.github/workflows/meson.yml |
CI workflow for testing Meson builds with various compilers and configurations |
.github/workflows/pipeline-ci.yml |
Updated main CI pipeline to include Meson workflow |
.github/workflows/pipeline-release.yml |
Fixed tar command argument ordering in release script |
.gitignore |
Added Meson build artifacts and Python cache to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ee03832 to
201a753
Compare
4ed6e5e to
efdefc3
Compare
| fail-fast: false | ||
| matrix: | ||
| compiler: | ||
| - {compiler: gcc, version: 13, modules: false} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding Meson compatibility tests seems unnecessary. This job targets compiler version coverage, and testing every toolchain combination isn't an effective use of resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 6dfbc08
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matrix build caught libc++ 16 has incomplete format support and not exposed (_LIBCPP_HAS_NO_INCOMPLETE_FORMAT is defined). should we test the feature using __cpp_lib_format instead of __has_include(<format>)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we support incomplete implementations of std::format? our code requires at least libc++ >= 17, but std::format support is incomplete until libc++ 19 (thus __cpp_lib_format is not defined before that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4dc8332 to
8d0d90b
Compare
984e89e to
c02d4db
Compare
With specialization for libc++. Fixes compilation on libc++ 16-18 that has incomplete std::format support.
| @@ -0,0 +1,13 @@ | |||
| [wrap-file] | |||
| directory = benchmark-1.8.4 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please align the version with CMake (1.9.4).
|
|
||
| @dataclass(frozen=True) | ||
| class Args: | ||
| input_dir: Path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid duplication, please invoke this script from docs/meson.build and iterate outputs.
meson.build
Outdated
| ) | ||
| modules_interface_specs = { | ||
| 'gcc': { | ||
| 'flags': ['-fmodules', '-fdeps-format=p1689r5'] + (cxx.has_header_symbol( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is -fdeps-format=p1689r5 still needed in GCC 15?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least on my Red Hat GCC 15.2.1-4 + ninja 1.13.1.
I completely removed modules support for now, will add it back once I fix it upstream.
too many compiler-specific workarounds. waiting for better upstream support.
… build benchmark on ci
… build benchmark on ci
|
|
||
| # - name: install meson | ||
| # # FIXME: install from upstream once https://github.com/mesonbuild/meson/pull/15353 is released | ||
| # run: pipx install git+https://github.com/mesonbuild/meson@02b85a846629090a0c7f18e860bab0a10ea4349b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that this change was released in 1.10.0. Thanks for the update! However, when building locally with NVHPC, I encountered several errors. I haven't had a chance to investigate in detail yet, but it seems related to the configuration.
In our CMake setup, we explicitly declare fmt as a SYSTEM dependency to suppress compiler warnings from third-party code, which helps avoid build failures with NVHPC. Does Meson offer a similar mechanism to mark dependencies as system includes?
CMake feature parity:
Extra features:
Tested on Fedora Linux 44 (gcc/clang/msvc-wine + ninja) and Windows 10 22H2 (msvc + vs2022).