本記事は「MIXI DEVELOPERS Advent Calendar 2024」の21日目の記事です。
株式会社MIXIでサロンスタッフ予約サービス「minimo」のバックエンドエンジニアをしている鈴木です。
実務で遭遇したreviewdogの不具合を修正した際の話を書いていきます。
実務を起点としたOSSへの貢献の事例として読んでいただければ幸いです。
reviewdogとは
Linterによる指摘事項をGitHubのレビューコメント等の形で出力してくれるツールです。
Linterによる指摘事項を分かりやすく表示してくれるので重宝しています。
また、一部のLinterについては、Linterとreviewdogと組み合わせたGitHub Actionsをreviewdog公式で公開しています。
これを使えば、Linterとreviewdogを連携させるための細かい設定を考えずに済み便利です。
不具合との遭遇
いつものように仕事に勤しんでいたある日、同僚から差分が生じているファイル数が多いPRにおいて、reviewdogのCIが落ちると相談がありました。
すでにreviewdogのリポジトリでIssueが立っていたので、その場ではいったんエラーを無視してreviewdog側の対応を待つことにしよう、という話になりました。
不具合を直そうと思い立つ
とはいえ、Issueでの議論を見ていると「GitHub起因のエラーではないか」という話で、すぐには解消されなさそうな雰囲気を感じました。
そのような中でreidab氏により修正案が示されました。
また、以前reviewdogのGitHub ActionsへPRを出した際に、reviewdogのGitHub Actionsのリポジトリ限定のメンテナーになっていた[1]ことを思い出しました。
これらのことから、一応reviewdogのメンテナーということになっているし、修正案も示されているのならそれを実装してみようと思い立ちました。
修正第1弾
reidab氏による修正案は、GitHubのAPIを使ったPRの差分取得に失敗した際の代替処理として git コマンドを使って取得する処理を設ける、というものでした。
これを github-pr-review reporterの場合において実装したものが上記のPRになります。
既存実装では、GitHub API GET /repos/{owner}/{repo}/pulls/{pull_number} を Accept: application/vnd.github.diff ヘッダー付きで実行し、 git diffコマンドの出力と同様の形式で差分を取得しています。
この処理がエラーになった場合、次の処理を行うように修正しました。
- GitHub API
GET /repos/{owner}/{repo}/pulls/{pull_number}をAcceptヘッダーなしで実行し、PR情報取得 -
git merge-base {PRのheadのコミットハッシュ} {PRのbaseのコミットハッシュ}でPRのheadとbaseの共通祖先となるコミットを見つける -
git diff --find-renames {PRのheadとbaseの共通祖先のコミットハッシュ} {PRのheadのコミットハッシュ}で差分を取得する (リネームにも対応)
この修正を通して、PRの差分を出すためにはheadとbaseの共通祖先とheadの差分を取る必要がある、ということを知りました。
reviewdogのコミッターのhaya14busa氏によるレビューに感謝です。
また、このPRはforkしたリポジトリから出したものでしたが、そのようなPRでreiewdogを動かすとエラーになるという事象に遭遇したので、それを修正する上記のPRも一緒に出せました。
修正第2弾
修正PRがリリースされ、それをreviewdogのGitHub Actionsに適用できたのでこれで良いだろうと思っていましたが、アップデートして動かなくなったという報告が上がってきました。
内容としては、まず前提として actions/checkout のデフォルトの fetch-depth は1になっています (CIを動かす対象のコミットのみ取得する) [2]。
そのため、PRのheadとbaseの共通祖先となるコミットを見つけられずエラーになる、というものでした。
これは fetch-depth: 0 (全てのコミットを取得する) を設定すれば回避できる問題であり、個人的にはこれを暗黙的な前提として置いていました。
そのため、最初は「actions/checkout に fetch-depth: 0 を設定するようREADMEに明示すれば良いのではないか」と考えていました。
しかし、次のような声が上がりました。
-
fetch-depth: 0にすると、ソースコードがCIのRunnerのディスクを圧迫したり、ソースコードを取得するのに時間がかかったりするケースがある -
actions/checkoutの修正をユーザーに要求するのは望ましくない
これを受けて考えを改め、次のような形で解決するPRを立てました。
- PRのheadとbaseの共通祖先となるコミットをGitHub API
GET /repos/{owner}/{repo}/compare/{basehead}を使って見つける - PRのheadとbaseの共通祖先とheadのコミットを
git fetchで取得する
これにより、 actions/checkout の fetch-depth がデフォルト値であっても、reviewdog側で最小限のコミットを取得して差分を取得できるようになりました。
また、この修正を通して、個人的には次のことを知りました。
- PRのheadとbaseの共通祖先となるコミットを見つけるGitHub APIがある
-
git fetchのリモートリポジトリ名の引数としてリポジトリのURLを入れられる
修正第3弾
今度こそ修正完了だろうと思っていたら、まだ再現するという報告が上がりました。
これを
github-pr-reviewreporterの場合において実装したものが上記のPRになります。
修正第1弾にて上記のように書きましたが、他のreporterの場合の対応が漏れていました。
そのため、github-check, github-pr-check, github-pr-annotations reporterにも修正を適用するPRを立てました。
これらのreporterの実装はDogHouseと呼ばれるGitHub Appのサーバーの実装と共通になっています。
そのため、 git コマンドがインストールされているか確認し、インストールされている場合のみ動くように実装しました。
そこにhaya14busa氏によるDogHouseサーバーでない場合のみ動作するようにする差分が入りました。
実装してみての感想としては、DogHouseサーバーの実装と共通化されていて、DogHouseサーバー側に影響を出さないように実装するのがなかなか難しいという感じでした。
この辺りはその後、上記PRで解消されたようです。
おまけ
haya14busa氏がX (旧Twitter) で私について言及してくださっていて嬉しかったです。
また、いつの間にかreviewdog本体のメンテナーにもなっていました。
最後に
修正にあたって考慮漏れが目立ち、何回かPRを出す形になってしまったというのは反省点としてありました。
しかし、実装やhaya14busa氏によるレビューを通して、GitやGitHub APIについて (少しだけですが) 詳しくなれたのはよかったです。
また、reviewdog本体への貢献という新たなチャレンジができ、不具合報告を通じてreviewdogのユースケースに触れられたのは良い経験でした。
皆さんも実務で使用しているOSSの不具合に直面した際には、そのOSSを修正することを視野に入れてみてはいかがでしょうか。
宣伝
MIXIやminimoでは一緒に働く仲間を募集中です！
リモート勤務も可能ですので、詳しくは下記採用ページをご確認ください。
-
reviewdogではGitHub ActionsのリポジトリにPRを出してマージされると、自動的にGitHub Actionsのリポジトリ限定のメンテナーに招待される ( https://github.com/reviewdog/inviter 参照)。 ↩︎
-
https://github.com/actions/checkout/blob/cbb722410c2e876e24abbe8de2cc27693e501dcb/action.yml#L74-L76 ↩︎
Discussion