Skip to content

refactor bootstrap path resolution #135058

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

Merged
merged 6 commits into from
Jan 4, 2025
Merged

Conversation

onur-ozkan
Copy link
Member

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 #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 #135022

Previously we removed paths as soon as we found the first intersection, which made
it impossible to find other intersecting paths. This patch changes that by marking
the intersecting paths instead, so we can collect them all and remove them together
when needed.

Signed-off-by: onur-ozkan <[email protected]>
@rustbot
Copy link
Collaborator

rustbot commented Jan 3, 2025

r? @albertlarsan68

rustbot has assigned @albertlarsan68.
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 Jan 3, 2025
@jieyouxu jieyouxu self-assigned this Jan 3, 2025
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. To make sure I properly understand the intention:

  • whereas previously the run logic adopted a "first matching Step consumes path filter" scheme,
  • now the run logic adopt a "all matching Steps will be executed" scheme.

Is my description accurate?

EDIT: it seems so.

@jieyouxu
Copy link
Member

jieyouxu commented Jan 3, 2025

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jan 3, 2025

📌 Commit baa7fce 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 3, 2025
@onur-ozkan
Copy link
Member Author

Thanks for the fix. To make sure I properly understand the intention:

  • whereas previously the run logic adopted a "first matching Step consumes path filter" scheme,
  • now the run logic adopt a "all matching Steps will be executed" scheme.

Is my description accurate?

EDIT: it seems so.

Yes, it is accurate.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request 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 added a commit to rust-lang-ci/rust that referenced this pull request Jan 3, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#133420 (Switch rtems target to panic unwind)
 - rust-lang#134965 (Make Boxy UwU)
 - rust-lang#135007 (Some type-outlives computation tweaks)
 - rust-lang#135036 (run-make-support: adjust assertion printing, add some basic sanity checks)
 - rust-lang#135043 (rustdoc: treat `allowed_through_unstable_modules` as deprecation)
 - rust-lang#135044 (Improve infer (`_`) suggestions in `const`s and `static`s)
 - rust-lang#135058 (refactor bootstrap path resolution)
 - rust-lang#135061 (crashes: add latest batch of tests)

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

Rollup of 7 pull requests

Successful merges:

 - rust-lang#133420 (Switch rtems target to panic unwind)
 - rust-lang#134965 (Make Boxy UwU)
 - rust-lang#135007 (Some type-outlives computation tweaks)
 - rust-lang#135036 (run-make-support: adjust assertion printing, add some basic sanity checks)
 - rust-lang#135043 (rustdoc: treat `allowed_through_unstable_modules` as deprecation)
 - rust-lang#135044 (Improve infer (`_`) suggestions in `const`s and `static`s)
 - rust-lang#135058 (refactor bootstrap path resolution)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c02499f into rust-lang:master Jan 4, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request 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
@rustbot rustbot added this to the 1.86.0 milestone Jan 4, 2025
@onur-ozkan onur-ozkan deleted the path-resolution branch January 4, 2025 04:22
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.

PathSet prefix matching breaks cli path filter consumption order
6 participants