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

x setup no longer goes through all setup steps before checking if config.toml exists #110471

Closed
jyn514 opened this issue Apr 18, 2023 · 2 comments · Fixed by #117708
Closed

x setup no longer goes through all setup steps before checking if config.toml exists #110471

jyn514 opened this issue Apr 18, 2023 · 2 comments · Fixed by #117708
Assignees
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 18, 2023

I tried this code:

x setup

I expected to see this happen: x setup prompts me to choose a profile and goes through the hook, vscode, etc. before exiting with an error.

Instead, this happened: x setup prompts me to choose a profile and immediately exits before running anything else:

; x setup
Building bootstrap
    Finished dev [unoptimized] target(s) in 0.14s
warning: `download-rustc` is enabled, but there are changes to compiler/ or library/
Welcome to the Rust project! What do you want to do with x.py?
a) library: Contribute to the standard library
b) compiler: Contribute to the compiler itself
c) codegen: Contribute to the compiler, and also modify LLVM or codegen
d) tools: Contribute to tools which depend on the compiler, but do not modify it directly (e.g. rustdoc, clippy, miri)
e) user: Install Rust from source
f) none: Do not modify `config.toml`
Please choose one (a/b/c/d/e/f): b

To get started, try one of the following commands:
- `x.py check`
- `x.py build`
- `x.py test`
For more suggestions, see https://rustc-dev-guide.rust-lang.org/building/suggested.html

error: you asked `x.py` to setup a new config file, but one already exists at `config.toml`
help: try adding `profile = "compiler"` at the top of config.toml
note: this will use the configuration in /home/jyn/src/rust2/src/bootstrap/defaults/config.compiler.toml
Build completed unsuccessfully in 0:00:01

This is a recent regression, I tested this at one point and it did work.

Meta

HEAD is branched from 9693b17

@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-bug Category: This is a bug. A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Apr 18, 2023
@albertlarsan68
Copy link
Member

I think this is good behavior, since the subcommands allow specific actions.
I think the check should happen at the start, and print a warning pointing to the different subcommands available.

@jyn514
Copy link
Member Author

jyn514 commented Apr 18, 2023

Hmm, ok, you're imagining something like this, where there's not an interactive prompt before the error?

; x setup
Building bootstrap
    Finished dev [unoptimized] target(s) in 0.14s
error: you asked `x.py` to setup a new config file, but one already exists at `config.toml`
help: to pick a specific profile, try adding `profile = "..."` at the top of config.toml
note: this will use the configuration in /home/jyn/src/rust2/src/bootstrap/defaults/config.compiler.toml
help: to see a list of available `setup` commands, run `x setup --help --verbose`

That seems like a good change 👍

@onur-ozkan onur-ozkan removed the A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself label Nov 8, 2023
@onur-ozkan onur-ozkan self-assigned this Nov 8, 2023
@bors bors closed this as completed in eae4135 Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants