Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

[インデックス 11563] ファイルの概要

このコミットは、Goプロジェクトで使用されているコードレビューツール（Rietveldベース）のcodereview.pyスクリプトにおける、デフォルトパスの検証ロジックの修正に関するものです。具体的には、コードレビュー機能が無効になっている場合に、不必要なデフォルトパスのURL形式チェックを行わないように変更されています。これにより、コードレビューが無効な状態でのスクリプトの異常終了（Abort）を防ぎ、より堅牢な動作を実現しています。

コミット

codereview: don't check default paths when codereview disabled

R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/5622043

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/e10150f96d9d7dd8f3ab45a04288a75dc1f3c218

元コミット内容

commit e10150f96d9d7dd8f3ab45a04288a75dc1f3c218
Author: Andrew Gerrand <adg@golang.org>
Date:   Thu Feb 2 14:25:13 2012 -0500

    codereview: don't check default paths when codereview disabled

    R=golang-dev, rsc
    CC=golang-dev
    https://golang.org/cl/5622043
---
 lib/codereview/codereview.py | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n

変更の背景

Goプロジェクトでは、RietveldというWebベースのコードレビューシステムをカスタマイズして使用していました。このシステムは、Mercurial (hg) リポジトリと連携し、変更セット（チェンジリスト）をレビューするためのツール群を提供しています。

lib/codereview/codereview.pyスクリプトは、Mercurialのリポジトリフックやコマンドとして機能し、コードレビュープロセスを自動化する役割を担っています。このスクリプトのreposetup関数は、リポジトリの初期設定やコードレビュー関連のオプションの読み込みを行います。

以前の実装では、reposetup関数内でコードレビュー機能が初期化される前に、ui.config("paths", "default", "")で取得したデフォルトパスがURL形式であるかどうかのチェックが行われていました。しかし、何らかの理由でコードレビュー機能自体が無効化されている場合（例えば、リポジトリにルートがない場合など）、このURLチェックは不要であり、かつ、無効なパスが設定されているとhg_util.Abort例外が発生し、スクリプトが異常終了してしまう問題がありました。

このコミットは、コードレビュー機能が無効化されている場合には、この不要なURL形式チェックをスキップすることで、スクリプトの堅牢性を高め、予期せぬエラーによる中断を防ぐことを目的としています。

前提知識の解説

  • Rietveld: Googleが開発したWebベースのコードレビューシステムです。PerforceのMondrianをベースにしており、主にSubversionやGit、Mercurialなどのバージョン管理システムと連携して動作します。Goプロジェクトでは、このRietveldをベースにしたカスタムツールが使用されていました。
  • Mercurial (hg): 分散型バージョン管理システムの一つで、Gitと同様に広く使われています。Goプロジェクトは初期にはMercurialを使用していましたが、後にGitに移行しました。このコミットが作成された2012年時点では、Mercurialが主要なバージョン管理システムでした。
  • codereview.py: Goプロジェクトのコードレビュープロセスをサポートするために書かれたPythonスクリプトです。Mercurialのリポジトリフックやコマンドとして実行され、チェンジリストの作成、Rietveldへのアップロード、レビューコメントの取得などを行います。
  • hg_util.Abort: MercurialのPython APIの一部で、Mercurialコマンドの実行を中断し、エラーメッセージを表示するために使用される例外です。この例外がスローされると、Mercurialの操作は失敗します。
  • ui.config("paths", "default", ""): Mercurialのui（ユーザーインターフェース）オブジェクトのメソッドで、Mercurialの設定ファイル（例: .hg/hgrc）から設定値を読み込むために使用されます。
    • "paths": 設定のセクション名。
    • "default": pathsセクション内のキー名。通常、リモートリポジトリのデフォルトパスを指します。
    • "": 指定されたキーが見つからなかった場合のデフォルト値。
    • この設定は、Mercurialがリモートリポジトリと通信する際に使用するURLを定義します。
  • remote.find("://") < 0: Pythonの文字列メソッドfind()を使用しています。文字列内に"://"（URLスキームの区切り文字）が含まれているかどうかをチェックしています。find()は部分文字列が見つからない場合に-1を返すため、< 0は"://"が見つからなかった、つまりURL形式ではないことを意味します。

技術的詳細

このコミットの技術的な核心は、lib/codereview/codereview.py内のreposetup関数におけるコードの移動です。

reposetup関数は、Mercurialリポジトリが初期化される際に呼び出されるフック関数です。この関数は、コードレビュー関連の設定を読み込み、Rietveldとの連携をセットアップします。

変更前のコードでは、codereview_initフラグがTrueでない場合に、まずremoteパスのURL形式チェックが行われていました。このチェックは、ui.config("paths", "default", "")で取得したパスが://を含まない場合にhg_util.Abort例外を発生させるものでした。

しかし、このURLチェックの直後に、リポジトリのルートが存在しない場合など、コードレビュー機能自体が無効化される可能性のあるロジックが続いていました。

# 変更前
def reposetup(ui, repo):
    if codereview_init:
        return
    codereview_init = True

    remote = ui.config("paths", "default", "")
    if remote.find("://") < 0:
        raise hg_util.Abort("codereview: default path '%s' is not a URL" % (remote,))

    # Read repository-specific options from lib/codereview/codereview.cfg or codereview.cfg.
    root = ''
    try:
        root = repo.root
    except hg_util.Abort:
        # Yes, repo might not have root; see issue 959.
        codereview_disabled = 'codereview disabled: repository has no root'
        return # ここでreturnされると、上記のURLチェックが無駄になる

この問題は、コードレビュー機能が無効化される条件（例: repo.rootが存在しない場合）が満たされたとしても、その前にURLチェックが実行されてしまう点にありました。もしremoteパスがURL形式でなかった場合、コードレビュー機能が無効化される前にhg_util.Abortがスローされ、不必要なエラーが発生していました。

このコミットでは、このURL形式チェックのコードブロックを、コードレビュー機能が無効化される可能性のあるロジックの後に移動しています。

# 変更後
def reposetup(ui, repo):
    if codereview_init:
        return
    codereview_init = True

    # Read repository-specific options from lib/codereview/codereview.cfg or codereview.cfg.
    root = ''
    try:
        root = repo.root
    except hg_util.Abort:
        # Yes, repo might not have root; see issue 959.
        codereview_disabled = 'codereview disabled: repository has no root'
        return # ここでreturnされるため、URLチェックは実行されない

    # ... その他の設定読み込みロジック ...

    remote = ui.config("paths", "default", "")
    if remote.find("://") < 0:
        raise hg_util.Abort("codereview: default path '%s' is not a URL" % (remote,))

    InstallMatch(ui, repo)
    RietveldSetup(ui, repo)

この変更により、reposetup関数が実行され、codereview_initがTrueでない場合でも、まずリポジトリのルートの有無など、コードレビュー機能の有効/無効を判断するロジックが先に実行されます。もしコードレビュー機能が無効化されると判断された場合、関数はそこでreturnするため、remoteパスのURL形式チェックは完全にスキップされます。これにより、コードレビューが無効な状態での不必要なエラー発生が回避され、スクリプトの安定性が向上します。

コアとなるコードの変更箇所

--- a/lib/codereview/codereview.py
+++ b/lib/codereview/codereview.py
@@ -2187,10 +2187,6 @@ def reposetup(ui, repo):\n     if codereview_init:\n         return\n     codereview_init = True\n-\t\n-\tremote = ui.config("paths", "default", "")\n-\tif remote.find("://") < 0:\n-\t\traise hg_util.Abort("codereview: default path '%s' is not a URL" % (remote,))\n \n     # Read repository-specific options from lib/codereview/codereview.cfg or codereview.cfg.\n     root = ''\n@@ -2200,7 +2196,7 @@ def reposetup(ui, repo):\n         # Yes, repo might not have root; see issue 959.\n         codereview_disabled = 'codereview disabled: repository has no root'\n         return\n-\n+\t\n     repo_config_path = ''\n     p1 = root + '/lib/codereview/coderereview.cfg'\n     p2 = root + '/codereview.cfg'\n@@ -2220,6 +2216,10 @@ def reposetup(ui, repo):\n         codereview_disabled = 'codereview disabled: cannot open ' + repo_config_path\n         return\n \n+\tremote = ui.config("paths", "default", "")\n+\tif remote.find("://") < 0:\n+\t\traise hg_util.Abort("codereview: default path '%s' is not a URL" % (remote,))\n+\n     InstallMatch(ui, repo)\n     RietveldSetup(ui, repo)\n

コアとなるコードの解説

このdiffは、lib/codereview/codereview.pyファイルのreposetup関数内の変更を示しています。

  • 削除された行 (-で始まる行):

    -	remote = ui.config("paths", "default", "")
-	if remote.find("://") < 0:
-		raise hg_util.Abort("codereview: default path '%s' is not a URL" % (remote,))

    これらの行は、reposetup関数の冒頭、codereview_init = Trueの直後に存在していました。ここで、Mercurialの設定からデフォルトパスを取得し、それがURL形式であるかをチェックしていました。URL形式でない場合はhg_util.Abort例外を発生させていました。

  • 追加された行 (+で始まる行):

    +	remote = ui.config("paths", "default", "")
+	if remote.find("://") < 0:
+		raise hg_util.Abort("codereview: default path '%s' is not a URL" % (remote,))

    全く同じコードブロックが、reposetup関数の後半、repo_config_pathの処理とcodereview_disabledの設定が完了した後に移動されています。

この変更の意図は、reposetup関数内でコードレビュー機能が有効であると最終的に判断された場合にのみ、このURL形式チェックを実行するようにすることです。もし、リポジトリにルートがないなどの理由でコードレビュー機能が早期に無効化され、関数がreturnする場合、このURLチェックは実行されなくなります。これにより、コードレビューが不要な状況で、無効なデフォルトパス設定による不必要なエラー発生を防ぎ、スクリプトのロバスト性が向上します。

関連リンク

  • https://golang.org/cl/5622043 - このコミットに対応するGoのコードレビューシステム（Gerrit/Rietveld）上のチェンジリスト。

参考にした情報源リンク