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

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

本ドキュメントは、Go言語プロジェクトの初期に利用されていたコードレビューツール codereview.py に対するコミット ceb59b069e0dfa54be9d57ece3c966da737d8be9 の技術的な詳細解説を提供します。このコミットは、Mercurial (hg) をバージョン管理システムとして利用していた当時のGoプロジェクトのコードレビュープロセスにおける、いくつかのバグ修正と改善を含んでいます。

コミット

このコミットは、Goプロジェクトのコードレビューツールである lib/codereview/codereview.py に対する複数の修正を適用しています。主な目的は、ツールの堅牢性と正確性を向上させることです。特に、Mercurialリポジトリとの連携部分における挙動の改善と、初期化処理の安全性の確保が含まれます。コミットメッセージにある「Python and Mercurial are a matched pair.」は、当時のGoプロジェクトのツールチェインにおいて、PythonスクリプトがMercurialと密接に連携して機能していたことを示唆しています。

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

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

元コミット内容

codereview: more fixes

Python and Mercurial are a matched pair.

R=adg
CC=golang-dev
https://golang.org/cl/5570065

変更の背景

Go言語プロジェクトは、その初期段階において、バージョン管理システムとしてGitではなくMercurial (hg) を採用していました。コードレビュープロセスも、Mercurialと連携するカスタムスクリプトによって支えられていました。lib/codereview/codereview.py は、その中心的な役割を担うPythonスクリプトであり、パッチの適用、リポジトリの同期、コミットのプッシュなど、コードレビューに関連する様々な操作を自動化していました。

このコミットは、そのようなツールが運用される中で発見されたいくつかの問題に対処するために作成されました。具体的には、ファイルの読み込みエラー時の変数未定義問題、Mercurialコマンドの実行結果の誤った解釈、そしてリポジトリの初期プッシュ時に不要な同期処理がトリガーされる問題などが挙げられます。コミットメッセージの「Python and Mercurial are a matched pair.」は、当時のGo開発環境におけるPythonとMercurialの密接な関係性、およびそれらが一体となって開発ワークフローを支えていた状況を端的に表現しています。

前提知識の解説

Mercurial (hg)

Mercurialは、Gitと同様に分散型バージョン管理システム (DVCS) の一つです。2000年代後半から2010年代初頭にかけて、多くのオープンソースプロジェクトで利用されていました。Gitと比較して、よりシンプルで直感的なコマンド体系を持つと評されることもあります。Go言語プロジェクトも、初期にはMercurialを主要なバージョン管理システムとして採用しており、その開発ワークフローはMercurialの機能に大きく依存していました。

Python

Pythonは、その高い可読性と豊富なライブラリ、そしてスクリプト言語としての柔軟性から、システム管理、Web開発、データ分析など幅広い分野で利用されています。Goプロジェクトの初期においては、Mercurialとの連携やコードレビュープロセスの自動化といった、開発ワークフローをサポートするツール群の多くがPythonで記述されていました。codereview.py もその一つであり、Pythonのスクリプト能力が最大限に活用されていました。

コードレビューツール

コードレビューは、ソフトウェア開発において品質を向上させ、バグを早期に発見し、知識を共有するための重要なプロセスです。コードレビューツールは、このプロセスを効率化するために設計されたソフトウェアです。パッチの適用、変更点の表示、コメントの追加、レビューの承認・却下といった機能を提供します。codereview.py は、Mercurialリポジトリと連携し、Goプロジェクトの特定のコードレビューワークフローに合わせてカスタマイズされたツールでした。

Go Change List (CL)

Goプロジェクトでは、変更の単位を「Change List (CL)」と呼びます。これは、Gitにおけるコミットやプルリクエストに似た概念ですが、Goプロジェクト独自のレビューシステム (Gerritベース) と密接に結びついています。開発者はCLを作成し、レビューアに提出し、承認された後にリポジトリにマージされます。コミットメッセージにある https://golang.org/cl/5570065 は、このコミットがGoのレビューシステムにおける特定のCLに対応していることを示しています。

技術的詳細

このコミットでは、lib/codereview/codereview.py ファイルに対して3つの主要な変更が加えられています。

  1. ReadContributors 関数における変数初期化の修正:

    • 変更前: contributors = {} の初期化が try ブロックの内部にありました。
    • 変更後: contributors = {} の初期化が try ブロックの外部、関数の冒頭に移動されました。
    • 詳細: ReadContributors 関数は、おそらく CONTRIBUTORS というファイルから貢献者情報を読み込むことを意図しています。元のコードでは、ファイルを開く際に ExceptionDetail() が発生した場合（例: ファイルが存在しない、アクセス権がないなど）、f のオープンに失敗し、try ブロックが終了します。このとき、contributors 変数は初期化されないままとなり、後続のコードで contributors を参照しようとすると NameError が発生する可能性がありました。初期化を try ブロックの外に移動することで、ファイルオープンに失敗した場合でも contributors が空の辞書として確実に存在し、プログラムの堅牢性が向上します。

  2. hg_heads 関数におけるエラーハンドリングの簡素化:

    • 変更前: hg_commands.heads の戻り値 ret をチェックし、ret が真（非空文字列など）であれば hg_util.Abort(ret) を発生させていました。
    • 変更後: hg_commands.heads(ui, repo) を直接呼び出し、戻り値をチェックしていません。
    • 詳細: hg_commands.heads はMercurialの hg heads コマンドを実行するラッパー関数です。元のコードは、このコマンドがエラーメッセージを標準出力に返す場合にそれを捕捉し、Abort 例外として再スローしようとしていたと考えられます。この変更は、以下のいずれかの理由によるものと推測されます。
      • hg_commands.heads 関数自体が、エラー発生時に適切なPython例外を発生させるように変更された。
      • uiwrap(ui) のメカニズムが、Mercurialコマンドの標準出力だけでなく、標準エラー出力も捕捉し、エラーを適切に処理するようになった。
      • この特定のコンテキストでは、hg heads コマンドがエラーを返すことは想定されておらず、常に成功するか、またはエラーが別のメカニズムで処理されるようになった。 いずれにせよ、この変更により、エラーハンドリングのロジックが簡素化され、よりPythonicな例外処理に依存するようになった可能性があります。

  3. submit 関数における need_sync 呼び出し条件の修正:

    • 変更前: if old_heads != new_heads:
    • 変更後: if old_heads != new_heads and not (old_heads == 0 and new_heads == 1):
    • 詳細: submit 関数は、コードレビューが承認された後に変更をリポジトリにプッシュする処理を担当しています。old_heads はプッシュ前のリポジトリのヘッド数、new_heads はプッシュ後のヘッド数を表します。
      • old_heads != new_heads という条件は、「新しいヘッドが作成された」ことを意味し、これは通常、ローカルリポジトリがリモートリポジトリに対して古くなっていた（つまり、他の変更がプッシュされていた）場合に発生します。このような場合、need_sync() を呼び出してリポジトリを同期する必要がありました。
      • しかし、この条件にはエッジケースがありました。old_heads == 0 (空のリポジトリ) から new_heads == 1 (最初のコミットをプッシュ) になる場合も old_heads != new_heads は真になります。このケースでは、新しいヘッドが作成されるのは当然のことであり、リポジトリが「古かった」わけではないため、need_sync() を呼び出す必要はありませんでした。
      • 追加された and not (old_heads == 0 and new_heads == 1) という条件は、この特定のエッジケースを除外します。これにより、空のリポジトリへの最初のプッシュ時に不要な同期処理がトリガーされるバグが修正され、submit 関数のロジックがより正確になりました。

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

--- a/lib/codereview/codereview.py
+++ b/lib/codereview/codereview.py
@@ -974,6 +974,7 @@ def ReadContributors(ui, repo):
 		ui.write("warning: cannot open %s: %s\n" % (opening, ExceptionDetail()))
 		return
 
+	contributors = {}
 	for line in f:
 		# CONTRIBUTORS is a list of lines like:
 		#	Person <email>
@@ -1106,9 +1107,7 @@ def hg_matchPattern(ui, repo, *pats, **opts):\n 
 def hg_heads(ui, repo):
 	w = uiwrap(ui)
-	ret = hg_commands.heads(ui, repo)
-	if ret:
-		raise hg_util.Abort(ret)
+	hg_commands.heads(ui, repo)
 	return w.output()
 
 noise = [
@@ -1928,7 +1927,7 @@ def submit(ui, repo, *pats, **opts):
 	# push to remote; if it fails for any reason, roll back
 	try:
 		new_heads = len(hg_heads(ui, repo).split())
-		if old_heads != new_heads:
+		if old_heads != new_heads and not (old_heads == 0 and new_heads == 1):
 			# Created new head, so we weren't up to date.
 			need_sync()

コアとなるコードの解説

ReadContributors 関数 (行 974-975)

 	contributors = {}
 	for line in f:
  • 変更前: contributors = {} の行は、この変更差分には直接表示されていませんが、元のコードでは try ブロックの内部にありました。
  • 変更後: try ブロックの外部、for line in f: ループの直前に contributors = {} が追加されました。
  • 解説: この変更は、CONTRIBUTORS ファイルの読み込みに失敗した場合に contributors 変数が未定義のままになる可能性を防ぐためのものです。ファイルを開く try ブロックが例外を発生させて終了した場合でも、contributors が空の辞書として確実に初期化されることで、後続のコードが NameError を起こすことなく安全に実行できるようになります。

hg_heads 関数 (行 1106-1109)

 def hg_heads(ui, repo):
 	w = uiwrap(ui)
-	ret = hg_commands.heads(ui, repo)
-	if ret:
-		raise hg_util.Abort(ret)
+	hg_commands.heads(ui, repo)
 	return w.output()
  • 変更前: hg_commands.heads(ui, repo) の戻り値 ret を受け取り、ret が存在すれば hg_util.Abort(ret) 例外を発生させていました。
  • 変更後: hg_commands.heads(ui, repo) を直接呼び出し、その戻り値を無視しています。
  • 解説: この修正は、hg_commands.heads 関数またはその呼び出し環境におけるエラーハンドリングの変更を反映しています。おそらく、hg_commands.heads がエラーを返す代わりに、直接例外を発生させるようになったか、または uiwrap(ui) がMercurialコマンドの標準エラー出力をより適切に捕捉・処理するようになったため、明示的な戻り値のチェックが不要になったと考えられます。これにより、コードが簡潔になり、エラー処理がより一貫した方法で行われるようになります。

submit 関数 (行 1928-1930)

 		new_heads = len(hg_heads(ui, repo).split())
-		if old_heads != new_heads:
+		if old_heads != new_heads and not (old_heads == 0 and new_heads == 1):
 			# Created new head, so we weren't up to date.
 			need_sync()
  • 変更前: old_heads と new_heads が異なる場合に need_sync() を呼び出していました。
  • 変更後: old_heads と new_heads が異なるかつ old_heads が0で new_heads が1ではない場合にのみ need_sync() を呼び出すように条件が追加されました。
  • 解説: この変更は、submit 関数がリポジトリにコミットをプッシュする際の同期ロジックを改善するものです。old_heads != new_heads は、プッシュによって新しいヘッドが作成されたことを示し、通常はローカルリポジトリがリモートに対して古くなっていたことを意味します。しかし、リポジトリが完全に空 (old_heads == 0) の状態で最初のコミットをプッシュ (new_heads == 1) する場合もこの条件は真になります。このケースでは、新しいヘッドが作成されるのは自然なことであり、リポジトリが「古かった」わけではないため、need_sync() を呼び出す必要はありません。追加された and not (old_heads == 0 and new_heads == 1) という条件は、この誤った同期トリガーを防ぎ、submit 関数の動作をより正確にしています。

関連リンク

参考にした情報源リンク