diff --git a/zip/testdata/check_dir/various.txt b/zip/testdata/check_dir/various.txt index 4dd757d..0ded5ac 100644 --- a/zip/testdata/check_dir/various.txt +++ b/zip/testdata/check_dir/various.txt @@ -6,6 +6,7 @@ $work/vendor/modules.txt omitted: $work/.hg_archival.txt: file is inserted by 'hg archive' and is always omitted $work/.git: directory is a version control repository +$work/pkg/vendor/vendor.go: file is in vendor directory $work/sub: directory is in another module $work/vendor/x/y: file is in vendor directory @@ -20,3 +21,4 @@ $work/invalid.go': malformed file path "invalid.go'": invalid char '\'' -- sub/go.mod -- -- .hg_archival.txt -- -- .git/x -- +-- pkg/vendor/vendor.go -- diff --git a/zip/testdata/check_dir/various_go123.txt b/zip/testdata/check_dir/various_go123.txt index 169c059..69102ac 100644 --- a/zip/testdata/check_dir/various_go123.txt +++ b/zip/testdata/check_dir/various_go123.txt @@ -7,6 +7,7 @@ $work/vendor/modules.txt omitted: $work/.hg_archival.txt: file is inserted by 'hg archive' and is always omitted $work/.git: directory is a version control repository +$work/pkg/vendor/vendor.go: file is in vendor directory $work/sub: directory is in another module $work/vendor/x/y: file is in vendor directory @@ -20,4 +21,5 @@ go 1.23 -- vendor/x/y -- -- sub/go.mod -- -- .hg_archival.txt -- --- .git/x -- \ No newline at end of file +-- .git/x -- +-- pkg/vendor/vendor.go -- diff --git a/zip/testdata/check_dir/various_go124.txt b/zip/testdata/check_dir/various_go124.txt index bea7bf4..43bed2e 100644 --- a/zip/testdata/check_dir/various_go124.txt +++ b/zip/testdata/check_dir/various_go124.txt @@ -1,6 +1,7 @@ -- want -- valid: $work/go.mod +$work/pkg/vendor/vendor.go $work/valid.go omitted: @@ -22,3 +23,4 @@ go 1.24 go 1.23 -- .hg_archival.txt -- -- .git/x -- +-- pkg/vendor/vendor.go -- diff --git a/zip/testdata/check_files/various.txt b/zip/testdata/check_files/various.txt index cb312bf..06ec791 100644 --- a/zip/testdata/check_files/various.txt +++ b/zip/testdata/check_files/various.txt @@ -7,6 +7,7 @@ omitted: vendor/x/y: file is in vendor directory sub/go.mod: file is in another module .hg_archival.txt: file is inserted by 'hg archive' and is always omitted +pkg/vendor/vendor.go: file is in vendor directory invalid: not/../clean: file path is not clean @@ -25,3 +26,4 @@ valid.go: multiple entries for file "valid.go" duplicate -- valid.go -- another duplicate +-- pkg/vendor/vendor.go -- diff --git a/zip/testdata/check_files/various_go123.txt b/zip/testdata/check_files/various_go123.txt index 7a34168..a28be93 100644 --- a/zip/testdata/check_files/various_go123.txt +++ b/zip/testdata/check_files/various_go123.txt @@ -8,6 +8,7 @@ omitted: vendor/x/y: file is in vendor directory sub/go.mod: file is in another module .hg_archival.txt: file is inserted by 'hg archive' and is always omitted +pkg/vendor/vendor.go: file is in vendor directory invalid: not/../clean: file path is not clean @@ -26,3 +27,4 @@ go 1.23 duplicate -- valid.go -- another duplicate +-- pkg/vendor/vendor.go -- diff --git a/zip/testdata/check_files/various_go124.txt b/zip/testdata/check_files/various_go124.txt index 3e8881d..94d4bfd 100644 --- a/zip/testdata/check_files/various_go124.txt +++ b/zip/testdata/check_files/various_go124.txt @@ -2,6 +2,7 @@ valid: valid.go go.mod +pkg/vendor/vendor.go omitted: vendor/x/y: file is in vendor directory @@ -27,3 +28,4 @@ go 1.23 duplicate -- valid.go -- another duplicate +-- pkg/vendor/vendor.go -- diff --git a/zip/testdata/create/exclude_vendor.txt b/zip/testdata/create/exclude_vendor.txt index 79b2c08..40ac449 100644 --- a/zip/testdata/create/exclude_vendor.txt +++ b/zip/testdata/create/exclude_vendor.txt @@ -12,3 +12,6 @@ see comment in isVendoredPackage and golang.org/issue/31562. excluded -- sub/vendor/sub.txt -- excluded +-- pkg/vendor/vendor.go -- +excluded +see comment in isVendoredPackage and golang.org/issue/37397 diff --git a/zip/testdata/create/exclude_vendor_go124.txt b/zip/testdata/create/exclude_vendor_go124.txt index 5559d66..940df51 100644 --- a/zip/testdata/create/exclude_vendor_go124.txt +++ b/zip/testdata/create/exclude_vendor_go124.txt @@ -1,6 +1,6 @@ path=example.com/m version=v1.0.0 -hash=h1:WxQ7ERgHLILawPCiuSwXp5UHul7CUSRRftO8b7GTnV8= +hash=h1:mJR1q75yiMK6+CDw6DuhMS4v4hNoLXqkyk2Cph4VS8Q= -- go.mod -- module example.com/m @@ -13,3 +13,6 @@ see comment in isVendoredPackage and golang.org/issue/31562. excluded -- sub/vendor/sub.txt -- excluded +-- pkg/vendor/vendor.go -- +included +see comment in isVendoredPackage and golang.org/issue/37397 diff --git a/zip/testdata/create_from_dir/exclude_vendor.txt b/zip/testdata/create_from_dir/exclude_vendor.txt index 79b2c08..40ac449 100644 --- a/zip/testdata/create_from_dir/exclude_vendor.txt +++ b/zip/testdata/create_from_dir/exclude_vendor.txt @@ -12,3 +12,6 @@ see comment in isVendoredPackage and golang.org/issue/31562. excluded -- sub/vendor/sub.txt -- excluded +-- pkg/vendor/vendor.go -- +excluded +see comment in isVendoredPackage and golang.org/issue/37397 diff --git a/zip/testdata/create_from_dir/exclude_vendor_go124.txt b/zip/testdata/create_from_dir/exclude_vendor_go124.txt index 5559d66..940df51 100644 --- a/zip/testdata/create_from_dir/exclude_vendor_go124.txt +++ b/zip/testdata/create_from_dir/exclude_vendor_go124.txt @@ -1,6 +1,6 @@ path=example.com/m version=v1.0.0 -hash=h1:WxQ7ERgHLILawPCiuSwXp5UHul7CUSRRftO8b7GTnV8= +hash=h1:mJR1q75yiMK6+CDw6DuhMS4v4hNoLXqkyk2Cph4VS8Q= -- go.mod -- module example.com/m @@ -13,3 +13,6 @@ see comment in isVendoredPackage and golang.org/issue/31562. excluded -- sub/vendor/sub.txt -- excluded +-- pkg/vendor/vendor.go -- +included +see comment in isVendoredPackage and golang.org/issue/37397 diff --git a/zip/vendor_test.go b/zip/vendor_test.go index e330401..9deb192 100644 --- a/zip/vendor_test.go +++ b/zip/vendor_test.go @@ -18,43 +18,40 @@ var pre124 []string = []string{ "go1.9", } +var after124 []string = []string{"go1.24.0", "go1.24", "go1.99.0"} + +var allVers []string = append(pre124, after124...) + 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? - versions []string + path string + want bool + versions []string }{ - {path: "vendor/foo/foo.go", want: true, versions: pre124}, - {path: "pkg/vendor/foo/foo.go", want: true, versions: pre124}, - {path: "longpackagename/vendor/foo/foo.go", want: true, versions: pre124}, - {path: "vendor/vendor.go", want: false, versions: pre124}, - {path: "vendor/foo/modules.txt", want: true, versions: pre124}, - {path: "modules.txt", want: false, versions: pre124}, - {path: "vendor/amodules.txt", want: false, versions: pre124}, + {path: "vendor/foo/foo.go", want: true, versions: allVers}, + {path: "pkg/vendor/foo/foo.go", want: true, versions: allVers}, + {path: "longpackagename/vendor/foo/foo.go", want: true, versions: allVers}, + {path: "vendor/vendor.go", want: false, versions: allVers}, + {path: "vendor/foo/modules.txt", want: true, versions: allVers}, + {path: "modules.txt", want: false, versions: allVers}, + {path: "vendor/amodules.txt", want: false, versions: allVers}, // These test cases were affected by https://golang.org/issue/63395 {path: "vendor/modules.txt", want: false, versions: pre124}, - {path: "vendor/modules.txt", want: true, versions: []string{"go1.24.0", "go1.24", "go1.99.0"}}, + {path: "vendor/modules.txt", want: true, versions: after124}, - // 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}, + // These test cases were affected by https://golang.org/issue/37397 + {path: "pkg/vendor/vendor.go", want: true, versions: pre124}, + {path: "pkg/vendor/vendor.go", want: false, versions: after124}, + {path: "longpackagename/vendor/vendor.go", want: true, versions: pre124}, + {path: "longpackagename/vendor/vendor.go", want: false, versions: after124}, } { for _, v := range tc.versions { got := isVendoredPackage(tc.path, v) want := tc.want - if tc.falsePositive { - want = true - } if got != want { t.Errorf("isVendoredPackage(%q, %s) = %t; want %t", tc.path, v, 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 9b351c5..3673db4 100644 --- a/zip/zip.go +++ b/zip/zip.go @@ -786,8 +786,6 @@ func (fi dataFileInfo) Sys() interface{} { return nil } // 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". // The 'vers' parameter specifies the Go version declared in the module's // go.mod file and must be a valid Go version according to the // go/version.IsValid function. @@ -803,13 +801,27 @@ func isVendoredPackage(name string, vers string) bool { if strings.HasPrefix(name, "vendor/") { i += len("vendor/") } else if j := strings.Index(name, "/vendor/"); j >= 0 { - // This offset looks incorrect; this should probably be + // Calculate the correct starting position within the import path + // to determine if a package is vendored. // - // i = j + len("/vendor/") + // Due to a bug in Go versions before 1.24 + // (see https://golang.org/issue/37397), the "/vendor/" prefix within + // a package path was not always correctly interpreted. // - // (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/") + // This bug affected how vendored packages were identified in cases like: + // + // - "pkg/vendor/vendor.go" (incorrectly identified as vendored in pre-1.24) + // - "pkg/vendor/foo/foo.go" (correctly identified as vendored) + // + // To correct this, in Go 1.24 and later, we skip the entire "/vendor/" prefix + // when it's part of a nested package path (as in the first example above). + // In earlier versions, we only skipped the length of "/vendor/", leading + // to the incorrect behavior. + if version.Compare(vers, "go1.24") >= 0 { + i = j + len("/vendor/") + } else { + i += len("/vendor/") + } } else { return false } @@ -950,8 +962,7 @@ func listFilesInDir(dir string) (files []File, omitted []FileError, err error) { } slashPath := filepath.ToSlash(relPath) - // Skip some subdirectories inside vendor, but maintain bug - // golang.org/issue/31562, described in isVendoredPackage. + // Skip some subdirectories inside vendor. // We would like Create and CreateFromDir to produce the same result // for a set of files, whether expressed as a directory tree or zip. if isVendoredPackage(slashPath, vers) {