Commit Series with Phabricator

Mozilla is transitioning to Phabricator as the primary code review system for Firefox, and as a member of the team rolling this change out I’ve been using Phabricator for most of my development. I tend to do a lot of history rewriting before posting code for review, crafting a very intentional series of commits. The default behaviour of arcanist, Phabricator’s CLI, is to squash my beautiful commits into a single monstrous diff, but with a little configuration it is possible to have a nice workflow for reviewing a series of commits.

Arcanist[1] defaults to a behaviour developers familiar with github would expect; each commit on a local branch is meant to be viewed squashed together as a single diff. This branch usually ends up with a number of “fixup” commits, modifying the initial change as feedback is received through review cycles.

I prefer to rewrite my commits and have them reviewed independently. Each commit acts as an atomic unit which is updated based on review feedback. While Phabricator has support for stacking revisions[2], Arcanist lacks the ability to automatically convert a series of commits into a stack of revisions. Even though each commit must be posted manually, the process is pretty smooth.

Configuring Arcanist’s Base Commit Rules

The first step in making this workflow possible is telling Phabricator to only post a single commit when running arc diff[3].

“In Git and Mercurial, many arc commands (notably, arc diff) operate on a range of commits beginning with some commit you specify and ending with the working copy state.” [4]

The commit you specify is called the base, and we can configure arc to always choose the currently checked-out commit as the base with a simple arc command:

$ arc set-config base "arc:this, arc:prompt"

Now whenever arc diff is run, only the currently checked-out commit will be used to create or update a revision. Rewrite history to your heart’s content and when a commit is ready for review git checkout <commit> and arc diff.

Linking Revisions into a “Stack”

Phabricator allows linking revisions through the web UI by clicking Edit Related Revisions… -> Edit Parent Revisions. When posting a commit that’s a descendent of something already up for review, the Edit Related Revisions UI will most likely have the parent as a suggested revision. While this works for linking our commits together, opening a browser and going through multiple clicks is a bit of a pain.

arc diff does not have an option to specify a parent revision to link up[5], but it will take hints from the commit itself. By adding Depends on D123 to the commit message[6], where D123 is the revision your commit depends on, Phabricator will modify the revision relationships adding D123 as a parent revision.

When arc diff creates a revision out of a commit for the first time it will prompt the user to fill out a set of fields; the configured editor will be launched, pre-populated by information from the commit message. These fields are then used to rewrite the commit message after it is succesfully posted to Phabricator, including a link to the revision that is created [7]. Because the revision ID is included in the modified commit message locally, it’s easy to add Depends on D... to new commits by looking at the commit message of their parent.

Updating the Stack with git rebase -i

Because arc diff likes to rewrite the commit it operates on, I tend to run all of my invocations while interactively rebasing. First I run git rebase -i and mark each commit I’m going to post as edit, causing the rebase to stop at that commit and allow modifications. Next, arc diff will upload my commit and possibly rewrite it. Running git rebase --continue will move to the next commit marked edit and I can continue until all commits have been posted.

If I reach a commit which is being posted for the first time, it’s the perfect opportunity to look at the parent commit and add a Depends on D... message. In the end all my commits are fully rebased locally and have been properly updated and stacked on Phabricator.

The Future

In the future I’m hoping that arc gains new functionality to update an entire revision stack with a single command. This would certainly make iterating on a commit series faster and simpler. The Mercurial team has built a custom mercurial extension[8] for themselves with this feature, which is quite nice if you’re using Phabricator with a mercurial repository. I wouldn’t be surprised to see this feature as part of any custom client Mozilla might develop or use for Firefox development.

[2]: Phabricator’s unit of code review, analogous to a PR.
[3]: arc diff is the arcanist command used to post code for review.
[5]: In fact, as of this writing, Phabricator’s API doesn’t provide a way to modify revision relationships.
[6]: Technically it should be in the revision’s summary, which is first populated by your commit message. If you are updating a revision make sure to run arc diff with --edit to modify the summary, as updates will not use amended commit messages.
[7]: This link is important as it tells arc diff which revision to update when running on this commit in the future.