Test the cargo args generated by bootstrap.py by jyn514 · Pull Request #112281 · rust-lang/rust

jyn514 · 2023-06-04T17:30:49Z

I recommend reviewing this commit-by-commit using the instructions in https://rustc-dev-guide.rust-lang.org/git.html#moving-large-sections-of-code.

Test cargo arguments passed by bootstrap.py
This moves a lot of code around, but the logic itself is not too terribly complicated.
- Move almost all logic in def bootstrap to the RustBuild class, to avoid mixing setting configuration with running commands
- Update various doctests to the new (more complete) RustBuild config. In particular, don't pretend that bin_root supports build being unset.
- Change parse_args not to use a global, to allow testing it
- Set BUILD_DIR appropriately so bootstrap.py doesn't panic because cargo isn't found
Allow passing arguments to bootstrap_test.py
Previous, it used the built-in test runner, which doesn't support options unless they're manually passed in the script.
Fix progress messages for configure in bootstrap_test.py
Before it would unconditionally print configure-args = [].

r? @albertlarsan68 cc #112089 #111979 (comment)

albertlarsan68

Looks mostly good to me, r=me with questions answered.

jyn514 · 2023-06-06T14:07:14Z

@bors r=albertlarsan68

bors · 2023-06-06T14:07:16Z

📌 Commit c2723c70b1fd31de5a57558ad1e373d0e0f36052 has been approved by albertlarsan68

It is now in the queue for this repository.

bors · 2023-06-08T03:54:59Z

⌛ Testing commit c2723c70b1fd31de5a57558ad1e373d0e0f36052 with merge bdae42044ea1244db04c548301491ce5f1919556...

bors · 2023-06-08T04:44:01Z

💔 Test failed - checks-actions

albertlarsan68 · 2023-06-08T05:30:57Z

It seems like the hack to allow for two levels of defaults is not working anymore.

jyn514 · 2023-06-08T13:31:52Z

lmao - ok what went wrong isn't that the hack broke, but that we started testing it at all 😂 before we didn't include the profile in the config the unit tests created, only the config from the main bootstrap.py process.

i think once i switch configure.py not to deserialize and reserialize the toml this problem will go away entirely, but in the meantime i'm going to change the hack back again so it's untested.

bors · 2023-06-24T00:14:36Z

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

Before it would unconditionally print `configure-args = []`.

Previous, it used the built-in test runner, which doesn't support options unless they're manually passed in the script.

This moves a lot of code around, but the logic itself is not too terribly complicated. - Move almost all logic in `def bootstrap` to the `RustBuild` class, to avoid mixing setting configuration with running commands - Update various doctests to the new (more complete) RustBuild config. In particular, don't pretend that `bin_root` supports `build` being unset. - Change `parse_args` not to use a global, to allow testing it - Set BUILD_DIR appropriately so bootstrap.py doesn't panic because cargo isn't found

These weren't being passed in to bootstrap consistently before; in particular `serialize_and_parse` forgot to pass them in.

It generates invalid TOML. I want to get rid of it eventually, but this avoids the issue in the meantime.

jyn514 · 2023-06-24T14:41:21Z

@bors r=albertlarsan68

bors · 2023-06-24T14:41:22Z

📌 Commit 24e67d5 has been approved by albertlarsan68

It is now in the queue for this repository.

bors · 2023-06-25T00:47:47Z

⌛ Testing commit 24e67d5 with merge b716bc479aaeb4143d9f05a9390fb9d659d57525...

bors · 2023-06-25T02:07:59Z

💔 Test failed - checks-actions

- fix tests when `--build` is set - don't leak `config.example.toml` fd - don't crash if `config.toml` doesn't exist yet

albertlarsan68 · 2023-06-25T15:12:57Z

Thanks for the PR!
@bors r+

bors · 2023-06-25T15:12:59Z

📌 Commit 1e7f037 has been approved by albertlarsan68

It is now in the queue for this repository.

Rollup of 3 pull requests Successful merges: - rust-lang#112281 (Test the cargo args generated by bootstrap.py) - rust-lang#113028 (rustdoc: handle assoc const equalities in cross-crate impl-Trait-in-arg-pos) - rust-lang#113029 (CI: do not run Bump dependencies workflow on forks) r? `@ghost` `@rustbot` modify labels: rollup

clubby789 · 2023-07-01T13:57:50Z

+ .env("BUILD_DIR", &builder.out)
+ .env("BUILD_PLATFORM", &builder.build.build.triple)
+ .current_dir(builder.src.join("src/bootstrap/"))
+ .args(builder.config.test_args());


This means that x test src/bootstrap -- <rust_test_name> no longer works, as unittest throws an exception when the test name doesn't exist as a module and stops the testing
We could maybe try and reproduce the logic pytest uses for finding modules, and remove arguments that aren't flags or valid modules but that seems a bit brittle 😕

i'm going to revert this for now - people can use python -m pytest manually if they want to pass test args.

…clubby789 Don't pass --test-args to `python -m unitest` The args for unittest and cargo test are mutually incompatible. Suggest that people use `python -m unittest ...` manually instead. This also changes `bootstrap_test.py` to be easier to run standalone; see the commit for details. r? `@clubby789` cc rust-lang#112281 (comment)

rustbot assigned albertlarsan68 Jun 4, 2023

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 4, 2023

This comment was marked as resolved.
Sign in to view

albertlarsan68 reviewed Jun 5, 2023
View reviewed changes

Comment threadsrc/bootstrap/bootstrap.py Outdated
Comment threadsrc/bootstrap/test.rs Outdated

jyn514 force-pushed the test-bootstrap-py branch from f51995e to c2723c7Compare June 6, 2023 14:04

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 6, 2023

This comment has been minimized.
Sign in to view

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 Jun 8, 2023

jyn514 mentioned this pull request Jun 9, 2023
Don't parse and reserialize toml in configure.py #112445
Closed

albertlarsan68 reviewed Jun 9, 2023
View reviewed changes

Comment threadsrc/bootstrap/bootstrap.py Outdated

jyn514 added 4 commits June 23, 2023 21:12

Fix progress messages for configure in bootstrap_test.py
7d2373e
Before it would unconditionally print `configure-args = []`.

Allow passing arguments to bootstrap_test.py
3508cde
Previous, it used the built-in test runner, which doesn't support options unless they're manually passed in the script.

Test color/verbose/warnings properly
c7af6fb
These weren't being passed in to bootstrap consistently before; in particular `serialize_and_parse` forgot to pass them in.

jyn514 force-pushed the test-bootstrap-py branch from 06dcbcd to db8f6a0Compare June 24, 2023 02:18

Don't test the profile override hack
24e67d5
It generates invalid TOML. I want to get rid of it eventually, but this avoids the issue in the meantime.

jyn514 force-pushed the test-bootstrap-py branch from db8f6a0 to 24e67d5Compare June 24, 2023 14:40

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 24, 2023

This comment has been minimized.
Sign in to view

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 Jun 25, 2023

fix some bugs
1e7f037
- fix tests when `--build` is set - don't leak `config.example.toml` fd - don't crash if `config.toml` doesn't exist yet

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 25, 2023

TaKO8Ki mentioned this pull request Jun 25, 2023
Rollup of 3 pull requests #113037
Merged

bors merged commit 96ab7e6 into rust-lang:masterJun 25, 2023

rustbot added this to the 1.72.0 milestone Jun 25, 2023

jyn514 deleted the test-bootstrap-py branch June 26, 2023 06:09

clubby789 reviewed Jul 1, 2023
View reviewed changes

jyn514 mentioned this pull request Jul 1, 2023
Don't pass --test-args to python -m unitest#113234
Merged

Uh oh!

Uh oh!

Conversation

jyn514 commented Jun 4, 2023

Uh oh!

This comment was marked as resolved.

albertlarsan68 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jyn514 commented Jun 6, 2023

Uh oh!

bors commented Jun 6, 2023

Uh oh!

bors commented Jun 8, 2023

Uh oh!

This comment has been minimized.

bors commented Jun 8, 2023

Uh oh!

albertlarsan68 commented Jun 8, 2023

Uh oh!

jyn514 commented Jun 8, 2023

Uh oh!

Uh oh!

bors commented Jun 24, 2023

Uh oh!

jyn514 commented Jun 24, 2023

Uh oh!

bors commented Jun 24, 2023

Uh oh!

bors commented Jun 25, 2023

Uh oh!

This comment has been minimized.

bors commented Jun 25, 2023

Uh oh!

albertlarsan68 commented Jun 25, 2023

Uh oh!

bors commented Jun 25, 2023

Uh oh!

clubby789Jul 1, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jyn514Jul 1, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

clubby789Jul 1, 2023•
edited
Loading