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

このコミットは、Goプロジェクトのコードレビューツールであるcodereview.pyにおけるMercurialインターフェースの修正に関するものです。具体的には、エラーハンドリングの改善と、Mercurialコマンドの戻り値の扱いに関する変更が含まれています。

コミット

commit eca37e1eed089bd6bfb6e929a64761435dae65ae
Author: Shenghou Ma <minux.ma@gmail.com>
Date:   Mon Oct 8 04:19:36 2012 +0800

    codereview: more mercurial interface fixes
            Fixes #4131.
    
    R=golang-dev, rsc
    CC=golang-dev
    https://golang.org/cl/6614061

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

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

元コミット内容

codereview: more mercurial interface fixes Fixes #4131.

このコミットは、Mercurialインターフェースに関するさらなる修正を行い、Issue #4131を解決することを目的としています。

変更の背景

Goプロジェクトは、2012年当時、バージョン管理システムとしてMercurialを使用していました。コードレビュープロセスは、golang-devメーリングリストとRietveld（codereview.appspot.com）というツールを中心に行われていました。codereview.pyは、このコードレビュープロセスをMercurialと連携させるための重要なスクリプトでした。

以前のバージョンでは、codereview.py内のMercurial関連の操作でエラーが発生した場合、単にエラーメッセージを文字列として返していました。しかし、これはMercurialの標準的なエラー処理メカニズムとは異なり、スクリプトの実行フローを適切に中断させることができませんでした。MercurialのPython APIでは、エラーや異常な終了を通知するためにhg_util.Abort例外を発生させるのが慣例です。

また、pendingやuploadといった一部の関数では、成功時にNoneを返していましたが、Mercurialコマンドの慣例として、成功時には0を返すことが一般的です。これにより、スクリプトの外部からこれらの関数の実行結果をより明確に判断できるようになります。

このコミットは、これらの不整合を修正し、codereview.pyがMercurialのインターフェースとより一貫性のある動作をするようにするためのものです。特に、Issue #4131で報告された問題に対処しています。

前提知識の解説

1. Mercurial (Hg): 分散型バージョン管理システムの一つで、Gitと同様にリポジトリをローカルに持ち、変更履歴を管理します。Goプロジェクトは初期にMercurialを採用していました。
2. hg_util.Abort: MercurialのPython APIにおける例外クラスです。Mercurialの操作中に致命的なエラーが発生した場合や、意図的に処理を中断させる必要がある場合に発生させられます。これにより、スクリプトの実行が即座に停止し、適切なエラーメッセージがユーザーに表示されます。これは、単にエラーメッセージを返すよりも、プログラムの制御フローを明確にする上で重要です。
3. Goプロジェクトのコードレビュープロセス (2012年頃): 2012年当時、Goプロジェクトは厳格なコードレビュープロセスを採用していました。
   • ツール: Rietveld (codereview.appspot.com) が主要なコードレビューツールとして使用されていました。これはGoogleが開発したWebベースのコードレビューシステムで、差分表示、コメント、承認などの機能を提供します。
   • バージョン管理: Mercurialが使用されており、codereview.pyのようなスクリプトがMercurialリポジトリとRietveldの間で変更をやり取りする役割を担っていました。
   • ワークフロー: 開発者は変更をコミットし、それをRietveldにアップロードしてレビューを依頼します。レビュー担当者はコードをレビューし、コメントや承認（LGTM: Looks Good To Me）を行います。変更が承認されると、コミットがメインリポジトリに統合されます。
4. Pythonの例外処理: Pythonでは、エラーや予期せぬ状況が発生した場合に例外（Exception）を発生させることで、プログラムの実行フローを中断し、エラーを適切に処理するメカニズムが提供されています。raiseキーワードを使って例外を発生させ、try...exceptブロックで例外を捕捉します。

技術的詳細

このコミットの主要な変更点は、codereview.py内の複数の関数において、エラー発生時の処理を文字列の返却からhg_util.Abort例外の発生に切り替えたことです。

• エラーハンドリングの統一: 以前は、CommandLineCLの呼び出しでエラーが発生した場合や、レビュー担当者が指定されていない場合、変更されたファイルがない場合などに、関数がエラーメッセージの文字列を返していました。これは、呼び出し元がその文字列をチェックして適切な処理を行う必要があり、エラー処理が煩雑になる可能性がありました。 今回の変更では、これらのエラーケースでraise hg_util.Abort(err)を使用することで、Mercurialの標準的なエラー処理メカニズムに準拠させました。これにより、エラーが発生した時点で即座に処理が中断され、MercurialのCLIツールが適切なエラーメッセージを表示できるようになります。これは、Pythonにおける例外ベースのエラー処理のベストプラクティスに沿ったものです。
• 関数の戻り値の統一: pending, submit, sync_changes, uploadといった関数では、成功時にNoneを返していました。Mercurialコマンドや多くのCLIツールでは、成功時に終了コード0を返すのが一般的です。この変更により、これらの関数が成功時にreturn 0を返すように修正され、より標準的なCLIの挙動に合わせられました。これにより、スクリプトの外部からこれらの関数の実行結果をチェックする際に、より明確な成功/失敗の判断が可能になります。

これらの変更は、codereview.pyの堅牢性とMercurialインターフェースとの一貫性を向上させることを目的としています。

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

--- a/lib/codereview/codereview.py
+++ b/lib/codereview/codereview.py
@@ -1794,7 +1794,7 @@ def mail(ui, repo, *pats, **opts):
 
 	cl, err = CommandLineCL(ui, repo, pats, opts, defaultcc=defaultcc)
 	if err != "":
-		return err
+		raise hg_util.Abort(err)
 	cl.Upload(ui, repo, gofmt_just_warn=True)
 	if not cl.reviewer:
 		# If no reviewer is listed, assign the review to defaultcc.
@@ -1802,15 +1802,15 @@ def mail(ui, repo, *pats, **opts):
 		# codereview.appspot.com/user/defaultcc
 		# page, so that it doesn't get dropped on the floor.
 		if not defaultcc:
-			return "no reviewers listed in CL"
+			raise hg_util.Abort("no reviewers listed in CL")
 		cl.cc = Sub(cl.cc, defaultcc)
 		cl.reviewer = defaultcc
 		cl.Flush(ui, repo)
 
 	if cl.files == []:
-		return "no changed files, not sending mail"
+		raise hg_util.Abort("no changed files, not sending mail")
 
-	cl.Mail(ui, repo)		
+	cl.Mail(ui, repo)
 
 #######################################################################
 # hg p / hg pq / hg ps / hg pending
@@ -1851,7 +1851,7 @@ def pending(ui, repo, *pats, **opts):
 			ui.write(cl.PendingText(quick=quick) + "\\n")
 
 	if short:
-		return
+		return 0
 	files = DefaultFiles(ui, repo, [])
 	if len(files) > 0:
 		s = "Changed files not in any CL:\\n"
@@ -1883,7 +1883,7 @@ def submit(ui, repo, *pats, **opts):
 
 	cl, err = CommandLineCL(ui, repo, pats, opts, defaultcc=defaultcc)
 	if err != "":
-		return err
+		raise hg_util.Abort(err)
 
 	user = None
 	if cl.copied_from:
@@ -1902,10 +1902,10 @@ def submit(ui, repo, *pats, **opts):
 	about += "CC=" + JoinComma([CutDomain(s) for s in cl.cc]) + "\\n"
 
 	if not cl.reviewer:
-		return "no reviewers listed in CL"
+		raise hg_util.Abort("no reviewers listed in CL")
 
 	if not cl.local:
-		return "cannot submit non-local CL"
+		raise hg_util.Abort("cannot submit non-local CL")
 
 	# upload, to sync current patch and also get change number if CL is new.
 	if not cl.copied_from:
@@ -1940,7 +1940,7 @@ def submit(ui, repo, *pats, **opts):
 	ret = hg_commit(ui, repo, *['path:'+f for f in cl.files], message=message, user=userline)
 	commit_okay = False
 	if ret:
-		return "nothing changed"
+		raise hg_util.Abort("nothing changed")
 	node = repo["-1"].node()
 	# push to remote; if it fails for any reason, roll back
 	try:
@@ -1993,7 +1993,7 @@ def submit(ui, repo, *pats, **opts):
 		err = hg_clean(repo, "default")
 		if err:
 			return err
-	return None
+	return 0
 
 #######################################################################
 # hg sync
@@ -2047,7 +2047,7 @@ def sync_changes(ui, repo):\n 			ui.warn("CL %s has no files; delete (abandon) with hg change -d %s\\n" % (cl.name, cl.name))\n 		else:\n 			ui.warn("CL %s has no files; delete locally with hg change -D %s\\n" % (cl.name, cl.name))\n-	return\n+	return 0\n \n #######################################################################\n # hg upload\n@@ -2064,12 +2064,12 @@ def upload(ui, repo, name, **opts):\n 	repo.ui.quiet = True\n 	cl, err = LoadCL(ui, repo, name, web=True)\n 	if err != "":\n-		return err\n+		raise hg_util.Abort(err)\n 	if not cl.local:\n-		return "cannot upload non-local change"\n+		raise hg_util.Abort("cannot upload non-local change")\n 	cl.Upload(ui, repo)\n 	print "%s%s\\n" % (server_url_base, cl.name)\n-	return\n+	return 0

コアとなるコードの解説

このコミットでは、lib/codereview/codereview.pyファイル内の複数の関数が変更されています。

1. mail 関数:

   • CommandLineCLの呼び出しでエラーが発生した場合、以前はreturn errとしていましたが、raise hg_util.Abort(err)に変更されました。これにより、エラーが例外として伝播し、MercurialのCLIが適切に処理できるようになります。
   • レビュー担当者が指定されていない場合（if not defaultcc:）や、変更されたファイルがない場合（if cl.files == []:）も同様に、エラーメッセージを返す代わりにhg_util.Abort例外を発生させるように変更されました。

2. pending 関数:

   • if short:ブロック内で、以前はreturn（暗黙的にNoneを返す）としていましたが、return 0と明示的に成功を示す終了コードを返すように変更されました。

3. submit 関数:

   • CommandLineCLの呼び出しでエラーが発生した場合、mail関数と同様にraise hg_util.Abort(err)に変更されました。
   • レビュー担当者が指定されていない場合（if not cl.reviewer:）や、ローカルの変更でない場合（if not cl.local:）も、エラーメッセージを返す代わりにhg_util.Abort例外を発生させるように変更されました。
   • hg_commitが何も変更をコミットしなかった場合（if ret:）も、return "nothing changed"からraise hg_util.Abort("nothing changed")に変更されました。
   • 関数の最後に、以前はreturn Noneとしていましたが、return 0と明示的に成功を示す終了コードを返すように変更されました。

4. sync_changes 関数:

   • 関数の最後に、以前はreturn（暗黙的にNoneを返す）としていましたが、return 0と明示的に成功を示す終了コードを返すように変更されました。

5. upload 関数:

   • LoadCLの呼び出しでエラーが発生した場合、以前はreturn errとしていましたが、raise hg_util.Abort(err)に変更されました。
   • ローカルの変更でない場合（if not cl.local:）も、エラーメッセージを返す代わりにraise hg_util.Abort("cannot upload non-local change")に変更されました。
   • 関数の最後に、以前はreturn（暗黙的にNoneを返す）としていましたが、return 0と明示的に成功を示す終了コードを返すように変更されました。

これらの変更により、codereview.pyはMercurialのPython APIとの連携において、より堅牢で一貫性のあるエラーハンドリングと戻り値の規約に従うようになりました。

関連リンク

• GoプロジェクトのIssue #4131: https://code.google.com/p/go/issues/detail?id=4131 (このコミットが修正したIssue)
• Goプロジェクトのコードレビューツールに関する情報 (2012年当時の状況): https://go.dev/doc/contribute#code_review (当時のドキュメントは直接参照できませんが、Goの貢献ガイドラインはコードレビューの重要性を強調しています)
• Mercurial公式ドキュメント: https://www.mercurial-scm.org/

参考にした情報源リンク

• Mercurial hg_util.Abortに関する情報:
  • https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/error.py
  • https://meulie.net/mercurial-hg_util-abort/
• Goプロジェクトのコードレビュープロセス (2012年):
  • https://go.dev/blog/go1 (Go 1リリースに関するブログ記事、当時の開発プロセスの一端が伺える)
  • https://go.dev/doc/effective_go (Goのコーディングスタイルと哲学、コード品質への意識の高さを示す)
• Mercurialコマンドの戻り値 (Python):
  • https://stackoverflow.com/questions/1000000/how-to-check-the-return-code-of-a-mercurial-command-in-python
  • https://www.mercurial-scm.org/wiki/PythonApi