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.

Sign up for GitHub

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

Jump to bottom

Gracefully exit if --keep-stage flag is used on a clean source tree #107397

Merged
merged 1 commit into from
Jan 29, 2023
Merged

Gracefully exit if --keep-stage flag is used on a clean source tree #107397

merged 1 commit into from
Jan 29, 2023

Conversation

Teapot4195
Copy link
Contributor

Instead of quitting with an obscure No such file or directory error, give the user a clearer and easier to understand error (as well as suggesting a possible cause for the error).

This is the first time I have written rust since 2020, and the first PR I will ever make to rust, so please do point out any mistakes I have made 😄 .

This fixes #107392

@rustbot
Copy link
Collaborator

rustbot commented Jan 28, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @albertlarsan68 (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 28, 2023
@albertlarsan68
Copy link
Member

I am not sure whether to land this, cc #107019 @jyn514 @GentBinaku.
Is there another way of reporting the errors without panicking?

jyn514
jyn514 reviewed Jan 28, 2023
View reviewed changes
src/bootstrap/lib.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Teapot4195
Copy link
Contributor Author

The no-merge policy got me a little confused, but hopefully these changes should fix the remaining issues :)

@albertlarsan68
Copy link
Member

Can you squash your commits please?
r=me once squashed and CI passes

@albertlarsan68 albertlarsan68 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 28, 2023
@albertlarsan68
Copy link
Member

Thanks for your PR!
@bors r+

@bors
Copy link
Contributor

bors commented Jan 28, 2023

📌 Commit 65186e0 has been approved by albertlarsan68

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 28, 2023
@Teapot4195 Teapot4195 closed this Jan 29, 2023
@Teapot4195 Teapot4195 deleted the issue-107392-fix branch January 29, 2023 00:37
@Teapot4195 Teapot4195 restored the issue-107392-fix branch January 29, 2023 00:37
@Teapot4195 Teapot4195 deleted the issue-107392-fix branch January 29, 2023 00:37
@Teapot4195 Teapot4195 restored the issue-107392-fix branch January 29, 2023 00:37
@Teapot4195
Copy link
Contributor Author

oops, think I accidentally deleted the wrong branch
Apologies in advance for whoever has to come back and approve for a second time (if necessary).

@Teapot4195 Teapot4195 reopened this Jan 29, 2023
@jyn514
Copy link
Member

jyn514 commented Jan 29, 2023

@bors r=albertlarsan68

@bors
Copy link
Contributor

bors commented Jan 29, 2023

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Jan 29, 2023

📌 Commit 65186e0 has been approved by albertlarsan68

It is now in the queue for this repository.

@matthiaskrgr matthiaskrgr mentioned this pull request Jan 29, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2023
@bors
Auto merge of rust-lang#107435 - matthiaskrgr:rollup-if5h6yu, r=matth…
a29efcc 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#106618 (Disable `linux_ext` in wasm32 and fortanix rustdoc builds.)
 - rust-lang#107097 (Fix def-use dominance check)
 - rust-lang#107154 (library/std/sys_common: Define MIN_ALIGN for m68k-unknown-linux-gnu)
 - rust-lang#107397 (Gracefully exit if --keep-stage flag is used on a clean source tree)
 - rust-lang#107401 (remove the usize field from CandidateSource::AliasBound)
 - rust-lang#107413 (make more pleasant to read)
 - rust-lang#107422 (Also erase substs for new infcx in pin move error)
 - rust-lang#107425 (Check for missing space between fat arrow and range pattern)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 40b63d0 into rust-lang:master Jan 29, 2023
@rustbot rustbot added this to the 1.69.0 milestone Jan 29, 2023
@Teapot4195 Teapot4195 deleted the issue-107392-fix branch January 29, 2023 13:42
jyn514
jyn514 reviewed Jan 29, 2023
View reviewed changes
src/bootstrap/lib.rs
Comment on lines +1435 to +1438
eprintln!(
"Warning: Unable to find the stamp file, did you try to keep a nonexistent build stage?"
);
crate::detail_exit(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have two comments here:

  1. This should say "error", not "warning"
  2. This should print the file path of the stamp file; currently it's a regression compared to the existing error message.

@Teapot4195 do you think you could make a follow-up PR with both those changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll make a follow up PR for those changes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@Teapot4195 Teapot4195 mentioned this pull request Jan 29, 2023
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 29, 2023
@matthiaskrgr
Rollup merge of rust-lang#107448 - Teapot4195:pr-107397-followup, r=M…
97e6aba 
…ark-Simulacrum

When stamp doesn't exist, should say Error, and print path to stamp file

Follow up pr for rust-lang#107397
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albertlarsan68 albertlarsan68 albertlarsan68 left review comments

@jyn514 jyn514 jyn514 left review comments

Assignees

@albertlarsan68 albertlarsan68

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.69.0
Development

Successfully merging this pull request may close these issues.

Confusing error message when using --keep-stage-std after cleaning
6 participants
@Teapot4195 @rustbot @albertlarsan68 @rust-log-analyzer @bors @jyn514