-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Do not print errors when repository rules are interrupted #20023
Conversation
When the user interrupts a build with Ctrl+C during the download or extraction of a file, no Starlark error should be printed. This is achieved by propagating `InterruptedException`s instead of failing the repository rule with an `IOException`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to trust that you did some testing, as I'm not really sure how the interaction works here :)
I tested interrupting downloads and extractions and they both interrupted cleanly. Based on the contract of I would still suggest not to cherry pick this into 7.0.0. |
Another case of #18629. |
@lberki While taking a deeper look at how Skyframe handles interrupts, I noticed that it seems to drop the Skyframe compute state only if a SkyFunction errors out (https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java;l=479;drc=69359279d4ff40b25b6a69219f0576bae19f0297;bpv=1;bpt=1?q=Skyfunc&ss=bazel%2Fbazel), but not when it is interrupted. Do you know what the precise invariant on state maintained through interrupts is? |
I don't, not off the bat. Maybe @justinhorvitz does? |
I think we haven't thought to clear the compute state on interrupt because an interrupted skyframe thread typically means that the whole evaluation is being interrupted, and the state cache is per-evaluation. Do you have a case where there's an interrupt but the overall evaluation proceeds? Tagging @haxorz because he was more involved in the state cache implementation. |
If that's the case then all is well: I was worried about state being preserved across evaluations, and with it the possibly invalid contents of the external repository. Just for the sake of my own education, could you point me to where this per-evaluation state clearing happens? |
|
@bazel-io flag |
@fmeum You can now do |
@bazel-io fork 7.1.0 |
When the user interrupts a build with Ctrl+C during the download or extraction of a file, no Starlark error should be printed. This is achieved by propagating `InterruptedException`s instead of failing the repository rule with an `IOException`. Closes bazelbuild#20023. PiperOrigin-RevId: 578869315 Change-Id: I9dd901ed87ed00c239599877816c6836688c1e16
…0662) When the user interrupts a build with Ctrl+C during the download or extraction of a file, no Starlark error should be printed. This is achieved by propagating `InterruptedException`s instead of failing the repository rule with an `IOException`. Closes #20023. Commit a2d3f20 PiperOrigin-RevId: 578869315 Change-Id: I9dd901ed87ed00c239599877816c6836688c1e16 Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im> Co-authored-by: Xùdōng Yáng <wyverald@gmail.com> Co-authored-by: Ian (Hee) Cha <heec@google.com>
The changes in this PR have been included in Bazel 7.1.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc. |
When the user interrupts a build with Ctrl+C during the download or extraction of a file, no Starlark error should be printed. This is achieved by propagating
InterruptedException
s instead of failing the repository rule with anIOException
.