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

PathSet prefix matching breaks cli path filter consumption order #135022

Closed
jieyouxu opened this issue Jan 2, 2025 · 0 comments · Fixed by #135058
Closed

PathSet prefix matching breaks cli path filter consumption order #135022

jieyouxu opened this issue Jan 2, 2025 · 0 comments · Fixed by #135058
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Jan 2, 2025

#133492 changed PathSet matching to also admit path prefixes, in addition to the previously-admitted suffixes. Unfortunately, this breaks how describe! handles some Steps. For the typical non-ShouldRun::suite_path Step, it will consume a path filter like "compiler" when eligible.

#133492 allowing prefix matching changed which Steps is permitted to "eat" the path filter first, which broke ./x test compiler --stage 0 and ./x build compiler --stage 0 due to the respective Step registration order and which Steps get to consume the path filter first:

cc discussions at https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Mismatch.20between.20arg-filter.20vs.20.60--exclude.60.20filter.

@jieyouxu jieyouxu added C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 2, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 2, 2025
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 2, 2025
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 2, 2025
bootstrap: flip `compile::Rustc` vs `compile::Assemble`

The `PathSet` prefix matching unfortunately also has implications for `./x build compiler --stage 0`, because the path filter `"compiler"` gets consumed by `compile::Rustc` step first after `PathSet` prefix matching, whereas before `PathSet` prefix matching, `compile::Rustc` would not have consumed `"compiler"`.

This merely papers over rust-lang#134970 to unblock contributors from using `./x build compiler --stage 0`.

The `PathSet` prefix matching behavior is tracked in rust-lang#135022.

Closes rust-lang#134970.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 3, 2025
…ouxu

refactor bootstrap path resolution

Previously we removed paths as soon as we found the first intersection, which made it impossible to find other intersecting paths (and that is the reason of rust-lang#135022).

This patch changes that by marking the intersecting paths instead, so we can collect them all and remove them together when needed. Which means, `x build compiler` would compile anything that ends or starts with `"compiler"` instead of picking the first matching `Step` from `builder::get_step_descriptions`.

Fixes rust-lang#135022
@bors bors closed this as completed in c02499f Jan 4, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 4, 2025
Rollup merge of rust-lang#135058 - onur-ozkan:path-resolution, r=jieyouxu

refactor bootstrap path resolution

Previously we removed paths as soon as we found the first intersection, which made it impossible to find other intersecting paths (and that is the reason of rust-lang#135022).

This patch changes that by marking the intersecting paths instead, so we can collect them all and remove them together when needed. Which means, `x build compiler` would compile anything that ends or starts with `"compiler"` instead of picking the first matching `Step` from `builder::get_step_descriptions`.

Fixes rust-lang#135022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants