CI: Verify commits in beta & stable are in upstream branches.

[yaymukund]

Closes #74721

I think this does the trick. #87597 is an example of it failing as it should.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pietroalbini (or someone else) soon.

Please see the contribution instructions for more information.

[pietroalbini]

I'll soon go on vacation and I won't have time to review PRs then. r? @Mark-Simulacrum?

[Mark-Simulacrum]

Could you add some documentation indicating what the outputs are / limitations here etc? For example, it looks like there are a number of commits different between the branches, but it's not quite clear to me what the expectations are for when those need to be updated and such.

We'll also want to be careful to avoid a situation where editing these files to include a commit is "impossible" because that would change the commit hash. At a quick glance, it seems like this could be a problem. Maybe instead of a single file tracking this information, we would prefer to ensure that each commit on the beta and stable branches have a line in them indicating the corresponding hash on the master branch, e.g.,

backport-of: <hash>

[yaymukund]

@Mark-Simulacrum That all makes sense.

I've implemented your backport-of: <sha> suggestion and added more details in the script & in the output (see a sample failure).

Edit: Thought about it some more...

This works fine if a single commit requires conflict resolution. However, if you merge an entire branch into (e.g.) beta but not master then each commit in the branch will need a backport-of: <sha> commit message. Is that acceptable?

[Mark-Simulacrum]

This works fine if a single commit requires conflict resolution. However, if you merge an entire branch into (e.g.) beta but not master then each commit in the branch will need a backport-of: commit message. Is that acceptable?

Yeah, this is expected. I usually cherry-pick commits one by one anyway, so it shouldn't really be a big deal.

Can we add a script like src/tools/cherry-pick.sh which is essentially a wrapper around git cherry-pick but additionally edits the commit message after picking to include the backport-of line? I'd suggest using the git cherry-pick -x option, but it looks like that doesn't include the upstream commit hash if the commit has conflicts when cherry picking, so it won't quite work for us...

[Mark-Simulacrum]

Could you refactor this to take multiple parameters (and just cherry-pick each in turn, just like the normal cherry pick)?

[yaymukund]

PLEASE NOTE: DO NOT MERGE: This is producing strange behavior in CI and should not be merged in its current state. I shall update when it's fixed.

(Specifically: I added parentheses to the backport-of annotation and it's failing on correct backports.)

Update: It's fixed, and should be good to review / possibly merge. ✔️

[yaymukund]

@Mark-Simulacrum If I understand your comment correctly, I don't think this script works if your cherry-pick has a conflict. You get dumped into the conflict resolution phase and the subsequent git commit --amend doesn't ever happen. I can try to make this atomic if you wish - I think there's an alternate approach where I set GIT_EDITOR=/some/script git cherry-pick -e.

This gets worse if you have a conflict in the middle of a long series of commits, as the first n commits may be successfully applied, but n+1 breaks. I don't see how to fix this easily.

[Mark-Simulacrum]

Hm. Yes, I see your point. I think it might be possible that we can make it work using --continue as an argument to our wrapper script, but that might be hard -- I guess the goal would be to store some state that we could use to sort of resume the script. It seems like git-cherry-pick itself does this via .git/sequencer based on the manpage:

Continue the operation in progress using the information in .git/sequencer

But I'm not sure whether that file is documented anywhere or if editing it is in anyway feasible.

It's probably not a huge deal for us to manually deal with conflicts and amend commits after the fact; CI will yell at us after all...

[Mark-Simulacrum]

I think if we can get it to work via GIT_EDITOR somehow, then let's try that, but if not, then no big deal.

[yaymukund]

Let's stick with the present approach.

I've just tested it and GIT_EDITOR doesn't seem to persist through conflict resolution. We'd need to export GIT_EDITOR, which means we'd also have to unset it. I don't think it's worth it.

[Mark-Simulacrum]

Oh, one more thought: we will have commits on beta/stable that are specific to those branches, such as 56397e9 -- those will want their own annotation like "backport-of: nothing" or something, still explicitly denoted.

Could you add that support?

I think that's the last thing.

[Mark-Simulacrum]

@bors r+ rollup

Thanks! Looking forward to seeing this work in practice so we can see how effective/painful it is.

[bors]

📌 Commit f19ba35 has been approved by Mark-Simulacrum

[yaymukund]

Thanks for reviewing. Feel free to tag me in followup work if it would be helpful.