1266863 - [meta] mozilla-central should have linear history with very few backouts

Status: RESOLVED WONTFIX

(Conduit Graveyard :: Transplant, defect, P3)

Description

[Gregory Szorc [:gps]]

This bug tracks a project to change the Firefox autoland landing workflow to rebase commits to mozilla-central instead of merging them in [via inbound].

Stage 1 (this bug): Stand up a new repository for autoland and start rebasing from it to central. We'll also strip backed out commits from the rebased history so they don't pollute the final repo history.
Stage 2 (much later): kill integration repositories and merge commits: mozilla-central is 100% linear.

I'll fill in more details later.

Comment 1

[Gregory Szorc [:gps]]

I thought I had written words on why integration repos and merge commits are bad. But a quick Google Docs search apparently says I haven't :/

Anyway, here are the main reasons we want to move to a merge commit free, linear history in mozilla-central:

1) Linear history is easier to reason about
2) Merge commits make bisect harder than it needs to be
3) Merge commits are commits that aren't reviewed by anyone / Difficult to audit
4) Conflicts on merge commits can be resolved incorrectly
5) Merging between two heads can result in a franken-failure
6) Straight merges leave things like backout commits in final history

Merge commits introduce multiple parents to commits. This turns repo history from a straight line into a weave. Run `gitk --all` on pretty much any GitHub project (https://github.com/mozilla-b2g/gaia is a great example) to see what I'm talking about. It can be extremely difficult to reason about the history of your repo when it is littered with merge commits. You can't argue that linear isn't simpler and easier to reason about (just one commit after the other).

Regarding bisecting, the bisection algorithm for a linear range of commits is simply binary search. When merge commits are involved, you have an amended binary search that needs to take the DAG and various merge points into consideration. It can be more difficult to reason about and more time consuming. Pure binary search from a 100% linear history is simpler. I like simplicity.

A merge commit is an actual commit performed by your version control tool. It takes the state of 2 sets of files and "merges" them together using complicated algorithms. Most of the time, you are lucky and it "just works" so you don't really think about the complications occurring. At the end of the day, your VCS is producing a commit that has a diff. The thing with merge commits is that it is pretty easy to sneak additional changes into those commits. For repos the size of Firefox, finding 1 thing in a merge commit diff is like finding a needle in a haystack. Auditing merge commits for "extra" changes would be a PITA. Furthermore, our code policy is that all commits are reviewed. Yet we more or less implicitly trust merge commits. That's undermining policy and security. When we start being more aggressive about signing commits, merge commits will undermine that as well. Merge commits and auditing/security just don't play well with each other.

If there is a conflict on a merge commit, resolving it can be performed incorrectly or can delay commit incorporation. Since people doing Firefox merge commits today don't typically write the code they are merging, there is a higher-than-desirable probability that they don't know the proper way to resolve a merge. Oftentimes it is easy. But when it isn't, disaster. Of course, you still have file merges when e.g. rebasing, so the issue of resolving conflicts doesn't go away when you eliminate merges. But at least the conflict is isolated to a single commit (as opposed to 2 heads of significantly diverged repos in the case of Firefox integration repos).

Fortunately it has been a rare occurrence for Firefox development, but a few times we've seen a franken-failure after a merge: fx-team and inbound were each passing tests but when they were merged into each other we got a new failure. I vividly recall one such failure in say 2012 or 2013 that required someone (I think dbaron) to bisect each side of the merge until he found the commits with the co-dependency that introduced a new failure. This apparently took him several hours. Meanwhile all trees were closed and it was a massive productivity hit. This touches on #1 about merge commits and bisect not being optimal. You still have the potential for new failures after rebase, but at least there is just a linear list of commits that could have introduced it on rebase, not 2. The multi-dimension bisect is what makes things especially gnarly and can be a real time sink.

A significant percentage of commits in Firefox are "backout" commits. These are commits that revert a previous commit. What typically happens is someone lands commit X, it is later discovered to be busted, so we revert X with a new "backout" commit Z. Unfortunately, commit Y landed in between. Unfortunately, Y couldn't be tested adequately because the build was failing because of X. So we have to wait until Z lands to see if Y was good. If Y wasn't good, then we back it out. You can see where this is going. By merging, these backed out commits and the backout commits are forever part of the repo history. The ranges in history that couldn't be built are forever baked into the history. When you bisect, you could land on one of these bad commits and you can't bisect properly. Doh. It is much more elegant to drop these backed out and backout commits from the final history before they make it to mainline. This leaves a history that has good code and is easier to reason about.

That's a summary of why merge commits and integration repos are bad.

Now, there are some good reasons for merge commits. And not all our problems go away by eliminating merge commits. We even introduce some new problems by rebasing! But, they are problems we'd much rather deal with. IMO, the security benefits of removing merge commits and the backout-free, linear history that is easier to bisect are the big benefits.

Comment 2

[Axel Hecht [:Pike]]

I like the goal of having a mostly-linear version control history.

I also like the idea of dropping broken landings on integration by rebasing them away.

I also like the goal of having fewer (eventuall one) integration repo.

I do want to challenge the idea of rebasing onto central, though. A rebase is the same as a merge, as far as the math goes on what we know about the end result. Putting things on central that never built and never got an automated test run sounds like something we wouldn't want, and also I don't think we need that.

Instead, we should work on as few integration repos as possible, and only ever merge to those integration repos. Also, rebasing within that integration branch is fine (if it's non-publishing-ish). And when that integration branch is green, push to central. No new commit, same shas, just a push. I'd go as far as switch off all builds on central aside from Nightlies.

I.e. (in a post-fx-tree world), merge inbound and autoland, back out anything that merge conflicts if necessary, push to both.
Once green, push to central. Three integration branches makes this harder to describe, which makes me suggest to get rid of fx-team as soon as possible, unless there's benefit to sheriffs in them.

Note, in this world, I think Gijs' concerns in bug 1283669 just go away.

Comment 3

[Gregory Szorc [:gps]]

Changing the summary of this bug to reflect the end goal, which is to eliminate merge commits from mozilla-central (100% linear history going forward) and to have very few "backout" commits in the history. There will still be backouts in the final history. But we shouldn't have commits that are known bad and e.g. fail to build, as this can make bisection difficult.

Given the work in 1288845, the autoland repo will soon be backout free. And, pushing the autoland repo to mozilla-central will result in linear history on mozilla-central. So the only source of merge commits will be inbound, fx-team, etc. These repos will eventually go away in favor of autoland and mozilla-central will then be linear.

Comment 4

[Aki Sasaki (not active)]

We may need to make adjustments to the vcs-sync process / configs as part of this work.

Comment 5

[:glob ✱]

Transplant has been decommissioned.