Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.0.1] Force output checking for incremental run commands without the bytes. #20881

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

bazel-io
Copy link
Member

When building without the bytes, outputs are only materialized when either (1) an action prefetches them, or (2) they've been explicitly requested. When both --remote_download_minimal and --noexperimental_check_output_files are set, this presents a problem for a build command followed by a run command for the same target: the build command won't download the outputs and the run command won't check for their presence, proceeding without them. A non-incremental run command works, because the outputs are considered top-level even in minimal mode.

This is only a problem for a run command because it exists outside of action execution, and doesn't get a chance to prefetch inputs; it would have been better to implement the run command by injecting a specially crafted action into the build graph, but that will have to wait for another day. The present fix might theoretically slow things down if output checking is truly unnecessary, but I doubt that would cause a significant regression in practice.

Fixes #20843.

Closes #20853.

Commit 2f899ef

PiperOrigin-RevId: 597909909
Change-Id: I66aedef4994fbda41fe5378c80940fa8ba637bfd

When building without the bytes, outputs are only materialized when either (1) an action prefetches them, or (2) they've been explicitly requested. When both --remote_download_minimal and --noexperimental_check_output_files are set, this presents a problem for a build command followed by a run command for the same target: the build command won't download the outputs and the run command won't check for their presence, proceeding without them. A non-incremental run command works, because the outputs are considered top-level even in minimal mode.

This is only a problem for a run command because it exists outside of action execution, and doesn't get a chance to prefetch inputs; it would have been better to implement the run command by injecting a specially crafted action into the build graph, but that will have to wait for another day. The present fix might theoretically slow things down if output checking is truly unnecessary, but I doubt that would cause a significant regression in practice.

Fixes bazelbuild#20843.

Closes bazelbuild#20853.

PiperOrigin-RevId: 597909909
Change-Id: I66aedef4994fbda41fe5378c80940fa8ba637bfd
@bazel-io bazel-io requested a review from a team as a code owner January 12, 2024 20:02
@bazel-io bazel-io added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Jan 12, 2024
@bazel-io bazel-io requested a review from coeuvre January 12, 2024 20:02
@tjgq tjgq added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jan 15, 2024
@meteorcloudy meteorcloudy merged commit f4da34d into bazelbuild:release-7.0.1 Jan 15, 2024
33 checks passed
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 15, 2024
iancha1992 pushed a commit that referenced this pull request Jan 18, 2024
Baseline:  d798ebd

Release Notes:

+ Consider MODULE.bazel for workspace detection in bazel.sh (#20594)
+ Auto-create deploy jars for Bazel `java_test` targets if requested (#20602)
+ `java_binary` wrapper should forward `restricted_to` (#20611)
+ Mount user-specified bind mounts before Bazel's own magic. (#20609)
+ Fix bootstrapped Bazel binary (#20612)
+ Attempt to make main repo mapping inverse more efficient (#20625)
+ Cherry-pick all presubmit.yml changes (#20733)
+ Print interactive sandboxed shell command with `--sandbox_debug` (#20734)
+ Fix two issues with --incompatible_sandbox_hermetic_tmp that manifested themselves when the output base was under /tmp (#20718)
+ Let module extensions track calls to `Label()` (#20750)
+ Add support for bind mounts under `/tmp` with hermetic tmp (#20749)
+ Fixes for Bazel's own integration tests fail locally on Linux  (#20821)
+ Fix NPE in BzlmodRepoRuleFunction (#20828)
+ Avoid emitting canonical labels into generated repos (#20840)
+ Let .bzl files record their usages of repo mapping (#20848)
+ Force output checking for incremental run commands without the bytes. (#20881)
+ Retry binding to ipv6 localhost (#20903)
+ Fix linker feature detection being performed on wrong linker (#20901)
+ Fix singlejar resource mapping for external repositories (#20904)

Acknowledgements:

This release contains contributions from many people at Google, as well as bazel.build machine account, David Ostrovsky, Fabian Meumertzheim, hvd, Siddhartha Bagaria, Tianyu Geng, Xdng Yng, Xùdōng Yáng.
copybara-service bot pushed a commit that referenced this pull request Jan 18, 2024
Baseline:  d798ebd

Release Notes:

+ Consider MODULE.bazel for workspace detection in bazel.sh (#20594)
+ Auto-create deploy jars for Bazel `java_test` targets if requested (#20602)
+ `java_binary` wrapper should forward `restricted_to` (#20611)
+ Mount user-specified bind mounts before Bazel's own magic. (#20609)
+ Fix bootstrapped Bazel binary (#20612)
+ Attempt to make main repo mapping inverse more efficient (#20625)
+ Cherry-pick all presubmit.yml changes (#20733)
+ Print interactive sandboxed shell command with `--sandbox_debug` (#20734)
+ Fix two issues with --incompatible_sandbox_hermetic_tmp that manifested themselves when the output base was under /tmp (#20718)
+ Let module extensions track calls to `Label()` (#20750)
+ Add support for bind mounts under `/tmp` with hermetic tmp (#20749)
+ Fixes for Bazel's own integration tests fail locally on Linux  (#20821)
+ Fix NPE in BzlmodRepoRuleFunction (#20828)
+ Avoid emitting canonical labels into generated repos (#20840)
+ Let .bzl files record their usages of repo mapping (#20848)
+ Force output checking for incremental run commands without the bytes. (#20881)
+ Retry binding to ipv6 localhost (#20903)
+ Fix linker feature detection being performed on wrong linker (#20901)
+ Fix singlejar resource mapping for external repositories (#20904)

Acknowledgements:

This release contains contributions from many people at Google, as well as bazel.build machine account, David Ostrovsky, Fabian Meumertzheim, hvd, Siddhartha Bagaria, Tianyu Geng, Xdng Yng, Xùdōng Yáng.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants