[CI] Disable C++20 module build to fix CI failures#91
[CI] Disable C++20 module build to fix CI failures#91
Conversation
The current CI configuration with OPFLOW_ENABLE_MODULE=ON is causing build failures due to MPI_Comm type conversion issues in module builds. This is blocking all PRs including documentation-only changes. This commit disables module builds in CI as a temporary measure to unblock PRs. Module support should be fixed and re-enabled in a dedicated PR that includes: - Fixing MPI_Comm type issues in Solver classes - Testing module builds with both MPI ON and OFF - Verifying compatibility across GCC 15 and Clang 21 Related issue: Module builds fail with -fpermissive errors Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe changes modify the CMake build configuration to conditionally manage compiler visibility and warning flags. A macro definition's visibility changes from PUBLIC to PRIVATE, compiler-specific warning options are introduced with conditional scope based on module enablement, and a C++20 comment updates to C++26. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66008bb5ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Remove redundant -B build flag (working-directory is already ./build) - Add .. as source directory argument - Change cmake --build from 'build' to '.' to use current directory This fixes: CMake Error: The source directory does not appear to contain CMakeLists.txt Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
When OPFLOW_ENABLE_MODULE=OFF, the opflow target is an INTERFACE library. CMake requires that INTERFACE libraries only use INTERFACE keyword when setting properties like target_include_directories, target_link_directories, and target_link_libraries. This fixes: - target_include_directories may only set INTERFACE properties on INTERFACE targets - target_link_directories may only set INTERFACE properties on INTERFACE targets - INTERFACE library can only be used with the INTERFACE keyword Changed all PUBLIC to INTERFACE in external/CMakeLists.txt for TecIO setup. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Import CMake configuration from fix-module branch that correctly handles
both module (PUBLIC scope) and header-only (INTERFACE scope) builds.
Key changes:
- Introduce OPFLOW_TARGET_SCOPE variable (PUBLIC for module, INTERFACE for header-only)
- Use ${OPFLOW_TARGET_SCOPE} throughout instead of hardcoded PUBLIC/INTERFACE
- Update target_sources with proper BASE_DIRS for header-only build
- Change to C++26 in header-only mode (was C++20)
- Add -Wall -Werror compiler flags with -Wno-psabi
- Fix all target_link_libraries calls to use correct scope
This resolves:
- target_sources/target_include_directories/target_link_libraries
INTERFACE library property errors
- Allows building with OPFLOW_ENABLE_MODULE=OFF
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Define OPFLOW_TARGET_SCOPE in main CMakeLists.txt right after OPFLOW_ENABLE_MODULE option - Remove duplicate definition from src/CMakeLists.txt - Ensures variable is available before use in src/ and external/ This fixes: target_include_directories called with invalid arguments Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem
The current CI configuration with
OPFLOW_ENABLE_MODULE=ONis causing build failures:These errors occur in multiple Solver classes when building with modules enabled and MPI support. This is blocking all PRs, including documentation-only changes like PR #88.
Solution
This PR temporarily disables module builds in CI by setting
-DOPFLOW_ENABLE_MODULE=OFF.Changes
OPFLOW_ENABLE_MODULE=OFFin.github/workflows/Build.ymlFollow-up Work
Module support should be fixed and re-enabled in a dedicated PR that includes:
MPI=ONandMPI=OFFTest Plan
Related Issues
🤖 Generated with Claude Code
Summary by CodeRabbit