[インデックス 18588] ファイルの概要
このコミットは、Go言語のツールチェインに含まれる cmd/pack コマンドにおけるバグ修正です。具体的には、アーカイブ内のファイルをマッチングする際のロジックが誤っていた点を修正しています。変更されたファイルは以下の通りです。
src/cmd/pack/pack.go:packコマンドの主要なロジックが含まれるファイルです。Archive構造体と、ファイルのマッチングを行うmatchメソッドが変更されています。src/cmd/pack/pack_test.go:packコマンドのテストファイルです。今回のバグ修正を検証するための新しいテストケースが追加されています。
コミット
commit 258c278e12ba90502bb4805343592a926b6d9a7a
Author: Russ Cox <rsc@golang.org>
Date: Thu Feb 20 15:50:30 2014 -0500
cmd/pack: fix match
Match used len(ar.files) == 0 to mean "match everything"
but it also deleted matched things from the list, so once you
had matched everything you asked for, match returned true
for whatever was left in the archive too.
Concretely, if you have an archive containing f1, f2, then
pack t foo.a f1
would match f1 and then, because len(ar.files) == 0 after
deleting f1 from the match list, also match f2.
Avoid the problem by recording explicitly whether match
matches everything.
LGTM=r, dsymonds
R=r, dsymonds
CC=golang-codereviews
https://golang.org/cl/65630046
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/258c278e12ba90502bb4805343592a926b6d9a7a
元コミット内容
cmd/pack: fix match
Match 関数は、len(ar.files) == 0 を「すべてをマッチさせる」という意味で使用していました。しかし、この関数はマッチした項目をリストから削除するため、要求されたすべての項目がマッチして削除されると、ar.files の長さが0になり、アーカイブ内に残っている他のすべての項目もマッチしてしまうという問題がありました。
具体的には、f1 と f2 を含むアーカイブがある場合、pack t foo.a f1 というコマンドを実行すると、まず f1 がマッチします。その後、マッチリストから f1 が削除されると len(ar.files) が0になるため、f2 も意図せずマッチしてしまいました。
この問題を回避するため、match 関数がすべてをマッチさせるべきかどうかを明示的に記録する新しいフィールドを導入しました。
変更の背景
このコミットは、Go言語の pack コマンドにおける論理的なバグを修正するために行われました。pack コマンドは、Goのツールチェインの一部として提供されるアーカイブユーティリティであり、Unix系のシステムにおける ar コマンドに似た機能を提供します。このコマンドは、複数のファイルを一つのアーカイブファイルにまとめたり、アーカイブからファイルを抽出したり、アーカイブの内容を一覧表示したりするために使用されます。
問題は、pack コマンドがアーカイブ内の特定ファイルを処理する際に、内部的なマッチングロジックに欠陥があったことです。具体的には、ユーザーが特定のファイルを指定してアーカイブを操作しようとした場合（例: pack t foo.a f1 で foo.a から f1 の内容を表示する）、pack は指定されたファイルだけでなく、アーカイブ内の他のファイルまで誤って処理してしまう可能性がありました。これは、内部で「すべてをマッチさせる」という状態を ar.files リストの空っぽさで判断していたためです。指定されたファイルが処理されてリストから削除されると、リストが空になり、その後の処理で意図せず「すべてをマッチさせる」モードに移行してしまっていたのです。
このバグは、ユーザーが期待する動作と異なる結果をもたらし、特にアーカイブの内容を一覧表示する t (table of contents) コマンドで顕著でした。この修正は、pack コマンドの正確性と信頼性を向上させることを目的としています。
前提知識の解説
cmd/pack とは
cmd/pack は、Go言語の標準ツールチェインに含まれるコマンドラインユーティリティの一つです。その主な機能は、Unix系のシステムで広く使われている ar (archiver) コマンドに似ています。ar コマンドは、複数のファイルを単一のアーカイブファイル（通常は .a 拡張子を持つ）にまとめるために使用されます。これらのアーカイブファイルは、特に静的ライブラリ（.lib や .a ファイル）を作成する際によく利用されます。
pack コマンドの典型的な用途は以下の通りです。
- ファイルのアーカイブ化: 複数のオブジェクトファイル（
.oファイルなど）を一つのライブラリファイルにまとめる。 - アーカイブからの抽出: アーカイブファイルから特定のファイルを抽出する。
- アーカイブの内容表示: アーカイブファイルに含まれるファイルの一覧を表示する。
Go言語のコンテキストでは、pack は主にコンパイラやリンカが内部的に使用するツールであり、Goプログラムのビルドプロセスにおいて静的ライブラリの管理に貢献しています。
アーカイブツールにおける「マッチング」の概念
アーカイブツールにおいて「マッチング」とは、ユーザーが指定した条件（ファイル名、パターンなど）に基づいて、アーカイブ内のどのファイルを操作対象とするかを決定するプロセスを指します。例えば、pack t myarchive.a file1 file2 のようにコマンドを実行した場合、ツールは myarchive.a の中から file1 と file2 を探し出し、それらに対して操作（この場合は内容の表示）を行います。
多くの場合、引数としてファイル名が指定されない場合、アーカイブツールはアーカイブ内の「すべてのファイル」を操作対象とします。これは、ユーザーがアーカイブ全体に対して操作を行いたい場合に便利な機能です。
今回のバグは、この「すべてをマッチさせる」という状態の判断方法に起因していました。従来の pack コマンドでは、内部的に処理すべきファイル名のリスト（ar.files）が空であることによって「すべてをマッチさせる」状態を判断していました。しかし、このリストは処理が進むにつれてマッチしたファイルが削除されていくため、意図せず空になってしまい、結果として「すべてをマッチさせる」状態に移行してしまうという副作用がありました。
技術的詳細
このコミットの技術的な核心は、cmd/pack がアーカイブ内のファイルをマッチングする際の内部状態管理の改善にあります。
既存の問題点
Archive 構造体には、処理すべきファイル名のリストを保持する files []string フィールドがありました。そして、match メソッド（アーカイブ内のエントリが現在の処理対象リストにマッチするかどうかを判断する）では、以下のようなロジックが使われていました。
func (ar *Archive) match(entry *Entry) bool {
if len(ar.files) == 0 {
return true // ar.filesが空なら、すべてをマッチさせる
}
// ... それ以外の場合は、ar.filesリスト内の名前とマッチするかどうかをチェックし、マッチしたらリストから削除する
}
このロジックの問題点は、ar.files が二つの異なる意味を持っていたことです。
- 明示的に指定されたファイルリスト: ユーザーが
pack t foo.a f1のように特定のファイルを指定した場合、ar.filesにはf1が含まれます。 - 「すべてをマッチさせる」フラグ: ユーザーがファイルを指定しなかった場合（例:
pack t foo.a）、ar.filesは初期状態で空になり、matchメソッドはtrueを返してすべてのファイルをマッチさせます。
しかし、match メソッドは、マッチしたファイルを ar.files リストから削除する副作用を持っていました。このため、以下のようなシナリオで問題が発生しました。
- アーカイブ
foo.aにf1とf2が含まれているとします。 - ユーザーが
pack t foo.a f1を実行します。 Archiveが初期化され、ar.filesには["f1"]がセットされます。matchメソッドがf1に対して呼び出されます。f1はar.filesにマッチし、ar.filesからf1が削除されます。この時点でar.filesは[](空) になります。- 次に
matchメソッドがf2に対して呼び出されます。この時、len(ar.files) == 0がtrueとなり、f2も意図せずマッチしてしまいます。
結果として、pack t foo.a f1 は f1 だけでなく f2 も表示してしまうというバグが発生していました。
修正内容
この問題を解決するため、コミットでは Archive 構造体に新しいブール型フィールド matchAll が追加されました。
type Archive struct {
fd *os.File // Open file descriptor.
files []string // Explicit list of files to be processed.
pad int // Padding bytes required at end of current archive file
matchAll bool // match all files in archive
}
この matchAll フィールドは、archive 関数（アーカイブを開くまたは作成する関数）で初期化されます。
func archive(name string, mode int, files []string) *Archive {
// ...
return &Archive{
fd: fd,
files: files,
matchAll: len(files) == 0, // ここでmatchAllを適切に設定
}
}
これにより、matchAll はアーカイブが初期化される時点で、ユーザーがファイルを明示的に指定したかどうか（つまり、files スライスが空かどうか）に基づいて一度だけ設定されます。
そして、match メソッドのロジックが変更され、len(ar.files) == 0 の代わりに ar.matchAll を参照するようになりました。
func (ar *Archive) match(entry *Entry) bool {
if ar.matchAll { // len(ar.files) == 0 の代わりに matchAll を使用
return true
}
for i, name := range ar.files {
// ...
}
}
この変更により、ar.files リストからファイルが削除されても matchAll の値は変化しないため、match メソッドが意図せず「すべてをマッチさせる」モードに移行することがなくなりました。matchAll は、ユーザーがコマンドラインでファイルを指定しなかった場合にのみ true となり、それ以外の場合は false のまま維持されます。
テストの追加
修正の正しさを検証するために、src/cmd/pack/pack_test.go に新しいテストケースが追加されました。このテストは、特定のファイルを指定してアーカイブの内容を表示するシナリオをシミュレートし、期待される出力（指定されたファイルのみが表示されること）が返されることを確認します。
// Do it again with file list arguments.
verbose = false
buf.Reset()
ar = archive(name, os.O_RDONLY, []string{helloFile.name}) // helloFile.nameのみを指定
ar.scan(ar.tableOfContents)
ar.fd.Close()
result = buf.String()
// Expect only helloFile.
expect = fmt.Sprintf("%s\n", helloFile.name)
if result != expect {
t.Fatalf("expected %q got %q", expect, result)
}
このテストは、helloFile.name という単一のファイルを指定してアーカイブをスキャンし、出力がそのファイル名のみであることを検証することで、以前のバグ（他のファイルも誤って表示される）が修正されたことを確認しています。
コアとなるコードの変更箇所
src/cmd/pack/pack.go
--- a/src/cmd/pack/pack.go
+++ b/src/cmd/pack/pack.go
@@ -132,9 +132,10 @@ const (
// An Archive represents an open archive file. It is always scanned sequentially
// from start to end, without backing up.
type Archive struct {
- fd *os.File // Open file descriptor.
- files []string // Explicit list of files to be processed.
- pad int // Padding bytes required at end of current archive file
+ fd *os.File // Open file descriptor.
+ files []string // Explicit list of files to be processed.
+ pad int // Padding bytes required at end of current archive file
+ matchAll bool // match all files in archive
}
// archive opens (or if necessary creates) the named archive.
@@ -148,8 +149,9 @@ func archive(name string, mode int, files []string) *Archive {
}\n mustBeArchive(fd)
return &Archive{
- fd: fd,
- files: files,
+ fd: fd,
+ files: files,
+ matchAll: len(files) == 0,
}
}
@@ -282,7 +284,7 @@ func (ar *Archive) skip(entry *Entry) {
// match reports whether the entry matches the argument list.
// If it does, it also drops the file from the to-be-processed list.
func (ar *Archive) match(entry *Entry) bool {
- if len(ar.files) == 0 {
+ if ar.matchAll {
return true
}
for i, name := range ar.files {
src/cmd/pack/pack_test.go
--- a/src/cmd/pack/pack_test.go
+++ b/src/cmd/pack/pack_test.go
@@ -90,10 +90,12 @@ func TestTableOfContents(t *testing.T) {
defer os.RemoveAll(dir)
name := filepath.Join(dir, "pack.a")
ar := archive(name, os.O_RDWR, nil)
+\n // Add some entries by hand.
// Add some entries by hand.
ar.addFile(helloFile.Reset())
ar.addFile(goodbyeFile.Reset())
ar.fd.Close()
+\n // Now print it.
// Now print it.
ar = archive(name, os.O_RDONLY, nil)
var buf bytes.Buffer
@@ -111,6 +113,7 @@ func TestTableOfContents(t *testing.T) {
if result != expect {
t.Fatalf("expected %q got %q", expect, result)
}
+\n // Do it again without verbose.
// Do it again without verbose.
verbose = false
buf.Reset()
@@ -123,6 +126,19 @@ func TestTableOfContents(t *testing.T) {
if result != expect {
t.Fatalf("expected %q got %q", expect, result)
}
+\n+\t// Do it again with file list arguments.
+\tverbose = false
+\tbuf.Reset()
+\tar = archive(name, os.O_RDONLY, []string{helloFile.name})
+\tar.scan(ar.tableOfContents)
+\tar.fd.Close()
+\tresult = buf.String()
+\t// Expect only helloFile.
+\texpect = fmt.Sprintf("%s\n", helloFile.name)
+\tif result != expect {
+\t\tt.Fatalf("expected %q got %q", expect, result)
+\t}
}\n \n // Test that we can create an archive, put some files in it, and get back a file.\n```
## コアとなるコードの解説
### `src/cmd/pack/pack.go` の変更点
1. **`Archive` 構造体への `matchAll` フィールドの追加**:
`Archive` 構造体に `matchAll bool` という新しいフィールドが追加されました。このフィールドは、アーカイブ内のすべてのファイルをマッチング対象とするべきかどうかを明示的に示すためのフラグです。これにより、`files` スライスの長さが0であるという曖昧な状態に依存することなく、意図を明確に表現できるようになりました。
2. **`archive` 関数での `matchAll` の初期化**:
`archive` 関数は、新しい `Archive` オブジェクトを作成する際に呼び出されます。この関数内で、`matchAll` フィールドが `len(files) == 0` の結果に基づいて初期化されるようになりました。
- もし `files` スライスが空（つまり、ユーザーが特定のファイルを指定しなかった）であれば、`matchAll` は `true` に設定されます。これは、すべてのファイルをマッチング対象とすることを意味します。
- もし `files` スライスが空でなければ（つまり、ユーザーが特定のファイルを指定した）、`matchAll` は `false` に設定されます。これは、指定されたファイルのみをマッチング対象とすることを意味します。
この初期化は一度だけ行われ、その後の `files` スライスの変更（マッチしたファイルの削除など）によって `matchAll` の値が変わることはありません。
3. **`match` メソッドでの `matchAll` の利用**:
`match` メソッドは、アーカイブ内の各エントリが現在のマッチング条件に合致するかどうかを判断します。このメソッドの冒頭の条件分岐が `if len(ar.files) == 0` から `if ar.matchAll` に変更されました。
- `ar.matchAll` が `true` であれば、そのエントリは常にマッチすると判断され、`true` が返されます。
- `ar.matchAll` が `false` であれば、従来のロジック（`ar.files` リスト内の名前とエントリの名前を比較し、マッチすればリストから削除する）が実行されます。
この変更により、`files` スライスが空になったとしても、`matchAll` が `false` であれば「すべてをマッチさせる」モードには移行せず、意図しないマッチングを防ぐことができます。
### `src/cmd/pack/pack_test.go` の変更点
1. **新しいテストケースの追加**:
`TestTableOfContents` 関数内に、新しいテストシナリオが追加されました。このテストは、`pack t foo.a f1` のような、特定のファイルを指定してアーカイブの内容を表示するコマンドの動作をシミュレートします。
- `ar = archive(name, os.O_RDONLY, []string{helloFile.name})` の行で、`archive` 関数に `helloFile.name` という単一のファイル名を渡して `Archive` オブジェクトを初期化しています。これにより、`matchAll` は `false` に設定され、`files` スライスには `helloFile.name` が含まれます。
- `ar.scan(ar.tableOfContents)` を呼び出してアーカイブをスキャンし、内容を `buf` に書き込みます。
- `result = buf.String()` で得られた出力が、`expect = fmt.Sprintf("%s\n", helloFile.name)` で定義された期待される出力（`helloFile.name` のみ）と一致するかどうかを検証しています。
このテストは、以前のバグ（指定したファイル以外も表示される）が修正され、正確に指定されたファイルのみが処理されることを保証します。
これらの変更により、`pack` コマンドのファイルマッチングロジックがより堅牢になり、意図しない動作が排除されました。
## 関連リンク
- Go CL 65630046: [https://golang.org/cl/65630046](https://golang.org/cl/65630046)
## 参考にした情報源リンク
- Go言語のソースコード (特に `src/cmd/pack` ディレクトリ)
- Unix `ar` コマンドのドキュメント (一般的なアーカイブツールの概念理解のため)
- コミットメッセージと差分