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

Fix running bootstrap tests with a local Rust toolchain as the stage0 #126476

Merged
merged 5 commits into from
Jul 10, 2024

Conversation

pietroalbini
Copy link
Member

@pietroalbini pietroalbini commented Jun 14, 2024

When configuring a local Rust toolchain as the stage0 (with build.rustc and build.cargo in config.toml) we noticed there were test failures (both on the Python and the Rust side) due to bootstrap not being able to find rustc and Cargo.

This was due to those two config.toml settings not being propagated in the tests. This PR fixes the issue by ensuring rustc and cargo are always configured in tests, using the parent bootstrap's initial_rustc and initial_cargo.

try-job: x86_64-msvc
Fixes #105766

@rustbot
Copy link
Collaborator

rustbot commented Jun 14, 2024

r? @onur-ozkan

rustbot has assigned @onur-ozkan.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jun 14, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 14, 2024

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@rust-log-analyzer

This comment has been minimized.

@pietroalbini pietroalbini force-pushed the pa-bootstrap-test-local-rustc branch from 4f30d21 to fcb6ff5 Compare June 14, 2024 11:00
@onur-ozkan
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 14, 2024

📌 Commit fcb6ff5 has been approved by onur-ozkan

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 Jun 14, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 14, 2024
…rustc, r=onur-ozkan

Fix running bootstrap tests with a local Rust toolchain as the stage0

When configuring a local Rust toolchain as the stage0 (with `build.rustc` and `build.cargo` in `config.toml`) we noticed there were test failures (both on the Python and the Rust side) due to bootstrap not being able to find rustc and Cargo.

This was due to those two `config.toml` settings not being propagated in the tests. This PR fixes the issue by ensuring rustc and cargo are always configured in tests, using the parent bootstrap's `initial_rustc` and `initial_cargo`.
@cuviper
Copy link
Member

cuviper commented Jun 14, 2024

I suppose this will fix #105766?

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125722 (Indicate in `non_local_defs` lint that the macro needs to change)
 - rust-lang#125829 (rustc_span: Add conveniences for working with span formats)
 - rust-lang#126192 (Various Redox OS fixes and add i686 Redox OS target)
 - rust-lang#126352 (ci: Update centos:7 to use vault repos)
 - rust-lang#126354 (Use `Variance` glob imported variants everywhere)
 - rust-lang#126469 (MIR Shl/Shr: the offset can be computed with rem_euclid)
 - rust-lang#126472 (build `libcxx-version` only when it doesn't exist)
 - rust-lang#126476 (Fix running bootstrap tests with a local Rust toolchain as the stage0)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125722 (Indicate in `non_local_defs` lint that the macro needs to change)
 - rust-lang#125829 (rustc_span: Add conveniences for working with span formats)
 - rust-lang#126192 (Various Redox OS fixes and add i686 Redox OS target)
 - rust-lang#126352 (ci: Update centos:7 to use vault repos)
 - rust-lang#126354 (Use `Variance` glob imported variants everywhere)
 - rust-lang#126469 (MIR Shl/Shr: the offset can be computed with rem_euclid)
 - rust-lang#126472 (build `libcxx-version` only when it doesn't exist)
 - rust-lang#126476 (Fix running bootstrap tests with a local Rust toolchain as the stage0)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r- I guess
#126486 (comment)

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 15, 2024
@onur-ozkan
Copy link
Member

Seems like it's missing .exe part.

Due to the way the paths initial_rustc and initial_cargo were
constructed before this commit, they mixed \ and / for path separators
and they omitted the .exe suffix.

This worked fine up until now, as Windows is capable of handling the
mixed path separators and the Command::new API adds the ".exe" suffix if
missing from the executable.

This resulted in paths that didn't actually exist on disk though, due to
the missing .exe suffix. This commit fixes that by adding the .exe
suffix to initial_rustc and initial_cargo when --build is Windows.
@pietroalbini
Copy link
Member Author

That failure seems to be caused by initial_rustc and initial_cargo actually not being valid file paths on Windows... They were valid command names, as Command::new automatically adds the .exe suffix on Windows, but they didn't point to anything on disk.

I pushed a commit to fix those paths, let's see if it works...

@bors try

@pietroalbini
Copy link
Member Author

I suppose this will fix #105766?

Yes!

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2024
…stc, r=<try>

Fix running bootstrap tests with a local Rust toolchain as the stage0

When configuring a local Rust toolchain as the stage0 (with `build.rustc` and `build.cargo` in `config.toml`) we noticed there were test failures (both on the Python and the Rust side) due to bootstrap not being able to find rustc and Cargo.

This was due to those two `config.toml` settings not being propagated in the tests. This PR fixes the issue by ensuring rustc and cargo are always configured in tests, using the parent bootstrap's `initial_rustc` and `initial_cargo`.

try-job: x86_64-msvc
Fixes rust-lang#105766
@bors
Copy link
Contributor

bors commented Jul 8, 2024

⌛ Trying commit 198c809 with merge b7d2a81...

@bors
Copy link
Contributor

bors commented Jul 8, 2024

☀️ Try build successful - checks-actions
Build commit: b7d2a81 (b7d2a8118e1fc850a3d8b4bf60c7ca265fc7f428)

@pietroalbini
Copy link
Member Author

@rustbot ready

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 8, 2024
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 8, 2024
@pietroalbini
Copy link
Member Author

@rustbot author

Figured out another case that'd need tweaking.

@rustbot rustbot 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 Jul 8, 2024
@rust-log-analyzer

This comment has been minimized.

@pietroalbini pietroalbini force-pushed the pa-bootstrap-test-local-rustc branch from aed27c5 to b4b2643 Compare July 9, 2024 08:13
@pietroalbini
Copy link
Member Author

Confirmed this fixes all of the problems with using custom rustc/cargo in an airgapped environment.

@rustbot review

@rustbot rustbot 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 Jul 9, 2024
@rustbot rustbot 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 Jul 9, 2024
@pietroalbini pietroalbini 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 Jul 10, 2024
@onur-ozkan
Copy link
Member

LGTM, thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jul 10, 2024

📌 Commit 9cd1d25 has been approved by onur-ozkan

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 Jul 10, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 10, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126476 (Fix running bootstrap tests with a local Rust toolchain as the stage0)
 - rust-lang#127094 (E0191 suggestion correction, inserts turbofish)
 - rust-lang#127554 ( do not run test where it cannot run)
 - rust-lang#127564 (Temporarily remove me from review rotation.)
 - rust-lang#127568 (instantiate higher ranked goals in candidate selection again)
 - rust-lang#127569 (Fix local download of Docker caches from CI)
 - rust-lang#127570 ( small normalization improvement)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d17e0cf into rust-lang:master Jul 10, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 10, 2024
Rollup merge of rust-lang#126476 - ferrocene:pa-bootstrap-test-local-rustc, r=onur-ozkan

Fix running bootstrap tests with a local Rust toolchain as the stage0

When configuring a local Rust toolchain as the stage0 (with `build.rustc` and `build.cargo` in `config.toml`) we noticed there were test failures (both on the Python and the Rust side) due to bootstrap not being able to find rustc and Cargo.

This was due to those two `config.toml` settings not being propagated in the tests. This PR fixes the issue by ensuring rustc and cargo are always configured in tests, using the parent bootstrap's `initial_rustc` and `initial_cargo`.

try-job: x86_64-msvc
Fixes rust-lang#105766
@pietroalbini pietroalbini deleted the pa-bootstrap-test-local-rustc branch July 10, 2024 22:14
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.

Some bootstrap tests fail with custom rustc
7 participants