[ML] Enforce zero warnings in debug Linux CI build by edsavage · Pull Request #3198 · elastic/ml-cpp

edsavage · 2026-09-17T01:24:26Z

Summary

Adds -DCMAKE_COMPILE_WARNING_AS_ERROR=ON to the debug Linux x86_64 CI build step
PR [ML] Fix compiler warnings across the codebase #2985 reduced compiler warnings to zero on this configuration; this makes any regression an immediate build failure rather than silent accumulation
Scoped to the existing debug PR build step only, the release and cross-compile configurations are unchanged

Test plan

Verify the debug Linux build step still passes with zero warnings
Introduce a deliberate warning locally to confirm the build fails with a clear error

Add -DCMAKE_COMPILE_WARNING_AS_ERROR=ON to the debug Linux x86_64 CI build step. PR elastic#2985 reduced compiler warnings to zero on this configuration; this flag makes any regression an immediate build failure rather than a silent accumulation. The flag is scoped to the existing debug PR build step only, leaving release and cross-compile configurations unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

elasticsearchmachine · 2026-09-17T01:44:02Z

Pinging @elastic/ml-core (Team:ML)

…t me)

…t (revert me)" This reverts commit 1eab514.

Copilot

🟢 Approval recommended

The remaining feedback is a non-blocking test-coverage nit.

Pull request overview

Enforces zero compiler warnings in the Linux x86_64 debug CI build.

Changes:

Enables CMAKE_COMPILE_WARNING_AS_ERROR=ON for the debug build.
Leaves release and cross-compile configurations unchanged.

File summaries

File	Summary
`.buildkite/pipelines/build_linux.json.py`	Adds warning-as-error enforcement; a non-blocking nit requests coverage verifying the flag remains scoped correctly.

Review details

Suppressed comments (1)

.buildkite/pipelines/build_linux.json.py:240

The existing pipeline-generator tests only assert that the debug step's label is present; they never inspect its environment. A later edit could remove this warning-as-error flag (or apply it to the wrong step) while all current tests still pass. Add coverage that finds the debug build step and asserts this flag is present, while the release/cross-compile steps remain unchanged.

              "CMAKE_FLAGS": "-DCMAKE_TOOLCHAIN_FILE=cmake/linux-x86_64.cmake -DML_FAST_DEBUG=ON -DCMAKE_UNITY_BUILD=ON -DML_PCH=ON -DCMAKE_COMPILE_WARNING_AS_ERROR=ON",

Files reviewed: 1/1 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The top-level build applies ml-cpp's strict warning flags (including -Wconversion) to every target via add_compile_options(${ML_CXX_FLAGS}), and the debug Linux CI build enables CMAKE_COMPILE_WARNING_AS_ERROR=ON (elastic#3198). Both leak into any vendored third-party sources compiled from the 3rd_party subtree, whose own diagnostics we do not control, so a warning there fails the whole build. Disable warnings-as-errors for that subtree only, matching the existing save/restore pattern in the block. ml-cpp's own targets are unaffected and still treat warnings as errors. Co-authored-by: Cursor <cursoragent@cursor.com>

ml-cpp applies its strict warning flags (including -Wconversion) to every target via add_compile_options(${ML_CXX_FLAGS}), and the debug Linux CI build enables CMAKE_COMPILE_WARNING_AS_ERROR=ON (elastic#3198). This surfaces as build failures in two distinct places once third-party dependencies are actually compiled from the 3rd_party subtree: 1. Compiling the third-party sources themselves, whose own warnings are promoted to errors. 2. Compiling our own targets that include third-party headers, where the warnings originate in those headers (pulled in via -I, not -isystem). Fix both without weakening linting of ml-cpp's own code: 1. Disable warnings-as-errors for the third-party subtree only, matching the existing save/restore pattern in the block. 2. Add the third-party include directories as SYSTEM so consuming targets compile those headers as system headers. ml-cpp's own targets keep -Wconversion -Werror. Co-authored-by: Cursor <cursoragent@cursor.com>

ml-cpp applies its strict warning flags (including -Wconversion) to every target via add_compile_options(${ML_CXX_FLAGS}), and the debug Linux CI build enables CMAKE_COMPILE_WARNING_AS_ERROR=ON (elastic#3198). Once third-party dependencies are actually compiled from the 3rd_party subtree this breaks the build in two places: 1. Compiling the third-party sources themselves, whose own warnings are promoted to errors. Fixed by disabling warnings-as-errors for the third-party subtree only (matching the existing save/restore pattern), so their warnings stay visible but non-fatal. 2. Compiling our own MlSandbox target, which includes Sandbox2/Abseil/ protobuf headers. Those headers are pulled in as normal (-I) includes - deliberately not -isystem, so they stay ahead of PyTorch's bundled protobuf in the search path - and their value-changing conversions trip -Werror. Fixed by downgrading only the conversion diagnostics from errors to warnings for MlSandbox; all other warning classes stay fatal. ml-cpp's own code keeps -Wconversion -Werror everywhere else. Co-authored-by: Cursor <cursoragent@cursor.com>

ml-cpp applies its strict warning flags (including -Wconversion, -Wunused-parameter, ...) to every target via add_compile_options(${ML_CXX_FLAGS}), and the debug Linux CI build enables CMAKE_COMPILE_WARNING_AS_ERROR=ON (elastic#3198). Once third-party dependencies are actually compiled from the 3rd_party subtree this breaks the build in two places: 1. Compiling the third-party sources themselves, whose own warnings are promoted to errors. Fixed by disabling warnings-as-errors for the third-party subtree only (matching the existing save/restore pattern), so their warnings stay visible but non-fatal. 2. Compiling our own targets that include Sandbox2/Abseil/protobuf headers (MlSandbox and its unit tests). Those headers are pulled in as normal (-I) includes - deliberately not -isystem, so they stay ahead of PyTorch's bundled protobuf in the search path - and are not warning-clean (conversion, unused-parameter, ...). Fixed by turning off warnings-as-errors on just those consuming targets, which is order-safe and covers every warning class the third-party headers may trip. ml-cpp's own code keeps warnings-as-errors everywhere else. Co-authored-by: Cursor <cursoragent@cursor.com>

ml-cpp applies its strict warning flags (including -Wconversion, -Wunused-parameter, ...) to every target via add_compile_options(${ML_CXX_FLAGS}), and the debug Linux CI build enables CMAKE_COMPILE_WARNING_AS_ERROR=ON (#3198). Once third-party dependencies are actually compiled from the 3rd_party subtree this breaks the build in two places: 1. Compiling the third-party sources themselves, whose own warnings are promoted to errors. Fixed by disabling warnings-as-errors for the third-party subtree only (matching the existing save/restore pattern), so their warnings stay visible but non-fatal. 2. Compiling our own targets that include Sandbox2/Abseil/protobuf headers (MlSandbox and its unit tests). Those headers are pulled in as normal (-I) includes - deliberately not -isystem, so they stay ahead of PyTorch's bundled protobuf in the search path - and are not warning-clean (conversion, unused-parameter, ...). Fixed by turning off warnings-as-errors on just those consuming targets, which is order-safe and covers every warning class the third-party headers may trip. ml-cpp's own code keeps warnings-as-errors everywhere else. Co-authored-by: Cursor <cursoragent@cursor.com>

elasticsearchmachine added the v9.6.0 label Sep 17, 2026

edsavage added >build :ml labels Sep 17, 2026

[ML] Temporary: deliberate warning to test -Werror enforcement (rever…

1eab514

…t me)

edsavage force-pushed the fix/werror-debug-linux-ci branch from 884b942 to 1eab514 Compare September 17, 2026 03:28

Revert "[ML] Temporary: deliberate warning to test -Werror enforcemen…

7818f32

…t (revert me)" This reverts commit 1eab514.

edsavage requested a lite review from Copilot September 17, 2026 03:46

Copilot started reviewing on behalf of edsavage September 17, 2026 03:46 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

valeriy42 self-requested a review September 21, 2026 06:46

valeriy42 approved these changes Sep 21, 2026

View reviewed changes

edsavage merged commit 045d226 into elastic:main Sep 21, 2026
24 checks passed

This was referenced Sep 22, 2026

[ML] Typed routing behind the established controller protocol #3188

Merged

[ML] Do not treat third-party warnings as errors #3208

Merged

Navigation Menu

[ML] Enforce zero warnings in debug Linux CI build - #3198

[ML] Enforce zero warnings in debug Linux CI build#3198
edsavage merged 3 commits into
elastic:mainfrom
edsavage:fix/werror-debug-linux-ci

edsavage commented Sep 17, 2026 •

edited

Loading

Uh oh!

elasticsearchmachine commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

edsavage commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

elasticsearchmachine commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

edsavage commented Sep 17, 2026 •

edited

Loading