rustfmt feature: Sync r-l/r milestones to r-l/rustfmt when merging subtree sync PRs by ytmimi · Pull Request #2499 · rust-lang/triagebot

ytmimi · 2026-09-10T18:33:53Z

This adds a new step to the milestone_prs::handle that tries to sync r-l/r milestones to r-l/rustfmt after merging r-l/r subtree sync PRs.

For this to work correctly the r-l/r PR needs to have both the T-rustfmt and the subtree-sync label. Then on the r-l/rustfmt side rustbot will only set the milestone on closed r-l/rustfmt PRs that were previously labeled with release-notes.

It's possible that some r-l/rustfmt PRs get merged into r-l/rustfmt's main branch between the time it takes to start the sync process and open the subtree sync PR in r-l/r. To mitigate issues around incorrectly setting a milestone on r-l/rustfmt PRs that didn't make it into the r-l/r sync PR, there's an additional filter that only sets milestones on r-l/rustfmt PRs that were merged at least one hour before the r-l/r sync PR was opened.

cc: @rust-lang/rustfmt @rust-lang/rustfmt-contributors

Note

I didn't know if there was a good way to test this change. GithubClient and Repository are both concrete types so I don't think there's a good way to mock out their behavior to directly test different scenarios.

Urgau · 2026-09-10T19:58:31Z

Instead of looking at the closed/merged PRs, I think we should be able to use the merge commits created in r-l/rustfmt to our advantage to have an accurate list of the merged PRs.

We know they have a title like this: Merge pull request #<pr_num> from <user>/<branch>.

I think we could use that to our advantage, by:

finding all the commits that match the merge title (inside event.issue.compare(gh)?.commits)
figure out if the commits that matched are in rust-lang/rustfmt
- can be done with GitHub API: https://api.github.com/repos/rust-lang/rustfmt/git/commits/a753af41d503edf1d3d0a568ac8d491f3b36724d (we probably have a method for that)
set the milestone for all the PRs we correlated back

What do you think?

An optimization to be done is to only to this when files under src/tools/rustfmt are being changed. milestone_submodules has such detection

ytmimi · 2026-09-10T20:29:20Z

@Urgau thank you for the feedback! Seems like a much more robust approach than what I was originally thinking. Taking the most recent subtree-sync as an example (rust-lang/rust#161964) this is what I think we could do.

Paginate through GET request to https://api.github.com/repos/rust-lang/rust/pulls/161964/commits
Pull out all merge commits that have Merge pull request #<pr_num>. rustfmt recently started using a merge queue so all new PRs should get merged into the main branch this way.
Apply the milestone to all PRs that we just found.

Urgau · 2026-09-11T05:51:03Z

this is what I think we could do

That seems broadly fine with the caveat that we need to check that the merge commits are in r-l/rustfmt, as the subtree sync could have been in a rollup and we would start milestoning the wrong PRs.

Btw, we already have a method to get all the commits of a PR.

ytmimi · 2026-09-11T14:03:13Z

Btw, we already have a method to get all the commits of a PR.

That's amazing!

That seems broadly fine with the caveat that we need to check that the merge commits are in r-l/rustfmt, as the subtree sync could have been in a rollup and we would start milestoning the wrong PRs.

I was thinking that we'd just look at the commit messages and use a regex to extract the PR number. In that case would we still need to check that the commits are in rustfmt? Maybe I'm misunderstanding something?

We typically don't include rustfmt subtree syncs in rollups, but I agree that we shouldn't rely on that here. Is the concern that if it's included in a rollup that we might accidentally assign milestones to PR numbers from other repos that just happen to have the same number as rustfmt? Maybe the T-rustfmt and subtree-sync label checks would help prevent that? Presumably the rollup wouldn't get tagged with subtree-sync?

My original thought was that this would only get triggered when the subtree-sync PR was closed and that we'd be able to reference that PR and grab all of its commit messages. At that point we'd know that all the PRs referenced in the Merge pull request #<pr_number> were rustfmt PRs. I didn't consider rollups when I first started thinking about this, so maybe there are some edge cases around those that I need to think through.

I'm also wondering if your original suggestion to use event.issue.compare(gh)?.commits would be easier for this? I'm just not familiar with the triagebot repo and what's the easiest way to go about this.

Urgau · 2026-09-11T17:06:45Z

Is the concern that if it's included in a rollup that we might accidentally assign milestones to PR numbers from other repos that just happen to have the same number as rustfmt?

Yes, that's my concern.

Not checking that the commits are indeed in r-l/rustfmt would effectively prevent doing rollups, which is not ideal, in particular since we don't have a way to prevent someone from accidentally rollup-ing future syncs.

Maybe the T-rustfmt and subtree-sync label checks would help prevent that? Presumably the rollup wouldn't get tagged with subtree-sync?

I rather avoid gating on labels when we can, they can be forgotten or missed by triagebot (we may miss a webhook); and as you mention it as well the rollup probably won't get tagged with subtree-sync.

I'm also wondering if your original suggestion to use event.issue.compare(gh)?.commits would be easier for this?

I think our implementation does not properly gather all the pages (it only does one call), so we would have to fix it first before using it. I will try fixing it this week-end.

But the reason I suggested compare is that it's already called in the handler (via diff) and that it's output is cached for all the handlers, so one less call.

ytmimi · 2026-09-11T17:41:07Z

I just asked a question in #t-rustfmt > Merge queue. I'm wondering if it would simplify things if we could update the merge commit messages from Merge pull request #{pr_number} -> Merge pull request rust-lang/rustfmt#{pr_number} so that we'd have more confidence that the commit came from rustfmt, and then we'd have an easier time pulling out the PR numbers that we need. Would probably also help us avoid issues with rollups.

Edit: Not sure how feasible this is, but if we could set something like that up then I think we might be able to take a similar approach for other subtrees.

Urgau · 2026-09-11T17:55:17Z

I'm wondering if it would simplify things if we could update the merge commit messages from Merge pull request #{pr_number} -> Merge pull request rust-lang/rustfmt#{pr_number}

That would certainly help, unfortunately it's not possible. GitHub doesn't allow setting a custom merge queue commit title (see https://github.com/orgs/community/discussions/15925).

But it's not necessary, we just need to make sure the SHA of the merge commit (the ones with Merge pull request #{pr_number}), is in rust-lang/rustfmt. Which can be done pretty easly with GitHub API by trying to fetch the details of the commit in rust-lang/rustfmt https://api.github.com/repos/rust-lang/rustfmt/git/commits/a753af41d503edf1d3d0a568ac8d491f3b36724d.

ytmimi · 2026-09-11T18:52:36Z

@Urgau Are the SHAs from the merge commits guaranteed to be the same? I thought they might be different, but I picked out one of the commits from rust-lang/rust#161964 to double check and it looks like the SHAs are consistent. From this message:

Merge pull request #7066 from ytmimi/issue_7062
fix: correct the span used when rewriting `ast::TyKind::FnPtr`

Which corresponds to this rustfmt PR rust-lang/rustfmt#7066

rustfmt commit: rust-lang/rustfmt@dbc566c
rust-lang/rust commit: rust-lang/rust@dbc566c

Another example from rust-lang/rust#161964 that's not a merge commit because it was merged before we started using the merge queue, but it looks like the SHAs are consistent there too:

rustfmt PR: rust-lang/rustfmt#6010

rustfmt commit: rust-lang/rustfmt@0675df6
rust-lang/rust commit rust-lang/rust@0675df6

Urgau · 2026-09-11T21:30:59Z

Are the SHAs from the merge commits guaranteed to be the same?

Good question. Seems like they are the same with git subtree, at least while looking at your subtree sync PRs.

However, looking at a Josh sync, I'm not seeing the same SHA, in rust-lang/rust rust-lang/rust@8173a82, in rust-lang/miri rust-lang/miri@8de2ef9.

Any hindsight to share @Kobzol?

Kobzol · 2026-09-11T21:57:38Z

With Josh, the commit SHAs in r-l/r and the subtree will be different.

Urgau · 2026-09-11T22:13:40Z

With Josh, the commit SHAs in r-l/r and the subtree will be different.

That's a good news, for rustfmt at least, less for Josh subtrees.

I guess there isn't an easy way to correlate them without running Josh?

However, looking at a Josh sync, I'm not seeing the same SHA, in rust-lang/rust rust-lang/rust@8173a82, in rust-lang/miri rust-lang/miri@8de2ef9.

Looking more that those two commits, the one is rust-lang/rust has it's path corrected to src/tools/miri, but that's not the same for the rustfmt ones rust-lang/rust@dbc566c, they are not corrected in rust-lang/rust with git subtree.

So what I think we can do for Josh subtrees, if we ever get one that wants milestones, is to look at which files the commit modifies.

If the commit only modifies files inside the subtree and is named Merge pull request #{pr_number}, then we milestone the corresponding PR.

This is of course hypothetical since rustfmt is not a Josh subtree, and we can just check the SHA with git subtree.

jieyouxu · 2026-09-14T07:00:55Z

This is of course hypothetical since rustfmt is not a Josh subtree, and we can just check the SHA with git subtree.

Right, but I think it's more "not yet", I think we'd like to move to become a Josh subtree to make the syncs easier, once the WIP Josh things are settled down.

ytmimi · 2026-09-15T06:43:31Z

Pushed a lot of changes. I applied all the feedback and then tried to follow the same structure used by milestone_submodules and milestone_submodule to generalize things for any other git subtree to use. As mentioned earlier, we'll need to take a different approach for josh subtrees since the SHAs won't match.

Urgau · 2026-09-15T19:01:01Z

Looking at #t-rustfmt > Migrating to Josh, seems like the migration to Josh is pretty imminent.

I'm fine merging the PR the meantime, but since it will only be used once (hopefully), it may be better to prepare instead for Josh?

Regarding what to do with Josh subtrees, we talked about it at #t-infra > saving the original commit SHAs with Josh subtree and the consensus is to filter all the "Merge pull request" commits and milestone those who only contain src/tools/rustfmt changes.

ytmimi · 2026-09-15T19:24:28Z

Looking at #t-rustfmt > Migrating to Josh, seems like the migration to Josh is pretty imminent.

I'm fine merging the PR the meantime, but since it will only be used once (hopefully), it may be better to prepare instead for Josh?

Regarding what to do with Josh subtrees, we talked about it at #t-infra > saving the original commit SHAs with Josh subtree and the consensus is to filter all the "Merge pull request" commits and milestone those who only contain src/tools/rustfmt changes.

The transition to a Josh Subtree is likely around the corner, but I'd like to test out this current version for rustfmt it you're okay with it. My hope with the current milestone_git_subtrees implementation is that it could be easily extended to support other git subtrees. Clippy comes to mind.

That said, once we transition rustfmt to a Josh subtree we can also remove the git subtree implementation if you don't want dead code kicking around in triagebot.

I'm happy to make those changes once the time comes!

Urgau · 2026-09-15T19:51:51Z

My hope with the current milestone_git_subtrees implementation is that it could be easily extended to support other git subtrees. Clippy comes to mind

Clippy is probably also around the corner with regards to using Josh: rust-lang/rust-clippy#17739; but point taken.

ytmimi · 2026-09-15T19:54:58Z

😅 Oh I didn't realize that clippy was planning to transition too. Well then maybe one of these other subtrees :)

Kobzol · 2026-09-15T19:55:56Z

Hopefully all of them will migrate soon, now that we have the new version of Josh working :)

Urgau

The logic looks good to me. Only a small nit and we should be ready to go.

View changes since this review

…ng subtree syncs This adds a new step to the `milestone_prs::handle` that tries to sync r-l/r milestones to r-l/rustfmt after merging r-l/r subtree sync PRs.

…MMIT_TITLE_RE`

…tfmt` Instead of using the `T-rustfmt` label, check that the PR modified files in `src/tools/rustfmt`.

This is still implemented for rustfmt, but the approach can be used for any rust-lang/rust git subtree that uses a merge queue to to standardize how PRs are merged.

rustbot · 2026-09-17T19:09:59Z

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.

Urgau

Let's try it.

View changes since this review

ytmimi force-pushed the rustfmt_milestones branch from 9d37c4d to f1208d1 Compare September 10, 2026 18:54

ytmimi mentioned this pull request Sep 10, 2026

Milestone PRs in core Rust tooling repos #813

Open

Urgau mentioned this pull request Sep 11, 2026

Fetch up to a 1000 commits when getting the comparison of the PR #2500

Merged

ytmimi force-pushed the rustfmt_milestones branch from f1208d1 to 832f5c2 Compare September 14, 2026 06:56

This comment has been minimized.

Sign in to view

ytmimi force-pushed the rustfmt_milestones branch from 832f5c2 to 849f814 Compare September 14, 2026 06:59

Urgau reviewed Sep 14, 2026

View reviewed changes

Comment thread src/handlers/milestone_prs.rs Outdated

Comment thread src/handlers/milestone_prs.rs Outdated

Comment thread src/handlers/milestone_prs.rs Outdated

Comment thread src/handlers/milestone_prs.rs Outdated

ytmimi force-pushed the rustfmt_milestones branch from 849f814 to d2c206c Compare September 15, 2026 06:41

This comment has been minimized.

Sign in to view

ytmimi force-pushed the rustfmt_milestones branch from d2c206c to b1f3ce9 Compare September 15, 2026 15:34

Urgau reviewed Sep 16, 2026

View reviewed changes

Comment thread src/handlers/milestone_prs.rs Outdated

milestones (rustfmt): Sync r-l/r milestones to r-l/rustfmt when mergi…

3228ec8

…ng subtree syncs This adds a new step to the `milestone_prs::handle` that tries to sync r-l/r milestones to r-l/rustfmt after merging r-l/r subtree sync PRs.

ytmimi added 5 commits September 17, 2026 15:09

refactor milestone(rustfmt): Use a LazyLock for the `MERGE_QUEUE_CO…

a37a46e

…MMIT_TITLE_RE`

refactor milestone(rustfmt): check that the PR touched `src/tools/rus…

efaf3b3

…tfmt` Instead of using the `T-rustfmt` label, check that the PR modified files in `src/tools/rustfmt`.

chore milestone(rustfmt): Add logging for PRs we're about to milestone.

d2b0aa4

refactor milestone(rustfmt): simplify subtree merge commit check

ba0f19f

refactor milestone: generalize milestone assignment for git subtrees

b6455bb

This is still implemented for rustfmt, but the approach can be used for any rust-lang/rust git subtree that uses a merge queue to to standardize how PRs are merged.

ytmimi force-pushed the rustfmt_milestones branch from b1f3ce9 to b6455bb Compare September 17, 2026 19:09

Urgau approved these changes Sep 17, 2026

View reviewed changes

Urgau added this pull request to the merge queue Sep 17, 2026

Merged via the queue into rust-lang:main with commit 8b2c2e0 Sep 17, 2026
4 checks passed

ytmimi deleted the rustfmt_milestones branch September 17, 2026 20:02

Conversation

ytmimi commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Urgau commented Sep 10, 2026

Uh oh!

ytmimi commented Sep 10, 2026

Uh oh!

Urgau commented Sep 11, 2026

Uh oh!

ytmimi commented Sep 11, 2026

Uh oh!

Urgau commented Sep 11, 2026

Uh oh!

ytmimi commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Urgau commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ytmimi commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Urgau commented Sep 11, 2026

Uh oh!

Kobzol commented Sep 11, 2026

Uh oh!

Urgau commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

jieyouxu commented Sep 14, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

ytmimi commented Sep 15, 2026

Uh oh!

Urgau commented Sep 15, 2026

Uh oh!

ytmimi commented Sep 15, 2026

Uh oh!

Urgau commented Sep 15, 2026

Uh oh!

ytmimi commented Sep 15, 2026

Uh oh!

Kobzol commented Sep 15, 2026

Uh oh!

Urgau 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!

Uh oh!

rustbot commented Sep 17, 2026

Uh oh!

Urgau 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ytmimi commented Sep 10, 2026 •

edited

Loading

ytmimi commented Sep 11, 2026 •

edited

Loading

Urgau commented Sep 11, 2026 •

edited

Loading

ytmimi commented Sep 11, 2026 •

edited

Loading

Urgau commented Sep 11, 2026 •

edited

Loading

Urgau left a comment •

edited by rustbot

Loading

Urgau left a comment •

edited by rustbot

Loading