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.

Sign up for GitHub

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

Jump to bottom

improvements on build_steps::test implementation #135887

Merged
merged 3 commits into from
Jan 23, 2025
Merged

improvements on build_steps::test implementation #135887

merged 3 commits into from
Jan 23, 2025

Conversation

onur-ozkan
Copy link
Member

Reviewing commits one-by-one should make it easier to understand.

@onur-ozkan
make bootstrap self test to use bootstrap cargo
c568da9 
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan
resolve clippy FIXME
17b2ede 
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan
reduce number of prepare_cargo_test args
e3a5314 
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@rustbot
Copy link
Collaborator

rustbot commented Jan 22, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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 A-testsuite Area: The testsuite used to check the correctness of rustc 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 Jan 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 22, 2025

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

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

onur-ozkan
onur-ozkan commented Jan 22, 2025
View reviewed changes
src/bootstrap/src/core/build_steps/test.rs
Comment on lines -3124 to -3129
if let Some(flags) = option_env!("RUSTFLAGS") {
// Use the same rustc flags for testing as for "normal" compilation,
// so that Cargo doesn’t recompile the entire dependency graph every time:
// /~https://github.com/rust-lang/rust/issues/49215
cmd.env("RUSTFLAGS", flags);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: this is already handled from Cargo with

rust/src/bootstrap/src/core/builder/cargo.rs

Lines 27 to 31 in b2728d5

fn new(target: TargetSelection) -> Rustflags {
let mut ret = Rustflags(String::new(), target);
ret.propagate_cargo_env("RUSTFLAGS");
ret
}

onur-ozkan
onur-ozkan commented Jan 22, 2025
View reviewed changes
src/bootstrap/src/core/config/tests.rs
@@ -92,7 +92,7 @@ fn detect_src_and_out() {
// `{build-dir}/bootstrap/debug/deps/bootstrap-c7ee91d5661e2804`
// `{build-dir}` can be anywhere, not just in the rust project directory.
let dep = Path::new(args.first().unwrap());
let expected_out = dep.ancestors().nth(4).unwrap();
let expected_out = dep.ancestors().nth(5).unwrap();
Copy link
Member Author

Choose a reason for hiding this comment

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

Output path now also includes the host triple as bootstrap passes that regardless even for non-cross builds

rust/src/bootstrap/src/core/builder/cargo.rs

Lines 415 to 416 in b2728d5

if cmd_kind != Kind::Install {
cargo.arg("--target").arg(target.rustc_target_arg());

@jieyouxu jieyouxu self-assigned this Jan 23, 2025
jieyouxu
jieyouxu approved these changes Jan 23, 2025
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, this is a nice cleanup. I tried some basic sanity checks like ./x test bootstrap and ./x test run-make-support, and they behaved as expected.

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 23, 2025

📌 Commit e3a5314 has been approved by jieyouxu

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 Jan 23, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 23, 2025
@jhpratt
Rollup merge of rust-lang#135887 - onur-ozkan:testing-improvements, r…
5c448d0 
…=jieyouxu

improvements on `build_steps::test` implementation

Reviewing commits one-by-one should make it easier to understand.
@jhpratt jhpratt mentioned this pull request Jan 23, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2025
@bors
Auto merge of rust-lang#135915 - jhpratt:rollup-m965rf3, r=jhpratt
26089bc 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#134746 (Don't ICE in coerce when autoderef fails to structurally normalize non-WF type in new solver)
 - rust-lang#135552 ([AIX] Lint on structs that have a different alignment in AIX's C ABI)
 - rust-lang#135764 (Fix tests on LLVM 20)
 - rust-lang#135779 (CI: free disk on linux arm runner)
 - rust-lang#135790 (Update windows-gnu targets to set `DebuginfoKind::DWARF`)
 - rust-lang#135879 (fix outdated file path ref in llvm)
 - rust-lang#135883 (Remove erroneous `unsafe` in `BTreeSet::upper_bound_mut`)
 - rust-lang#135884 (remove implied end of slice)
 - rust-lang#135887 (improvements on `build_steps::test` implementation)
 - rust-lang#135898 (rustdoc-json-types: Finalize dyn compatibility renaming)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 23, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2025
@bors
Auto merge of rust-lang#135921 - matthiaskrgr:rollup-5mzn76m, r=matth…
99768c8 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#134746 (Don't ICE in coerce when autoderef fails to structurally normalize non-WF type in new solver)
 - rust-lang#135552 ([AIX] Lint on structs that have a different alignment in AIX's C ABI)
 - rust-lang#135779 (CI: free disk on linux arm runner)
 - rust-lang#135790 (Update windows-gnu targets to set `DebuginfoKind::DWARF`)
 - rust-lang#135879 (fix outdated file path ref in llvm)
 - rust-lang#135883 (Remove erroneous `unsafe` in `BTreeSet::upper_bound_mut`)
 - rust-lang#135884 (remove implied end of slice)
 - rust-lang#135887 (improvements on `build_steps::test` implementation)
 - rust-lang#135898 (rustdoc-json-types: Finalize dyn compatibility renaming)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 76e68cc into rust-lang:master Jan 23, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 23, 2025
@onur-ozkan onur-ozkan deleted the testing-improvements branch January 23, 2025 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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
Milestone
1.86.0
Development

Successfully merging this pull request may close these issues.
5 participants
@onur-ozkan @rustbot @jieyouxu @bors @Mark-Simulacrum