wiki:org/teams/NetworkTeam/MergePolicy

Version 3 (modified by teor, 21 months ago) (diff)

Add backport construction info

-1. Meta

Hi!

This is a draft. If we think this draft is good enough, let's try following it for a few weeks, and then deciding whether to amend it or adopt it as policy.

In general, let's have the above be our policy-adoption mechanism: let's have policies start out as "draft" like this one is now. If we think it could maybe work out, let's call them "experimental" and try them out for a limited amount. And then let's see whether we need to amend them or adopt them or discard them entirely based on network-team rough consensus.

I've written this draft from my memory of things we've discussed in the past, but I have probably missed a bunch of good ideas.

  1. Goals and General principles

Here are some goals that we'd like to achieve:

  • We should keep merging Tor patches regularly.
  • We should make sure that all patches merged are of high quality, and conform to our standards.
  • We should make the Tor development process resilient to any one or two people being unavailable.
  • We should avoid developer burn-out.

Here are some principles for achieving those goals:

  • For patch quality: We should make sure that every commit to Tor is reviewed and double-checked for conformance.
  • For patch quality: We should do as much automatic testing as possible.
  • For process resilience: We should avoid single points of failure.
  • For process resilience: Nobody should have commit privileges without using them regularly. (Otherwise, they tend to fall out of practice, or get out of sync with others in their understanding of our coding standards.)
  • To avoid burnout, and to keep old series of high quality: Older release series should have a different maintainer who makes backport decisions.
  • To keep merging patches: We should create exceptions to the above rules when it makes sense to do so.
  • To keep merging patches: We should not deadlock development because somebody is on vacation or unavailable.
  1. Proposed policies

We treat "current" and "old" release series differently. For the purposes of merging: all non-stable release series are "current", and the most recent stable release is also "current" for the first 2 months of its lifetime.

Patches that are going to get merged into a current series should, if possible, be merged by somebody who did not write the patch or review the patch. (This helps us improve our chances of noticing bugs before they get merged.) If no such merger exists, anybody with merge permissions can merge. See section 2 below for what the merger should check before merging.

Most patches that are going to be merged into an old series should first have been merged into a current series, and had a reasonable time for testing in the wild. How much testing time is "reasonable" will depend on the severity of the issue that the patch fixes, and the risk introduced by the patch. The backport merge to an old series should be done by the stable release maintainer.

For security bugs of "medium" severity or higher, and other urgent patches (see 3 below) we will want to patch them simultaneously on all supported series that are affected. The stable release maintainer should coordinate with the other mergers to make sure that the patch poses as little risk as possible.

  1. What the expectations for a merger?

The merger should make sure that the review exists, that the patch exists, that CI passes (or has only unrelated failures), and that the reviewer checked the right things.

The merger should at least skim over the patch to make sure it "looks okay" and touches reasonable parts of the codebase.

The merger think about whether there are other planned branches that will touch the same parts of the codebase, and whether the patch will likely introduce conflicts that we need to prepare for.

The merger should check formal requirements that we do not currently have automated tests for:

  • Is there a changes file?
  • Is the patch based against the correct branch?
  • Is there a spec branch corresponding to any protocol change?

The merger should have the appropriate pre-push hook script installed (from scripts/maint/pre-push.git-hook), so that they do not accidentally push any fixup! or squash! commits.

Mergers should skim the tor-commits mailing list for commits to tor.git, and keep an eye on the other mergers.

The merger should, if possible, be on IRC when they're merging things.

Only merge when you are sober, awake, and alert.

If you feel funny about something, don't merge it, and tell the other team members.

Remember, above all, that Tor exists for its users, not for its developers: we all have the duty to protect user security and privacy. [This should be obvious, but it's useful to have a policy saying so.]

  1. What patches are urgent to backport?

The following items can be backported without first maturing for a while in a stable release.

Medium or higher security issues, as defined in our security problem.

Major bugs that seriously threaten network stability.

Regressions that break compilation or CI on important platforms.

  1. What patches can be merged without review?

The following items do not need a reviewer or a separate merger:

Putting out releases:

  • Creating the ChangeLog and ReleaseNotes
  • Bumping the version number
  • Creating a signed tag

Typo fixes in comments.

The following items need CI, but don't need a reviewer or separate merger:

Trivial cherry-picks and rebases for backports

Configuration changes to tests and tooling

  • Adding or removing releases from the merge scripts (for example, #29606)
  • Updating exceptions to make tests pass (for example, the final fixup in #29221)

Reverts, when the commit was merged and introduced a bug

  1. How do you do a merge?

5.1. Configuring your environment

[draft]

Make sure you have the pre-commit and pre-push hooks installed.

Make sure you are using worktrees.

Make sure you have the merge-forward and push-everything scripts installed.

Make sure have merge permissions.

Make sure you have your git repository configured with git-rw.torproject.org:tor.git as "origin" and github.com/torproject/tor.git as "tor-github".

Make sure you have PRs from tor-github set up to be named tor-github/pr/NNN, where NNN is the number of the PR.

5.2. Finding out what to merge

If you are Nick, or you want to do it like Nick does, you should regularly look at the list of all merge_ready tickets in the Tor component where the Owner is not you, and the Reviewer is not you. These are tickets that you have permission to merge into newer series.

Additionally, Nick will add tags like "asn-merge" or "dgoulet-merge" to assign a merger for the tickets he shouldn't be merging himself. You should check for tickets tagged (yourname)-merge.

5.3. Merging a branch.

Make sure you have the latest version of all the stable branches checked out in your worktrees.

Make sure that you have the latest version of the branch you're merging.

Check all the things you're supposed to check.

Go into the earliest target directory where you are merging today, and merge the branch.

[TODO: Write a standard for merge commits]

Run "merge-forward" and verify that there are no conflicts.

Run "push-everything".

If you are merging a patch into the earliest series to which it applies, close the ticket after you merge, and say where you merged it. If you are merging a patch that is a candidate for backport, move the ticket into the earliest open milestone to which it applies.

  1. How do we create backport branches?

To make the merges as simple as possible, here's what we usually do:

  • when we know we want to backport, we find the earliest maint branch (N) that needs a backport
  • if we're writing the bugfix, we write the fix on branch N
  • if we have a bugfix on master, we cherry-pick or rebase to N
  • once the backport branch is done, we merge the branch into N, and then merge N+branch forward
  • if there are any significant conflicts, we find the earliest maint branch with a conflict (M)
  • we make a separate branch based on M that resolves the conflict (How? We still need to write a merge policy.)
  • then we continue to merge forward

We deal with required changes that don't conflict in a similar way: in a separate branch, on the earliest maint branch for that change.

So we should be able to merge all these branches in the right places, then merge forward without conflicts.

  1. TODO:

We should integrate this document with the stable maintainer guide.

We should describe a standard for merge commits.

We should figure out whether we can do automatic checks for our process above.