Change "error finalizing incremental compilation" text and emit it as a note, not a warning#154110
Conversation
The warning has no error code, so in a `-D warnings` environment, it's impossible to ignore if it consistently breaks your build. Change it to a note so it is still visible, but doesn't break the build
Remove the confusing word "error". The diagnostic is already prefixed
with a level when it is displayed, so this is redundant and possibly
confusing ("warning: error ...").
Add some help text summarizing the impact of what happened: the next
build won't be able to reuse work from the current run.
|
r? @wesleywiser rustbot has assigned @wesleywiser. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
I like new wording, but I'm wondering if that's possible to add a test for this? |
Came up with a convoluted way to test this. Make a proc macro that poisons the incremental
I'm not sure this a great idea I'll push it to the PR, though |
…e fail Use a proc macro to observe the incremental session directory and do something platform specific so that renaming the '-working' session directory during finalize_session_directory will fail. On Unix, change the permissions on the parent directory to be read-only. On Windows, open and leak a file inside the `-working` directory.
525cb88 to
0cc4946
Compare
| #[derive(Diagnostic)] | ||
| #[diag("error finalizing incremental compilation session directory `{$path}`: {$err}")] | ||
| #[diag("did not finalize incremental compilation session directory `{$path}`: {$err}")] | ||
| #[help("the next build will not be able to reuse work from this compilation")] |
There was a problem hiding this comment.
I like that this focuses the issue directly on the impact the user might care about 👍
|
@bors r+ rollup |
…r=wesleywiser Change "error finalizing incremental compilation" text and emit it as a note, not a warning As mentioned in rust-lang#151181 (comment) and rust-lang#151181 (comment) the current message could be improved: 1. Right now it displays as "warning: error ..." which is confusing (is it an error or a warning) 2. It doesn't give the user a clear indication of what the consequences are 3. The _current_ build is successful. The _next_ build might be slower The new message is now ```text note: did not finalize incremental compilation session directory ... | = help: the next build will not be able to reuse work from this compilation ``` I started a zulip thread [#t-compiler/incremental > Ergonomics of "error finalizing incremental session"](https://rust-lang.zulipchat.com/#narrow/channel/241847-t-compiler.2Fincremental/topic/Ergonomics.20of.20.22error.20finalizing.20incremental.20session.22/with/580191447)
…r=wesleywiser Change "error finalizing incremental compilation" text and emit it as a note, not a warning As mentioned in rust-lang#151181 (comment) and rust-lang#151181 (comment) the current message could be improved: 1. Right now it displays as "warning: error ..." which is confusing (is it an error or a warning) 2. It doesn't give the user a clear indication of what the consequences are 3. The _current_ build is successful. The _next_ build might be slower The new message is now ```text note: did not finalize incremental compilation session directory ... | = help: the next build will not be able to reuse work from this compilation ``` I started a zulip thread [#t-compiler/incremental > Ergonomics of "error finalizing incremental session"](https://rust-lang.zulipchat.com/#narrow/channel/241847-t-compiler.2Fincremental/topic/Ergonomics.20of.20.22error.20finalizing.20incremental.20session.22/with/580191447)
Rollup of 7 pull requests Successful merges: - #152457 (Pass -pg to linker when using -Zinstrument-mcount) - #154031 (Remove divergence check from check_expr_array) - #154418 (move many tests out of `ui/unsafe`) - #153662 (Suggest fully qualified path on method name collision) - #153675 (simd_add/sub/mul/neg: document overflow behavior) - #154110 (Change "error finalizing incremental compilation" text and emit it as a note, not a warning) - #154430 (Create GPU target notification group)
@bors r- |
|
This pull request was unapproved. This PR was contained in a rollup (#154440), which was unapproved. |
|
ok, #154440 (comment) makes sense: we need the proc macro compiled for the host platform, not the target.
|
The test needs proc-macros to function
|
@bors try jobs=test-various |
|
@lambdageek: 🔑 Insufficient privileges: not in try users |
|
@wesleywiser PTAL (and re-run "test-various"?) |
|
@bors try jobs=test-various |
Change "error finalizing incremental compilation" text and emit it as a note, not a warning try-job: test-various
This comment has been minimized.
This comment has been minimized.
|
@bors r+ |
Rollup of 9 pull requests Successful merges: - #154357 (uefi: extend comment for TcpStream Send impl) - #154410 (Clean up the API for opening/checking incremental-compilation files ) - #154081 (format safety doc of Rc/Arc::from_raw/from_raw_in) - #154110 (Change "error finalizing incremental compilation" text and emit it as a note, not a warning) - #154196 (Make `Ipv6Addr::multicast_scope()` exhaustive) - #154221 (`vec::as_mut_slice()`: use lowercase "isize" in safety comment) - #154234 (Use common Timestamp impl in Hermit (attempt 2)) - #154396 (chore(deps): update rust crate tar to v0.4.45) - #154488 (Revert "Unstable book options parser")
Rollup of 9 pull requests Successful merges: - #154357 (uefi: extend comment for TcpStream Send impl) - #154410 (Clean up the API for opening/checking incremental-compilation files ) - #154081 (format safety doc of Rc/Arc::from_raw/from_raw_in) - #154110 (Change "error finalizing incremental compilation" text and emit it as a note, not a warning) - #154196 (Make `Ipv6Addr::multicast_scope()` exhaustive) - #154221 (`vec::as_mut_slice()`: use lowercase "isize" in safety comment) - #154234 (Use common Timestamp impl in Hermit (attempt 2)) - #154396 (chore(deps): update rust crate tar to v0.4.45) - #154488 (Revert "Unstable book options parser")
Rollup merge of #154110 - lambdageek:fix/incr-compile-note, r=wesleywiser Change "error finalizing incremental compilation" text and emit it as a note, not a warning As mentioned in #151181 (comment) and #151181 (comment) the current message could be improved: 1. Right now it displays as "warning: error ..." which is confusing (is it an error or a warning) 2. It doesn't give the user a clear indication of what the consequences are 3. The _current_ build is successful. The _next_ build might be slower The new message is now ```text note: did not finalize incremental compilation session directory ... | = help: the next build will not be able to reuse work from this compilation ``` I started a zulip thread [#t-compiler/incremental > Ergonomics of "error finalizing incremental session"](https://rust-lang.zulipchat.com/#narrow/channel/241847-t-compiler.2Fincremental/topic/Ergonomics.20of.20.22error.20finalizing.20incremental.20session.22/with/580191447)
|
The newly-added run-make test works very hard to trigger a real rename failure, via a custom proc-macro and some platform-specific code. But for the sake of testing diagnostics I wonder if it might have been more appropriate to instead add a perma-unstable compiler flag that simply fakes an error in the appropriate place. |
|
By the way, I originally asked for the test to be added just to see this diagnostic in action. However, looking at the test, I can't see any output, such as .stderr. How is this test testing this diagnostic? |
|
Looks like there are some hardcoded non-blessable assertions in |
As mentioned in #151181 (comment) and #151181 (comment) the current message could be improved:
The new message is now
I started a zulip thread #t-compiler/incremental > Ergonomics of "error finalizing incremental session"