microsoft / go

The Microsoft build of the Go toolset
BSD 3-Clause "New" or "Revised" License
267 stars 26 forks source link

Improve the process to apply infra fixes from `microsoft/main` onto release branches #145

Open dagood opened 3 years ago

dagood commented 3 years ago

As of writing (starting to add release branch infra: https://github.com/microsoft/go/issues/128), it seems that the most reasonable way to apply infra fixes from microsoft/main onto release branches is cherry-picking. A few ideas to improve that process:


Automate the cherry-picks.

Periodically copy all build infra files from microsoft/main to all release branches.

Make "fake" merges that carry infrastructure changes but ignore changes from the upstream commits.

dagood commented 2 years ago

Here's what I have in mind for this. A pipeline that runs periodically (or you can trigger it) that will:

  1. Find all merged PRs with base:microsoft/main and the Backport label.
  2. For each release branch:
    1. Check for a "backport blocked!" alert issue for this branch. If one exists, skip the branch.
    2. Check for an existing open auto-backport PR for this branch. If it's older than (say) three days, raise a block alert issue and skip the branch.
    3. Check for a merged or closed auto-backport PR for this branch marked Needs Backport Bot Followup. If one exists, then:
      1. If the PR was merged, add Backported {version} to each PR listed in the PR description.
      2. Remove Needs Backport Bot Followup from the merged/closed PR.
    4. Cherry-pick each PR squash/merge commit into a temporary branch based on the release branch, from oldest to newest PR, where the PR doesn't have the Backported {version} tag.
      • (The commits may be editing the same areas--they're likely to be infra commits or tightly focused on one feature--so cherry-picking each one individually would be unlikely to work.)
    5. If a cherry-pick doesn't work, raise a "backport blocked!" issue listing the unpickable PR and all PRs after that one that haven't been attempted yet. Ping a human to fix it up.
    6. If we didn't manage to cherry-pick any commits, skip this branch.
    7. Submit a PR, "Port pull requests to {version}". Include the list of PRs being ported in the PR description. Add the tag Needs Backport Bot Followup.
    8. Enable auto-merge in "create a merge commit" mode. (Not squash mode.) At this point the PR is waiting for CI to complete and for a human to approve.
  3. For any PR with the Backport label and also one Backported {version} label for every release branch, remove the PR's Backport label, because it's done.

That's what I come up with when I avoid external databases and webhooks and aim for something that primarily works well for infra changes.

There's probably more needed to make it work well for other situations, like backporting FIPS fixes or other feature work. (E.g. if you make a fix PR on 1.16, you probably want to cherry-pick it to 1.15, 1.14, etc. The change probably won't apply to 1.17 or 1.18, so the bot shouldn't even attempt it. Or, maybe you make a fix to 1.18 that you know applies to 1.17, but is inapplicable to <= 1.16 because the feature being fixed didn't exist yet.) I'm not sure which of these scenarios will end up mattering. If we end up maintaining only two release branches like upstream Go, we probably don't need to worry about this.

@microsoft/golang-compiler Does this sound roughly reasonable?

The alert issues I mentioned should be done for the upstream sync and go-docker updates, too. It just seems more important here, because we can't just retry a cherry-pick and hope it works next time.

dagood commented 2 years ago

We talked about this again in today's sync. Now that we're using submodules, we can use git merges!

The process would be this:

  1. Make a change in microsoft/main.
  2. Cherry-pick the change to the oldest applicable release branch.
  3. Wait for (or trigger) the automatic branch merge process.
  4. If the branch merge hits a merge conflict, the bot might be able to resolve it automatically, but maybe not:
    • Submodule pointer change. This should never be updated by a branch merge. (You don't want to update the 1.17 branch to point at a "new" 1.16 commit.) The bot can deal with this.
    • Patch file change. A conflict here would probably need to be manually resolved. Note that even if the merge has no conflicts, the patch may fail to apply, which is another thing that someone would need to manually resolve.
      • Patch files are tricky to merge, if not impossible due to changing chunks and underlying source code. I think we might need more tooling to help out. E.g. look at the state of git merge-base 1.16 1.17, apply the patches at that point onto the Go source code, examine the diff from there to latest 1.16, and apply that diff onto 1.17.
    • Infra change. If everything works right, these should never conflict: we aim to have the same infra work on every branch. It might end up being more practical to allow small differences (e.g. different test matrix that matches which branches support which features), and those could require manual resolution.

A cherry-pick-only approach is still valid, but merges make it a lot easier to trace fixes through all branches.

This is what a complete merge flow might look like:

image

(Note the boring branches merging from two places... the idea is that 1.17-boring should be up to date with all boring-specific changes from 1.16-boring as well as version-specific changes from 1.17.)

When someone has to manually resolve conflicts, we can put a safeguard in place for bad submodule updates. A PR CI job can check whether the current PR is a merge PR (bot author, matching title) and make sure the submodule commit is not changed. Submodules are a little more awkward to handle than normal files, so some protection against mistakes will be helpful.