diff --git a/src/cmd/go/internal/modload/buildlist.go b/src/cmd/go/internal/modload/buildlist.go index def9c489e9..58ef80bfe5 100644 --- a/src/cmd/go/internal/modload/buildlist.go +++ b/src/cmd/go/internal/modload/buildlist.go @@ -11,6 +11,7 @@ import ( "cmd/go/internal/par" "cmd/go/internal/slices" "context" + "errors" "fmt" "os" "reflect" @@ -758,8 +759,8 @@ func updateWorkspaceRoots(ctx context.Context, rs *Requirements, add []module.Ve // roots) until the set of roots has converged. func tidyPrunedRoots(ctx context.Context, mainModule module.Version, direct map[string]bool, pkgs []*loadPkg) (*Requirements, error) { var ( - roots []module.Version - pathIncluded = map[string]bool{mainModule.Path: true} + roots []module.Version + pathIsRoot = map[string]bool{mainModule.Path: true} ) // We start by adding roots for every package in "all". // @@ -779,9 +780,9 @@ func tidyPrunedRoots(ctx context.Context, mainModule module.Version, direct map[ if !pkg.flags.has(pkgInAll) { continue } - if pkg.fromExternalModule() && !pathIncluded[pkg.mod.Path] { + if pkg.fromExternalModule() && !pathIsRoot[pkg.mod.Path] { roots = append(roots, pkg.mod) - pathIncluded[pkg.mod.Path] = true + pathIsRoot[pkg.mod.Path] = true } queue = append(queue, pkg) queued[pkg] = true @@ -813,11 +814,12 @@ func tidyPrunedRoots(ctx context.Context, mainModule module.Version, direct map[ queue = append(queue, pkg.test) queued[pkg.test] = true } - if !pathIncluded[m.Path] { + + if !pathIsRoot[m.Path] { if s := mg.Selected(m.Path); cmpVersion(s, m.Version) < 0 { roots = append(roots, m) + pathIsRoot[m.Path] = true } - pathIncluded[m.Path] = true } } @@ -827,10 +829,62 @@ func tidyPrunedRoots(ctx context.Context, mainModule module.Version, direct map[ } } + roots = tidy.rootModules _, err := tidy.Graph(ctx) if err != nil { return nil, err } + + // We try to avoid adding explicit requirements for test-only dependencies of + // packages in external modules. However, if we drop the explicit + // requirements, that may change an import from unambiguous (due to lazy + // module loading) to ambiguous (because lazy module loading no longer + // disambiguates it). For any package that has become ambiguous, we try + // to fix it by promoting its module to an explicit root. + // (See https://go.dev/issue/60313.) + q := par.NewQueue(runtime.GOMAXPROCS(0)) + for { + var disambiguateRoot sync.Map + for _, pkg := range pkgs { + if pkg.mod.Path == "" || pathIsRoot[pkg.mod.Path] { + // Lazy module loading will cause m to be checked before any other modules + // that are only indirectly required. It is as unambiguous as possible. + continue + } + pkg := pkg + q.Add(func() { + skipModFile := true + _, _, _, _, err := importFromModules(ctx, pkg.path, tidy, nil, skipModFile) + if aie := (*AmbiguousImportError)(nil); errors.As(err, &aie) { + disambiguateRoot.Store(pkg.mod, true) + } + }) + } + <-q.Idle() + + disambiguateRoot.Range(func(k, _ any) bool { + m := k.(module.Version) + roots = append(roots, m) + pathIsRoot[m.Path] = true + return true + }) + + if len(roots) > len(tidy.rootModules) { + module.Sort(roots) + tidy = newRequirements(pruned, roots, tidy.direct) + _, err = tidy.Graph(ctx) + if err != nil { + return nil, err + } + // Adding these roots may have pulled additional modules into the module + // graph, causing additional packages to become ambiguous. Keep iterating + // until we reach a fixed point. + continue + } + + break + } + return tidy, nil } diff --git a/src/cmd/go/testdata/script/mod_tidy_issue60313.txt b/src/cmd/go/testdata/script/mod_tidy_issue60313.txt index 1ae2c13b74..cd704ce34c 100644 --- a/src/cmd/go/testdata/script/mod_tidy_issue60313.txt +++ b/src/cmd/go/testdata/script/mod_tidy_issue60313.txt @@ -2,12 +2,9 @@ # dependencies needed to prevent 'ambiguous import' errors in external test # dependencies. +cp go.mod go.mod.orig go mod tidy -cp go.mod tidy1.mod - -! go mod tidy # BUG: This should succeed and leave go.mod unchanged. - # cmp go.mod tidy1.mod -stderr 'ambiguous import' +cmp go.mod go.mod.orig -- go.mod -- module example