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.

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).

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 added 3 commits July 30, 2026 17:04

Avoid inappropriate --skip handling in run_default_doc_steps

a6b6d9b

Don't pass Kind into PathSet::check

6eb5393

Don't include Kind in TaskPath

c3c4090

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

		// 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

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!

jieyouxu Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Zalathar Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Zalathar Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jul 30, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

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