PhabricatorでのFirefoxへのパッチ投稿方法

以前の記事で、Firefoxへのパッチ投稿の一手段としてMozReviewという仕組みがあることと、その使用方法を紹介しました。しかし、その後すぐにMozillaのコードレビューシステムがPhabricatorに完全移行してしまい、MozReviewの運用は止まってしまったようです。記事公開時点ではまだMozReviewを使う開発者が大半のように見えていたため、MozReviewの運用停止が近づいているということを把握できていませんでした。

今回は改めてPhabricatorでのパッチ投稿を実践してみたため、その方法を紹介します。

セットアップ

本記事で紹介するセットアップ方法はMozilla Phabricator User Guideを元にしています。詳細はそちらを参照して下さい。

事前に用意するもの

BMO(bugzilla.mozilla.org)のアカウント
- まだBMOアカウントを取得していない場合は https://bugzilla.mozilla.org/createaccount.cgi で作成します。
- Real nameにはYour Name [:ircnick]のような形で末尾にニックネームを付けておきましょう。
  - Phabricator上ではこのニックネームがユーザー名として表示されます。
  - ただし、既に存在するニックネームは使用できません。
- Phabricatorにアクセスするためには、Bugzilla側で2段階認証を有効化しておく必要があります。
mozilla-centralのワーキングコピー
- Firefoxへのフィードバックの仕方：Windows編等を参考に、hg clone https://hg.mozilla.org/mozilla-centralで取得して下さい。
- 本記事ではバージョン管理システムとしてMercurialを使用する場合のみを対象とします
  - PhabricatorはGitでも使用できるようですが、本記事では対象としていません。

Phabricatorへのログイン確認

MozillaのPhabricatorは https://phabricator.services.mozilla.com/ でアクセスできます。 Loginボタンを押して、ログインを試みます。

Phablicatorログイン

認証はBugzilla側で行われるため、以下のようなボタンが表示されます。

Bugzilla認証

最初のアクセス時には、Bugzilla側での認証成功後、Phabricator上でのアカウント登録を促されます。

Phabricatorユーザー登録

アカウントの登録を完了させて、Phabricatorを使用できる状態にしておきましょう。

Arcanistおよびmoz-phabのセットアップ

パッチをコマンドラインから投稿するためには、ArchanistというPhablicatorのコマンドラインツールと、そのラッパーコマンドであるmoz-phabをインストール必要があります。

Archanist

Archanistのインストール方法は以下に記載されています。

https://phabricator.services.mozilla.com/book/phabricator/article/arcanist_quick_start/

Ubuntuの場合は比較的簡単で、依存パッケージをインストール後、ArchanistのGitリポジトリをcloneして、パスを通します。

$ sudo apt install php php-curl
$ mkdir somewhere/
$ cd somewhere/
$ git clone https://github.com/phacility/libphutil.git
$ git clone https://github.com/phacility/arcanist.git
$ export PATH="$PATH:/somewhere/arcanist/bin/"

Windowsの場合のインストール方法は以下に記載されています。

https://moz-conduit.readthedocs.io/en/latest/arcanist-windows.html

~~こちらについては筆者の手元で検証できていないため、本記事では省略します。~~

MozillaBuild を使用している場合の具体的な手順は以下の通りです。

Visual Studio 2017 の Microsoft Visual C++ 再頒布可能パッケージをダウンロードし、インストールする。
Windows版PHPのzipファイルをダウンロードし、展開した物をC:\PHPに置く。（検証はPHP 7.2の「VC15 x64 Non Thread Safe」と書かれている物で行いました）
C:\PHP\php.ini-development の位置にあるファイルを C:\PHP\php.ini にコピーし、以下の通り編集する。
- ;extension=php_curl.dll または ;extension=curl と書かれた行の行頭の ; を削除する。
- ;extension_dir = "ext" と書かれた行を extension_dir = "C:\PHP\ext" に書き換える（行頭の ; を削除し、実際のパスを記入する）。
MozillaBuild のシェル上で /c/PHP/php.exe -i | grep curl と実行し、結果に curl という行が含まれている事を確認する。
Windows版Gitをダウンロードし、インストールする。

GitBash を起動し、以下の操作で必要なツールをダウンロードする。

$ mkdir ~/phabricator
$ cd ~/phabricator
$ git clone https://github.com/phacility/libphutil.git
$ git clone https://github.com/phacility/arcanist.git

MozillaBuild で以下のコマンド列を実行し、各ツールにパスを通す。

$ echo 'export PATH=${PATH}:/c/PHP:${HOME}/phabricator/arcanist/bin:/c/Program\ Files/Git/bin' >> ~/.bash_profile
$ echo 'export EDITOR=/usr/bin/vim' >> ~/.bash_profile
$ source ~/.bash_profile

MozillaのドキュメントにはテキストエディタとしてVim以外を使用する場合の手順も書かれていますので、他のテキストエディタを使いたい場合はそちらも併せて参照して下さい。

Ubuntu、Windowsのそれぞれの方法でArchanistのインストールが完了したら、APIキーを設定します。mozilla-centralのソースディレクトリ下で以下のコマンドを実行します。

$ arc install-certificate

表示されたURLをブラウザで開いてログインするとAPIキーが表示されますので、そのAPIキーをコマンドラインにコピー&ペーストすると、APIキーが取り込まれます。 APIキーは以下のようなJSON形式で~/.arcrcに書き込まれます。

{
  "hosts": {
    "https://phabricator.services.mozilla.com/api/": {
      "token": "xxx-xxxxxxxxxxxxxxxxxxxxxxxxxxxx"
    }
  }
}

moz-phab

moz-phabのインストール方法は、moz-phabのREADME.mdに記載されていますので、そちらを参照して下さい。

単にmoz-phabコマンドをパスの通ったディレクトリにコピーし、実行権限を付けるだけで良いようです。以下はその操作例です。

$ cd ~/phabricator
$ git clone https://github.com/mozilla-conduit/review.git
$ echo 'export PATH=${PATH}:${HOME}/phabricator/review' >> ~/.bash_profile
$ source ~/.bash_profile

レビューリクエストの作成

以前の記事で紹介したように、Firefoxで何かパッチを投稿したい場合は、全てBugzilla上の該当Bugを起点に作業を行います。何か新機能を追加したり、不具合修正を行いたい場合は、まず該当するBugが既に存在するかどうかを確認し、無い場合は新たに自分で新しいBugをfileします。

該当Bugでソースコードに対して何か変更を行って、いざPhabricatorにパッチを投稿したいという状況になった場合、まずはMercurialで手元のリポジトリに変更をコミットします。

$ hg branch fix-bug-xxxxx
$ hg commit

このとき、コミットメッセージの形式に注意しましょう。具体的には以下のような形式にする必要があります。

Bug [Bug番号] - [Bugの概要（一行）]

以下、Bugの詳細についての記述...

Mercurialでリモートリポジトリにpushする際、上記のコミットメッセージのBug番号から自動的にBugzillaの該当Bugにパッチが投稿されます。

また、末尾にr?ircnickという形式でレビュアーを指定すると、push後に自動的に該当レビュアーにレビューリクエストを投げることもできます。このレビュアーの指定は、パッチを送信した後にPhabricatorのWeb UIから行うこともできますので、必ずしもコミットメッセージに含める必要はありません。

以下に、筆者が実際にパッチを投稿した際のコミットメッセージを示します。

Bug 1502786 - Break cycle between PureOmxPlatformLayer and OmxDataDecoder r?jya

OmxDataDecoder, OmxPromiseLayer and PureOmxPlatformLayer consist
circular reference by RefPtr, and no one sever the reference. As a
result their refcount never decrease to 0.
This commit sever it at PureOmxPlatformLayer::Shutdown() which is
called by OmxDataDecoder.

詳細な議論はBug番号から辿ることができるため、コミットメッセージには必ずしも詳細な記述は必要ないようです。有った方が好ましいとは思いますが、慣れていない場合には、まずはBug番号と一行サマリを適切に記載することに注力すると良いでしょう。

ローカルリポジトリへのコミットが完了したら、リモートリポジトリにsubmitします。

$ moz-phab submit

submitが完了した後、先ほどコミットした内容をhg exportで確認してみると、以下のようにDifferential Revision:という行が追加されていることに気が付きます。

# HG changeset patch
# User Takuro Ashie <ashie@clear-code.com>
# Date 1541472583 -32400
#      Tue Nov 06 11:49:43 2018 +0900
# Node ID 25c8e78baa9aa8189ca7026d7ac7868c69d483f3
# Parent  9f9a9234959f114825f58beee0cffbab82d0bb29
Bug 1502786 - Break cycle between PureOmxPlatformLayer and OmxDataDecoder r?jya

OmxDataDecoder, OmxPromiseLayer and PureOmxPlatformLayer consist
circular reference by RefPtr, and no one sever the reference. As a
result their refcount never decrease to 0.
This commit sever it at PureOmxPlatformLayer::Shutdown() which is
called by OmxDataDecoder.

Differential Revision: https://phabricator.services.mozilla.com/D10028

...

この行はレビュー結果を受けてパッチを修正する際に必要になります。また、この行に記載されているURLをブラウザで開くと、Phabricator上でレビューリクエストを参照することができます。以後、レビュアーとのやりとりはこのページで行うことになります。

パッチの修正

レビュアーによってパッチがレビューされ、Phabricator上で修正箇所を指摘されたら、パッチを修正して再度Phabricatorにsubmitすることになります。この際、同一のレビューリクエストに対する修正であることを指定するために、先ほどと同じDifferential Revisionをコミットメッセージに含めてhg commitし、moz-phab submitします。

Mercurialでのパッチ管理方法は本記事のスコープ外のため割愛しますが、パッチ（コミット）が１つのみで、ローカルリポジトリに過去のバージョンが不要である場合、もっとも簡単な修正方法はhg commit --amendで前回のコミットをやり直す方法でしょう。この方法の場合、コミットメッセージは特に修正しなければ前回のままとなりますので、Differential Revisionも前回と同じものが使用されます。ローカルリポジトリの修正は上書きされてしまいますが、リモートリポジトリ上では過去のバージョンも管理され、その差分を確認することもできます。

Phabricator diffリビジョン

修正をsubmitしたら、Phabricator上でその修正に対応するコメントの「Done」にチェックを入れ、レビュアーのコメントに返信をします。この際も、最後にSubmitボタンを押すことを忘れないで下さい。なお、MozReviewの時とは違い、Phabricator上での会話が自動的にBugzillaにも投稿されるという機能は無いようです。

Bugzilla上でパッチを添付する場合とは事なり、Phabricator上でレビュアーの情報が紐付けられているため、変更の度に改めてレビュー依頼をし直す必要はありません。再度レビューしてもらえるのをおとなしく待ちましょう。

レビューが通ったら

レビュアーによってパッチに問題ないと判断された場合、以下のようにAcceptedのマークが付きます。

Phabricator Accepted

この状態になったら、パッチのランドが可能になります。Mozilla Phabricator User GuideのLanding Patchesの項によると、パッチのランドにはLandoというシステムを使うことを強く推奨するとなっていますが、mozilla-centralへのコミット権限が無い場合、このシステムを使用することはできません。実際に試してみたところ、以下のように弾かれてしまいました（筆者の権限はLevel 1）。

Landoエラー

コミット権限が無い場合は、これまでと同様に、Bugzilla側で「Keywords」欄にcheckin-neededというキーワードを付加しておいて、権限のある開発者にコミットしてもらえば良いようです。この際、Bugzilla側ではレビュー承認済みであるr=ではなくレビューリクエスト中であるr?のマークのままになっていることがあるようですが、Phabricator側でAcceptedになっていれば、構わずcheckin-neededにしてしまって問題無いようです。

Accept後のパッチ修正

単にAcceptされただけであればそのままランドしてしまえば良いだけですが、場合によっては「Acceptするけど、こことここだけは修正しておいてね」と言われる場合があります。この場合はAcceptedのマークは付きますが、パッチは修正して再度送信する必要があります。すると、マークが以下のように変わります。

Phabricator Other Diff Accepted

この場合、修正版のパッチを再度レビューしてもらう必要があるのか疑問に思うところでしょう。結論から言えば、特にレビューしてもらう必要は無いようです。自分で修正できたと判断すれば、そのままランドしてしまうことができます。ただし、指摘された箇所は全て「Done」にチェックを入れておきましょう。

Phabricator Doneフラグ

この時も、「Done」のチェック後にSubmitボタンを押す必要があります（チェックを付けただけでは送信されません）。

Backoutされたパッチの修正

一旦ランドされたパッチが自動テストの失敗によってBackoutされる事もあります。ここのような場合、Phabricator上のパッチは既に一旦Closedになってしまっているため、そのままでは修正を継続できません。

FAQによると、このようなケースでは以下の手順でパッチをReopenする必要があります。

ページ最下部のコメント入力欄までスクロールする。
「Add Action...」のドロップダウンリストを開き、「Revision Actions」配下の「Reopen Revision」を選択する。
「Submit」をクリックしてアクションを確定する。

この操作を行うとパッチが「Accepted（レビュー完了済み）」の状態に戻りますので、hg commit --amendでパッチを修正してmoz-phab submitし、再度レビュアーの反応を待つ事になります。

まとめ

PhabricatorでのFirefoxへのパッチ投稿方法について紹介しました。

なお、本記事内で紹介した実例はBug 1502786: Memory leaks in OpenMAX PDMになります。以前Firefox本体にフィードバックしたOpenMAX IL対応パッチにバグがあることを発見したので、その修正を再度フィードバックしています。

元となるOpenMAX対応パッチについては、特にレビュアーを指定せずにとりあえずMozReviewで上げてみただけだったのですが、Mozillaの開発者の目に止まって勝手にレビューされ、本体にマージされるところまで進みました。やはりコードレビューシステムで登録しておいた方が開発者としてもレビューが捗るのかもしれませんね。

ククログ