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

[インデックス 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 7232047issue 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
}

このコードには以下の特徴がありました。

  1. ctxt.Import(".", dir, mode) を呼び出してパッケージ情報を取得します。
  2. if false && ... という条件文があります。これは false が含まれているため、この if ブロック内のコードは決して実行されません
  3. コメントで TODO(rsc,adg): breaks godoc net/http. Not sure why. See CL 7232047 and issue 4696. とあり、このコードが godoc net/http を壊す問題があり、その原因が不明であること、そして関連するCLとIssueが参照されています。
  4. 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 テストを削除することです。

  1. ImportDir 関数の簡素化:

    • 以前の ImportDir 関数は、ctxt.Import を呼び出した後、if false で囲まれたブロックで追加のディレクトリ存在チェックを行っていました。このチェックは godoc net/http を壊すという問題(issue 4696 に関連)があったため、if false によって無効化されていました。
    • このコミットでは、その無効化されたブロック全体が削除され、ImportDir は単に ctxt.Import の結果を返すだけのシンプルな関数になりました。これは、Import 関数自体がパッケージのインポートに必要なすべてのロジック(エラーハンドリングを含む)を処理すべきであり、ImportDir が追加のチェックを行う必要がない、という設計思想への回帰を示唆しています。これにより、コードの重複が避けられ、保守性が向上します。
  2. TestBogusDirectory テストの削除:

    • このテストは、存在しないディレクトリを ImportDir に渡した場合の挙動を検証しようとしていましたが、return // See issue 4696. によって常にスキップされていました。
    • issue 4696 の議論が示すように、ImportDir が存在しないディレクトリに対して特定のエラーを返すことを期待するこのテストは、もはやGoのビルドシステムの意図する挙動と一致しなくなっていました。
    • テストの削除は、この特定の挙動がもはや期待されていないこと、そして無効化されたテストコードをコードベースから取り除くことで、テストスイートをクリーンに保つことを意味します。

これらの変更は、Goのビルドシステムがより堅牢で予測可能な挙動をするように、そしてコードベースが不要な複雑さから解放されるようにするための、継続的な改善の一環です。特に、ImportDir がディレクトリの存在チェックを行わないようにすることで、godoc のようなツールがより柔軟に動作できるようになり、Goのツールチェイン全体の整合性が向上します。

関連リンク

参考にした情報源リンク

  • Google 検索: "golang CL 7232047"
  • Google 検索: "golang issue 4696"
  • Go言語の公式ドキュメント (go/buildパッケージに関する一般的な情報)