]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: with -mod=vendor, don't panic if there are duplicate requirements
authorJay Conrod <jayconrod@google.com>
Thu, 5 Aug 2021 22:18:42 +0000 (15:18 -0700)
committerJay Conrod <jayconrod@google.com>
Mon, 9 Aug 2021 20:06:35 +0000 (20:06 +0000)
In loadModFile with -mod=vendor, load the vendor list and use it to
initialize the module graph before calling updateRoots.

In updateLazyRoots with any mode other than "mod", return the original
*Requirements if no roots needed to be upgraded, even if there are
inconsistencies. This means 'go list -m -mod=readonly' and -mod=vendor
may succeed if there are duplicate requirements or requirements on
versions of the main module.

Fixes #47565

Change-Id: I4640dffc4a7359367cc910da0d29e3538bfd1ca4
Reviewed-on: https://go-review.googlesource.com/c/go/+/340252
Trust: Jay Conrod <jayconrod@google.com>
Trust: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
src/cmd/go/internal/modload/buildlist.go
src/cmd/go/internal/modload/init.go
src/cmd/go/testdata/script/mod_tidy_lazy_self.txt
src/cmd/go/testdata/script/mod_vendor_redundant_requirement.txt [new file with mode: 0644]

index 604a57b4373b28051b0fdf0235b1e52f9051d9c3..bf6956731676d39e66e74642f6a43b40607d619a 100644 (file)
@@ -191,6 +191,19 @@ func (rs *Requirements) rootSelected(path string) (version string, ok bool) {
        return "", false
 }
 
+// hasRedundantRoot returns true if the root list contains multiple requirements
+// of the same module or a requirement on any version of the main module.
+// Redundant requirements should be pruned, but they may influence version
+// selection.
+func (rs *Requirements) hasRedundantRoot() bool {
+       for i, m := range rs.rootModules {
+               if m.Path == Target.Path || (i > 0 && m.Path == rs.rootModules[i-1].Path) {
+                       return true
+               }
+       }
+       return false
+}
+
 // Graph returns the graph of module requirements loaded from the current
 // root modules (as reported by RootModules).
 //
@@ -882,6 +895,12 @@ func updateLazyRoots(ctx context.Context, direct map[string]bool, rs *Requiremen
                // and (trivially) version.
 
                if !rootsUpgraded {
+                       if cfg.BuildMod != "mod" {
+                               // The only changes to the root set (if any) were to remove duplicates.
+                               // The requirements are consistent (if perhaps redundant), so keep the
+                               // original rs to preserve its ModuleGraph.
+                               return rs, nil
+                       }
                        // The root set has converged: every root going into this iteration was
                        // already at its selected version, although we have have removed other
                        // (redundant) roots for the same path.
index a8cbd9fe16d1862d673de85f4f9e301461ab07d3..45f724d5e3658f5d21d757d7a955b24af6b89be9 100644 (file)
@@ -449,13 +449,22 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) {
        }
 
        setDefaultBuildMod() // possibly enable automatic vendoring
-       rs = requirementsFromModFile(ctx)
-
+       rs = requirementsFromModFile()
        if cfg.BuildMod == "vendor" {
                readVendorList()
                checkVendorConsistency()
                rs.initVendor(vendorList)
        }
+       if rs.hasRedundantRoot() {
+               // If any module path appears more than once in the roots, we know that the
+               // go.mod file needs to be updated even though we have not yet loaded any
+               // transitive dependencies.
+               rs, err = updateRoots(ctx, rs.direct, rs, nil, nil, false)
+               if err != nil {
+                       base.Fatalf("go: %v", err)
+               }
+       }
+
        if index.goVersionV == "" {
                // TODO(#45551): Do something more principled instead of checking
                // cfg.CmdName directly here.
@@ -530,7 +539,12 @@ func CreateModFile(ctx context.Context, modPath string) {
                base.Fatalf("go: %v", err)
        }
 
-       commitRequirements(ctx, modFileGoVersion(), requirementsFromModFile(ctx))
+       rs := requirementsFromModFile()
+       rs, err = updateRoots(ctx, rs.direct, rs, nil, nil, false)
+       if err != nil {
+               base.Fatalf("go: %v", err)
+       }
+       commitRequirements(ctx, modFileGoVersion(), rs)
 
        // Suggest running 'go mod tidy' unless the project is empty. Even if we
        // imported all the correct requirements above, we're probably missing
@@ -641,9 +655,8 @@ func initTarget(m module.Version) {
 
 // requirementsFromModFile returns the set of non-excluded requirements from
 // the global modFile.
-func requirementsFromModFile(ctx context.Context) *Requirements {
+func requirementsFromModFile() *Requirements {
        roots := make([]module.Version, 0, len(modFile.Require))
-       mPathCount := map[string]int{Target.Path: 1}
        direct := map[string]bool{}
        for _, r := range modFile.Require {
                if index != nil && index.exclude[r.Mod] {
@@ -656,28 +669,12 @@ func requirementsFromModFile(ctx context.Context) *Requirements {
                }
 
                roots = append(roots, r.Mod)
-               mPathCount[r.Mod.Path]++
                if !r.Indirect {
                        direct[r.Mod.Path] = true
                }
        }
        module.Sort(roots)
        rs := newRequirements(modDepthFromGoVersion(modFileGoVersion()), roots, direct)
-
-       // If any module path appears more than once in the roots, we know that the
-       // go.mod file needs to be updated even though we have not yet loaded any
-       // transitive dependencies.
-       for _, n := range mPathCount {
-               if n > 1 {
-                       var err error
-                       rs, err = updateRoots(ctx, rs.direct, rs, nil, nil, false)
-                       if err != nil {
-                               base.Fatalf("go: %v", err)
-                       }
-                       break
-               }
-       }
-
        return rs
 }
 
index ffcea186035574da7373494a69c3dc3ce050298c..9abbabd2ebe47345e3e1cbe9067a9e98fcf76b7f 100644 (file)
@@ -2,18 +2,13 @@
 # 'go mod tidy' should not panic if the main module initially
 # requires an older version of itself.
 
+# A module may require an older version of itself without error. This is
+# inconsistent (the required version is never selected), but we still get
+# a reproducible build list.
+go list -m all
+stdout '^golang.org/issue/46078$'
 
-# A module that explicitly requires an older version of itself should be
-# rejected as inconsistent: we enforce that every explicit requirement is the
-# selected version of its module path, but the selected version of the main
-# module is always itself — not some explicit version.
-
-! go list -m all
-stderr '^go: updates to go\.mod needed; to update it:\n\tgo mod tidy$'
-
-
-# The suggested 'go mod tidy' command should succeed (not crash).
-
+# 'go mod tidy' should fix this (and not crash).
 go mod tidy
 
 
diff --git a/src/cmd/go/testdata/script/mod_vendor_redundant_requirement.txt b/src/cmd/go/testdata/script/mod_vendor_redundant_requirement.txt
new file mode 100644 (file)
index 0000000..3f6f5c5
--- /dev/null
@@ -0,0 +1,29 @@
+# 'go list -mod=vendor' should succeed even when go.mod contains redundant
+# requirements. Verifies #47565.
+go list -mod=vendor
+
+-- go.mod --
+module m
+
+go 1.17
+
+require example.com/m v0.0.0
+require example.com/m v0.0.0
+
+replace example.com/m v0.0.0 => ./m
+-- m/go.mod --
+module example.com/m
+
+go 1.17
+-- m/m.go --
+package m
+-- use.go --
+package use
+
+import _ "example.com/m"
+-- vendor/example.com/m/m.go --
+package m
+-- vendor/modules.txt --
+# example.com/m v0.0.0 => ./m
+## explicit; go 1.17
+example.com/m