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

diff --git a/src/cmd/pack/pack.go b/src/cmd/pack/pack.go
index 99e22ed7ea3367923a93e8e29c322188b76980fb..72760832642759c9e259ff2bcefd1b4648df8e88 100644 (file)
--- 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 {
        }
        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 {

diff --git a/src/cmd/pack/pack_test.go b/src/cmd/pack/pack_test.go
index b54b0ae432e7c3a6b9422d477d3e929d9962e8ce..33abe45a2046b859e0101cffbb218fb758cab329 100644 (file)
--- 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)
+
        // Add some entries by hand.
        ar.addFile(helloFile.Reset())
        ar.addFile(goodbyeFile.Reset())
        ar.fd.Close()
+
        // 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)
        }
+
        // 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)
        }
+
+       // Do it again with file list arguments.
+       verbose = false
+       buf.Reset()
+       ar = archive(name, os.O_RDONLY, []string{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)
+       }
 }
 
 // Test that we can create an archive, put some files in it, and get back a file.