Phabricator

Phabricator is a FreeBSD-hosted service that provides pre-commit code review workflows. Revisions are created via a command-line tool (arc), reviewed and committed once accepted.

This page describes how to use the FreeBSD Code Review Service.

Admins can be contacted at <phabric-admin AT FreeBSD DOT org>

Contents

Phabricator
Setup
Create a Revision
Reviewer Workflow
Using Diffusion
Grey areas
Future Plans
1. Short Term
2. Medium Term
Links

Setup

Before being able to create revisions for code review, you need to create an account and install the command-line client tools on your machine. This section details the process.

Create an Account

Visit Phabricator Registration to create your account.

If you have one, please use your FreeBSD username as email address so that the Phabricator admins can track who you are.

Install Command Line Client

Arcanist is the command-line client for Phabricator. The most convenient way to install it is:

# pkg install devel/arcanist

If you prefer using ports:

# make -C /usr/ports/devel/arcanist install clean

If you need a specific version of PHP for other reasons, specify it as a package flavor:

# pkg install devel/arcanist@php80

If archivers/arc is installed (for example, for ClamAV), then instead:

# pkg install devel/arcanist-lib

– and then add to .cshrc or .shrc:

# .cshrc
alias arc       /usr/local/lib/php/arcanist/bin/arc
# .shrc
alias arc="/usr/local/lib/php/arcanist/bin/arc"

Configure Client

Set up ~/.arcrc with the certificates required to access the online service. This process requires manually copy/pasting a cookie provided by the online service. Do this by typing the command below and following the online instructions:

$ arc install-certificate https://reviews.freebsd.org

After completion, configure Arcanist to use https://reviews.freebsd.org as the default URI:

$ arc set-config default https://reviews.freebsd.org/

Create a Revision

Once you have finished preparing a change locally, you are ready to send it out for review.

Please make sure that your changeset does one thing (and one thing only) so that the review process goes smoothly. Small and self-contained changes are much easier to review!

Please also make sure that your changeset adheres to the project's style guide, documented in the style(9) manpage.

Phabricator has two groups of participants in a review: Reviewers and Subscribers. Individuals and project groups may be Reviewers, and at least one reviewer must accept the revision for it to proceed. Subscribers are notified of the revision and any changes, but are not required to accept the revision. Subscribers may be individuals, project groups, and mailing lists.

Create a Revision via command line (using git-arc)

In an effort to streamline code review for the FreeBSD project, a wrapper known as git-arc(1) has been made by MarkJohnston and it can be utilized to standardize much of the workflow outlined below in this article.

As such, it can also be considered the best recommendation for people looking to start contributing via Phabricator, as well as people looking to streamline their existing workflow.

This wrapper can be found in the devel/freebsd-git-devtools port, and a detailed overview can be found in the November 2021 issue of the FreeBSD Journal written by JohnBaldwin.

Create a Revision via command line (if using git)

You have to be in your local repository clone, on the branch where the changes you want reviewed are. Issue arc diff --create command using a commit range such as main.. (if you have made no updates to the main branch since you started working on your changes) or HEAD^, HEAD^^, etc., depending on how many commits you made in that branch (basically, it's the same argument you'd give to git diff to get the changes you want reviewed). Be sure to commit your changes locally using git commit first, then do:

$ arc diff --create main..

This command will open your favorite editor with the following template to fill in:

<<Replace this line with your Revision Title>>

Summary:

Test Plan:

Reviewers:

Subscribers:

# NEW DIFFERENTIAL REVISION
# Describe the changes in this new revision.
#
# arc could not identify any existing revision in your working copy.
# If you intended to update an existing revision, use:
#
#   $ arc diff --update <revision>

Use these guidelines:

The Revision Title is mandatory. This should be a one-liner description of your change.
The Summary is optional. However, you should fill it in with the verbatim contents of the commit message you will later use when submitting the change.
The Test Plan is optional. Describe here how you have tested the patch and/or how the reviewer can test it. If you cannot come up with anything, think twice...
The Reviewers is optional. However, you should indicate the list of FreeBSD committers that you expect to review the patch. If such committers have accounts on Phabricator, they will automatically be notified of your submission. You can also directly add reviewers using --reviewers <nicks> when using arc diff command. e.g. --reviewers #portmgr. reviewers can be either <nick> or #<project>
The Differential Revision cannot be added here. Yet you must add it to the git commit message.

After saving and exiting your editor, the changeset will be created in Phabricator and Arcanist will print a tracking link for your revision:

https://reviews.freebsd.org/DXXX

For git patch series, the scripts under https://cgit.freebsd.org/src/tree/tools/tools/git may help. The process described in https://docs.freebsd.org/en/articles/committers-guide/#_collaborating_with_others is also useful to create a live branch that the reviewers can pull from and test.

Create a Revision via Web Interface

Revisions can be created without having Arcanist (arc) installed by uploading a patch via the web interface. Patches can be created by diff, git or other tools.

To make reviewing changes easier, always include as much context as possible with your diff. Phabricator will automatically reduce the context in the default display and when sending emails, but it allows the full context to be displayed on demand in the web interface. To make committing easier, include the metadata. This can be done by using by using git format-patch

To create a full diff, use one of the following commands:

    git format-patch -U999999 --stdout other-branch > change.diff

To upload a new patch:

Go to https://reviews.freebsd.org/differential/
Click Create Diff in the upper right hand corner.
Upload the patch file (change.diff in the examples above) or copy and paste the text diff.
Click Create Diff in the lower right hand corner.
Leave the drop down on Create a new Revision... and click Continue.
Enter a descriptive title and summary
Add reviewers and mailing lists that you want to be included in the review. Mailing lists are added as subscribers:
Click Save.

To submit an updated patch:

Go to https://reviews.freebsd.org/differential/
Click Create Diff.
Paste or upload the updated diff.
Select the review you want to update from the Attach To dropdown and click Continue.
Click Save.

Address Revision Feedback

If the reviewer does not accept your change and asks you to modify stuff in your patch, edit the files in your local client accordingly.

While doing so, fill in replies to any line-specific comments that the reviewer may have left on the site. Don't worry: saving those replies does not cause them to be sent out yet. You must click the Submit button at the very bottom to cause these to be sent out.

After addressing all comments, reupload the patch. path should included all paths that are part of the review, even if their contents haven't changed as part of this update.

arc diff --update DXXX [<range>]

Note that this will trigger a notification to the reviewers mentioning that the patch has been updated.

Obtain Approval and Commit

Once the reviewers are happy with your change, they will accept your patch (which will be denoted by a green tick on the web site). At this point, you are free to submit the change to git. Just use or git push as usual.

NOTE: You must add the "Differential Revision" in the commit message, it is not in the summary.

NOTE: Do not use arc commit because it will not allow you to edit the commit message, and will use a non-standard (for FreeBSD) format.

If you use git you can also now do "arc amend".

Close the Revision

NOTE: If you have added the "Differential Revision" reference correctly to the commit message, Phabricator closes the revision automatically

It is important that you close the revision once you are done with it so that it does not clutter your dashboard nor your reviewers'. Do this by scrolling down to the bottom of the revision page, select the Close option from the Action dropdown menu just above the Comment field, and click Submit. The Close option will not be visible unless the revision is in the Accepted state.

You can close a revision (in any state) from the command line with the following command:

arc close-revision DXXX

Reviewer Workflow

This workflow applies to people reviewing patches sent to them via Phabricator.

Open the Revision

Click on the link that appears in the notification email you received to open Phabricator.

Review Revision and Add Feedback

Review the commit message provided along the patch and the diff for all affected files.

If you want to leave line-specific comments, do so by clicking on the line number. This will open a little popup under the line, allowing you to type whichever comment you may like.

If you have change-wide comments, scroll down to the bottom of the page and type them in the provided box. Note that line-specific comments are vastly preferred over change-wide ones!

Once you are done with your review, don't forget to click on the Submit button at the end.

Optional: Apply the Revision Locally

If you wish to test the patch on your machine, you can use Arcanist to download and apply it. Simply do:

$ arc patch D32

Check for changes between drafts

To see changes between two drafts, for instance to check how the revision author addressed questions or concerns:

Open the revision in your web browser.
Under Revision Contents, click on the History tab.
Using the two columns of radio buttons on the right, select the versions to compare from (left column, red) and too (right column, green).
Press the Show Diff button.
(Optional) submit line comments (with or without suggestions) and a general comment as described above.

Using Diffusion

Diffusion is the repository browser component of Phabricator. You can use it to add comments to and begin a review for existing code in the source tree. The use is similar to reviewing a proposed patch. For example, you want to make comments on a source file.

1. Go to Diffusion which you can find on the left side of your Phabricator main page.

2. Click on the repository where the file is located (doc, src, ports).

3. Use the list of recent commits to locate the commit and click on it (for example r12345). You can also use "Browse" to navigate to the file.

4. Once you are in the phabricator view where you see the graphical diffs, make your comments on the source lines, same as when doing reviews.

5. On the action combo box at the bottom, choose "Raise concern".

Grey areas

Selection

misbehaviour in response to actions such as a triple-click may cause commentary to be misplaced (example).

Done

things that are done may be misrepresented as not done, to the public (example)
after you mark a number of inline comments as done, the number shown by Phabricator may be incorrect
after you mark all inline comments as done, and submit, the record of your subsequent submission – without you marking any comment – may falsely state that you marked a comment as done (example).

Submission

things that are submitted may be misrepresented as unsubmitted (example).

Future Plans

Short Term

Integrate FreeBSD templates located into subversion ports (devel/subversion/files/extra-patch-fbsd-template)

FreeBSD templates in arcanist diff (src/differential/ArcanistDiffWorkflow.php)

Medium Term

Review reference in commit log:

Some commits have used CR:<tab><tab>DXXX as done with PR:<tab><tab>category/id (see r355492)
Phabricator's canonical notation is "Differential Revision: <URL>" which causes commits to update the review automatically.

Warner Losh / FreeBSD workflow working group ▶ Phabricator