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

Move some more bootstrap logic from python to rust #92260

Merged
merged 4 commits into from
Mar 8, 2022

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 24, 2021

Same rationale as #76544; it would be nice to make python entirely optional at some point.

This also removes $ROOT as an option for the build directory; I haven't been using it, and like Alex
said in #76544 (comment) it seems like a misfeature.

This allows running cargo run from src/bootstrap, although that still gives
lots of compile errors if you don't use the beta toolchain. It's not exactly the same as using x.py, since it won't have BOOTSTRAP_DOWNLOAD_RUSTC set, but it's pretty close. Doing this from the top-level directory requires rust-lang/cargo#7290 to be fixed, or using cargo run -p bootstrap.

The next steps for making python optional are to move download-ci-llvm and download-rustc support into rustbuild, likely be shelling out as the python scripts do today.

It would also be nice (although not required) to move submodule support there, but that would require taking bootstrap out of the workspace to avoid errors from crates that haven't been cloned yet.

r? @Mark-Simulacrum

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Dec 24, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 24, 2021
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member Author

jyn514 commented Dec 24, 2021

I am confused. canonicalize() is failing because /checkout/build doesn't exist yet in CI, but creating it fails because /checkout is read-only. How did this possibly work before? Something needs to create the build directory or you can't build the compiler.

@ehuss
Copy link
Contributor

ehuss commented Dec 25, 2021

The build-dir directory is relative to the current working directory (not the checkout directory). The docker scripts set the workdir to /checkout/obj and use relative paths like python3 ../x.py. The default build-dir of build then becomes /checkout/obj/build. The /checkout/obj volume is mounted read-write.

@jyn514 jyn514 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 Dec 25, 2021
@bors
Copy link
Contributor

bors commented Jan 2, 2022

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

@JohnCSimon JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 23, 2022
@jyn514 jyn514 force-pushed the less-python-logic branch 2 times, most recently from eb96446 to b3929be Compare February 6, 2022 23:48
@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 6, 2022
@jyn514
Copy link
Member Author

jyn514 commented Feb 6, 2022

It's not exactly the same as using x.py, since it won't have BOOTSTRAP_CONFIG or BOOTSTRAP_DOWNLOAD_RUSTC set, but it's pretty close.

actually, let me switch rustbuild to using RUST_BOOTSTRAP_CONFIG instead of its own custom env var while I'm at it, seems simple enough and I don't see how it could break anything

@jyn514 jyn514 force-pushed the less-python-logic branch from b3929be to d68021d Compare February 6, 2022 23:51
@jyn514 jyn514 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 Feb 6, 2022
@rust-log-analyzer

This comment has been minimized.

@jyn514 jyn514 force-pushed the less-python-logic branch from d68021d to f2426ba Compare February 7, 2022 00:12
@jyn514
Copy link
Member Author

jyn514 commented Feb 7, 2022

    # Read from `RUST_BOOTSTRAP_CONFIG`, then `--config`, then fallback to `config.toml` (if it
    # exists).
    toml_path = os.getenv('RUST_BOOTSTRAP_CONFIG') or args.config

uhhh it seems pretty strange to me that the env variable would take precedence over --config, but I don't want to change it here

@jyn514 jyn514 force-pushed the less-python-logic branch from f2426ba to c91ca29 Compare February 7, 2022 00:15
@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 7, 2022
@bors
Copy link
Contributor

bors commented Mar 8, 2022

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 8, 2022
@Mark-Simulacrum
Copy link
Member

@bors retry test various timed out

@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 Mar 8, 2022
@bors
Copy link
Contributor

bors commented Mar 8, 2022

⌛ Testing commit 477cae3 with merge 64187b8...

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@@ -440,3 +440,112 @@ fn fail(s: &str) -> ! {
println!("\n\n{}\n\n", s);
std::process::exit(1);
}

/// Copied from `std::path::absolute` until it stabilizes.
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't we just have bootstrap use the nightly feature until it's stabilized?

@bjorn3
Copy link
Member

bjorn3 commented Mar 8, 2022

#92260 (comment)

No, bootstrap shouldn't use unstable features, it causes problems when we are switching between self-bootstrap (i.e., building 1.x with 1.x) and regular bootstrap; most code avoids that problem through switching cfg(bootstrap) in rustbuild off and on depending on whether we're self bootstrapping but since rustbuild (in the long run) wants to be buildable with just a beta/stable compiler, we want to avoid unstable feature use since there's no one to do that switching. Copying the code from std should be fine (it's not a lot of code and isn't particularly likely to change).

@bors
Copy link
Contributor

bors commented Mar 8, 2022

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 64187b8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 8, 2022
@bors bors merged commit 64187b8 into rust-lang:master Mar 8, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 8, 2022
@jyn514 jyn514 deleted the less-python-logic branch March 8, 2022 19:53
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (64187b8): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

jyn514 added a commit to jyn514/rust that referenced this pull request Mar 10, 2022
This was part of Mark's original PR in rust-lang@ecb424f,
but I missed it when writing rust-lang#92260.
@jyn514 jyn514 mentioned this pull request Mar 10, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 10, 2022
…Simulacrum

Allow `cargo run` instead of `cargo run -p bootstrap`

This was part of `@Mark-Simulacrum` 's original PR in rust-lang@ecb424f,
but I missed it when writing rust-lang#92260.

This also has the side effect of allowing `cargo build --bins` instead of `cargo build -p bootstrap --bins`. I'm not sure when you would want to run cargo build/check/test without going through bootstrap, but this still allows you to do so as long as you pass `-p` for all the crates you want to build.
jyn514 added a commit to jyn514/rust that referenced this pull request Mar 10, 2022
When I implemented rust-only bootstrapping in rust-lang#92260,
I neglected to test stage0 tools - it turns out they were broken because
they couldn't find the sysroot of the initial bootstrap compiler.

This fixes stage0 tools by using `rustc --print sysroot` instead of assuming rustc is already in a
sysroot and hard-coding the relative directory.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 30, 2022
…acrum

Fix `cargo run tidy`

When I implemented rust-only bootstrapping in rust-lang#92260,
I neglected to test stage0 tools - it turns out they were broken because
they couldn't find the sysroot of the initial bootstrap compiler.

This fixes stage0 tools by using `rustc --print sysroot` instead of assuming rustc is already in a
sysroot and hard-coding the relative directory.

Fixes rust-lang#94797 (properly, without having to change rustup).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 30, 2022
…acrum

Fix `cargo run tidy`

When I implemented rust-only bootstrapping in rust-lang#92260,
I neglected to test stage0 tools - it turns out they were broken because
they couldn't find the sysroot of the initial bootstrap compiler.

This fixes stage0 tools by using `rustc --print sysroot` instead of assuming rustc is already in a
sysroot and hard-coding the relative directory.

Fixes rust-lang#94797 (properly, without having to change rustup).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 30, 2022
…acrum

Fix `cargo run tidy`

When I implemented rust-only bootstrapping in rust-lang#92260,
I neglected to test stage0 tools - it turns out they were broken because
they couldn't find the sysroot of the initial bootstrap compiler.

This fixes stage0 tools by using `rustc --print sysroot` instead of assuming rustc is already in a
sysroot and hard-coding the relative directory.

Fixes rust-lang#94797 (properly, without having to change rustup).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 30, 2022
…acrum

Fix `cargo run tidy`

When I implemented rust-only bootstrapping in rust-lang#92260,
I neglected to test stage0 tools - it turns out they were broken because
they couldn't find the sysroot of the initial bootstrap compiler.

This fixes stage0 tools by using `rustc --print sysroot` instead of assuming rustc is already in a
sysroot and hard-coding the relative directory.

Fixes rust-lang#94797 (properly, without having to change rustup).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.