zip: document isVendoredPackage false-positives
I had thought that the known bug in isVendoredPackage was strictly
conservative, but it turns out not to be.
Updates golang/go#37397
Updates golang/go#31562
Change-Id: I60f6062c41ec2d116766930f751d7df083535355
Reviewed-on: https://go-review.googlesource.com/c/mod/+/220657
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
diff --git a/zip/vendor_test.go b/zip/vendor_test.go
new file mode 100644
index 0000000..5eb9535
--- /dev/null
+++ b/zip/vendor_test.go
@@ -0,0 +1,39 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package zip
+
+import "testing"
+
+func TestIsVendoredPackage(t *testing.T) {
+ for _, tc := range []struct {
+ path string
+ want bool
+ falsePositive bool // is this case affected by https://golang.org/issue/37397?
+ }{
+ {path: "vendor/foo/foo.go", want: true},
+ {path: "pkg/vendor/foo/foo.go", want: true},
+ {path: "longpackagename/vendor/foo/foo.go", want: true},
+
+ {path: "vendor/vendor.go", want: false},
+
+ // We ideally want these cases to be false, but they are affected by
+ // https://golang.org/issue/37397, and if we fix them we will invalidate
+ // existing module checksums. We must leave them as-is-for now.
+ {path: "pkg/vendor/vendor.go", falsePositive: true},
+ {path: "longpackagename/vendor/vendor.go", falsePositive: true},
+ } {
+ got := isVendoredPackage(tc.path)
+ want := tc.want
+ if tc.falsePositive {
+ want = true
+ }
+ if got != want {
+ t.Errorf("isVendoredPackage(%q) = %t; want %t", tc.path, got, tc.want)
+ if tc.falsePositive {
+ t.Logf("(Expected a false-positive due to https://golang.org/issue/37397.)")
+ }
+ }
+ }
+}
diff --git a/zip/zip.go b/zip/zip.go
index 6bdc01f..6865895 100644
--- a/zip/zip.go
+++ b/zip/zip.go
@@ -316,6 +316,12 @@
func (f dirFile) Lstat() (os.FileInfo, error) { return f.info, nil }
func (f dirFile) Open() (io.ReadCloser, error) { return os.Open(f.filePath) }
+// isVendoredPackage attempts to report whether the given filename is contained
+// in a package whose import path contains (but does not end with) the component
+// "vendor".
+//
+// Unfortunately, isVendoredPackage reports false positives for files in any
+// non-top-level package whose import path ends in "vendor".
func isVendoredPackage(name string) bool {
var i int
if strings.HasPrefix(name, "vendor/") {
@@ -325,15 +331,8 @@
//
// i = j + len("/vendor/")
//
- // (See https://golang.org/issue/31562.)
- //
- // Unfortunately, we can't fix it without invalidating checksums.
- // Fortunately, the error appears to be strictly conservative: we'll retain
- // vendored packages that we should have pruned, but we won't prune
- // non-vendored packages that we should have retained.
- //
- // Since this defect doesn't seem to break anything, it's not worth fixing
- // for now.
+ // (See https://golang.org/issue/31562 and https://golang.org/issue/37397.)
+ // Unfortunately, we can't fix it without invalidating module checksums.
i += len("/vendor/")
} else {
return false
