Add @triagebot squash command

[jyn514]

A common challenge new contributors have is squashing commits into one:

• This requires having a local clone of the repo (especially for doc PRs, most people just use the web interface)
• Knowing how to use git rebase, including that you should use -i to edit individual commits
• Knowing that git push -f is ok (this last step trips a lot of people up - they'll rebase and then 'merge branch into master' which defeats the whole point)

Even for existing contributors squashing is annoying because it messes up your build cache, since cargo works on mtimes and not hashes.

It would be great to have a squash command that automatically squashes all commits in the current PR into one. The way to do this locally is

git checkout https://github.com/rust-lang/rust/pull/NNNNN && git rebase -i origin/master && git push -f [email protected]:CONTRIBUTOR/rust.git HEAD:BRANCH

NNNNN is the pr number, CONTRIBUTOR is the author, BRANCH is the branch name; all of these are easily available from the PR itself.

[jyn514]

This first made me annoyed enough to open an issue in rust-lang/rust#76289, but it's happened a lot before that too.

[camelid]

git checkout https://github.com/rust-lang/rust/pull/NNNNN && git rebase -i origin/master && git push -f [email protected]:CONTRIBUTOR/rust.git HEAD:BRANCH

@jyn514 Hmm, are you sure this command is correct? Running (for example)

$ git checkout https://github.com/rust-lang/rust/pull/77508
error: pathspec 'https://github.com/rust-lang/rust/pull/77508' did not match any file(s) known to git

fails for me.

Also:

• We probably need to git fetch first
• We should use git rebase -i $(git merge-base master HEAD) so we don't introduce merge conflicts
• We should use --force-with-lease so we don't overwrite new commits

[jyn514]

Sure, all of those seem fine. I never actually tested these commands.

[Mark-Simulacrum]

You need to install hub (GitHub integration for git) if you want the command to work without fetching.

[camelid]

Or you can use the newly-stable gh tool!

[kellda]

What about https://docs.github.com/en/free-pro-team@latest/github/administering-a-repository/about-merge-methods-on-github#squashing-your-merge-commits ? (GitHub allows to squash commits while merging)

[pietroalbini]

We need to merge PRs through @bors, we can't use the merge button on rust-lang/rust.

[kellda]

cc servo/homu#70

[kellda]

I implemented this in @bors (rust-lang/homu#122) because it already has a local git clone and is performing the merge. Is this still wanted in triagebot ?

[Mark-Simulacrum]

I think the homu support is great but I am a bit wary of r+'ing PRs that are then going to get squashed by homu at a later point. But generally I think support in homu is probably enough, so I'm going to close this. We can reopen if there's problems with the homu support at some point.

[cassaundra]

I'll start taking a look at this.

I'm curious if the required "allow edits from maintainer" option is enabled by default in GitHub's UI. It is for me, but that might be because that setting is saved between PRs.

[Mark-Simulacrum]

I believe we can make an assumption that it is enabled for the purposes of this feature; if it's not, we will error (ideally in a nice way, perhaps providing instructions on what to do: e.g., the author pushing a given commit hash to their branch).

In terms of making this happen:

• https://github.com/rust-lang/triagebot/blob/master/parser/src/command/second.rs is a sample argument-less command, which is what I think we want here
• Once a command is parsed, that particular one is handled by https://github.com/rust-lang/triagebot/blob/master/src/handlers/major_change.rs

You will want to create a new parser for this and wire up a new handler; hopefully easy to find to bits with those two pointers to wire this up.

In terms of handling the command, I think my expectation is that we will use the git database API to create a new commit -- this will probably mean adding to github.rs to support the API, with:

• message consisting of PR title and PR description (we likely want to steal some of the code from bors/homu to escape mentions in the PR description; see a test for sample escapes, probably can dig the code up from there)
• author should be the PR author -- may need to use the API to retrieve email (which is IIRC possible), failing that we can probably try to guess by using the first commit in the PR or something like that. It's fine to start with just throwing an error if we can't get the information we need easily.
• tree I think should work fine if we pull from the PR's last (in parent, not date order) commit the tree
• parents should just be one, probably easiest to point at master (default branch), but maybe slightly better is to use the first parent of the first PR commit.

Once we create the commit, I think we'd use the update a reference API to force push their branch into the right commit, and leave a comment with what we did.

If any of that fails, as I suggested above, I would post a comment saying that we couldn't squash because of X but still generated the commit sha X which they can make use of by running something like git fetch https://github.com/rust-lang/rust.git <SHA> && git reset --hard <SHA> && git push --force-with-lease? (Ideally we'd probably add some kind of check they're on the right branch; it's also reasonable for rust-lang/rust to have a script added to src/etc that does some more expansive checking that we can opportunistically reference here).

Hopefully that all helps! Please do let me know if there are any questions, and also feel free to say some of this is wrong. This is just a sketch of a design literally half-invented right now.

[cassaundra]

Sounds good. Thank you for the detailed outline.