Fix Step Skipping Caused by Using the `--exclude` Option #115088

LuuuXXX · 2023-08-22T07:59:37Z

The original code was overreacting to the --exclude option,

Lines 257 to 260 in eadf69a

    
           fn maybe_run(&self, builder: &Builder<'_>, pathsets: Vec<PathSet>) { 
        
               if pathsets.iter().any(|set| self.is_excluded(builder, set)) { 
        
                   return; 
        
               }

For example:
When x test --exclude alloc or x test --exclude library/alloc is passed, the entire libraray test is skipped.

Related issues:
#112009

rustbot · 2023-08-22T07:59:44Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @albertlarsan68 (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

albertlarsan68

Thanks for the PR!
Just a little change.

src/bootstrap/builder.rs

albertlarsan68

Another nit, r=me once fixed!

src/bootstrap/builder.rs

LuuuXXX · 2023-09-05T10:30:59Z

Thanks for the review! I've addressed the phrasing comments
@bors r=albertlarsan68

bors · 2023-09-05T10:31:01Z

@LuuuXXX: 🔑 Insufficient privileges: Not in reviewers

albertlarsan68 · 2023-09-05T10:32:28Z

Thanks for the PR!
@bors r+

bors · 2023-09-05T10:32:30Z

📌 Commit ab28e3f has been approved by albertlarsan68

It is now in the queue for this repository.

bors · 2023-09-05T11:17:41Z

⌛ Testing commit ab28e3f with merge 9fd55f30de49d862e6e89e172babc1ef775dfbd4...

bors · 2023-09-05T11:50:25Z

💔 Test failed - checks-actions

LuuuXXX · 2023-09-08T03:34:12Z

When I run the x test --stage 2 --host='--target wasm32-unknown-emscripten library/std command, the following error occurs (same to the error in CI):
test io::copy::tests::io_benches::bench_copy_buf_reader... FAILED
I noticed that wasm32 used --skip library/alloc to temporarily handle this situation when testing library/alloc, so I did the same for library/std.

rust/src/ci/docker/host-x86_64/wasm32/Dockerfile

Lines 60 to 62 in feb0673

    
           # Exclude library/alloc due to OOM in benches. 
        
           ENV SCRIPT python3 ../x.py test --stage 2 --host='' --target $TARGETS \ 
        
               --skip library/alloc

Maybe another PR is needed to fix the problem in the wasm32 test.

LuuuXXX · 2023-09-08T03:47:13Z

@bors r=albertlarsan68

bors · 2023-09-08T03:47:14Z

@LuuuXXX: 🔑 Insufficient privileges: Not in reviewers

albertlarsan68

Leave a FIXME for the WASM test

src/ci/docker/host-x86_64/wasm32/Dockerfile

albertlarsan68

Can you squash the PR?
r=me once done.

modify fuction clond() -> cloned() optimize the code Handle the problem that the pathset is empty and modify the judgment of the builder::tests::test_exclude_kind Delete unnecessary judegment conditions skip test for library/std duo to OOM in benches as library/alloc Add FIXME for WASM32

LuuuXXX · 2023-09-08T09:45:07Z

@bors r=albertlarsan68

bors · 2023-09-08T09:45:09Z

@LuuuXXX: 🔑 Insufficient privileges: Not in reviewers

albertlarsan68 · 2023-09-08T09:49:53Z

Thanks for the PR!
@bors r+

bors · 2023-09-08T09:49:55Z

📌 Commit 45abd8c has been approved by albertlarsan68

It is now in the queue for this repository.

…llaumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#104299 (Clarify stability guarantee for lifetimes in enum discriminants) - rust-lang#115088 (Fix Step Skipping Caused by Using the `--exclude` Option) - rust-lang#115201 (rustdoc: list matching impls on type aliases) - rust-lang#115633 (Lint node for `PRIVATE_BOUNDS`/`PRIVATE_INTERFACES` is the item which names the private type) - rust-lang#115638 (`-Cllvm-args` usability improvement) - rust-lang#115643 (fix: return early when has tainted in mir-lint) r? `@ghost` `@rustbot` modify labels: rollup

bors · 2023-09-08T15:32:46Z

⌛ Testing commit 45abd8c with merge 309af34...

Rollup merge of rust-lang#115088 - LuuuXXX:issue-112009, r=albertlarsan68 Fix Step Skipping Caused by Using the `--exclude` Option The original code was overreacting to the `--exclude` option, https://github.com/rust-lang/rust/blob/eadf69a6c6edfe220fc5b1b659e46e271d75a3a1/src/bootstrap/builder.rs#L257-L260 For example: When `x test --exclude alloc` or `x test --exclude library/alloc` is passed, the entire libraray test is skipped. Related issues: rust-lang#112009

rustbot assigned albertlarsan68 Aug 22, 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 Aug 22, 2023

This comment has been minimized.

Sign in to view

albertlarsan68 requested changes Aug 22, 2023

View reviewed changes

src/bootstrap/builder.rs Outdated Show resolved Hide resolved

src/bootstrap/builder.rs Outdated Show resolved Hide resolved

albertlarsan68 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 Aug 22, 2023

This comment has been minimized.

Sign in to view

LuuuXXX requested a review from albertlarsan68 August 22, 2023 11:34

albertlarsan68 requested changes Sep 5, 2023

View reviewed changes

src/bootstrap/builder.rs Outdated Show resolved Hide resolved

LuuuXXX requested a review from albertlarsan68 September 5, 2023 10:31

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 5, 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 Sep 5, 2023

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Sep 8, 2023

albertlarsan68 reviewed Sep 8, 2023

View reviewed changes

src/ci/docker/host-x86_64/wasm32/Dockerfile Outdated Show resolved Hide resolved

LuuuXXX requested a review from albertlarsan68 September 8, 2023 08:29

albertlarsan68 approved these changes Sep 8, 2023

View reviewed changes

LuuuXXX force-pushed the issue-112009 branch from 114f624 to 45abd8c Compare September 8, 2023 09:13

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

GuillaumeGomez mentioned this pull request Sep 8, 2023

Rollup of 6 pull requests #115672

Merged

bors merged commit dcb4659 into rust-lang:master Sep 8, 2023

rustbot added this to the 1.74.0 milestone Sep 8, 2023

	fn maybe_run(&self, builder: &Builder<'_>, pathsets: Vec<PathSet>) {
	if pathsets.iter().any(\|set\| self.is_excluded(builder, set)) {
	return;
	}

Fix Step Skipping Caused by Using the --exclude Option #115088

Fix Step Skipping Caused by Using the --exclude Option #115088

Uh oh!

Conversation

LuuuXXX commented Aug 22, 2023

Uh oh!

rustbot commented Aug 22, 2023

Uh oh!

This comment has been minimized.

This comment has been minimized.

albertlarsan68 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

albertlarsan68 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LuuuXXX commented Sep 5, 2023

Uh oh!

bors commented Sep 5, 2023

Uh oh!

albertlarsan68 commented Sep 5, 2023

Uh oh!

bors commented Sep 5, 2023

Uh oh!

bors commented Sep 5, 2023

Uh oh!

This comment has been minimized.

bors commented Sep 5, 2023

Uh oh!

LuuuXXX commented Sep 8, 2023

Uh oh!

LuuuXXX commented Sep 8, 2023

Uh oh!

bors commented Sep 8, 2023

Uh oh!

albertlarsan68 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

albertlarsan68 left a comment

Choose a reason for hiding this comment

Uh oh!

LuuuXXX commented Sep 8, 2023

Uh oh!

bors commented Sep 8, 2023

Uh oh!

albertlarsan68 commented Sep 8, 2023

Uh oh!

bors commented Sep 8, 2023

Uh oh!

bors commented Sep 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fix Step Skipping Caused by Using the `--exclude` Option #115088

Fix Step Skipping Caused by Using the `--exclude` Option #115088