]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: make 'go get' match patterns against packages, not modules
authorJay Conrod <jayconrod@google.com>
Thu, 9 May 2019 19:42:55 +0000 (15:42 -0400)
committerJay Conrod <jayconrod@google.com>
Mon, 13 May 2019 19:29:47 +0000 (19:29 +0000)
This is a follow-up to CL 174099, fixing an important TODO.
The 'go help modget' documentation will be clarified in anotehr CL,
pending further discussion.

When invoked without -m, 'go get' will no longer match arguments
containing "..." against module paths. If a module's path matches a
pattern but no packages within the module match the pattern, the
module should not be upgraded. For example, if
golang.org/x/tools/playground and golang.org/x/tools are separate
modules, and only golang.org/x/tools is in the build list,
'go get golang.org/x/tools/playground/...' should add
golang.org/x/tools/playground to the build list and leave
golang.org/x/tools alone.

Updates #26902

Change-Id: I2bd18c7950db1aa7bd8527210c1baf2c7d174375
Reviewed-on: https://go-review.googlesource.com/c/go/+/176578
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
src/cmd/go/internal/modget/get.go
src/cmd/go/testdata/mod/example.com_nest_sub_v1.0.0.txt [new file with mode: 0644]
src/cmd/go/testdata/mod/example.com_nest_v1.0.0.txt [new file with mode: 0644]
src/cmd/go/testdata/mod/example.com_nest_v1.1.0.txt [new file with mode: 0644]
src/cmd/go/testdata/script/mod_get_main.txt
src/cmd/go/testdata/script/mod_get_patterns.txt

index 62519fd5faf418f7c1dd2ed8558a9c0119ede485..bf87c4a0d103c9f7e9556e8f593066e1940a1727 100644 (file)
@@ -336,37 +336,32 @@ func runGet(cmd *base.Command, args []string) {
                        }
 
                case strings.Contains(path, "..."):
-                       // Find modules in the build list matching the pattern, if any.
-                       match := search.MatchPattern(path)
-                       matched := false
-                       for _, m := range modload.BuildList() {
-                               // TODO: If we have matching packages already in the build list and we
-                               // know which module(s) they are in, then we should not upgrade the
-                               // modules that do *not* contain those packages, even if the module path
-                               // is a prefix of the pattern.
-                               //
-                               // For example, if we have modules golang.org/x/tools and
-                               // golang.org/x/tools/playground, and all of the packages matching
-                               // golang.org/x/tools/playground... are in the
-                               // golang.org/x/tools/playground module, then we should not *also* try
-                               // to upgrade golang.org/x/tools if the user says 'go get
-                               // golang.org/x/tools/playground...@latest'.
-                               if match(m.Path) || str.HasPathPrefix(path, m.Path) {
-                                       queries = append(queries, &query{querySpec: querySpec{path: m.Path, vers: vers, prevM: m, forceModulePath: true}, arg: arg})
-                                       matched = true
+                       // If we're using -m, look up modules in the build list that match
+                       // the pattern. Report an error if no modules match.
+                       if *getM {
+                               match := search.MatchPattern(path)
+                               matched := false
+                               for _, m := range modload.BuildList() {
+                                       if match(m.Path) || str.HasPathPrefix(path, m.Path) {
+                                               queries = append(queries, &query{querySpec: querySpec{path: m.Path, vers: vers, prevM: m, forceModulePath: true}, arg: arg})
+                                               matched = true
+                                       }
+                               }
+                               if !matched {
+                                       base.Errorf("go get %s: pattern matches no modules in build list", arg)
+                                       continue
                                }
-                       }
-                       // If matched, we're done.
-                       // If we're using -m, report an error.
-                       // Otherwise, look up a module containing packages that match the pattern.
-                       if matched {
                                break
                        }
-                       if *getM {
-                               base.Errorf("go get %s: pattern matches no modules in build list", arg)
-                               continue
-                       }
-                       queries = append(queries, &query{querySpec: querySpec{path: path, vers: vers}, arg: arg})
+
+                       // If we're not using -m, wait until we load packages to look up modules.
+                       // We don't know yet whether any modules in the build list provide
+                       // packages matching the pattern. For example, suppose
+                       // golang.org/x/tools and golang.org/x/tools/playground are separate
+                       // modules, and only golang.org/x/tools is in the build list. If the
+                       // user runs 'go get golang.org/x/tools/playground/...', we should
+                       // add a requirement for golang.org/x/tools/playground. We should not
+                       // upgrade golang.org/x/tools.
 
                case path == "all":
                        // This is the package pattern "all" not the module pattern "all",
@@ -463,8 +458,16 @@ func runGet(cmd *base.Command, args []string) {
        var matches []*search.Match
        var install []string
        for {
-               var queries []*query
                var seenPkgs map[string]bool
+               seenQuery := make(map[querySpec]bool)
+               var queries []*query
+               addQuery := func(q *query) {
+                       if !seenQuery[q.querySpec] {
+                               seenQuery[q.querySpec] = true
+                               queries = append(queries, q)
+                       }
+               }
+
                if len(pkgPatterns) > 0 {
                        // Don't load packages if pkgPatterns is empty. Both
                        // modload.ImportPathsQuiet and ModulePackages convert an empty list
@@ -474,16 +477,21 @@ func runGet(cmd *base.Command, args []string) {
                        } else {
                                matches = modload.ImportPathsQuiet(pkgPatterns)
                        }
-                       seenQuery := make(map[querySpec]bool)
                        seenPkgs = make(map[string]bool)
                        install = make([]string, 0, len(pkgPatterns))
                        for i, match := range matches {
-                               if len(match.Pkgs) == 0 {
-                                       // We'll print a warning at the end of the outer loop to avoid
-                                       // repeating warnings on multiple iterations.
+                               arg := pkgGets[i]
+
+                               if !*getM && len(match.Pkgs) == 0 {
+                                       // If the pattern did not match any packages, look up a new module.
+                                       // If the pattern doesn't match anything on the last iteration,
+                                       // we'll print a warning after the outer loop.
+                                       if !search.IsRelativePath(arg.path) && !match.Literal && arg.path != "all" {
+                                               addQuery(&query{querySpec: querySpec{path: arg.path, vers: arg.vers}, arg: arg.raw})
+                                       }
                                        continue
                                }
-                               arg := pkgGets[i]
+
                                install = append(install, arg.path)
                                allStd := true
                                for _, pkg := range match.Pkgs {
@@ -501,11 +509,7 @@ func runGet(cmd *base.Command, args []string) {
                                                continue
                                        }
                                        allStd = false
-                                       spec := querySpec{path: m.Path, vers: arg.vers}
-                                       if !seenQuery[spec] {
-                                               seenQuery[spec] = true
-                                               queries = append(queries, &query{querySpec: querySpec{path: m.Path, vers: arg.vers, forceModulePath: true, prevM: m}, arg: arg.raw})
-                                       }
+                                       addQuery(&query{querySpec: querySpec{path: m.Path, vers: arg.vers, forceModulePath: true, prevM: m}, arg: arg.raw})
                                }
                                if allStd {
                                        if *getM {
diff --git a/src/cmd/go/testdata/mod/example.com_nest_sub_v1.0.0.txt b/src/cmd/go/testdata/mod/example.com_nest_sub_v1.0.0.txt
new file mode 100644 (file)
index 0000000..90f1459
--- /dev/null
@@ -0,0 +1,12 @@
+Written by hand.
+Test case for nested modules without an explicit relationship.
+This is nested below the top-level module.
+
+-- .mod --
+module example.com/nest/sub
+-- .info --
+{"Version": "v1.0.0"}
+-- go.mod --
+module example.com/nest/sub
+-- y/y.go --
+package y
diff --git a/src/cmd/go/testdata/mod/example.com_nest_v1.0.0.txt b/src/cmd/go/testdata/mod/example.com_nest_v1.0.0.txt
new file mode 100644 (file)
index 0000000..593caf1
--- /dev/null
@@ -0,0 +1,12 @@
+Written by hand.
+Test case for nested modules without an explicit relationship.
+This is the top-level module.
+
+-- .mod --
+module example.com/nest
+-- .info --
+{"Version": "v1.0.0"}
+-- go.mod --
+module example.com/nest
+-- sub/x/x.go --
+package x
diff --git a/src/cmd/go/testdata/mod/example.com_nest_v1.1.0.txt b/src/cmd/go/testdata/mod/example.com_nest_v1.1.0.txt
new file mode 100644 (file)
index 0000000..5a01550
--- /dev/null
@@ -0,0 +1,12 @@
+Written by hand.
+Test case for nested modules without an explicit relationship.
+This is the top-level module.
+
+-- .mod --
+module example.com/nest
+-- .info --
+{"Version": "v1.1.0"}
+-- go.mod --
+module example.com/nest
+-- sub/x/x.go --
+package x
index 0acb7179646417dfec08a816b50db30f1c0da141..06f9f238777eaead008bb410304e14241eaa91fd 100644 (file)
@@ -16,9 +16,10 @@ stderr '^go get rsc.io@v0.1.0: can.t get a specific version of the main module$'
 ! go get -d rsc.io/x@v0.1.0
 stderr '^go get rsc.io/x@v0.1.0: can.t query specific version for package rsc.io/x in the main module \(rsc.io\)$'
 
-# TODO: upgrading a package pattern not contained in the main module should not
+# Upgrading a package pattern not contained in the main module should not
 # attempt to upgrade the main module.
-! go get rsc.io/quote/...@v1.5.1
+go get rsc.io/quote/...@v1.5.1
+grep 'rsc.io/quote v1.5.1' go.mod
 
 -- go.mod.orig --
 module rsc.io
index 1642ff2d107af10b28b0b536b4cb5653c757b527..733d4452d7af00800265fd3fc7f1d09791c65519 100644 (file)
@@ -22,6 +22,14 @@ stderr 'go get rsc.io/quote/x...: module rsc.io/quote@latest \(v1.5.2\) found, b
 stderr 'go get rsc.io/quote/x/...: module rsc.io/quote@latest \(v1.5.2\) found, but does not contain packages matching rsc.io/quote/x/...'
 ! grep 'require rsc.io/quote' go.mod
 
+# If a pattern matches no packages within a module, the module should not
+# be upgraded, even if the module path matches the pattern.
+cp go.mod.orig go.mod
+go mod edit -require example.com/nest@v1.0.0
+go get example.com/nest/sub/y...
+grep 'example.com/nest/sub v1.0.0' go.mod
+grep 'example.com/nest v1.0.0' go.mod
+
 -- go.mod.orig --
 module m