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

このコミットは、Goコマンドラインツール（cmd/go）において、テストコードによって引き起こされるインポートサイクルをビルド前に検出する機能を追加するものです。これにより、ユーザーはバイナリをビルドする手間をかけずに、より早くインポートサイクルの問題を特定できるようになります。

コミット

commit 2497c430d846d52dbfd2e8150c51e1ad59aeee3f
Author: Russ Cox <rsc@golang.org>
Date:   Mon May 12 16:52:55 2014 -0400

    cmd/go: detect import cycle caused by test code

    The runtime was detecting the cycle already,
    but we can give a better error without even
    building the binary.

    Fixes #7789.

    LGTM=iant
    R=golang-codereviews, iant
    CC=golang-codereviews
    https://golang.org/cl/96290043

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

https://github.com/golang/go/commit/2497c430d846d52dbfd2e8150c51e1ad59aeee3f

元コミット内容

cmd/go: detect import cycle caused by test code

このコミットは、テストコードによって引き起こされるインポートサイクルを検出する機能を追加します。以前はランタイムが既にサイクルを検出していましたが、この変更によりバイナリをビルドする前に、より良いエラーメッセージを提供できるようになります。 Issue #7789 を修正します。

変更の背景

Go言語では、パッケージ間の循環参照（インポートサイクル）は許可されていません。これは、コンパイル時の依存関係の解決を単純化し、コードベースの健全性を保つために重要な制約です。しかし、通常のアプリケーションコードだけでなく、テストコードもパッケージをインポートします。

このコミット以前は、テストコードが原因でインポートサイクルが発生した場合、Goランタイムは最終的にそのサイクルを検出してエラーを報告していました。しかし、この検出はバイナリのビルドプロセス中、またはビルドが完了した後に発生するため、開発者は問題を特定し、修正するまでに余分な時間とリソースを費やす必要がありました。

このコミットの目的は、go testコマンドがテストバイナリをビルドする前に、テストコードに起因するインポートサイクルを早期に検出し、より明確なエラーメッセージをユーザーに提供することです。これにより、開発サイクルが短縮され、デバッグの効率が向上します。

Goのプログラムはパッケージに分割され、各パッケージは他のパッケージをインポートしてその機能を利用できます。Goのビルドシステムでは、パッケージ間の依存関係は有向非巡回グラフ（DAG）を形成する必要があります。つまり、AがBをインポートし、BがCをインポートする場合、CがAを直接的または間接的にインポートすることはできません。このような循環参照が「インポートサイクル」です。

インポートサイクルは、以下のような問題を引き起こす可能性があります。

コンパイルエラー: Goコンパイラはインポートサイクルを解決できないため、コンパイルエラーとなります。
初期化順序の複雑化: パッケージのinit関数が呼び出される順序が不明確になり、予期せぬ動作を引き起こす可能性があります。
コードの結合度の増加: パッケージ間の依存関係が密になりすぎ、コードの再利用性や保守性が低下します。

`go test`コマンドとテストパッケージ

go testコマンドは、Goプロジェクトのテストを実行するための主要なツールです。go testは、テスト対象のパッケージとその依存関係をビルドし、テスト関数を実行します。

Goのテストコードは、通常、テスト対象のパッケージと同じディレクトリに配置され、ファイル名が_test.goで終わる必要があります。これらのテストファイルは、2つの異なる方法でパッケージをインポートできます。

内部テスト (Internal Tests): テストファイルがテスト対象のパッケージと同じパッケージ名（例: package mypackage）を持つ場合、そのテストファイルはテスト対象のパッケージの内部に属すると見なされます。これらのテストは、テスト対象パッケージの内部の（エクスポートされていない）識別子にアクセスできます。
外部テスト (External Tests): テストファイルがテスト対象のパッケージとは異なるパッケージ名（例: package mypackage_test）を持つ場合、そのテストファイルは外部テストパッケージに属すると見なされます。これらのテストは、テスト対象パッケージのエクスポートされた識別子のみにアクセスできます。外部テストパッケージは、XTestImportsという概念に関連しています。

このコミットは、特に外部テストパッケージが引き起こすインポートサイクルに焦点を当てています。

`cmd/go`の役割

cmd/goは、Go言語のビルド、テスト、依存関係管理などを行うための主要なコマンドラインツールです。このツールは、Goソースコードを解析し、パッケージの依存関係グラフを構築します。インポートサイクル検出は、この依存関係グラフの構築フェーズで行われます。

技術的詳細

このコミットの主要な変更は、cmd/goのtestサブコマンドがパッケージの依存関係をロードする際に、テストコードが引き起こすインポートサイクルを積極的にチェックする点にあります。

具体的には、src/cmd/go/test.go内のbuilder.test関数が変更されています。この関数は、テスト対象のパッケージとそのテスト依存関係をロードする役割を担っています。

変更前は、テストパッケージのインポートパスをスタックにプッシュする際に、単にp.ImportPath + "_test"という文字列を使用していました。これは、テストパッケージが通常のパッケージとは異なる依存関係を持つ可能性があることを考慮していませんでした。

変更後、builder.test関数は、テストパッケージのインポートをロードする際に、loadImport関数が返すパッケージの依存関係（p1.Deps）をチェックします。もし、ロードしようとしているテストパッケージの依存関係の中に、テスト対象のパッケージ（p.ImportPath）が含まれている場合、それはテストコードによるインポートサイクルを示唆します。

この検出が行われた場合、新しいPackageErrorが生成され、isImportCycleフラグがtrueに設定されます。このエラーは、"import cycle not allowed in test"というメッセージを含み、ビルドプロセスを中断してユーザーに即座に通知します。

また、インポートサイクルのパスをより正確に表示するために、新しいヘルパー関数testImportStackが導入されました。この関数は、テストパッケージのインポートスタックを構築し、サイクルが発生した正確なパスをユーザーに提示します。これにより、エラーメッセージがより分かりやすくなります。

src/cmd/go/pkg.goのPackageError.Error()メソッドも修正され、インポートサイクルエラーメッセージのフォーマットが改善されています。以前はp.Pos（ファイル位置）が含まれていましたが、テストコードによるインポートサイクルでは、ファイル位置よりもインポートスタック自体が重要であるため、p.Posが省略されるようになりました。

さらに、src/cmd/go/test.bashには、この新しい検出機能を検証するための新しいテストケースが追加されています。testcycle/p3というテストデータディレクトリが作成され、p1がp2をインポートし、p2がp3をインポートし、p3のテストコードがp1をインポートするという循環参照を意図的に作成しています。このテストは、go test -c testcycle/p3コマンドが期待通りにインポートサイクルエラーを報告することを確認します。

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

`src/cmd/go/pkg.go`

--- a/src/cmd/go/pkg.go
+++ b/src/cmd/go/pkg.go
@@ -144,7 +144,7 @@ type PackageError struct {
 func (p *PackageError) Error() string {
 	// Import cycles deserve special treatment.
 	if p.isImportCycle {
-		return fmt.Sprintf("%s: %s\\npackage %s\\n", p.Pos, p.Err, strings.Join(p.ImportStack, "\\n\\timports "))
+		return fmt.Sprintf("%s\\npackage %s\\n", p.Err, strings.Join(p.ImportStack, "\\n\\timports "))
 	}
 	if p.Pos != "" {
 		// Omit import stack.  The full path to the file where the error

`src/cmd/go/test.go`

--- a/src/cmd/go/test.go
+++ b/src/cmd/go/test.go
@@ -538,14 +538,27 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
 
 	var imports, ximports []*Package
 	var stk importStack
-	stk.push(p.ImportPath + "_test")
+	stk.push(p.ImportPath + " (test)")
 	for _, path := range p.TestImports {
 		p1 := loadImport(path, p.Dir, &stk, p.build.TestImportPos[path])
 		if p1.Error != nil {
 			return nil, nil, nil, p1.Error
 		}
+		if contains(p1.Deps, p.ImportPath) {
+			// Same error that loadPackage returns (via reusePackage) in pkg.go.
+			// Can't change that code, because that code is only for loading the
+			// non-test copy of a package.
+			err := &PackageError{
+				ImportStack:   testImportStack(stk[0], p1, p.ImportPath),
+				Err:           "import cycle not allowed in test",
+				isImportCycle: true,
+			}
+			return nil, nil, nil, err
+		}
 		imports = append(imports, p1)
 	}
+	stk.pop()
+	stk.push(p.ImportPath + "_test")
 	for _, path := range p.XTestImports {
 		if path == p.ImportPath {
 			continue
@@ -777,6 +790,24 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
 	return pmainAction, runAction, printAction, nil
 }
 
+func testImportStack(top string, p *Package, target string) []string {
+	stk := []string{top, p.ImportPath}
+Search:
+	for p.ImportPath != target {
+		for _, p1 := range p.imports {
+			if p1.ImportPath == target || contains(p1.Deps, target) {
+				stk = append(stk, p1.ImportPath)
+				p = p1
+				continue Search
+			}
+		}
+		// Can't happen, but in case it does...
+		stk = append(stk, "<lost path to cycle>")
+		break
+	}
+	return stk
+}
+
 func recompileForTest(pmain, preal, ptest *Package, testDir string) {
 	// The "test copy" of preal is ptest.
 	// For each package that depends on preal, make a "test copy"

`src/cmd/go/test.bash`

--- a/src/cmd/go/test.bash
+++ b/src/cmd/go/test.bash
@@ -770,6 +770,19 @@ elif ! grep 'no buildable Go' testdata/err.out >/dev/null; then
 fi
 rm -f testdata/err.out
 
+TEST 'go test detects test-only import cycles'
+export GOPATH=$(pwd)/testdata
+if ./testgo test -c testcycle/p3 2>testdata/err.out; then
+	echo "go test testcycle/p3 succeeded, should have failed"
+	ok=false
+elif ! grep 'import cycle not allowed in test' testdata/err.out >/dev/null; then
+	echo "go test testcycle/p3 produced unexpected error:"
+	cat testdata/err.out
+	ok=false
+fi
+rm -f testdata/err.out
+unset GOPATH
+
 # clean up
 if $started; then stop; fi
 rm -rf testdata/bin testdata/bin1

新規追加されたテストデータ (`src/cmd/go/testdata/src/testcycle/`)

src/cmd/go/testdata/src/testcycle/p1/p1.go
src/cmd/go/testdata/src/testcycle/p1/p1_test.go
src/cmd/go/testdata/src/testcycle/p2/p2.go
src/cmd/go/testdata/src/testcycle/p3/p3.go
src/cmd/go/testdata/src/testcycle/p3/p3_test.go

これらのファイルは、p1 -> p2 -> p3 -> p1 (テストコード経由) というインポートサイクルを意図的に作成し、新しい検出機能が正しく動作することを確認するために使用されます。

コアとなるコードの解説

`src/cmd/go/pkg.go`の変更

PackageError構造体のError()メソッドは、Goコマンドがエラーメッセージをフォーマットする方法を定義します。この変更は、インポートサイクルエラーメッセージの表示を改善します。

変更前: fmt.Sprintf("%s: %s\\npackage %s\\n", p.Pos, p.Err, strings.Join(p.ImportStack, "\\n\\timports "))
- エラーメッセージにファイル位置（p.Pos）が含まれていました。
変更後: fmt.Sprintf("%s\\npackage %s\\n", p.Err, strings.Join(p.ImportStack, "\\n\\timports "))
- p.Posが削除されました。テストコードによるインポートサイクルでは、具体的なファイル位置よりも、どのパッケージがどのように循環参照しているかを示すインポートスタックが重要であるため、メッセージを簡潔かつ分かりやすくするために変更されました。

`src/cmd/go/test.go`の変更

このファイルには、go testコマンドのロジックが含まれており、インポートサイクル検出の核心部分がここにあります。

stk.push(p.ImportPath + " (test)"):
- 以前はp.ImportPath + "_test"としていましたが、より汎用的な" (test)"という表記に変更されました。これは、インポートスタックを構築する際に、現在のパッケージがテストコンテキストでロードされていることを示すためのものです。
インポートサイクル検出ロジックの追加:
```
if contains(p1.Deps, p.ImportPath) {
    // Same error that loadPackage returns (via reusePackage) in pkg.go.
    // Can't change that code, because that code is only for loading the
    // non-test copy of a package.
    err := &PackageError{
        ImportStack:   testImportStack(stk[0], p1, p.ImportPath),
        Err:           "import cycle not allowed in test",
        isImportCycle: true,
    }
    return nil, nil, nil, err
}
```
- p1は現在ロードされているテストインポートパッケージです。
- p1.Depsはp1が直接的または間接的に依存するすべてのパッケージのリストです。
- contains(p1.Deps, p.ImportPath)は、現在ロードしているテストインポートパッケージ（p1）の依存関係の中に、テスト対象の元のパッケージ（p.ImportPath）が含まれているかどうかをチェックします。もし含まれていれば、それはテストコードによって引き起こされるインポートサイクルが存在することを示します。
- この条件が真の場合、PackageErrorが作成され、Errフィールドに"import cycle not allowed in test"というメッセージが設定され、isImportCycleがtrueに設定されます。これにより、go testコマンドはビルドを続行せずにエラーを報告します。
testImportStack関数の追加:
```
func testImportStack(top string, p *Package, target string) []string {
    stk := []string{top, p.ImportPath}
Search:
    for p.ImportPath != target {
        for _, p1 := range p.imports {
            if p1.ImportPath == target || contains(p1.Deps, target) {
                stk = append(stk, p1.ImportPath)
                p = p1
                continue Search
            }
        }
        // Can't happen, but in case it does...
        stk = append(stk, "<lost path to cycle>")
        break
    }
    return stk
}
```
- この新しいヘルパー関数は、インポートサイクルのパスを再構築し、エラーメッセージに含めるためのインポートスタックを生成します。
- topはスタックの開始点（例: p.ImportPath + " (test)"）です。
- pは現在のパッケージ、targetはサイクルを形成している目的のパッケージ（テスト対象のパッケージ）です。
- この関数は、pからtargetへのパスを逆順にたどり、インポートスタックを構築します。これにより、ユーザーはどのパッケージがどのように循環参照を引き起こしているかを正確に理解できます。

`src/cmd/go/test.bash`の変更

このファイルは、Goコマンドのテストスイートの一部であり、新しい機能が正しく動作することを確認するためのシェルスクリプトテストが追加されています。

TEST 'go test detects test-only import cycles'ブロックが追加されました。
このテストは、testdata/src/testcycle/p3という意図的にインポートサイクルを持つパッケージに対して./testgo test -c（-cはテストバイナリのコンパイルのみを行うオプション）を実行します。
期待される動作は、コマンドが失敗し、標準エラー出力に'import cycle not allowed in test'というメッセージが含まれることです。これにより、新しい検出機能がビルド前にインポートサイクルを正しく捕捉していることが検証されます。

参考にした情報源リンク

Go言語公式ドキュメント: パッケージとモジュールに関する情報
Go言語のビルドプロセスに関する一般的な知識
go testコマンドの動作に関する一般的な知識
Go言語のインポートサイクルに関する一般的な知識
コミットメッセージとコードの変更点自体

comemo