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

Fixes error count display is different when there's only one error left #114759

Closed
wants to merge 12 commits into from

Conversation

adrianEffe
Copy link
Contributor

What does this PR try to resolve?

Relates to Cargo issue #12390 and the intention is to align rustc and cargo.
Related cargo PR #12484

When there's only 1 error left, the number 1 appears in the output so that it scans the same as the output when there's more than 1 error, so:

error: could not compile `crate` (lib test) due to 1 previous error

instead of the current:

error: could not compile `crate` (lib test) due to a previous error

@rustbot
Copy link
Collaborator

rustbot commented Aug 12, 2023

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 12, 2023
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Aug 12, 2023

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

The Miri subtree was changed

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Aug 14, 2023

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

@Noratrieb
Copy link
Member

thank you for your PR. I think it's likely that your PR will not be accepted, simply due to being extremely hard to review (because the diff is so huge). This is especially conflict heavy and you'll probably have a bad time just keeping up with conflicts.

I think the way to do this change (which is a good change imo) is to have a trusted member from the compiler (contributors) team do the change and then quickly get it reviewed (without reviewing the entire diff, trusting that the author didn't make big mistakes). But I'm not sure whether this would be a good solution either.

I currently don't have much time for Rust so I definitely can't help here.

Just trying to set expectations^^

@adrianEffe
Copy link
Contributor Author

Hi @Nilstrieb, thanks for your comment 🙏.
Totally agree, I was waiting for someone to acknowledge this pr really, as you said it doesn't make much sense for me to keep up with changes.
It would be great if someone from the compiler team is willing to help with this, goes without saying I'm willing to help see this through however I can. ☺️

@wesleywiser
Copy link
Member

r? wesleywiser

@rustbot rustbot assigned wesleywiser and unassigned cjgillot Sep 28, 2023
@WaffleLapkin
Copy link
Member

@wesleywiser fyi, I'm willing to help with this (either by re-blessing the tests as a trusted contributor as suggested by @Nilstrieb, or by reviewing you, if you want to do this, etc)

@apiraino
Copy link
Contributor

Given the time elapsed since your question @WaffleLapkin I can safely assume that you can take on the review of this one. I'll reassign this PR to you. Thank you for your help! In case, please @wesleywiser feel free to chime in and revert the review reassignment.

@apiraino apiraino assigned WaffleLapkin and unassigned wesleywiser Nov 21, 2023
@Noratrieb Noratrieb self-assigned this Nov 21, 2023
@Noratrieb
Copy link
Member

In collaboration with @WaffleLapkin, I'm going to implement my plan above by basically re-doing your PR (crediting you as a co-author of course!) and then putting up a new PR that waffle will approve.

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 21, 2023
…leLapkin

Fixes error count display is different when there's only one error left

Supersedes rust-lang#114759

### What did I do?

I did the small change in `rustc_errors` by hand. Then I did the other changes in `/compiler` by hand, those were just find replace on `*.rs` in the workspace. The changes in run-make are find replace for `run-make` in the workspace.

All other changes are blessed using `x test TEST --bless`. I blessed the tests that were blessed in rust-lang#114759.

### how to review this nightmare

ping bors with an `r+`. You should check that my logic is sound and maybe quickly scroll through the diff, but fully verifying it seems fairly hard to impossible. I did my best to do this correctly.

Thank you `@adrianEffe` for bringing this up and your initial implementation.

cc `@flip1995,` you said you want to do a subtree sync asap
cc `@RalfJung` maybe you want to do a quick subtree sync afterwards as well for Miri

r? `@WaffleLapkin`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 22, 2023
…leLapkin

Fixes error count display is different when there's only one error left

Supersedes rust-lang#114759

### What did I do?

I did the small change in `rustc_errors` by hand. Then I did the other changes in `/compiler` by hand, those were just find replace on `*.rs` in the workspace. The changes in run-make are find replace for `run-make` in the workspace.

All other changes are blessed using `x test TEST --bless`. I blessed the tests that were blessed in rust-lang#114759.

### how to review this nightmare

ping bors with an `r+`. You should check that my logic is sound and maybe quickly scroll through the diff, but fully verifying it seems fairly hard to impossible. I did my best to do this correctly.

Thank you `@adrianEffe` for bringing this up and your initial implementation.

cc `@flip1995,` you said you want to do a subtree sync asap
cc `@RalfJung` maybe you want to do a quick subtree sync afterwards as well for Miri

r? `@WaffleLapkin`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 24, 2023
…leLapkin

Fixes error count display is different when there's only one error left

Supersedes rust-lang#114759

### What did I do?

I did the small change in `rustc_errors` by hand. Then I did the other changes in `/compiler` by hand, those were just find replace on `*.rs` in the workspace. The changes in run-make are find replace for `run-make` in the workspace.

All other changes are blessed using `x test TEST --bless`. I blessed the tests that were blessed in rust-lang#114759.

### how to review this nightmare

ping bors with an `r+`. You should check that my logic is sound and maybe quickly scroll through the diff, but fully verifying it seems fairly hard to impossible. I did my best to do this correctly.

Thank you `@adrianEffe` for bringing this up and your initial implementation.

cc `@flip1995,` you said you want to do a subtree sync asap
cc `@RalfJung` maybe you want to do a quick subtree sync afterwards as well for Miri

r? `@WaffleLapkin`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 24, 2023
…leLapkin

Fixes error count display is different when there's only one error left

Supersedes rust-lang#114759

### What did I do?

I did the small change in `rustc_errors` by hand. Then I did the other changes in `/compiler` by hand, those were just find replace on `*.rs` in the workspace. The changes in run-make are find replace for `run-make` in the workspace.

All other changes are blessed using `x test TEST --bless`. I blessed the tests that were blessed in rust-lang#114759.

### how to review this nightmare

ping bors with an `r+`. You should check that my logic is sound and maybe quickly scroll through the diff, but fully verifying it seems fairly hard to impossible. I did my best to do this correctly.

Thank you `@adrianEffe` for bringing this up and your initial implementation.

cc `@flip1995,` you said you want to do a subtree sync asap
cc `@RalfJung` maybe you want to do a quick subtree sync afterwards as well for Miri

r? `@WaffleLapkin`
@WaffleLapkin
Copy link
Member

Closing since #118138 was merged.

bors added a commit to rust-lang/cargo that referenced this pull request Nov 29, 2023
Fixes error count display is different when there's only one error left

### What does this PR try to resolve?

When there's only 1 error left, the number 1 appears in the output so that it scans the same as the output when there's more than 1 error, so:

```
error: could not compile `crate` (lib test) due to 1 previous error
```
instead of the current:
```
error: could not compile `crate` (lib test) due to a previous error
```

Fixes #12390

rustc related PR [#114759](rust-lang/rust#114759)
@WaffleLapkin WaffleLapkin removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants