PHABRICATORの使い方

FreeBSDではコードレビューにPhabricator(ファブリケーター)を使っています。PhabricatorはFacebookで作られたオープンソースのようです。

githubではpullreqでレビューを行いますが、FreeBSDではsvnでもコードを管理しているので、githubのpullreqではなく、Phabricatorを使っているのだと思います。

このページはgitベースでPhabricatorのコードレビューを出してみて操作のメモです。もっと良い方法があるかもしれませんが、複雑な処理が嫌いなので、以下のような単純な操作にしています。

Phabricatorはarcというphpで作られたコマンドで処理を行うので、まずこれをインストールしておきます。

11.2Rでは以下のパッケージのどれかを入れます。

% pkg search arcanist
arcanist-php56-20180420_1      Command line interface for Phabricator
arcanist-php70-20180420_1      Command line interface for Phabricator
arcanist-php71-20180420_1      Command line interface for Phabricator
arcanist-php72-20180420_1      Command line interface for Phabricator

FreeBSDのCodeReviewのページにあるようにFreeBSDのPhabricatorのアカウントを作って、arcの初期設定を行います。

これで準備はできたので、実際のレビューを出します。

ローカルでレビュー用のブランチを作成します。

% git checkout master
% git checkout -b review_branch

これで下の図のAの時点のcommitでreview_branchがcheckoutされました。

レビューを依頼する修正を反映します。普通はワーク用の別ブランチから下記のようにファイルをコピーしてくる事になると思います。

% git checkout work_branch -- hogehoge.c

レビューのファイルがすべて準備できたら、git commitはせず、下記のarcコマンドを実行します。(B)

% arc diff --create

いくつかの質問項目を入力すると、レビューが作成されます。レビュー作成されるとURLが表示されブラウザベースで確認できます。

レビューについてコメントが付き修正したときは下記のようにarcコマンドを実行します。(C)

% git commit -a -m "review refict"
% arc diff

diffの--updateオプション使っていたら、おかしなことになって使わないようにしました。

レビューの差分すなわちAとreview_branchの差分は以下のコマンドで確認できます。

% git diff `git merge-base origin/master HEAD`..HEAD

レビュー中のdiffを別のブランチに反映したい場合はexportしてpatchコマンドでできます。patchコマンドに-V noneオプションをつけるとBackupファイルを作りません。

% arc export --revision D10740 --git | patch
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/sys/dev/etherswitch/rtl8366/rtl8366rbvar.h b/sys/dev/etherswitch/r
tl8366/rtl8366rbvar.h
|--- a/sys/dev/etherswitch/rtl8366/rtl8366rbvar.h
|+++ b/sys/dev/etherswitch/rtl8366/rtl8366rbvar.h
--------------------------
File to patch: ^Cmicroserver % arc export --revision D10740 --git | patch -p1
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/sys/dev/etherswitch/rtl8366/rtl8366rbvar.h b/sys/dev/etherswitch/r
tl8366/rtl8366rbvar.h
|--- a/sys/dev/etherswitch/rtl8366/rtl8366rbvar.h
|+++ b/sys/dev/etherswitch/rtl8366/rtl8366rbvar.h
--------------------------
Patching file sys/dev/etherswitch/rtl8366/rtl8366rbvar.h using Plan A...
Hunk #1 succeeded at 30.
Hunk #2 succeeded at 176.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/sys/dev/etherswitch/rtl8366/rtl8366rb.c b/sys/dev/etherswitch/rtl8
366/rtl8366rb.c
|--- a/sys/dev/etherswitch/rtl8366/rtl8366rb.c
|+++ b/sys/dev/etherswitch/rtl8366/rtl8366rb.c
--------------------------
Patching file sys/dev/etherswitch/rtl8366/rtl8366rb.c using Plan A...
Hunk #1 succeeded at 54.
Hunk #2 succeeded at 76.
Hunk #3 succeeded at 149.
Hunk #4 succeeded at 219.
Hunk #5 succeeded at 273.
Hunk #6 succeeded at 329.
Hunk #7 succeeded at 375.
Hunk #8 succeeded at 390.
Hunk #9 succeeded at 406.
Hunk #10 succeeded at 482.
Hunk #11 succeeded at 658.
Hunk #12 succeeded at 702.
Hunk #13 succeeded at 717.
Hunk #14 succeeded at 742.
Hunk #15 succeeded at 768.
Hunk #16 succeeded at 781.
Hunk #17 succeeded at 927.
Hunk #18 succeeded at 952.
Hmm...  Ignoring the trailing garbage.
done

レビューの一覧は以下のように見れます。

% arc list
* Needs Review D6796: Add RTL8366SR support at etherswitch driver
* Needs Review D6832: enable phy4 and mac4
* Needs Review D7238: Add support Ralink RT1310 soc
* Accepted     D8344: add marvell 88e6060 support on etherswitch

レビューが終わってheadにcommitされCloseになると、リストから消えます。

その後、上の図でmasterをwork_branchにmargeするとコンフリクトしてしまうようです。とりあえず自分が作ってたものをいったんmvでリネームしてmasterのファイルをcheckoutして、それからマージすると良いようです。確認できたらリネームしたファイルを消します。

review_branchブランチは不要になるので、下記のコマンドで削除します。

% git branch -D review_branch

作業しているツリーでbranchを作って、checkoutしてレビュー出して、作業branchに戻ってビルドすると、フルビルドになってしまうようです。レビューのツリーと作業ツリーは別にした方が良いです。