Replace "error" references in incremental compilation warnings

[ssg]

Context: #151181 (comment)

When errors are encountered during incremental compilation, rustc displays warnings, but some of the warnings use the word "error" in the messages despite that it's still a warning. This might cause devs to incorrectly assume that the build has failed even though it's not necessarily the case.

This PR replaces instances of "error"s in incremental compilation with a more common phrasing of "failed to"s, so the severity of the message is clear.

There is a small regression risk if the behavior of tooling depends on the actual wording of these messages which would be quite unlikely, I believe.

[rustbot]

r? @tiif

rustbot has assigned @tiif.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Kivooeo]

personally, i dont like the changes, it seems perfectly correct and clear to me as it is

[Kivooeo]

i guess i could elaborate on what exactly i dont i like about it

so this pr basically changes "error" with "failed to" in warnings

but what will change for a person who reads it whether it "errored while trying do something" or it "failed while trying do something"

to me word "fail" imply that some "error" already occurred before, so to me "error deleting" and "failed to delete" is absolute the same phrases

but, the "error" seems more technical correct because the real error is occurred while trying do something

[Kivooeo]

for author: it doesn't mean im blocking this or declining it, just opinion from a side

r? @ChrisDenton

[ssg]

but what will change for a person who reads it whether it "errored while trying do something" or it "failed while trying do something"

The reason I picked "failed to" was that it was already the most common phrasing in relevant messages.ftl, and I don't see any confusion you mentioned in the instances where "failed to" was used.

My greatest gripe with the existing wording is with "warning" and "error" being used next to each other, confusing the user about the severity of the problem.

Any better recommendation given the circumstances is always welcome of course.

[ChrisDenton]

I do agree that "failed" could be considered a synonym of "error" but in the context of diagnostics, "error" has a special meaning. The exact words "info", "warning", "error" are severity levels so when you have a log line that reads "warning: error ..." then I don't think it's great.

I do want to quickly loop in @estebank here since it seems we're at least somewhat deciding policy here (even if it's easily changed in the future).

[psandana]

Just my $0.02 here, but I would rather have these messages to state what happened objectively (i.e., could not delete a file), than giving it an assessment of what occurred (failed, err, etc.). Although I also prefer failed over error as this PR is doing.

Also, not for this PR, but for the meta issue in the incremental compilation warnings: Is this a retriable error? Why it is "fine" that it occurs, and we are not breaking the build after it happens?

If this is a retriable "failure", then I would like to see the warning stating this is going to be retried. So, we would have something like:

1. Success case, no warnings: Finished dev profile [unoptimized + debuginfo] target(s) in 2.78s
2. One warning, retried and worked:

   warning: could not delete file ...... . Retrying.
   Finished dev profile [unoptimized + debuginfo] target(s) in 2.78s 
   

3. One warning, retried and failed:

   warning: could not delete file ...... . Retrying.
   fatal: failed to delete file ..... .
   

I am mentioning this in case this is relevant to adjust the wording of the messages.

[ssg]

to state what happened objectively (i.e., could not delete a file),

Isn't that already the case though? Instead of "could not delete", it's "failed to delete". I don't see the distinction.

Is this a retriable error?

It's a warning that's only shown after all retries have been exhausted in the case of #151181. I'm not sure about the others.

Why it is "fine" that it occurs, and we are not breaking the build after it happens?

I think it only leaves garbage around as "-working" path is never renamed to the actual path. But, I'm not sure how it impacts incremental build itself. Perhaps, the compiler fails to utilize incremental compilation completely because of this. I'm not familiar with the inner workings of incremental compilation, so, I can't really tell at the moment.

[ChrisDenton]

Yes, it's "fine" to leave old incremental files lying around (drive space aside). It would be a compiler bug if it reused them when it shouldn't (admittedly we have had such bugs in the past).

[rust-bors]

☔ The latest upstream changes (presumably #152075) made this pull request unmergeable. Please resolve the merge conflicts.

[ssg]

This PR isn't applicable anymore after 4cacfc0 - so, closing it. Let's see how the new changes will affect UX.