wiki:org/teams/NetworkTeam/MergePolicy

Version 24 (modified by teor, 14 months ago) (diff)

Link to help for SSH keys

Merge Policy

Network team merge policy, setup instructions, and notes.

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.

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.

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. Historically, we have asked the stable maintainer to merge tickets when a mainline merger is away. See section 2 below for what the merger should check before merging.

(Other repositories like torspec, chutney, and sbws work on a 2-person rule: the author and reviewer should be different people, and anyone can merge.)

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.

What are 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.]

What Patches are Urgent to Backport?

In general, tickets that fix more urgent, important, or severe issues should be merged first. Having a large merge backlog that takes a long time to prioritise is a sign of process failure.

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.

What Patches Need Less 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
  • Reverts, when the commit was merged and introduced a bug
    • Includes reverts that make CI pass again
  • Edits to changes files

The following changes need 2 different people, rather than 3: (A separate author and reviewer, then the merger can be anyone.)

  • 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)
  • Obviously correct minor fixes on a merged branch that has already been reviewed
    • Includes small changes that make CI pass again

How do you do a Merge?

Configuring your Environment

[draft]

Master and Backport Merges

  1. Make sure you have the git hooks installed.

You can find the hooks in tor/scripts/git/. They need to be copied into tor/.git/hooks, without the extension:

  • tor/scripts/git/post-merge.git-hook -> .git/hooks/post-merge
  • (repeat for the other hooks)

Q: How do I keep the git hooks up to date?

A: Install the post-merge (post pull) hook, so git logs a message when the hooks are updated in master. (See #29588.)

  1. 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".
$ git remote add tor-github https://github.com/torproject/tor.git

Piece of advise here. Because git default behavior of "git push" is to use "origin", then mistake can happen where one of us can push a personal branch to our upstream tor.git.

For that I nullify the "pushurl" of origin with:

[remote "origin"]
  fetch = +refs/heads/*:refs/remotes/origin/*
  url = https://git.torproject.org/tor.git
  pushurl = "Don't push personal branches to origin!"

To push now, I created an "upstream" remote so when I push commits upstream, I have to explicitly type in:

$ git push upstream <branch>

(The git-push-all.sh allows you to specific the name of the "upstream" branch).

To add the upstream remote properly:

$ git remote add upstream https://git.torproject.org/tor.git
$ git remote set-url --push upstream ssh://git@git-rw.torproject.org/tor.git

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.

In your gitconfig:

[remote "tor-github"]
   fetch = +refs/heads/*:refs/remotes/tor-github/*
   fetch = +refs/pull/*:refs/remotes/tor-github/pr/*

This sets up pull requests as:

  • tor-github/pr/NNNN/head - the branch you want to merge
  • tor-github/pr/NNNN/merge - the branch github made as a test merge for CI. Only exists if the merge is clean.

Newer git versions (>> 2.20) default to head when you use tor-github/pr/NNNN.

  1. Make sure you have merge permissions

Set up SSH keys using Tor's LDAP: https://db.torproject.org/doc-mail.html

Check your access:

$ ssh git@git-rw.torproject.org | grep W

Backport Merges Only

  1. Make sure you have the git-pull-all.sh, git-merge-forward.sh and git-push-all.sh scripts installed.

You can find the hooks in tor/scripts/git/. They need to be copied into your $PATH:

  • tor/scripts/git/git-pull-all.sh -> ~/bin/git-pull-all.sh
  • (repeat for the other scripts)

You will need to customise these scripts to your own environment, using the environmental variables listed in the script's --help.

These scripts require a recent bash that supports arrays.

  1. Make sure you are using worktrees.

You might find it easier to use a separate git repository for merges and regular coding.

To use the git-* scripts, you need worktrees. Here is an example on how to do it properly for let say the "maint-0.3.5" branch and your worktrees are in "tor-wkt" (relative to tor.git/ you have):

$ git worktree add ../tor-wkt/maint-0.3.5 origin/maint-0.3.5
$ cd ../tor-wkt/maint-0.3.5
$ git checkout -b maint-0.3.5
$ git branch --set-upstream-to=origin/maint-0.3.5 maint-0.3.5

So then now within the worktree, you can simply do "git pull" and it will always fetch the latest from origin/maint-0.3.5 which is important because the git-pull-all.sh script requires that. These worktrees need to follow upstream at all times.

Basically, a maintainer should do the above for all maintained branches (including releases-*).

We want to make a script that sets up all the directories (#29603).

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.

Merging a Branch

Master Merges

  1. Make sure your environment is set up
  • git hooks
  • tor-github custom remote
  1. Make sure you have the latest version of master checked out in your git directory.
$ git fetch upstream
  1. Make sure that you have the latest version of the branch you're merging.
$ git fetch tor-github
  1. Check all the things you're supposed to check.
  1. Merge the branch
# If you have an older git version, you'll need to merge tor-github/pr/NNNNN/head
$ git merge tor-github/pr/NNNNN
  1. Push to master
$ git push upstream master
  1. Update the ticket, and wait for CI

See "Backport Merges" Steps 8 & 9 for details.

Backport Merges

  1. Make sure your environment is set up
  • git hooks
  • tor-github custom remote
  • git scripts
  • worktrees
  1. Make sure you have the latest version of all supported branches checked out in your worktrees.
  1. Make sure that you have the latest version of the branch you're merging.

You can get the supported branches and github PRs using:

$ git-pull-all.sh
  1. Check all the things you're supposed to check.
  1. Go into the earliest target directory where you are merging today, and merge the branch.
  1. If the merge isn't clean, ask the patch author or a stable maintainer what to do.
  1. Run git-merge-forward.sh and verify that there are no conflicts.

The merge-forward script will stop after any conflicts, so you'll have to go in the worktree and fix the conflict yourself. It *does* happen so not to worry :).

  1. Run git-push-all.sh
  1. Update the ticket

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. Wait for the CI to finish on master.

If the CI fails due to practracker errors, update the practracker exceptions file, and push to master again. (practracker fixes don't need review.) Ask the person who wrote the code to fix the exception.

Make a note on the trac ticket for the PR that caused the error.

Merging Different Branches into Different Releases

Follow the "Backport Merges" steps 3-5 for each branch you need to merge.

Finding Branches to Merge

  1. Work out all the maint branches you are merging pull requests into today:
    • if you're doing a mainline merge, the maint branches might be:
      • the alpha maint branch (if there is one), or
      • master (if you'll backport to alpha later)
    • if you're doing a backport merge, the maint branches might be:
      • the oldest LTS release, or
      • newer supported/LTS releases (but not the mainline releases)
    • if you're doing a security fix merge:
      • merge into all maint branches with the vulnerability
  1. Work out the pull request for the earliest maint branch:
    1. If there is a pull request based on that maint branch, use that pull request
    2. Otherwise, check for a pull request for the next earliest version of Tor, and use that pull request
    3. Repeat b. with the Nth earlier version of Tor
  1. Work out the maint branches for each other pull request:
    1. If there is a pull request based on that maint branch, use that pull request
    2. Otherwise, don't merge any pull request into that maint branch
      • the pull request for an earlier version should merge forward cleanly

To simplify this process, we usually:

  • Merge the change to master
  • Wait for testing in an alpha release
  • Merge the change to all other versions

Doing the Merges

Here's how you do the merges:

  1. Find the earliest maint branch you want to merge into today, and the pull request for that maint branch
  2. Change to the directory for the maint branch
  3. Merge the pull request in to the maint branch
  4. Repeat from 2 for the next pull request and its maint branch
  5. Once all the pull requests are merged, merge all branches forward

Dealing with Merge Conflicts

If there are any merge conflicts, stop! Don't push anything! Reset your merge directories to the current branches on torproject.org. Ask for help on the ticket. Include details of the merge conflict.

Worked Example

Here is a worked example from https://trac.torproject.org/projects/tor/ticket/29806#comment:18

We want to do a mainline merge into:

  • maint-0.4.0
  • master

We have these pull requests:

The earliest branch and pull request are:

The other pull requests are:

Here are the exact steps on the command-line:

$ cd ../maint-0.4.0
$ git merge tor-github/pr/821
Auto-merging src/test/test_dir.c
Auto-merging src/feature/dirauth/bwauth.c
Merge made by the 'recursive' strategy.
 changes/ticket29806          |  7 ++++++
 src/feature/dirauth/bwauth.c |  8 ++++++-
 src/test/test_dir.c          | 57 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 changes/ticket29806
$ cd ../tor
# If you are using git ~2.20, you will need to merge tor-github/pr/822/head
$ git merge tor-github/pr/822
Merge made by the 'recursive' strategy.
 changes/ticket29806          |  7 ++++++
 src/feature/dirauth/bwauth.c |  8 +++++-
 src/test/test_dir.c          | 59 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 changes/ticket29806
$ git-merge-forward.sh
  [+] Fetching origin...success
[+] Handling branch 
release-0.2.9 Handling branch 
  [+] Switching branch to release-0.2.9...success
  [+] Merging branch origin/release-0.2.9...success
  [+] Merging branch maint-0.2.9 into release-0.2.9...success
[+] Handling branch 
maint-0.3.4 Handling branch 
  [+] Switching branch to maint-0.3.4...success
  [+] Merging branch origin/maint-0.3.4...success
  [+] Merging branch maint-0.2.9 into maint-0.3.4...success
[+] Handling branch 
release-0.3.4 Handling branch 
  [+] Switching branch to release-0.3.4...success
  [+] Merging branch origin/release-0.3.4...success
  [+] Merging branch maint-0.3.4 into release-0.3.4...success
[+] Handling branch 
maint-0.3.5 Handling branch 
  [+] Switching branch to maint-0.3.5...success
  [+] Merging branch origin/maint-0.3.5...success
  [+] Merging branch maint-0.3.4 into maint-0.3.5...success
[+] Handling branch 
release-0.3.5 Handling branch 
  [+] Switching branch to release-0.3.5...success
  [+] Merging branch origin/release-0.3.5...success
  [+] Merging branch maint-0.3.5 into release-0.3.5...success
[+] Handling branch 
maint-0.4.0 Handling branch 
  [+] Switching branch to maint-0.4.0...success
  [+] Merging branch origin/maint-0.4.0...success
  [+] Merging branch maint-0.3.5 into maint-0.4.0...success
[+] Handling branch 
release-0.4.0 Handling branch 
  [+] Switching branch to release-0.4.0...success
  [+] Merging branch origin/release-0.4.0...success
  [+] Merging branch maint-0.4.0 into release-0.4.0...success
[+] Handling branch 
master Handling branch 
  [+] Switching branch to master...success
  [+] Merging branch origin/master...success
  [+] Merging branch maint-0.4.0 into master...success

These forward merges were clean merges:

  [+] Merging branch maint-0.4.0 into release-0.4.0...success
  [+] Merging branch maint-0.4.0 into master...success

The rest of the merges did not add a merge commit, because there were no changes.

How do we Create Backport Branches?

Use git-merge-forward.sh -t bugNNNNN to automatically create merge forward branches from the current contents of your maint-* and master working directories.

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

Even if there are no conflicts, the patch may require changes to work on later versions. We deal with these changes in a similar way:

  • merge forward the changes for previous versions,
  • add a commit with the new required changes, on the earliest maint branch that needs the change.

Just merge lines in merge commits. Don't put semantic changes in merge commits. Instead, put them in a new commit after the merge commit.

If we create merge branches this way, we should be able to merge all these branches in the right places, then merge forward without conflicts.

TODO

We should integrate this document with the stable maintainer guide.

We should decide if we need more comprehensive instructions for unclean merge commits.

We should automate as much of this process as possible.