Skip to content

Commit 9cd0e4c

Browse files
committed
x/mod: remove vendor/modules.txt from module download
This fixes a bug where vendor/modules.txt was accidently included during a module download. This CL trims this file for 1.24 modules and beyond. We cannot remove this for earlier Go versions because this would alter checksums and cause a checksum failure. This CL also adds the ability to case on the Go version in the root's go.mod file, enabling future behavior changes if necessary. Fixes: golang/go#63395 Updates: golang/go#37397 Change-Id: I4a4f2174b0f5e79c7e5c516e0db3c91e7d2ae4d9 Reviewed-on: https://go-review.googlesource.com/c/mod/+/584635 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
1 parent 46a3137 commit 9cd0e4c

File tree

10 files changed

+221
-16
lines changed

10 files changed

+221
-16
lines changed

zip/testdata/check_dir/various.txt

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
-- want --
22
valid:
33
$work/valid.go
4+
$work/vendor/modules.txt
45

56
omitted:
67
$work/.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
@@ -14,6 +15,7 @@ $work/invalid.go': malformed file path "invalid.go'": invalid char '\''
1415
-- valid.go --
1516
-- GO.MOD --
1617
-- invalid.go' --
18+
-- vendor/modules.txt --
1719
-- vendor/x/y --
1820
-- sub/go.mod --
1921
-- .hg_archival.txt --
+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
-- want --
2+
valid:
3+
$work/go.mod
4+
$work/valid.go
5+
$work/vendor/modules.txt
6+
7+
omitted:
8+
$work/.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
9+
$work/.git: directory is a version control repository
10+
$work/sub: directory is in another module
11+
$work/vendor/x/y: file is in vendor directory
12+
13+
invalid:
14+
$work/invalid.go': malformed file path "invalid.go'": invalid char '\''
15+
-- valid.go --
16+
-- go.mod --
17+
go 1.23
18+
-- invalid.go' --
19+
-- vendor/modules.txt --
20+
-- vendor/x/y --
21+
-- sub/go.mod --
22+
-- .hg_archival.txt --
23+
-- .git/x --
+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
-- want --
2+
valid:
3+
$work/go.mod
4+
$work/valid.go
5+
6+
omitted:
7+
$work/.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
8+
$work/.git: directory is a version control repository
9+
$work/sub: directory is in another module
10+
$work/vendor/modules.txt: file is in vendor directory
11+
$work/vendor/x/y: file is in vendor directory
12+
13+
invalid:
14+
$work/invalid.go': malformed file path "invalid.go'": invalid char '\''
15+
-- go.mod --
16+
go 1.24
17+
-- valid.go --
18+
-- invalid.go' --
19+
-- vendor/modules.txt --
20+
-- vendor/x/y --
21+
-- sub/go.mod --
22+
go 1.23
23+
-- .hg_archival.txt --
24+
-- .git/x --

zip/testdata/check_files/various.txt

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
-- want --
22
valid:
33
valid.go
4+
vendor/modules.txt
45

56
omitted:
67
vendor/x/y: file is in vendor directory
@@ -17,6 +18,7 @@ valid.go: multiple entries for file "valid.go"
1718
-- GO.MOD --
1819
-- invalid.go' --
1920
-- vendor/x/y --
21+
-- vendor/modules.txt --
2022
-- sub/go.mod --
2123
-- .hg_archival.txt --
2224
-- valid.go --
+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
-- want --
2+
valid:
3+
valid.go
4+
go.mod
5+
vendor/modules.txt
6+
7+
omitted:
8+
vendor/x/y: file is in vendor directory
9+
sub/go.mod: file is in another module
10+
.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
11+
12+
invalid:
13+
not/../clean: file path is not clean
14+
invalid.go': malformed file path "invalid.go'": invalid char '\''
15+
valid.go: multiple entries for file "valid.go"
16+
-- valid.go --
17+
-- not/../clean --
18+
-- go.mod --
19+
go 1.23
20+
-- invalid.go' --
21+
-- vendor/x/y --
22+
-- vendor/modules.txt --
23+
-- sub/go.mod --
24+
-- .hg_archival.txt --
25+
-- valid.go --
26+
duplicate
27+
-- valid.go --
28+
another duplicate
+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
-- want --
2+
valid:
3+
valid.go
4+
go.mod
5+
6+
omitted:
7+
vendor/x/y: file is in vendor directory
8+
vendor/modules.txt: file is in vendor directory
9+
sub/go.mod: file is in another module
10+
.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
11+
12+
invalid:
13+
not/../clean: file path is not clean
14+
invalid.go': malformed file path "invalid.go'": invalid char '\''
15+
valid.go: multiple entries for file "valid.go"
16+
-- valid.go --
17+
-- not/../clean --
18+
-- go.mod --
19+
go 1.24
20+
-- invalid.go' --
21+
-- vendor/x/y --
22+
-- vendor/modules.txt --
23+
-- sub/go.mod --
24+
go 1.23
25+
-- .hg_archival.txt --
26+
-- valid.go --
27+
duplicate
28+
-- valid.go --
29+
another duplicate
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
path=example.com/m
2+
version=v1.0.0
3+
hash=h1:WxQ7ERgHLILawPCiuSwXp5UHul7CUSRRftO8b7GTnV8=
4+
-- go.mod --
5+
module example.com/m
6+
7+
go 1.24
8+
modules.txt is excluded in 1.24+. See golang.org/issue/63395
9+
-- vendor/modules.txt --
10+
excluded
11+
see comment in isVendoredPackage and golang.org/issue/31562.
12+
-- vendor/example.com/x/x.go --
13+
excluded
14+
-- sub/vendor/sub.txt --
15+
excluded
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
path=example.com/m
2+
version=v1.0.0
3+
hash=h1:WxQ7ERgHLILawPCiuSwXp5UHul7CUSRRftO8b7GTnV8=
4+
-- go.mod --
5+
module example.com/m
6+
7+
go 1.24
8+
modules.txt is excluded in 1.24+. See golang.org/issue/63395
9+
-- vendor/modules.txt --
10+
excluded
11+
see comment in isVendoredPackage and golang.org/issue/31562.
12+
-- vendor/example.com/x/x.go --
13+
excluded
14+
-- sub/vendor/sub.txt --
15+
excluded

zip/vendor_test.go

+32-11
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,52 @@ package zip
66

77
import "testing"
88

9+
var pre124 []string = []string{
10+
"",
11+
"go1.14",
12+
"go1.21.0",
13+
"go1.22.4",
14+
"go1.23",
15+
"go1.23.1",
16+
"go1.2",
17+
"go1.7",
18+
"go1.9",
19+
}
20+
921
func TestIsVendoredPackage(t *testing.T) {
1022
for _, tc := range []struct {
1123
path string
1224
want bool
1325
falsePositive bool // is this case affected by https://golang.org/issue/37397?
26+
versions []string
1427
}{
15-
{path: "vendor/foo/foo.go", want: true},
16-
{path: "pkg/vendor/foo/foo.go", want: true},
17-
{path: "longpackagename/vendor/foo/foo.go", want: true},
28+
{path: "vendor/foo/foo.go", want: true, versions: pre124},
29+
{path: "pkg/vendor/foo/foo.go", want: true, versions: pre124},
30+
{path: "longpackagename/vendor/foo/foo.go", want: true, versions: pre124},
31+
{path: "vendor/vendor.go", want: false, versions: pre124},
32+
{path: "vendor/foo/modules.txt", want: true, versions: pre124},
33+
{path: "modules.txt", want: false, versions: pre124},
34+
{path: "vendor/amodules.txt", want: false, versions: pre124},
1835

19-
{path: "vendor/vendor.go", want: false},
36+
// These test cases were affected by https://golang.org/issue/63395
37+
{path: "vendor/modules.txt", want: false, versions: pre124},
38+
{path: "vendor/modules.txt", want: true, versions: []string{"go1.24.0", "go1.24", "go1.99.0"}},
2039

2140
// We ideally want these cases to be false, but they are affected by
2241
// https://golang.org/issue/37397, and if we fix them we will invalidate
2342
// existing module checksums. We must leave them as-is-for now.
2443
{path: "pkg/vendor/vendor.go", falsePositive: true},
2544
{path: "longpackagename/vendor/vendor.go", falsePositive: true},
2645
} {
27-
got := isVendoredPackage(tc.path)
28-
want := tc.want
29-
if tc.falsePositive {
30-
want = true
31-
}
32-
if got != want {
33-
t.Errorf("isVendoredPackage(%q) = %t; want %t", tc.path, got, tc.want)
46+
for _, v := range tc.versions {
47+
got := isVendoredPackage(tc.path, v)
48+
want := tc.want
49+
if tc.falsePositive {
50+
want = true
51+
}
52+
if got != want {
53+
t.Errorf("isVendoredPackage(%q, %s) = %t; want %t", tc.path, v, got, tc.want)
54+
}
3455
if tc.falsePositive {
3556
t.Logf("(Expected a false-positive due to https://golang.org/issue/37397.)")
3657
}

zip/zip.go

+51-5
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import (
5050
"bytes"
5151
"errors"
5252
"fmt"
53+
"go/version"
5354
"io"
5455
"os"
5556
"os/exec"
@@ -60,6 +61,7 @@ import (
6061
"unicode"
6162
"unicode/utf8"
6263

64+
"golang.org/x/mod/modfile"
6365
"golang.org/x/mod/module"
6466
)
6567

@@ -193,6 +195,20 @@ func CheckFiles(files []File) (CheckedFiles, error) {
193195
return cf, cf.Err()
194196
}
195197

198+
// parseGoVers extracts the Go version specified in the given go.mod file.
199+
// It returns an empty string if the version is not found or if an error
200+
// occurs during file parsing.
201+
//
202+
// The version string is in Go toolchain name syntax, prefixed with "go".
203+
// Examples: "go1.21", "go1.22rc2", "go1.23.0"
204+
func parseGoVers(file string, data []byte) string {
205+
mfile, err := modfile.ParseLax(file, data, nil)
206+
if err != nil || mfile.Go == nil {
207+
return ""
208+
}
209+
return "go" + mfile.Go.Version
210+
}
211+
196212
// checkFiles implements CheckFiles and also returns lists of valid files and
197213
// their sizes, corresponding to cf.Valid. It omits files in submodules, files
198214
// in vendored packages, symlinked files, and various other unwanted files.
@@ -217,6 +233,7 @@ func checkFiles(files []File) (cf CheckedFiles, validFiles []File, validSizes []
217233
// Files in these directories will be omitted.
218234
// These directories will not be included in the output zip.
219235
haveGoMod := make(map[string]bool)
236+
var vers string
220237
for _, f := range files {
221238
p := f.Path()
222239
dir, base := path.Split(p)
@@ -226,8 +243,21 @@ func checkFiles(files []File) (cf CheckedFiles, validFiles []File, validSizes []
226243
addError(p, false, err)
227244
continue
228245
}
229-
if info.Mode().IsRegular() {
230-
haveGoMod[dir] = true
246+
if !info.Mode().IsRegular() {
247+
continue
248+
}
249+
haveGoMod[dir] = true
250+
// Extract the Go language version from the root "go.mod" file.
251+
// This ensures we correctly interpret Go version-specific file omissions.
252+
// We use f.Open() to handle potential custom Open() implementations
253+
// that the underlying File type might have.
254+
if base == "go.mod" && dir == "" {
255+
if file, err := f.Open(); err == nil {
256+
if data, err := io.ReadAll(file); err == nil {
257+
vers = version.Lang(parseGoVers("go.mod", data))
258+
}
259+
file.Close()
260+
}
231261
}
232262
}
233263
}
@@ -257,7 +287,7 @@ func checkFiles(files []File) (cf CheckedFiles, validFiles []File, validSizes []
257287
addError(p, false, errPathNotRelative)
258288
continue
259289
}
260-
if isVendoredPackage(p) {
290+
if isVendoredPackage(p, vers) {
261291
// Skip files in vendored packages.
262292
addError(p, true, errVendored)
263293
continue
@@ -758,7 +788,17 @@ func (fi dataFileInfo) Sys() interface{} { return nil }
758788
//
759789
// Unfortunately, isVendoredPackage reports false positives for files in any
760790
// non-top-level package whose import path ends in "vendor".
761-
func isVendoredPackage(name string) bool {
791+
// The 'vers' parameter specifies the Go version declared in the module's
792+
// go.mod file and must be a valid Go version according to the
793+
// go/version.IsValid function.
794+
// Vendoring behavior has evolved across Go versions, so this function adapts
795+
// its logic accordingly.
796+
func isVendoredPackage(name string, vers string) bool {
797+
// vendor/modules.txt is a vendored package but was included in 1.23 and earlier.
798+
// Remove vendor/modules.txt only for 1.24 and beyond to preserve older checksums.
799+
if version.Compare(vers, "go1.24") >= 0 && name == "vendor/modules.txt" {
800+
return true
801+
}
762802
var i int
763803
if strings.HasPrefix(name, "vendor/") {
764804
i += len("vendor/")
@@ -894,6 +934,12 @@ func (cc collisionChecker) check(p string, isDir bool) error {
894934
// files, as well as a list of directories and files that were skipped (for
895935
// example, nested modules and symbolic links).
896936
func listFilesInDir(dir string) (files []File, omitted []FileError, err error) {
937+
// Extract the Go language version from the root "go.mod" file.
938+
// This ensures we correctly interpret Go version-specific file omissions.
939+
var vers string
940+
if data, err := os.ReadFile(filepath.Join(dir, "go.mod")); err == nil {
941+
vers = version.Lang(parseGoVers("go.mod", data))
942+
}
897943
err = filepath.Walk(dir, func(filePath string, info os.FileInfo, err error) error {
898944
if err != nil {
899945
return err
@@ -908,7 +954,7 @@ func listFilesInDir(dir string) (files []File, omitted []FileError, err error) {
908954
// golang.org/issue/31562, described in isVendoredPackage.
909955
// We would like Create and CreateFromDir to produce the same result
910956
// for a set of files, whether expressed as a directory tree or zip.
911-
if isVendoredPackage(slashPath) {
957+
if isVendoredPackage(slashPath, vers) {
912958
omitted = append(omitted, FileError{Path: slashPath, Err: errVendored})
913959
return nil
914960
}

0 commit comments

Comments
 (0)