[インデックス 15019] ファイルの概要
このコミットは、Go言語のビルドシステムを扱う go/build パッケージにおけるクリーンアップ作業です。具体的には、以前の変更(おそらくロールバックされたもの)によって残された不要なコードとテストを削除し、ImportDir 関数の実装を簡素化しています。
コミット
- コミットハッシュ:
3bf3ba2d8179cfbc73f535d3ccc28bc8f46dc45e - Author: Andrew Gerrand adg@golang.org
- Date: Wed Jan 30 09:10:58 2013 +1100
- コミットメッセージ:
go/build: clean up after rollback R=rsc CC=golang-dev https://golang.org/cl/7237049
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/3bf3ba2d8179cfbc73f535d3ccc28bc8f46dc45e
元コミット内容
go/build: clean up after rollback
R=rsc
CC=golang-dev
https://golang.org/cl/7237049
変更の背景
このコミットの背景には、Go言語の go/build パッケージにおける以前の変更とその後のロールバック(または部分的な取り消し)があります。コミットメッセージの "clean up after rollback"(ロールバック後のクリーンアップ)がその意図を明確に示しています。
具体的には、src/pkg/go/build/build.go 内の ImportDir 関数に存在していた以下のコードブロックが削除されています。
// TODO(rsc,adg): breaks godoc net/http. Not sure why.
// See CL 7232047 and issue 4696.
if false && err == nil && !ctxt.isDir(p.Dir) {
err = fmt.Errorf("%q is not a directory", p.Dir)
}
このコードブロックは if false という条件によって常に実行されない状態になっていました。これは、このコードが何らかの理由で問題を引き起こし、一時的に無効化されていたことを示唆しています。コメントには TODO(rsc,adg): breaks godoc net/http. Not sure why. See CL 7232047 and issue 4696. とあり、godoc net/http を壊す問題があり、その原因が不明であったこと、そして関連する変更リスト (CL) とIssueが参照されています。
また、src/pkg/go/build/build_test.go から TestBogusDirectory テスト関数が削除されています。このテスト関数も return // See issue 4696. というコメントと共に無効化されていました。
これらの無効化されたコードやテストは、以前の変更が意図した通りに機能しなかったか、予期せぬ副作用を引き起こしたため、一時的に無効化され、最終的にこのコミットで完全に削除されたと考えられます。このコミットは、その「ロールバック」の結果として残された不要なコードの痕跡をきれいに取り除くことを目的としています。
前提知識の解説
Go言語の go/build パッケージ
go/build パッケージは、Goのソースコードを解析し、パッケージの依存関係を解決するための機能を提供します。Goのビルドツール (go build, go install など) や、ドキュメンテーションツール (godoc) など、Goのツールチェインの多くの部分で利用されています。
Context: ビルド環境に関する情報(GOPATH、GOOS、GOARCHなど)を保持する構造体です。Import関数: 指定されたパスにあるGoパッケージをインポートし、そのパッケージに関する情報(ファイルリスト、依存関係など)を返します。ImportDir関数: 特定のディレクトリにあるGoパッケージをインポートするためのヘルパー関数です。内部的にはImport関数を呼び出します。ImportMode: パッケージのインポート時にどのような情報を取得するかを制御するフラグです。例えばFindOnlyは、パッケージの存在確認のみを行い、詳細な解析は行いません。
Goの変更リスト (CL) と Issue
Goプロジェクトでは、コードの変更は「変更リスト (Change List, CL)」として提出され、レビューされます。CLは Gerrit というコードレビューシステムで管理されており、https://golang.org/cl/XXXXXXX のようなURLで参照されます。
「Issue」は、バグ報告や機能要望などを追跡するためのものです。GoプロジェクトのIssueは https://golang.org/issue/XXXX のようなURLで参照されます。
このコミットで参照されている CL 7232047 と issue 4696 は、この変更の背景にある具体的な問題や議論を示しています。
CL 7232047: 検索結果によると、これはgo/types: don't crash on invalid type parametersというタイトルで、go/typesパッケージにおける不正な型パラメータによるクラッシュを防ぐための変更のようです。直接go/buildとは関係ないように見えますが、Goのツールチェイン全体で共有される型システムに関連する問題が、ビルドプロセスに影響を与えた可能性も考えられます。issue 4696: 検索結果によると、これはgo/build: ImportDir should not check for directory existenceというタイトルで、ImportDirがディレクトリの存在チェックを行うべきではないという議論に関連しています。このIssueは、ImportDirが存在しないディレクトリに対してエラーを返す動作が、godocのようなツールで問題を引き起こす可能性を指摘しています。
ロールバック (Rollback)
ソフトウェア開発における「ロールバック」とは、以前に適用された変更を元に戻すことを指します。これは、新しい変更がバグを引き起こしたり、予期せぬ問題を生じさせたりした場合に、安定した状態に戻すために行われます。このコミットは、ロールバックによって無効化されたコードやテストの「痕跡」を完全に削除するクリーンアップ作業です。
技術的詳細
このコミットは、go/build パッケージの ImportDir 関数と、それに関連するテストの変更に焦点を当てています。
src/pkg/go/build/build.go の変更
ImportDir 関数の実装が簡素化されました。
変更前:
func (ctxt *Context) ImportDir(dir string, mode ImportMode) (*Package, error) {
p, err := ctxt.Import(".", dir, mode)
// TODO(rsc,adg): breaks godoc net/http. Not sure why.
// See CL 7232047 and issue 4696.
if false && err == nil && !ctxt.isDir(p.Dir) {
err = fmt.Errorf("%q is not a directory", p.Dir)
}
return p, err
}
このコードには以下の特徴がありました。
ctxt.Import(".", dir, mode)を呼び出してパッケージ情報を取得します。if false && ...という条件文があります。これはfalseが含まれているため、このifブロック内のコードは決して実行されません。- コメントで
TODO(rsc,adg): breaks godoc net/http. Not sure why. See CL 7232047 and issue 4696.とあり、このコードがgodoc net/httpを壊す問題があり、その原因が不明であること、そして関連するCLとIssueが参照されています。 fmt.Errorfを使用して、ディレクトリが存在しない場合にエラーを生成する可能性のあるロジックが含まれていました(ただしif falseのため実行されません)。
変更後:
func (ctxt *Context) ImportDir(dir string, mode ImportMode) (*Package, error) {
return ctxt.Import(".", dir, mode)
}
変更後は、ImportDir 関数は単に ctxt.Import(".", dir, mode) を呼び出し、その結果をそのまま返します。これにより、以前の if false ブロック、TODO コメント、および fmt.Errorf の呼び出しが完全に削除され、コードが大幅に簡素化されました。
この変更は、ImportDir がディレクトリの存在チェックを行うべきではないという issue 4696 の議論と一致しています。Import 関数自体が適切なエラーハンドリングを行うため、ImportDir での追加のチェックは不要であり、むしろ問題を引き起こす可能性があったと考えられます。
src/pkg/go/build/build_test.go の変更
TestBogusDirectory テスト関数が完全に削除されました。
変更前:
// golang.org/issue/3248
func TestBogusDirectory(t *testing.T) {
return // See issue 4696.
const dir = "/foo/bar/baz/gopher"
_, err := ImportDir(dir, FindOnly)
want := fmt.Sprintf("%q is not a directory", filepath.FromSlash(dir))
if err == nil || err.Error() != want {
t.Errorf("got error %q, want %q", err, want)
}
}
このテスト関数は、存在しないディレクトリを ImportDir に渡した場合に特定のエラーが返されることを期待していました。しかし、return // See issue 4696. という行があるため、このテストは常にスキップされていました。これは、issue 4696 で議論されているように、ImportDir が存在しないディレクトリに対してエラーを返す動作が問題であると認識されていたためです。
変更後:
TestBogusDirectory 関数は完全に削除されました。これにより、無効化されていたテストコードがコードベースから取り除かれ、クリーンアップが完了しました。
また、fmt パッケージのインポートも不要になったため、src/pkg/go/build/build_test.go から削除されています。
コアとなるコードの変更箇所
src/pkg/go/build/build.go
--- a/src/pkg/go/build/build.go
+++ b/src/pkg/go/build/build.go
@@ -321,13 +321,7 @@ func (p *Package) IsCommand() bool {
// ImportDir is like Import but processes the Go package found in
// the named directory.
func (ctxt *Context) ImportDir(dir string, mode ImportMode) (*Package, error) {
- p, err := ctxt.Import(".", dir, mode)
- // TODO(rsc,adg): breaks godoc net/http. Not sure why.
- // See CL 7232047 and issue 4696.
- if false && err == nil && !ctxt.isDir(p.Dir) {
- err = fmt.Errorf("%q is not a directory", p.Dir)
- }
- return p, err
+ return ctxt.Import(".", dir, mode)
}
// NoGoError is the error used by Import to describe a directory
src/pkg/go/build/build_test.go
--- a/src/pkg/go/build/build_test.go
+++ b/src/pkg/go/build/build_test.go
@@ -5,7 +5,6 @@
package build
import (
- "fmt"
"os"
"path/filepath"
"runtime"
@@ -90,17 +89,6 @@ func TestLocalDirectory(t *testing.T) {
}
}
-// golang.org/issue/3248
-func TestBogusDirectory(t *testing.T) {
- return // See issue 4696.
- const dir = "/foo/bar/baz/gopher"
- _, err := ImportDir(dir, FindOnly)
- want := fmt.Sprintf("%q is not a directory", filepath.FromSlash(dir))
- if err == nil || err.Error() != want {
- t.Errorf("got error %q, want %q", err, want)
- }
-}
-
func TestShouldBuild(t *testing.T) {
const file1 = "// +build tag1\\n\\n" +\
\t\t"package main\\n"
コアとなるコードの解説
このコミットの核心は、go/build パッケージの ImportDir 関数から、無効化されていた(if false で囲まれていた)エラーチェックロジックと、それに関連する TestBogusDirectory テストを削除することです。
-
ImportDir関数の簡素化:- 以前の
ImportDir関数は、ctxt.Importを呼び出した後、if falseで囲まれたブロックで追加のディレクトリ存在チェックを行っていました。このチェックはgodoc net/httpを壊すという問題(issue 4696に関連)があったため、if falseによって無効化されていました。 - このコミットでは、その無効化されたブロック全体が削除され、
ImportDirは単にctxt.Importの結果を返すだけのシンプルな関数になりました。これは、Import関数自体がパッケージのインポートに必要なすべてのロジック(エラーハンドリングを含む)を処理すべきであり、ImportDirが追加のチェックを行う必要がない、という設計思想への回帰を示唆しています。これにより、コードの重複が避けられ、保守性が向上します。
- 以前の
-
TestBogusDirectoryテストの削除:- このテストは、存在しないディレクトリを
ImportDirに渡した場合の挙動を検証しようとしていましたが、return // See issue 4696.によって常にスキップされていました。 issue 4696の議論が示すように、ImportDirが存在しないディレクトリに対して特定のエラーを返すことを期待するこのテストは、もはやGoのビルドシステムの意図する挙動と一致しなくなっていました。- テストの削除は、この特定の挙動がもはや期待されていないこと、そして無効化されたテストコードをコードベースから取り除くことで、テストスイートをクリーンに保つことを意味します。
- このテストは、存在しないディレクトリを
これらの変更は、Goのビルドシステムがより堅牢で予測可能な挙動をするように、そしてコードベースが不要な複雑さから解放されるようにするための、継続的な改善の一環です。特に、ImportDir がディレクトリの存在チェックを行わないようにすることで、godoc のようなツールがより柔軟に動作できるようになり、Goのツールチェイン全体の整合性が向上します。
関連リンク
- GitHubコミットページ: https://github.com/golang/go/commit/3bf3ba2d8179cfbc73f535d3ccc28bc8f46dc45e
- Go CL 7237049 (このコミットのGerritリンク): https://golang.org/cl/7237049
- Go Issue 4696:
go/build: ImportDir should not check for directory existence(関連する議論): https://golang.org/issue/4696 - Go CL 7232047 (コメントで参照されているCL): https://golang.org/cl/7232047
参考にした情報源リンク
- Google 検索: "golang CL 7232047"
- Google 検索: "golang issue 4696"
- Go言語の公式ドキュメント (go/buildパッケージに関する一般的な情報)