-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Incremental bazel run
is incompatible with BwoB + --noexperimental_check_output_files
#20843
Labels
P1
I'll work on this now. (Assignee required)
team-Remote-Exec
Issues and PRs for the Execution (Remote) team
type: bug
Comments
19 tasks
tjgq
added
P1
I'll work on this now. (Assignee required)
type: bug
team-Remote-Exec
Issues and PRs for the Execution (Remote) team
labels
Jan 10, 2024
I would vote for (1), but I understand that the effort might not be perceived as worth it at this time. The thing I like about (1) is that it actually makes |
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Jan 11, 2024
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; it would be 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 would theoretically slow things down if output checking is truly unnecessary, but I doubt that would cause a significant regression in practice. Fixes bazelbuild#20843.
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Jan 11, 2024
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.
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Jan 11, 2024
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.
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Jan 11, 2024
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.
bazel-io
pushed a commit
to bazel-io/bazel
that referenced
this issue
Jan 12, 2024
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
meteorcloudy
pushed a commit
that referenced
this issue
Jan 15, 2024
…e bytes. (#20881) 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 Co-authored-by: Tiago Quelhas <[email protected]>
A fix for this issue has been included in Bazel 7.0.1 RC2. Please test out the release candidate and report any issues as soon as possible. Thanks! |
bazel-io
pushed a commit
to bazel-io/bazel
that referenced
this issue
Jan 22, 2024
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
github-merge-queue bot
pushed a commit
that referenced
this issue
Jan 27, 2024
…e bytes. (#20988) 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 Co-authored-by: Tiago Quelhas <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
P1
I'll work on this now. (Assignee required)
team-Remote-Exec
Issues and PRs for the Execution (Remote) team
type: bug
This repro was gently provided by @purkhusid in #6862:
https://github.com/purkhusid/bazel_runfiles_repro/blob/main/reproduce.sh
In short: target T1 has a dependency on T2, which has associated runfiles. First populate a disk cache with the result of building T1. Then, starting clean, first build T1 against the disk cache without the bytes (
--remote_download_minimal
), then run it. The result is that the runfiles for T2 are missing at runtime (as in, the runfile symlinks dangle). Disabling output checking (--noexperimental_check_output_files
) is required to reproduce.The issue is that building without the bytes works by lying to the Skyframe machinery about the existence of files on disk; instead, the files are materialized when either (1) a consuming action prefetches them, or (2) a top-level target forces its outputs to be produced.
bazel run
is not a build action, so it can't rely on prefetching; instead, it relies on the target being considered top-level even in MINIMAL mode (see RemoteOutputChecker). Disabling output checking prevents Skyframe from doing the dirtyness check on the outputs and realize they're not actually there, which would cause the target to be rebuilt.There are two possible fixes:
bazel run
--noexperimental_check_output_files
a no-op forbazel run
commands.(1) is more correct but much trickier to implement, since all of the normal action-running machinery has already been torn down by the time the command is run; it would likely involve reimplementing
bazel run
as a special action injected into the build graph. I think the pragmatic thing to do is implement (2) until there's more evidence that (1) is worthwhile. (This will makebazel run
slightly slower in cases where output checking is truly unnecessary, but it's not obvious that that would lead to a major regression; we're also covered by the fact that--noexperimental_check_output_files
is, well, experimental and subject to change behavior at any time.)The text was updated successfully, but these errors were encountered: