[インデックス 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パッケージに関する一般的な情報)