ci: add Clang compilation warning check to build workflow#1386
ci: add Clang compilation warning check to build workflow#1386messere1 wants to merge 2 commits into
Conversation
Add a new 'build-clang' job to .github/workflows/build.yml that mirrors the existing GCC warning-check job but builds with CC=clang and a set of Clang-supported -Werror warning flags. This mirrors the GCC-side mechanism introduced in IvorySQL#999 for parity, as requested in issue IvorySQL#1221. The new job runs in parallel with the existing GCC build job so it does not increase overall CI latency. Closes IvorySQL#1221.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a ChangesClang CI Build
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Pull request overview
This PR updates the CI build workflow to add a parallel Clang build job, intended to provide compiler-diversity warning coverage alongside the existing GCC-based build and -Werror checks.
Changes:
- Added a new
build-clangjob to run./configurewithCC=clangand compile with a Clang-oriented-Werrorflag set. - Extended workflow dependencies installation to include
clangfor the new job.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --with-ossp-uuid --with-libxml --with-libxslt --with-perl \ | ||
| --with-icu --with-libnuma --enable-injection-points --enable-nls | ||
| - name: compile | ||
| run: make CFLAGS="$CFLAGS -Werror=unused-value -Werror=unused-but-set-variable -Werror=missing-prototypes -Werror=unused-variable -Werror=maybe-uninitialized -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Werror=incompatible-pointer-types -Werror=format -Werror=empty-body -Werror=parentheses -Werror=pointer-sign -Werror=unsequenced" |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: dependancy - linux |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/build.yml (2)
59-59: 📐 Maintainability & Code Quality | 🔵 TrivialConsider setting
CXX=clang++alongsideCC=clangWhile PostgreSQL is primarily C, explicitly setting
CXX=clang++ensures any C++ components or extensions are also compiled with Clang, maintaining compiler consistency across the build.🤖 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 @.github/workflows/build.yml at line 59, The build workflow currently sets only CC=clang in the configure step, so C++ compilation may still use a different default compiler. Update the configure invocation in the build workflow to set CXX=clang++ alongside CC=clang, keeping compiler selection consistent for any C++ components or extensions referenced by the configure/build process.
50-51: 📐 Maintainability & Code Quality | 🔵 TrivialConsider pinning the Clang version for reproducibility
The GCC job pins
gcc-14/g++-14and sets up alternatives, but the Clang job installs the unversionedclangpackage. As theubuntu-latestimage updates, the Clang version may change, potentially introducing new warnings or behavioral shifts. Consider installing a specific version (e.g.,clang-18) for consistency.🤖 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 @.github/workflows/build.yml around lines 50 - 51, The Clang setup in the build workflow is unpinned, so the version can drift on ubuntu-latest. Update the Clang installation in the workflow job that installs build tools to use a specific version package (for example, a versioned clang package) and make sure the same version is used consistently in the Clang job alongside the existing GCC pinning approach.
🤖 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 @.github/workflows/build.yml:
- Line 66: The clang build configuration in the build workflow is using the
GCC-only warning group -Werror=maybe-uninitialized, which causes an
unknown-warning diagnostic under clang. Update the CFLAGS definition in the
workflow job so the clang path does not include this flag, and use the
clang-compatible equivalent warning option (or omit it entirely) while keeping
the rest of the compiler warnings unchanged.
---
Nitpick comments:
In @.github/workflows/build.yml:
- Line 59: The build workflow currently sets only CC=clang in the configure
step, so C++ compilation may still use a different default compiler. Update the
configure invocation in the build workflow to set CXX=clang++ alongside
CC=clang, keeping compiler selection consistent for any C++ components or
extensions referenced by the configure/build process.
- Around line 50-51: The Clang setup in the build workflow is unpinned, so the
version can drift on ubuntu-latest. Update the Clang installation in the
workflow job that installs build tools to use a specific version package (for
example, a versioned clang package) and make sure the same version is used
consistently in the Clang job alongside the existing GCC pinning approach.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54dadd95-5cef-4aee-98a2-c75dca3ca981
📒 Files selected for processing (1)
.github/workflows/build.yml
|
Thanks for your patch! |
Per review feedback on PR IvorySQL#1386: - Replace GCC-only -Werror=maybe-uninitialized with Clang's -Werror=uninitialized (which enables -Wsometimes-uninitialized) - Pass CXX=clang++ to configure so C++ compilation also uses Clang - Pin the Clang version to clang-18 via update-alternatives, mirroring how the GCC job pins gcc-14, to avoid toolchain drift on ubuntu-latest Assisted-by: Claude Code:claude-fable-5 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Related Issue
Closes #1221
Motivation
Issue #1221 requests adding a Clang compilation warning check to the CI workflow, mirroring the GCC-side mechanism introduced in #999. Currently
.github/workflows/build.ymlonly builds with GCC 14 and GCC-specific-Werrorflags. This PR adds a parallel Clang build job for compiler-diversity warning coverage.Changes
Add a new
build-clangjob to.github/workflows/build.ymlthat:clang(ubuntu-latest default version) alongside the existing build dependencies../configurewithCC=clangand the same feature flags as the GCC job.makewith a set of Clang-supported-Werrorwarning flags.The flag list intentionally omits GCC-only flags (
-Wshadow=compatible-local,-Werror=missing-variable-declarations) that Clang does not support, and instead includes Clang-equivalent diagnostics that catch the same classes of bugs (unused values/variables, missing prototypes, implicit declarations, type mismatches, format strings, etc.).The new job runs in parallel with the existing
build(GCC) job, so it does not increase overall CI latency.How Verified
CC=clangand the warning flag set.Summary by CodeRabbit