Skip to content

Commit d27ebc7

Browse files
author
Bryan C. Mills
committed
cmd/go/internal/modload: implement the "all" pattern for lazy loading
The new semantics of the "all" package pattern can be implemented without actually changing module loading per se. This change implements those semantics, so that the change can be decoupled from the changes to the module requirement graph. For #36460 Change-Id: I0ee8b17afa8b728dc470a42a540fcc01764a4442 Reviewed-on: https://go-review.googlesource.com/c/go/+/240623 Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jay Conrod <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
1 parent b4944ef commit d27ebc7

File tree

6 files changed

+154
-12
lines changed

6 files changed

+154
-12
lines changed

doc/go1.16.html

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ <h3 id="go-command">Go command</h3>
5252
TODO: write and link to tutorial or blog post
5353
</p>
5454

55-
<p><!-= golang.org/issue/29062 -->
55+
<p><!-- golang.org/issue/29062 -->
5656
When using <code>go test</code>, a test that
5757
calls <code>os.Exit(0)</code> during execution of a test function
5858
will now be considered to fail.
@@ -62,6 +62,18 @@ <h3 id="go-command">Go command</h3>
6262
that is still considered to be a passing test.
6363
</p>
6464

65+
<h4 id="all-pattern">The <code>all</code> pattern</h4>
66+
67+
<p><!-- golang.org/cl/240623 -->
68+
When the main module's <code>go.mod</code> file
69+
declares <code>go</code> <code>1.16</code> or higher, the <code>all</code>
70+
package pattern now matches only those packages that are transitively imported
71+
by a package or test found in the main module. (Packages imported by <em>tests
72+
of</em> packages imported by the main module are no longer included.) This is
73+
the same set of packages retained
74+
by <code>go</code> <code>mod</code> <code>vendor</code> since Go 1.11.
75+
</p>
76+
6577
<p>
6678
TODO
6779
</p>

src/cmd/go/internal/modcmd/tidy.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,18 @@ func runTidy(ctx context.Context, cmd *base.Command, args []string) {
4040
base.Fatalf("go mod tidy: no arguments allowed")
4141
}
4242

43+
// Tidy aims to make 'go test' reproducible for any package in 'all', so we
44+
// need to include test dependencies. For modules that specify go 1.15 or
45+
// earlier this is a no-op (because 'all' saturates transitive test
46+
// dependencies).
47+
//
48+
// However, with lazy loading (go 1.16+) 'all' includes only the packages that
49+
// are transitively imported by the main module, not the test dependencies of
50+
// those packages. In order to make 'go test' reproducible for the packages
51+
// that are in 'all' but outside of the main module, we must explicitly
52+
// request that their test dependencies be included.
53+
modload.LoadTests = true
54+
4355
modload.LoadALL(ctx)
4456
modload.TidyBuildList()
4557
modload.TrimGoSum()

src/cmd/go/internal/modcmd/why.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ func runWhy(ctx context.Context, cmd *base.Command, args []string) {
6565
loadALL := modload.LoadALL
6666
if *whyVendor {
6767
loadALL = modload.LoadVendor
68+
} else {
69+
modload.LoadTests = true
6870
}
6971
if *whyM {
7072
listU := false

src/cmd/go/internal/modload/load.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func ImportPathsQuiet(ctx context.Context, patterns []string, tags map[string]bo
231231
loaded = loadFromRoots(loaderParams{
232232
tags: tags,
233233
allPatternIsRoot: allPatternIsRoot,
234-
allClosesOverTests: true, // until lazy loading in Go 1.16+
234+
allClosesOverTests: index.allPatternClosesOverTests(),
235235

236236
listRoots: func() (roots []string) {
237237
updateMatches(nil)
@@ -450,7 +450,7 @@ func ImportFromFiles(ctx context.Context, gofiles []string) {
450450
roots = append(roots, testImports...)
451451
return roots
452452
},
453-
allClosesOverTests: true, // until lazy loading.
453+
allClosesOverTests: index.allPatternClosesOverTests(),
454454
})
455455
WriteGoMod()
456456
}
@@ -501,7 +501,7 @@ func ReloadBuildList() []module.Version {
501501
loaded = loadFromRoots(loaderParams{
502502
tags: imports.Tags(),
503503
listRoots: func() []string { return nil },
504-
allClosesOverTests: true, // until lazy loading, but doesn't matter because the root list is empty.
504+
allClosesOverTests: index.allPatternClosesOverTests(), // but doesn't matter because the root list is empty.
505505
})
506506
return buildList
507507
}
@@ -512,9 +512,13 @@ func ReloadBuildList() []module.Version {
512512
// It adds modules to the build list as needed to satisfy new imports.
513513
// This set is useful for deciding whether a particular import is needed
514514
// anywhere in a module.
515+
//
516+
// In modules that specify "go 1.16" or higher, ALL follows only one layer of
517+
// test dependencies. In "go 1.15" or lower, ALL follows the imports of tests of
518+
// dependencies of tests.
515519
func LoadALL(ctx context.Context) []string {
516520
InitMod(ctx)
517-
return loadAll(ctx, true)
521+
return loadAll(ctx, index.allPatternClosesOverTests())
518522
}
519523

520524
// LoadVendor is like LoadALL but only follows test dependencies
@@ -523,7 +527,9 @@ func LoadALL(ctx context.Context) []string {
523527
// This set is useful for identifying the which packages to include in a vendor directory.
524528
func LoadVendor(ctx context.Context) []string {
525529
InitMod(ctx)
526-
return loadAll(ctx, false)
530+
// 'go mod vendor' has never followed test dependencies since Go 1.11.
531+
const closeOverTests = false
532+
return loadAll(ctx, closeOverTests)
527533
}
528534

529535
func loadAll(ctx context.Context, closeOverTests bool) []string {

src/cmd/go/internal/modload/modfile.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ import (
2525
"golang.org/x/mod/semver"
2626
)
2727

28+
// lazyLoadingVersion is the Go version (plus leading "v") at which lazy module
29+
// loading takes effect.
30+
const lazyLoadingVersionV = "v1.16"
31+
const go116EnableLazyLoading = true
32+
2833
var modFile *modfile.File
2934

3035
// A modFileIndex is an index of data corresponding to a modFile
@@ -249,6 +254,23 @@ func indexModFile(data []byte, modFile *modfile.File, needsFix bool) *modFileInd
249254
return i
250255
}
251256

257+
// allPatternClosesOverTests reports whether the "all" pattern includes
258+
// dependencies of tests outside the main module (as in Go 1.11–1.15).
259+
// (Otherwise — as in Go 1.16+ — the "all" pattern includes only the packages
260+
// transitively *imported by* the packages and tests in the main module.)
261+
func (i *modFileIndex) allPatternClosesOverTests() bool {
262+
if !go116EnableLazyLoading {
263+
return true
264+
}
265+
if i != nil && semver.Compare(i.goVersionV, lazyLoadingVersionV) < 0 {
266+
// The module explicitly predates the change in "all" for lazy loading, so
267+
// continue to use the older interpretation. (If i == nil, we not in any
268+
// module at all and should use the latest semantics.)
269+
return true
270+
}
271+
return false
272+
}
273+
252274
// modFileIsDirty reports whether the go.mod file differs meaningfully
253275
// from what was indexed.
254276
// If modFile has been changed (even cosmetically) since it was first read,

src/cmd/go/testdata/script/mod_all.txt

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -187,17 +187,105 @@ stdout '^example.com/main_test \[example.com/main.test\]$'
187187
stdout '^example.com/main/testonly.test$'
188188
stdout '^example.com/main/testonly_test \[example.com/main/testonly.test\]$'
189189

190-
# TODO(#36460):
190+
rm vendor
191+
192+
# Convert all modules to go 1.16 to enable lazy loading.
193+
go mod edit -go=1.16 a/go.mod
194+
go mod edit -go=1.16 b/go.mod
195+
go mod edit -go=1.16 c/go.mod
196+
go mod edit -go=1.16 d/go.mod
197+
go mod edit -go=1.16 q/go.mod
198+
go mod edit -go=1.16 r/go.mod
199+
go mod edit -go=1.16 s/go.mod
200+
go mod edit -go=1.16 t/go.mod
201+
go mod edit -go=1.16 u/go.mod
202+
go mod edit -go=1.16 w/go.mod
203+
go mod edit -go=1.16 x/go.mod
204+
go mod edit -go=1.16
205+
206+
# With lazy loading, 'go list all' with neither -mod=vendor nor -test should
207+
# match -mod=vendor without -test in 1.15.
191208

192-
# With lazy loading, 'go list all' without -mod=vendor should match
193-
# 'go mod vendor'.
209+
go list -f $PKGFMT all
210+
stdout -count=8 '^.'
211+
stdout '^example.com/a$'
212+
stdout '^example.com/b$'
213+
stdout '^example.com/main$'
214+
stdout '^example.com/main/testonly$'
215+
stdout '^example.com/q$'
216+
stdout '^example.com/r$'
217+
stdout '^example.com/t$'
218+
stdout '^example.com/u$'
194219

195-
# 'go list -test all' should expand that to cover test dependencies
196-
# of packages imported by the main module.
220+
# 'go list -test all' should expand that to include the test variants of the
221+
# packages in 'all', but not the dependencies of outside tests.
197222

198-
# 'go list -m all' should cover the packages in 'go list -test all'.
223+
go list -test -f $PKGFMT all
224+
stdout -count=25 '^.'
225+
stdout '^example.com/a$'
226+
stdout '^example.com/b$'
227+
stdout '^example.com/main$'
228+
stdout '^example.com/main/testonly$'
229+
stdout '^example.com/q$'
230+
stdout '^example.com/r$'
231+
stdout '^example.com/t$'
232+
stdout '^example.com/u$'
233+
stdout '^example.com/a.test$'
234+
stdout '^example.com/a_test \[example.com/a.test\]$'
235+
stdout '^example.com/b.test$'
236+
stdout '^example.com/b_test \[example.com/b.test\]$'
237+
stdout '^example.com/main.test$'
238+
stdout '^example.com/main \[example.com/main.test\]$'
239+
stdout '^example.com/main_test \[example.com/main.test\]$'
240+
stdout '^example.com/main/testonly.test$'
241+
stdout '^example.com/main/testonly_test \[example.com/main/testonly.test\]$'
242+
stdout '^example.com/q.test$'
243+
stdout '^example.com/q_test \[example.com/q.test\]$'
244+
stdout '^example.com/r.test$'
245+
stdout '^example.com/r_test \[example.com/r.test\]$'
246+
stdout '^example.com/t.test$'
247+
stdout '^example.com/t_test \[example.com/t.test\]$'
248+
stdout '^example.com/u.test$'
249+
stdout '^example.com/u_test \[example.com/u.test\]$'
250+
251+
# 'go list -test -deps all' should include the dependencies of those tests,
252+
# but not the tests of the dependencies of outside tests.
253+
254+
go list -test -deps -f $PKGFMT all
255+
stdout -count=28 '^.'
256+
stdout '^example.com/a$'
257+
stdout '^example.com/b$'
258+
stdout '^example.com/c$'
259+
stdout '^example.com/main$'
260+
stdout '^example.com/main/testonly$'
261+
stdout '^example.com/q$'
262+
stdout '^example.com/r$'
263+
stdout '^example.com/s$'
264+
stdout '^example.com/t$'
265+
stdout '^example.com/u$'
266+
stdout '^example.com/w$'
267+
stdout '^example.com/a.test$'
268+
stdout '^example.com/a_test \[example.com/a.test\]$'
269+
stdout '^example.com/b.test$'
270+
stdout '^example.com/b_test \[example.com/b.test\]$'
271+
stdout '^example.com/main.test$'
272+
stdout '^example.com/main \[example.com/main.test\]$'
273+
stdout '^example.com/main_test \[example.com/main.test\]$'
274+
stdout '^example.com/main/testonly.test$'
275+
stdout '^example.com/main/testonly_test \[example.com/main/testonly.test\]$'
276+
stdout '^example.com/q.test$'
277+
stdout '^example.com/q_test \[example.com/q.test\]$'
278+
stdout '^example.com/r.test$'
279+
stdout '^example.com/r_test \[example.com/r.test\]$'
280+
stdout '^example.com/t.test$'
281+
stdout '^example.com/t_test \[example.com/t.test\]$'
282+
stdout '^example.com/u.test$'
283+
stdout '^example.com/u_test \[example.com/u.test\]$'
199284

200285

286+
# TODO(#36460):
287+
# 'go list -m all' should exactly cover the packages in 'go list -test all'.
288+
201289
-- go.mod --
202290
module example.com/main
203291

0 commit comments

Comments
 (0)