[edsavage]

Summary

The debug Linux CI build (which enables -DCMAKE_COMPILE_WARNING_AS_ERROR=ON, #3198) fails once third-party dependencies are actually compiled from the 3rd_party subtree. ml-cpp applies its strict warning flags (-Wconversion, -Wunused-parameter, ...) to every target via add_compile_options(${ML_CXX_FLAGS}), and these leak into third-party code. This surfaces in two distinct places:

1. Compiling the third-party sources themselves — their own value-changing conversions are promoted to errors, e.g.:

   ... error: conversion from 'int64_t' to 'double' may change value [-Werror=conversion]
   cc1plus: all warnings being treated as errors
   

2. Compiling our own targets that include Sandbox2/Abseil/protobuf headers (MlSandbox and its unit tests ml_test_sandbox). Those headers are not warning-clean (conversion, unused-parameter, ...) and, under -Werror, fail the build of our own targets.

Fix

Two complementary changes, neither of which weakens linting of ml-cpp's own code:

1. 3rd_party/CMakeLists.txt: disable warnings-as-errors for the third-party subtree only (CMAKE_COMPILE_WARNING_AS_ERROR OFF), matching the block's existing save/restore of BUILD_TESTING, BUILD_SHARED_LIBS, and CMAKE_UNITY_BUILD. Third-party warnings stay visible but non-fatal.
2. lib/sandbox/CMakeLists.txt + lib/sandbox/unittest/CMakeLists.txt: set COMPILE_WARNING_AS_ERROR OFF on just the two ml-cpp targets that include third-party sandbox headers. This is order-safe and covers every warning class those headers may trip (rather than chasing individual -Wno-error=<class> flags). All other ml-cpp targets keep -Werror.

Why not SYSTEM includes?

Marking the third-party include directories SYSTEM (so consumers get -isystem) was tried and rejected: it demotes the vendored protobuf/Abseil source includes from -I to -isystem, which places them after PyTorch's bundled protobuf in the search path. Protobuf then compiles against a mix of its own generated headers and PyTorch's older protobuf, producing version-mismatch errors (FeatureSetDefaults not declared, PROTOBUF_NODISCARD does not name a type, ...). Keeping the includes as -I avoids this collision entirely.

Test plan

• CI: the debug Linux build compiles the third-party sources, MlSandbox, and ml_test_sandbox without failing on their warnings.
• Verify ml-cpp's own targets still treat warnings as errors in the debug build (unchanged behaviour).
• Introduce a (temporary) change that emits a warning in 3rd party code. The build should still succeed. (Verified on an earlier run: a probe target and the real Abseil/Sandbox2 sources emitted -Wconversion warnings and the build proceeded past them.)

Note: the modified blocks are Linux-only, so this is validated by CI rather than my local macOS build.

[elasticsearchmachine]

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

[Copilot]

Copilot review overview

🔵 Needs a closer look

Two moderate findings remain regarding target-wide suppression of warning enforcement.

Review effort: Lite
Findings: None

What changed in this PR

This PR prevents third-party warnings from failing Linux debug builds while preserving strict warning handling elsewhere.

Changes:

• Disable warnings-as-errors for third-party dependencies.
• Disable it for MlSandbox and ml_test_sandbox.

File	Summary
lib/​sandbox/​unittest/​CMakeLists.txt	Disables warning failures for the sandbox test target; this also affects ml-cpp test sources. Moderate finding (1 vote).
lib/​sandbox/​CMakeLists.txt	Disables warning failures for MlSandbox; this also affects ml-cpp production sources. Moderate finding (1 vote).
3rd_party/​CMakeLists.txt	Disables and restores warnings-as-errors for third-party targets.

---

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