bootstrap: Don't pass `Kind` to some places that don't need it by Zalathar · Pull Request #160191 · rust-lang/rust

Zalathar · 2026-07-30T02:21:07Z

The current CLI step's Kind was being passed deep into some selector-matching code that doesn't seem to actually need it. It seems that the kind associated with a step's ShouldRun paths/aliases always comes from that step, so proceeding to compare it against the step's kind later can never fail and doesn't achieve anything.

As far as I can tell, this is a relic of #91965 that (due to subsequent changes) doesn't do anything useful and also doesn't make a lot of conceptual sense.

This PR therefore removes the Kind parameter from PathSet::check, and removes the Kind field from TaskPath.

Touching TaskPath causes a lot of churn in snapshot tests, but there don't seem to be any changes in actual behaviour.

rustbot · 2026-07-30T02:21:12Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: bootstrap
bootstrap expanded to 6 candidates
Random selection from Mark-Simulacrum, clubby789, jieyouxu

jieyouxu

Thanks, this seems right.

I wonder if Kind was previously used to do... Kind-dependent resolution 😰

@bors r+

View changes since this review

jieyouxu · 2026-07-30T02:26:20Z

+        // This order is important for retro-compatibility, as `starts_with` was introduced later.
+        p.path.ends_with(needle) || p.path.starts_with(needle)


Remark (unrelated to this PR): I really dislike this behavior of both prefix and postfix matching, it's a nightmare, but alas

It makes sense that you would want both ./x test compiler and ./x test rustc_middle to test the crate compiler/rustc_middle, for example.

But yeah, selector matching is several layers of inside-out spaghetti stacked on top of each other, so it's a nightmare to touch anything at the moment.

./x test rustc_middle already matches through the crate name rather than path, right? Or did crate name matching get removed at some point?

IIRC, we currently don’t have any crate-specific behaviour in selector/path matching.

jieyouxu · 2026-07-30T02:29:06Z

@@ -1,137 +1,137 @@
 ---
 source: src/bootstrap/src/core/builder/cli_paths/tests.rs
-expression: test --skip=tests --skip=coverage-map --skip=coverage-run --skip=library --skip=tidyselftest


Remark: hm, interesting

I think this weirdness is because the “expression” isn't actually checked by insta, so it became stale after #159131 which changed the expression but didn't invalidate the actual snapshotted results.

Ah, yeah, makes sense

rust-bors · 2026-07-30T02:30:45Z

📌 Commit 8740040 has been approved by jieyouxu

It is now in the queue for this repository.

jieyouxu · 2026-07-30T02:31:14Z

Actually, r=me once PR CI is 🍏, just in case
@bors r-

rust-bors · 2026-07-30T02:31:19Z

This pull request was unapproved.

View changes since this unapproval

jieyouxu

Since apparently I failed to click the "Approve" button... 😆

View changes since this review

jieyouxu · 2026-07-30T03:06:54Z

Oh interesting, does this change linkcheck skipping? 🤔 (Can't dig into this rn, but can check later)

Zalathar · 2026-07-30T03:08:39Z

Oh interesting, does this change linkcheck skipping? 🤔 (Can't dig into this rn, but can check later)

Given that #91965 mentions linkchecker specifically, this might be a real problem. 😿

jieyouxu · 2026-07-30T03:10:54Z

For linkcheck I think we generally need to skip it for stage 1... 🤔

EDIT: i.e. #156792 (but I forgot to backlink to the original PR where this was discussed, argh)

Zalathar · 2026-07-30T03:34:05Z

I think something relevant is happening in Builder::run_default_doc_steps, where we end up creating a bunch of step descriptions that are based on Kind::Doc rather than self.kind.

EDIT: I think it's bogus for that function to be calling self.run_step_descriptions.

Zalathar · 2026-07-30T04:40:34Z

Further notes:

There are few steps (including ./x test linkchecker) that want to do the equivalent of ./x doc before proceeding, so they call Builder::run_default_doc_steps.
The implementation of run_default_doc_steps ends up re-invoking some of the command-line handling stuff with a fake subcommand Kind::Doc and a fake empty list of command-line selectors &[].
The underlying command-line-handling code is not prepared to actually deal with that, so details from the “real” arguments leak into things like --skip handling, causing some crates to not be documented.
Passing around the Kind then serves as a bizarre inside-out hack to prevent some of that interference, so getting rid of the hack breaks things.

Zalathar · 2026-07-30T04:50:09Z

I'm contemplating a narrower hack where we pass a two-value enum down through run_step_descriptions into maybe_run that signals “this is actually for run_default_doc_steps, so don't do any --skip handling”.

That's still super gross, but it might be a net improvement.

Zalathar · 2026-07-30T06:07:40Z

Pushed an update with a hack to forcibly disable --skip handling during run_default_doc_steps (diff).

Not sure if we actually want to do this, but I figure we might as well check whether it would work.

Zalathar · 2026-07-30T07:16:29Z

Pushed a different workaround that avoids the skip_mode enum, and instead duplicates just the relevant parts of run_step_descriptions (diff).

jieyouxu

Thanks. I wonder if there are ways to go about untangling this mess, but the changes here still feel like a net improvement.

@bors r+ rollup

View changes since this review

jieyouxu · 2026-07-31T14:04:19Z

+        // It's important that we don't just call `run_step_descriptions` here,
+        // because that would cause `--skip` handling for actual command-line
+        // arguments to inappropriately skip these steps.


I suspect the longer-term fix is to have some shared source of truth for “default doc steps” that is then shared by this code and by the actual steps.

rust-bors · 2026-08-02T10:43:43Z

📌 Commit 10a3cc5 has been approved by jieyouxu

It is now in the queue for this repository.

bootstrap: Don't pass `Kind` to some places that don't need it The current CLI step's `Kind` was being passed deep into some selector-matching code that doesn't seem to actually need it. It seems that the kind associated with a step's `ShouldRun` paths/aliases always comes from that step, so proceeding to compare it against the step's kind later can never fail and doesn't achieve anything. As far as I can tell, this is a relic of rust-lang#91965 that (due to subsequent changes) doesn't do anything useful and also doesn't make a lot of conceptual sense. This PR therefore removes the `Kind` parameter from `PathSet::check`, and removes the `Kind` field from `TaskPath`. Touching `TaskPath` causes a lot of churn in snapshot tests, but there don't seem to be any changes in actual behaviour.

JonathanBrouwer · 2026-08-02T11:36:19Z

💔 I suspect this PR failed tests as part of a rollup
@bors r-

After fixing the problem, consider running a try job for the failed job before re-approving.

Link to failure: #160381 (comment)

rust-bors · 2026-08-02T11:36:23Z

This pull request was unapproved.

View changes since this unapproval

rustbot · 2026-08-02T11:44:16Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Zalathar · 2026-08-02T11:59:33Z

I guess Windows didn't want me to switch path-printing from .display() to Debug?

@bors try jobs=i686-msvc-2,aarch64-msvc-2

bootstrap: Don't pass `Kind` to some places that don't need it try-job: i686-msvc-2 try-job: aarch64-msvc-2

rust-bors · 2026-08-02T13:46:34Z

☀️ Try build successful (CI)
Build commit: 4d54108 (4d54108c62d034f434fd7000572e314b1c794742)
Base parent: 8c3a200 (8c3a200b704adb5f110249b68daa4ff386b36e29)

Zalathar · 2026-08-03T01:01:19Z

Actually, maybe I can get quoted output by doing .display().to_string() and then debug-printing the string. Now the question is whether I prefer the quoted output. (Yes?)

Zalathar · 2026-08-03T02:24:59Z

I think I'll stick with the unquoted output after all. It's visually cleaner, and it seems unlikely that we'll encounter any ambiguity that requires quoting.

Since the post-approval fix was simple (and closer to the status-quo), I feel OK about re-approving.

@bors r=jieyouxu rollup=iffy

rust-bors · 2026-08-03T02:25:02Z

📌 Commit 4ca76aa has been approved by jieyouxu

It is now in the queue for this repository.

Rollup of 5 pull requests Successful merges: - #160143 (Add NEON support for is_ascii and eq_ignore_ascii_case) - #160191 (bootstrap: Don't pass `Kind` to some places that don't need it) - #160386 (rustc_data_structures: Cleanup jobserver initialization) - #160409 (Remove redundant target check from check_eii_impl) - #160410 (Update GitHub Actions to v7.0.1)

Rollup merge of #160191 - Zalathar:unkind, r=jieyouxu bootstrap: Don't pass `Kind` to some places that don't need it The current CLI step's `Kind` was being passed deep into some selector-matching code that doesn't seem to actually need it. It seems that the kind associated with a step's `ShouldRun` paths/aliases always comes from that step, so proceeding to compare it against the step's kind later can never fail and doesn't achieve anything. As far as I can tell, this is a relic of #91965 that (due to subsequent changes) doesn't do anything useful and also doesn't make a lot of conceptual sense. This PR therefore removes the `Kind` parameter from `PathSet::check`, and removes the `Kind` field from `TaskPath`. Touching `TaskPath` causes a lot of churn in snapshot tests, but there don't seem to be any changes in actual behaviour.

rustbot assigned jieyouxu Jul 30, 2026

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 Jul 30, 2026

jieyouxu reviewed Jul 30, 2026

View reviewed changes

rust-bors Bot 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 Jul 30, 2026

rust-bors Bot 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 30, 2026

jieyouxu approved these changes Jul 30, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Zalathar force-pushed the unkind branch from 8740040 to 3121de3 Compare July 30, 2026 06:06

Zalathar force-pushed the unkind branch from 3121de3 to c3c4090 Compare July 30, 2026 07:14

jieyouxu added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 31, 2026

This comment has been minimized.

Sign in to view

Zalathar force-pushed the unkind branch from c3c4090 to 10a3cc5 Compare July 31, 2026 14:00

This comment has been minimized.

Sign in to view

jieyouxu approved these changes Aug 2, 2026

View reviewed changes

rust-bors Bot 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 Aug 2, 2026

JonathanBrouwer mentioned this pull request Aug 2, 2026

Rollup of 5 pull requests #160381

Closed

rust-bors Bot 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 2, 2026

Zalathar added 2 commits August 2, 2026 21:44

Avoid inappropriate --skip handling in run_default_doc_steps

dc6e0ad

Don't pass Kind into PathSet::check

1cadc4b

Zalathar force-pushed the unkind branch from 10a3cc5 to a0584f8 Compare August 2, 2026 11:44

Don't include Kind in TaskPath

4ca76aa

Zalathar force-pushed the unkind branch from a0584f8 to 4ca76aa Compare August 2, 2026 11:57

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Aug 2, 2026

Auto merge of #160191 - Zalathar:unkind, r=<try>

4d54108

bootstrap: Don't pass `Kind` to some places that don't need it try-job: i686-msvc-2 try-job: aarch64-msvc-2

rust-bors Bot 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 Aug 3, 2026

Zalathar mentioned this pull request Aug 3, 2026

Rollup of 5 pull requests #160411

Merged

rust-bors Bot merged commit e64546e into rust-lang:main Aug 3, 2026
14 checks passed

rustbot added this to the 1.99.0 milestone Aug 3, 2026

Zalathar deleted the unkind branch August 3, 2026 06:31

		// This order is important for retro-compatibility, as `starts_with` was introduced later.
		p.path.ends_with(needle) \|\| p.path.starts_with(needle)

Uh oh!

Uh oh!

Conversation

Zalathar commented Jul 30, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 30, 2026

Uh oh!

jieyouxu left a comment • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Jul 30, 2026

Uh oh!

jieyouxu commented Jul 30, 2026

Uh oh!

rust-bors Bot commented Jul 30, 2026

Uh oh!

jieyouxu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

jieyouxu commented Jul 30, 2026

Uh oh!

Zalathar commented Jul 30, 2026

Uh oh!

jieyouxu commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zalathar commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zalathar commented Jul 30, 2026

Uh oh!

Zalathar commented Jul 30, 2026

Uh oh!

Zalathar commented Jul 30, 2026

Uh oh!

Zalathar commented Jul 30, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

jieyouxu left a comment • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Aug 2, 2026

Uh oh!

JonathanBrouwer commented Aug 2, 2026

Uh oh!

rust-bors Bot commented Aug 2, 2026

Uh oh!

rustbot commented Aug 2, 2026

Uh oh!

Zalathar commented Aug 2, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Aug 2, 2026

Uh oh!

Zalathar commented Aug 3, 2026

Uh oh!

Zalathar commented Jul 30, 2026 •

edited by rustbot

Loading

jieyouxu left a comment •

edited

Loading

jieyouxu left a comment •

edited by rustbot

Loading

jieyouxu commented Jul 30, 2026 •

edited

Loading

Zalathar commented Jul 30, 2026 •

edited

Loading

jieyouxu left a comment •

edited

Loading