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

Streamline the user experience for x.py setup #104952

Merged
merged 3 commits into from
Dec 6, 2022
Merged

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Nov 26, 2022

Don't update submodules for x setup

Before, the submodule handling was very jank and would update between two interactive prompts:

; x setup
Building rustbuild
    Finished dev [unoptimized] target(s) in 0.05s
Welcome to the Rust project! What do you want to do with x.py?
a) library: Contribute to the standard library
Please choose one (a/b/c/d/e): a
Updating submodule library/backtrace
Submodule 'library/backtrace' (https://github.com/rust-lang/backtrace-rs.git) registered for path 'library/backtrace'
error: you asked `x.py` to setup a new config file, but one already exists at `config.toml`
Build completed unsuccessfully in 0:00:02

That's not a great user experience because you need to wait a long time between prompts.
It would be possible to move the submodule handling either before or after the prompt, but it seems
better to just not require submodules to be checked out at all, to minimize the time spend waiting
just to create a new configuration.

Revamp the order setup executes

  • Create config.toml last. It's the most likely to error, and used to stop later steps from executing
  • Don't print an error message + exit if the git hook already exists; that's expected

Before, the submodule handling was very jank and would update *between two interactive prompts*:
```
; x setup
Building rustbuild
    Finished dev [unoptimized] target(s) in 0.05s
Welcome to the Rust project! What do you want to do with x.py?
a) library: Contribute to the standard library
Please choose one (a/b/c/d/e): a
Updating submodule library/backtrace
Submodule 'library/backtrace' (https://github.com/rust-lang/backtrace-rs.git) registered for path 'library/backtrace'
error: you asked `x.py` to setup a new config file, but one already exists at `config.toml`
Build completed unsuccessfully in 0:00:02
```

That's not a great user experience because you need to wait a long time between prompts.
It would be possible to move the submodule handling either before or after the prompt, but it seems
better to just not require submodules to be checked out at all, to minimize the time spend waiting
just to create a new configuration.
- Create `config.toml` last. It's the most likely to error, and used to stop later steps from executing
- Don't print an error message + exit if the git hook already exists; that's expected
@rustbot
Copy link
Collaborator

rustbot commented Nov 26, 2022

r? @Mark-Simulacrum

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

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

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 5, 2022

📌 Commit b771d90 has been approved by Mark-Simulacrum

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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 5, 2022
Streamline the user experience for `x.py setup`

## Don't update submodules for x setup

Before, the submodule handling was very jank and would update *between two interactive prompts*:
```
; x setup
Building rustbuild
    Finished dev [unoptimized] target(s) in 0.05s
Welcome to the Rust project! What do you want to do with x.py?
a) library: Contribute to the standard library
Please choose one (a/b/c/d/e): a
Updating submodule library/backtrace
Submodule 'library/backtrace' (https://github.com/rust-lang/backtrace-rs.git) registered for path 'library/backtrace'
error: you asked `x.py` to setup a new config file, but one already exists at `config.toml`
Build completed unsuccessfully in 0:00:02
```

That's not a great user experience because you need to wait a long time between prompts.
It would be possible to move the submodule handling either before or after the prompt, but it seems
better to just not require submodules to be checked out at all, to minimize the time spend waiting
just to create a new configuration.

## Revamp the order setup executes

- Create `config.toml` last. It's the most likely to error, and used to stop later steps from executing
- Don't print an error message + exit if the git hook already exists; that's expected
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 5, 2022
…Mark-Simulacrum

Ensure required submodules at the same time as updating existing submodules

In practice, this would always happen at the same time, but putting them next to each other makes that more obvious and ensures it doesn't change in the future. It also avoids the difference affecting `cargo metadata` somehow.

This is based on rust-lang#104952 for convenience to avoid merge conflicts, but doesn't depend on that PR.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 5, 2022
…Mark-Simulacrum

Ensure required submodules at the same time as updating existing submodules

In practice, this would always happen at the same time, but putting them next to each other makes that more obvious and ensures it doesn't change in the future. It also avoids the difference affecting `cargo metadata` somehow.

This is based on rust-lang#104952 for convenience to avoid merge conflicts, but doesn't depend on that PR.
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 5, 2022
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#104912 (PartialEq: PERs are homogeneous)
 - rust-lang#104952 (Streamline the user experience for `x.py setup`)
 - rust-lang#104953 (Ensure required submodules at the same time as updating existing submodules)
 - rust-lang#105180 (Use proper HirId for async track_caller attribute check)
 - rust-lang#105222 (std update libc version and freebsd image build dependencies)
 - rust-lang#105223 (suggest parenthesis around ExprWithBlock BinOp ExprWithBlock)
 - rust-lang#105230 (Skip recording resolution for duplicated generic params.)
 - rust-lang#105301 (update Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4b6010c into rust-lang:master Dec 6, 2022
@rustbot rustbot added this to the 1.67.0 milestone Dec 6, 2022
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Dec 9, 2022
…lacrum

Ensure required submodules at the same time as updating existing submodules

In practice, this would always happen at the same time, but putting them next to each other makes that more obvious and ensures it doesn't change in the future. It also avoids the difference affecting `cargo metadata` somehow.

This is based on rust-lang/rust#104952 for convenience to avoid merge conflicts, but doesn't depend on that PR.
@jyn514 jyn514 deleted the setup branch December 26, 2022 23:35
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
Streamline the user experience for `x.py setup`

## Don't update submodules for x setup

Before, the submodule handling was very jank and would update *between two interactive prompts*:
```
; x setup
Building rustbuild
    Finished dev [unoptimized] target(s) in 0.05s
Welcome to the Rust project! What do you want to do with x.py?
a) library: Contribute to the standard library
Please choose one (a/b/c/d/e): a
Updating submodule library/backtrace
Submodule 'library/backtrace' (https://github.com/rust-lang/backtrace-rs.git) registered for path 'library/backtrace'
error: you asked `x.py` to setup a new config file, but one already exists at `config.toml`
Build completed unsuccessfully in 0:00:02
```

That's not a great user experience because you need to wait a long time between prompts.
It would be possible to move the submodule handling either before or after the prompt, but it seems
better to just not require submodules to be checked out at all, to minimize the time spend waiting
just to create a new configuration.

## Revamp the order setup executes

- Create `config.toml` last. It's the most likely to error, and used to stop later steps from executing
- Don't print an error message + exit if the git hook already exists; that's expected
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#104912 (PartialEq: PERs are homogeneous)
 - rust-lang#104952 (Streamline the user experience for `x.py setup`)
 - rust-lang#104953 (Ensure required submodules at the same time as updating existing submodules)
 - rust-lang#105180 (Use proper HirId for async track_caller attribute check)
 - rust-lang#105222 (std update libc version and freebsd image build dependencies)
 - rust-lang#105223 (suggest parenthesis around ExprWithBlock BinOp ExprWithBlock)
 - rust-lang#105230 (Skip recording resolution for duplicated generic params.)
 - rust-lang#105301 (update Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants