Treat warnings in Bazel builds as errors #1779
Conversation
There was a problem hiding this comment.
+@jaroeichler Follow-up on #1778 (review)
Reviewable status: 0 of 1 LGTMs obtained, and 0 of 1 files reviewed, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / macos-15, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-15, Cargo Dev / ubuntu-24.04, Coverage, Installation / macos-14, Installation / macos-15, Installation / ubuntu-22.04, Installation / ubuntu-24.04, Local / bazel / ubuntu-24.04, Local / lre-cc / ubuntu-24.04, Local / lre-rs / macos-15, Local / lre-rs / ubuntu-24.04, NativeLink.com Cloud / Remote Cache / macos-15, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / lre-cc / xlarge-ubuntu-24.04, Remote / lre-rs / xlarge-ubuntu-24.04, asan / ubuntu-24.04, buildstream, integration-tests (24.04), macos-15, pre-commit-checks, ubuntu-24.04, ubuntu-24.04 / stable, vale, windows-2022 / stable (waiting on @jaroeichler)
aaronmondal
force-pushed
the
fix-bazel-clippy
branch
from
147a8a7 to
513ae15
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 1 LGTMs obtained, and 0 of 2 files reviewed, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / macos-15, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-15, Cargo Dev / ubuntu-24.04, Coverage, Installation / macos-14, Installation / macos-15, Installation / ubuntu-22.04, Installation / ubuntu-24.04, Local / bazel / ubuntu-24.04, Local / lre-cc / ubuntu-24.04, Local / lre-rs / macos-15, Local / lre-rs / ubuntu-24.04, NativeLink.com Cloud / Remote Cache / macos-15, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / lre-cc / xlarge-ubuntu-24.04, Remote / lre-rs / xlarge-ubuntu-24.04, asan / ubuntu-24.04, buildstream, integration-tests (24.04), macos-15, pre-commit-checks, ubuntu-24.04, ubuntu-24.04 / stable, vale, windows-2022 / stable (waiting on @jaroeichler)
nativelink-util/src/fs.rs line 215 at r1 (raw file):
let (permit, os_file) = call_with_permit(move |permit| { let mut os_file = std::fs::File::open(&path).err_tip(|| format!("Could not open {}", path.display()))?;
FYI these hare hotfixes for https://github.com/TraceMachina/nativelink/actions/runs/15086221164/job/42409066332?pr=1779.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained, and all files reviewed, and pending CI: asan / ubuntu-24.04
There was a problem hiding this comment.
Bazel's incremental caching doesn't rerun actions that only emit warnings.
I presume there's no easy(ish) way to make this be the case?
Reviewable status: 1 of 1 LGTMs obtained, and all files reviewed, and pending CI: asan / ubuntu-24.04
There was a problem hiding this comment.
I presume there's no easy(ish) way to make this be the case?
Not that I'm aware of. I believe this is the same issue that requires C++ builds to run with -Werror (warnings-as-errors) so that they don't get lost in the cache.
Reviewable status: 1 of 1 LGTMs obtained, and all files reviewed, and pending CI: asan / ubuntu-24.04
aaronmondal
force-pushed
the
fix-bazel-clippy
branch
from
513ae15 to
cfc7106
Compare
aaronmondal
force-pushed
the
fix-bazel-clippy
branch
from
cfc7106 to
6f93078
Compare
Bazel's incremental caching doesn't rerun actions that only emit warnings. That's inconvenient as these warnings will only be shown once during the initial invocation but not in following invocations which treat the action as successfull run. This leads to friction where a passing local build won't necessarily pass CI. We now treat all warnings as hard errors. The more natural approach to temporarily skip over warnings in Bazel is via `-k` (or `--keep_going`).
aaronmondal
force-pushed
the
fix-bazel-clippy
branch
from
6f93078 to
79ec677
Compare
|
The remaining build issues here appear to be due to rust-lang/rust-clippy#14850 and so we probably just need to set |
|
Closed in favour of #1840 |
Bazel's incremental caching doesn't rerun actions that only emit warnings. That's inconvenient as these warnings will only be shown once during the initial invocation but not in following invocations which treat the action as successfull run. This leads to friction where a passing local build won't necessarily pass CI.
We now treat all warnings as hard errors. The more natural approach to temporarily skip over warnings in Bazel is via
-k(or--keep_going).This change is