build: add Meson build support and export a C++20 module interface - #240
build: add Meson build support and export a C++20 module interface#240lozkoev wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe pull request adds a C++20 ChangesC++20 module and build integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CMake
participant miniocpp_target
participant Install
CMake->>miniocpp_target: Register modules/miniocpp.cc for supported C++20 builds
miniocpp_target->>Install: Install the CXX_MODULES file set
CMake->>Install: Install public and vendored headers
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@meson.build`:
- Line 1: Update the project configuration in meson.build to require C++20 for
the miniocpp module build, and make the cpp_args setting conditional so
/interface is supplied only when meson.get_compiler('cpp').get_id() is msvc; use
no MSVC-specific flag for GCC or native Clang builds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 229fd75c-ae3d-4238-a93b-13861489acae
📒 Files selected for processing (4)
CMakeLists.txtmeson.buildminiocpp.pc.inmodules/miniocpp.cc
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
CMakeLists.txt (2)
322-337: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExport C++ module files with the target export.
When
MINIO_CPP_HAS_CXX_MODULEis ON, theinstall(TARGETS miniocpp EXPORT miniocpp-targets ... FILE_SET CXX_MODULES ...)installs module files, butinstall(EXPORT miniocpp-targets ...)does not setCXX_MODULES_DIRECTORY. AddCXX_MODULES_DIRECTORYto the export installation for module-enabled builds; the project compatibility branch still supports CMake older than 3.28, so do not add it outside this branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CMakeLists.txt` around lines 322 - 337, Update the module-enabled CMake installation branch for the miniocpp target so its corresponding install(EXPORT miniocpp-targets ...) call sets CXX_MODULES_DIRECTORY. Keep this option restricted to the MINIO_CPP_HAS_CXX_MODULE branch and leave the compatibility branch unchanged for older CMake versions.
182-200: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGate C++ module registration on supported toolchain and generator combinations.
target_sources(miniocpp ... FILE_SET CXX_MODULES ...)is registered when onlyCMAKE_VERSION >= 3.28andMINIO_CPP_STD == "20"are true. File type registration and dependency scanning can then run for unsupported GCC/Clang versions or Makefile generators. Add checks forCMAKE_CXX_COMPILER_ID, minimum compiler version, andCMAKE_GENERATOR, or make this truly opt-in with a clear diagnostic whenmodules/miniocpp.cccannot be supported.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CMakeLists.txt` around lines 182 - 200, Update the MINIO_CPP_HAS_CXX_MODULE guard around target_sources(miniocpp) so module registration occurs only for supported C++20 compiler and generator combinations: validate CMAKE_CXX_COMPILER_ID, the compiler’s minimum supported version, and CMAKE_GENERATOR in addition to the existing CMake and standard checks. Keep the module disabled otherwise and provide a clear diagnostic when modules/miniocpp.cc is requested but unsupported.meson.build (1)
46-57: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAdd non-MSVC module support or restrict the Meson target.
cpp_std=c++20does not enable GCC module compilation, and the non-MSVC branch does not generate or export a BMI. Clang supports C++20 modules but still needs module artifact propagation for consumers. Add a Meson test that usesimport miniocpp;with GCC and Clang. If these toolchains are supported, add their compiler-specific module flags and propagate the module artifact throughminiocpp_dep; otherwise, disable this target for unsupported non-MSVC toolchains.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@meson.build` around lines 46 - 57, Update the miniocpp_module target and miniocpp_dep to either implement compiler-specific C++20 module support for GCC and Clang—including BMI generation, a Meson compile test using import miniocpp;, and propagation of the module artifact—or restrict/disable the target for unsupported non-MSVC compilers. Preserve the existing MSVC configuration and ensure consumers of miniocpp_dep receive the required module metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@CMakeLists.txt`:
- Around line 322-337: Update the module-enabled CMake installation branch for
the miniocpp target so its corresponding install(EXPORT miniocpp-targets ...)
call sets CXX_MODULES_DIRECTORY. Keep this option restricted to the
MINIO_CPP_HAS_CXX_MODULE branch and leave the compatibility branch unchanged for
older CMake versions.
- Around line 182-200: Update the MINIO_CPP_HAS_CXX_MODULE guard around
target_sources(miniocpp) so module registration occurs only for supported C++20
compiler and generator combinations: validate CMAKE_CXX_COMPILER_ID, the
compiler’s minimum supported version, and CMAKE_GENERATOR in addition to the
existing CMake and standard checks. Keep the module disabled otherwise and
provide a clear diagnostic when modules/miniocpp.cc is requested but
unsupported.
In `@meson.build`:
- Around line 46-57: Update the miniocpp_module target and miniocpp_dep to
either implement compiler-specific C++20 module support for GCC and
Clang—including BMI generation, a Meson compile test using import miniocpp;, and
propagation of the module artifact—or restrict/disable the target for
unsupported non-MSVC compilers. Preserve the existing MSVC configuration and
ensure consumers of miniocpp_dep receive the required module metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3b9fbe6b-5d18-4bf2-ab3c-6de2919f705e
📒 Files selected for processing (3)
CMakeLists.txtmeson.buildminiocpp.pc.in
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CMakeLists.txt`:
- Around line 198-201: Restrict the condition enabling FILE_SET CXX_MODULES to
supported toolchains: require CMake 3.28+, C++20, Ninja version 1.11 or newer
for Ninja generators, or Visual Studio 17.4+ with MSVC 14.34+; keep Ninja
Multi-Config supported as specified. Ensure older Ninja, MSVC, and Visual Studio
combinations use the existing module-less fallback.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 32cdcf69-1005-46c6-b5fc-a416ef50bd37
📒 Files selected for processing (1)
CMakeLists.txt
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CMakeLists.txt`:
- Around line 206-235: Update the MINIO_CPP_MODULE_TOOLCHAIN_OK compatibility
gate before the target_sources(miniocpp) CXX_MODULES registration to require
supported compiler versions: MSVC 19.34+, Clang 16+, and GNU GCC 14+. Disable
the gate for older or unsupported compiler IDs while preserving the existing
Ninja and Visual Studio generator checks, so unsupported toolchains use the
module-less fallback.
- Around line 230-235: Add installed-consumer coverage for the exported miniocpp
C++ module, using a supported Ninja or clang-cl toolchain, and verify that the
installed package can be discovered and consumed through miniocpp-targets. If
Visual Studio generators cannot support this test, document that limitation in
the CMake configuration or test setup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5ac2b6dc-3206-407b-ab5e-2f969adb93f0
📒 Files selected for processing (1)
CMakeLists.txt
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
What
meson.build(and a Meson-native.wrap-friendly layout) so thislibrary can be consumed directly by Meson-based projects, without going
through CMake at all.
modules/miniocpp.cc- a C++20 module interface (export module miniocpp;) that re-exports the public API (minio::s3::Client,BaseUrl,BucketExistsArgs/BucketExistsResponse,minio::creds:: StaticProvider,minio::Result), so consumers canimport miniocpp;instead of
#include <miniocpp/client.h>.miniocppCMake target viaFILE_SET CXX_MODULES(CMake 3.28+), gated behindMINIO_CPP_STD=20andthe CMake version check, so CMake/MSBuild consumers get
import miniocpp;too, not only Meson ones.
miniocpp.pc.in: the generated.pchad an emptyRequires:field,so pkg-config consumers never picked up curlpp/OpenSSL/pugixml/zlib
transitively and failed to link. Added the missing
Requires:.Why
Building this project with Meson previously required either wrapping it
in a manual
dependency(method: 'pkg-config')/method: 'cmake'call withhand-written link flags (fragile, breaks whenever a dependency version
changes), or going through
cmake.subproject(), which currently dropsinclude paths that live outside the subproject's own directory tree - a
known Meson limitation (see mesonbuild/meson#12451, #6079, #12351) that
makes it unusable for a library with external (e.g. vcpkg) dependencies.
A real
meson.buildavoids both problems.The module interface is a small addition on top since C++20 modules are
increasingly the expected way to consume a library that already supports
C++20 (
MINIO_CPP_STD=20, added in #237).Example usage
Testing
MINIO_CPP_STD=20):FILE_SET CXX_MODULESpath)subproject('minio-cpp'), full source build, not the vcpkgbinary package)
check-style.shclean (clang-format --style=Google)check-version.pypasses (no version bump in this change)MINIO_CPP_ENABLE_RDMAstill defaultsOFF; module/Meson additions don't touch it)
Summary by CodeRabbit
New Features
Build & Installation
Bug Fixes