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

Ensure required submodules at the same time as updating existing submodules #104953

Merged
merged 4 commits into from
Dec 6, 2022

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Nov 26, 2022

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 #104952 for convenience to avoid merge conflicts, but doesn't depend on that PR.

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
…odules

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 avoiding `cargo metadata` somehow.
@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 ab89c17 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
…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 f4643f5 into rust-lang:master Dec 6, 2022
@rustbot rustbot added this to the 1.67.0 milestone Dec 6, 2022
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…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.
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
@jyn514 jyn514 deleted the fewer-submodule-updates branch February 25, 2023 18:32
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