tidy: add triagebot checks #137885

klensy · 2025-03-02T10:09:24Z

Validates triagebot.toml to have existing paths:

[mentions."*"] sections, i.e.

[mentions."compiler/rustc_const_eval/src/"]

or

[assign.owners]
"/.github/workflows" = ["infra-ci"]

or

trigger_files = [
 "src/librustdoc/html/static/js/search.js",
 "tests/rustdoc-js",
 "tests/rustdoc-js-std",
 ]

Looked at #137876 and implemented check.

rustbot · 2025-03-02T10:09:28Z

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

rustbot · 2025-03-02T10:09:31Z

triagebot.toml has been modified, there may have been changes to the review queue.

cc @davidtwco, @wesleywiser

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

klensy · 2025-03-02T10:10:17Z

Few errors left, how to fix them?

tidy error: triagebot.toml [mentions.*] contains path 'compiler/rustc_attr_validation' which doesn't exist
tidy error: triagebot.toml [mentions.*] contains path 'compiler/rustc_lint/src/context/diagnostics/check_cfg.rs' which doesn't exist
tidy error: triagebot.toml [assign.owners] contains path 'compiler/rustc_const_eval/src/transform' which doesn't exist

@rustbot blocked #137876 the same fix

Will add more fixes...

fmease · 2025-03-02T10:28:06Z

tidy error: triagebot.toml [mentions.*] contains path 'compiler/rustc_attr_validation' which doesn't exist

Can be removed for now (cc #136643 (comment)).

tidy error: triagebot.toml [mentions.*] contains path 'compiler/rustc_lint/src/context/diagnostics/check_cfg.rs' which doesn't exist

Change it to compiler/rustc_lint/src/early/diagnostics/check_cfg.rs.

klensy · 2025-03-02T10:33:57Z

Added check for trigger_files and pushed fix from #137885 (comment):

tidy error: triagebot.toml [assign.owners] contains path 'compiler/rustc_const_eval/src/transform' which doesn't exist
tidy error: triagebot.toml [autolabel.A-meta] contains trigger_files path '.reuse' which doesn't exist
tidy error: triagebot.toml [autolabel.O-apple] contains trigger_files path 'library/std/src/sys/pal/unix/thread_parking/darwin.rs' which doesn't exist
tidy error: triagebot.toml [autolabel.O-wasm] contains trigger_files path 'library/std/src/os/wasm' which doesn't exist
tidy error: triagebot.toml [autolabel.T-libs] contains trigger_files path 'library/term' which doesn't exist
tidy error: triagebot.toml [autolabel.WG-trait-system-refactor] contains trigger_files path 'compiler/rustc_middle/src/traits/solve' which doesn't exist

fmease · 2025-03-02T10:45:48Z

There's one thing to discuss:

With this check theoretically being r-l/r agnostic why don't we move it into triagebot itself? Ofc, triagebot wouldn't be able to make CI fail then (well, it could cancel it I guess lol).

For context, in the past if triagebot.toml wasn't valid TOML all checks would still pass and triagebot would essentially ignore it / no longer consider it. Triagebot (not tidy) was then changed to at least post an error message as a comment (example) but iinm it still doesn't block a PR, so hypothetically a reviewer could still miss it and approve the PR regardless.

jieyouxu · 2025-03-02T10:59:55Z

I do agree it feels like the check belongs in triagebot (since it should be generalizable to repos that use triagebot), but I'm also okay with it being in tidy if it's a hassle to implement in triagebot.

cc @apiraino for vibes

Noratrieb · 2025-03-02T11:02:20Z

This should make CI fail, and triagebot can't make CI fail, so needs to be in tidy. Even if conceptually the code fits more into triagebot. I would much rather merge this instead of trying very hard to figure out a solution to make this more repo agnostic. We can always put something into triagebot in the future.

klensy · 2025-03-02T11:05:04Z

Implementing checks in triagebot is right thing (and should be there), but current impl is "just works" TM.

fmease · 2025-03-02T11:07:36Z

This should make CI fail, and triagebot can't make CI fail, so needs to be in tidy

I do see that. On the other hand as I mentioned above, a PR can simply disable triagebot for the entire repository (arguably more severe than dead paths) by introducing a TOML syntax error or a schema error without it affecting CI or bors (iinm). /perfect is the enemy of good

Noratrieb · 2025-03-02T11:09:39Z

tidy error: triagebot.toml [assign.owners] contains path 'compiler/rustc_const_eval/src/transform' which doesn't exist

you can delete this

tidy error: triagebot.toml [autolabel.A-meta] contains trigger_files path '.reuse' which doesn't exist

change to REUSE.toml

tidy error: triagebot.toml [autolabel.O-apple] contains trigger_files path 'library/std/src/sys/pal/unix/thread_parking/darwin.rs' which doesn't exist

change to library/std/src/sys/sync/thread_parking/darwin.rs

tidy error: triagebot.toml [autolabel.O-wasm] contains trigger_files path 'library/std/src/os/wasm' which doesn't exist

change to library/std/src/os/wasi and library/std/src/os/wasip2

tidy error: triagebot.toml [autolabel.T-libs] contains trigger_files path 'library/term' which doesn't exist

remove

tidy error: triagebot.toml [autolabel.WG-trait-system-refactor] contains trigger_files path 'compiler/rustc_middle/src/traits/solve' which doesn't exist

remove

src/tools/tidy/src/triagebot.rs

triagebot.toml

bors · 2025-03-03T08:12:38Z

☔ The latest upstream changes (presumably #137900) made this pull request unmergeable. Please resolve the merge conflicts.

apiraino · 2025-03-03T14:09:23Z

This should make CI fail, and triagebot can't make CI fail, so needs to be in tidy.

This seems a convincing argument to keep this check in tidy rather than moving it to the triagebot server

klensy · 2025-03-03T14:46:03Z

@rustbot label -S-blocked
#137876 merged

jieyouxu

Thanks for the PR. I have two requests:

Ensure that for rust-lang/rust this tidy check fails if triagebot.toml is missing [assign] or [mentions] sections.
Squash the commits into 2: 1 commit for tidy changes, and 1 commit for triagebot.toml fixes. Or just squish everything.

src/tools/tidy/src/triagebot.rs

klensy · 2025-03-05T17:40:04Z

@rustbot ready

jieyouxu

Thanks!

jieyouxu · 2025-03-09T02:29:28Z

@bors r+ rollup

bors · 2025-03-09T02:29:30Z

📌 Commit aa72de9 has been approved by jieyouxu

It is now in the queue for this repository.

tidy: add triagebot checks Validates triagebot.toml to have existing paths: `[mentions."*"]` sections, i.e. ```toml [mentions."compiler/rustc_const_eval/src/"] ``` or ```toml [assign.owners] "/.github/workflows" = ["infra-ci"] ``` or ```toml trigger_files = [ "src/librustdoc/html/static/js/search.js", "tests/rustdoc-js", "tests/rustdoc-js-std", ] ``` Looked at rust-lang#137876 and implemented check.

Rollup of 16 pull requests Successful merges: - rust-lang#122790 (Apply dllimport in ThinLTO) - rust-lang#136127 (Allow `*const W<dyn A> -> *const dyn A` ptr cast) - rust-lang#136968 (Turn order dependent trait objects future incompat warning into a hard error) - rust-lang#137147 (Add exclude to config.toml) - rust-lang#137319 (Stabilize `const_vec_string_slice`) - rust-lang#137885 (tidy: add triagebot checks) - rust-lang#138040 (compiler: Use `size_of` from the prelude instead of imported) - rust-lang#138052 (strip `-Wlinker-messages` wrappers from `rust-lld` rmake test) - rust-lang#138084 (Use workspace lints for crates in `compiler/`) - rust-lang#138158 (Move more layouting logic to `rustc_abi`) - rust-lang#138160 (depend more on attr_data_structures and move find_attr! there) - rust-lang#138192 (crashes: couple more tests) - rust-lang#138216 (bootstrap: Fix stack printing when a step cycle is detected) - rust-lang#138232 (Reduce verbosity of GCC build log) - rust-lang#138233 (Windows: Don't link std (and run-make) against advapi32, except on win7) - rust-lang#138242 (Revert "Don't test new error messages with the stage 0 compiler") r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#136127 (Allow `*const W<dyn A> -> *const dyn A` ptr cast) - rust-lang#136968 (Turn order dependent trait objects future incompat warning into a hard error) - rust-lang#137319 (Stabilize `const_vec_string_slice`) - rust-lang#137885 (tidy: add triagebot checks) - rust-lang#138040 (compiler: Use `size_of` from the prelude instead of imported) - rust-lang#138084 (Use workspace lints for crates in `compiler/`) - rust-lang#138158 (Move more layouting logic to `rustc_abi`) - rust-lang#138160 (depend more on attr_data_structures and move find_attr! there) - rust-lang#138192 (crashes: couple more tests) - rust-lang#138216 (bootstrap: Fix stack printing when a step cycle is detected) - rust-lang#138232 (Reduce verbosity of GCC build log) - rust-lang#138242 (Revert "Don't test new error messages with the stage 0 compiler") r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#137885 - klensy:tidy-triagebot, r=jieyouxu tidy: add triagebot checks Validates triagebot.toml to have existing paths: `[mentions."*"]` sections, i.e. ```toml [mentions."compiler/rustc_const_eval/src/"] ``` or ```toml [assign.owners] "/.github/workflows" = ["infra-ci"] ``` or ```toml trigger_files = [ "src/librustdoc/html/static/js/search.js", "tests/rustdoc-js", "tests/rustdoc-js-std", ] ``` Looked at rust-lang#137876 and implemented check.

rustbot assigned jieyouxu Mar 2, 2025

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2025

This comment has been minimized.

# to view

Noratrieb reviewed Mar 2, 2025

View reviewed changes

src/tools/tidy/src/triagebot.rs Show resolved Hide resolved

Noratrieb reviewed Mar 2, 2025

View reviewed changes

triagebot.toml Outdated Show resolved Hide resolved

klensy force-pushed the tidy-triagebot branch from 520152b to 65fca9a Compare March 3, 2025 10:34

rustbot removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 3, 2025

jieyouxu approved these changes Mar 3, 2025

View reviewed changes

src/tools/tidy/src/triagebot.rs Show resolved Hide resolved

src/tools/tidy/src/triagebot.rs Show resolved Hide resolved

jieyouxu added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 3, 2025

tidy: add check to verify paths in triagebot.toml

8edf528

fix triagebot.toml

aa72de9

klensy force-pushed the tidy-triagebot branch from 65fca9a to aa72de9 Compare March 5, 2025 09:31

rustbot 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 Mar 5, 2025

jieyouxu approved these changes Mar 9, 2025

View reviewed changes

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 Mar 9, 2025

jieyouxu mentioned this pull request Mar 9, 2025

Rollup of 16 pull requests #138255

Closed

matthiaskrgr mentioned this pull request Mar 9, 2025

Rollup of 12 pull requests #138267

Merged

bors merged commit cdd97ba into rust-lang:master Mar 9, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 9, 2025

jieyouxu mentioned this pull request Apr 15, 2025

[WIP] tidy: skip triagebot.toml path checks for local submodule directories that we can't read #139860

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tidy: add triagebot checks #137885

tidy: add triagebot checks #137885

klensy commented Mar 2, 2025 •

edited

Loading

rustbot commented Mar 2, 2025

rustbot commented Mar 2, 2025

klensy commented Mar 2, 2025 •

edited

Loading

This comment has been minimized.

fmease commented Mar 2, 2025

klensy commented Mar 2, 2025

This comment has been minimized.

fmease commented Mar 2, 2025 •

edited

Loading

jieyouxu commented Mar 2, 2025

Noratrieb commented Mar 2, 2025 •

edited

Loading

klensy commented Mar 2, 2025

fmease commented Mar 2, 2025 •

edited

Loading

Noratrieb commented Mar 2, 2025

bors commented Mar 3, 2025

apiraino commented Mar 3, 2025

klensy commented Mar 3, 2025

jieyouxu left a comment •

edited

Loading

klensy commented Mar 5, 2025

jieyouxu left a comment

jieyouxu commented Mar 9, 2025

bors commented Mar 9, 2025

tidy: add triagebot checks #137885

tidy: add triagebot checks #137885

Conversation

klensy commented Mar 2, 2025 • edited Loading

rustbot commented Mar 2, 2025

rustbot commented Mar 2, 2025

klensy commented Mar 2, 2025 • edited Loading

This comment has been minimized.

fmease commented Mar 2, 2025

klensy commented Mar 2, 2025

This comment has been minimized.

fmease commented Mar 2, 2025 • edited Loading

jieyouxu commented Mar 2, 2025

Noratrieb commented Mar 2, 2025 • edited Loading

klensy commented Mar 2, 2025

fmease commented Mar 2, 2025 • edited Loading

Noratrieb commented Mar 2, 2025

bors commented Mar 3, 2025

apiraino commented Mar 3, 2025

klensy commented Mar 3, 2025

jieyouxu left a comment • edited Loading

Choose a reason for hiding this comment

klensy commented Mar 5, 2025

jieyouxu left a comment

Choose a reason for hiding this comment

jieyouxu commented Mar 9, 2025

bors commented Mar 9, 2025

klensy commented Mar 2, 2025 •

edited

Loading

klensy commented Mar 2, 2025 •

edited

Loading

fmease commented Mar 2, 2025 •

edited

Loading

Noratrieb commented Mar 2, 2025 •

edited

Loading

fmease commented Mar 2, 2025 •

edited

Loading

jieyouxu left a comment •

edited

Loading