[go] cmd/go: do not rely on Import to find the module for a new package
Bryan C. Mills (Gerrit)
Bryan C. Mills has uploaded this change for review.
cmd/go: do not rely on Import to find the module for a new package
TODO: Further detail.
Fixes #26602.
DO NOT SUBMIT: 'go list' error-checking is currently broken.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/import.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/internal/modload/query.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
7 files changed, 88 insertions(+), 101 deletions(-)
diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index 666b53d..d19cc32 100644
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -32,13 +32,13 @@
ModInit func()
// module hooks; nil if module use is disabled
- ModBinDir func() string // return effective bin directory
- ModLookup func(parentPath, path string) (dir, realPath string, err error) // lookup effective meaning of import
- ModPackageModuleInfo func(path string) *modinfo.ModulePublic // return module info for Package struct
- ModImportPaths func(args []string) []string // expand import paths
- ModPackageBuildInfo func(main string, deps []string) string // return module info to embed in binary
- ModInfoProg func(info string) []byte // wrap module info in .go code for binary
- ModImportFromFiles func([]string) // update go.mod to add modules for imports in these files
+ ModBinDir func() string // return effective bin directory
+ ModLookup func(path string) (dir, realPath string, err error) // lookup effective meaning of import
+ ModPackageModuleInfo func(path string) *modinfo.ModulePublic // return module info for Package struct
+ ModImportPaths func(args []string) []string // expand import paths
+ ModPackageBuildInfo func(main string, deps []string) string // return module info to embed in binary
+ ModInfoProg func(info string) []byte // wrap module info in .go code for binary
+ ModImportFromFiles func([]string) // update go.mod to add modules for imports in these files
)
var IgnoreImports bool // control whether we ignore imports in packages
@@ -488,7 +488,7 @@
importPath = dirToImportPath(filepath.Join(srcDir, path))
} else if cfg.ModulesEnabled {
var p string
- modDir, p, modErr = ModLookup(parentPath, path)
+ modDir, p, modErr = ModLookup(path)
if modErr == nil {
importPath = p
}
@@ -628,11 +628,7 @@
// Go 1.11 module legacy conversion (golang.org/issue/25069).
func ResolveImportPath(parent *Package, path string) (found string) {
if cfg.ModulesEnabled {
- parentPath := ""
- if parent != nil {
- parentPath = parent.ImportPath
- }
- if _, p, e := ModLookup(parentPath, path); e == nil {
+ if _, p, e := ModLookup(path); e == nil {
return p
}
return path
diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index e8b0857..be75f76 100644
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -513,10 +513,11 @@
}
if len(install) > 0 {
+ // XXX: This is always true because search.CleanImportPaths returns "." for len(args)==0.
work.BuildInit()
var pkgs []string
for _, p := range load.PackagesAndErrors(install) {
- if p.Error == nil || !strings.HasPrefix(p.Error.Err, "no Go files") {
+ if p.Error == nil {
pkgs = append(pkgs, p.ImportPath)
}
}
@@ -557,22 +558,12 @@
return module.Version{}, err
}
- // Otherwise, interpret the package path as an import
- // and determine what module that import would address
- // if found in the current source code.
- // Then apply the version to that module.
- m, _, err := modload.Import(path)
+ // Otherwise, try a package path.
+ m, _, err := modload.QueryPackage(path, vers, modload.Allowed)
if err != nil {
- return module.Version{}, err
+ return module.Version{}, fmt.Errorf("%s is not a known package or module", path)
}
- if m.Path == "" {
- return module.Version{}, fmt.Errorf("package %q is not in a module", path)
- }
- info, err = modload.Query(m.Path, vers, modload.Allowed)
- if err != nil {
- return module.Version{}, err
- }
- return module.Version{Path: m.Path, Version: info.Version}, nil
+ return m, nil
}
// isModulePath reports whether path names an actual module,
diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go
index f0e7d86..71705a5 100644
--- a/src/cmd/go/internal/modload/import.go
+++ b/src/cmd/go/internal/modload/import.go
@@ -10,7 +10,6 @@
"fmt"
"go/build"
"os"
- pathpkg "path"
"path/filepath"
"strings"
@@ -42,7 +41,9 @@
// If the package cannot be found in the current build list,
// Import returns an ImportMissingError as the error.
// If Import can identify a module that could be added to supply the package,
-// the ImportMissingErr records that module.
+// the ImportMissingError records that module.
+// If the package does not exist but the path is a valid module path,
+// Import returns that module but the ImportMissingError does not include it.
func Import(path string) (m module.Version, dir string, err error) {
if strings.Contains(path, "@") {
return module.Version{}, "", fmt.Errorf("import path should not have @version")
@@ -124,24 +125,6 @@
return module.Version{}, "", errors.New(buf.String())
}
- // Special case: if the path matches a module path,
- // and we haven't found code in any module on the build list
- // (since we haven't returned yet),
- // force the use of the current module instead of
- // looking for an alternate one.
- // This helps "go get golang.org/x/net" even though
- // there is no code in x/net.
- for _, m := range buildList {
- if m.Path == path {
- root, isLocal, err := fetch(m)
- if err != nil {
- return module.Version{}, "", err
- }
- dir, _ := dirInModule(path, m.Path, root, isLocal)
- return m, dir, nil
- }
- }
-
// Not on build list.
// Look up module containing the package, for addition to the build list.
@@ -150,43 +133,11 @@
return module.Version{}, "", fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod)
}
- for p := path; p != "."; p = pathpkg.Dir(p) {
- // We can't upgrade the main module.
- // Note that this loop does consider upgrading other modules on the build list.
- // If that's too aggressive we can skip all paths already on the build list,
- // not just Target.Path, but for now let's try being aggressive.
- if p == Target.Path {
- // Can't move to a new version of main module.
- continue
- }
-
- info, err := Query(p, "latest", Allowed)
- if err != nil {
- continue
- }
- m := module.Version{Path: p, Version: info.Version}
- root, isLocal, err := fetch(m)
- if err != nil {
- continue
- }
- _, ok := dirInModule(path, m.Path, root, isLocal)
- if ok {
- return module.Version{}, "", &ImportMissingError{ImportPath: path, Module: m}
- }
-
- // Special case matching the one above:
- // if m.Path matches path, assume adding it to the build list
- // will either add the right code or the right code doesn't exist.
- if m.Path == path {
- return module.Version{}, "", &ImportMissingError{ImportPath: path, Module: m}
- }
+ m, _, err = QueryPackage(path, "latest", Allowed)
+ if err != nil {
+ return m, "", &ImportMissingError{ImportPath: path}
}
-
- // Did not resolve import to any module.
- // TODO(rsc): It would be nice to return a specific error encountered
- // during the loop above if possible, but it's not clear how to pick
- // out the right one.
- return module.Version{}, "", &ImportMissingError{ImportPath: path}
+ return m, "", &ImportMissingError{ImportPath: path, Module: m}
}
// maybeInModule reports whether, syntactically,
diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go
index 9c55044..b5292a7 100644
--- a/src/cmd/go/internal/modload/load.go
+++ b/src/cmd/go/internal/modload/load.go
@@ -93,6 +93,7 @@
} else if path := pathInModuleCache(dir); path != "" {
pkg = path
} else {
+ // No such package. Leave the error for Import.
base.Errorf("go: directory %s outside available modules", base.ShortPath(dir))
continue
}
@@ -337,10 +338,13 @@
return loaded.direct[path]
}
-// Lookup XXX TODO.
-func Lookup(parentPath, path string) (dir, realPath string, err error) {
- realPath = ImportMap(path)
- if realPath == "" {
+// Lookup returns the source directory, import path, and any loading error for
+// the package at path.
+// Lookup requires that one of the Load functions in this package has already
+// been called.
+func Lookup(path string) (dir, realPath string, err error) {
+ pkg, ok := loaded.pkgCache.Get(path).(*loadPkg)
+ if !ok {
if isStandardImportPath(path) {
dir := filepath.Join(cfg.GOROOT, "src", path)
if _, err := os.Stat(dir); err == nil {
@@ -349,7 +353,7 @@
}
return "", "", fmt.Errorf("no such package in module")
}
- return PackageDir(realPath), realPath, nil
+ return pkg.dir, pkg.path, pkg.err
}
// A loader manages the process of loading information about
@@ -457,11 +461,7 @@
}
continue
}
- if pkg.err != nil {
- base.Errorf("go: %s: %s", pkg.stackText(), pkg.err)
- }
}
- base.ExitIfErrors()
if numAdded == 0 {
break
}
@@ -473,7 +473,6 @@
base.Fatalf("go: %v", err)
}
}
- base.ExitIfErrors()
// Compute directly referenced dependency modules.
ld.direct = make(map[string]bool)
@@ -504,9 +503,6 @@
}
}
}
-
- // Check for visibility violations.
- // TODO!
}
// pkg returns the *loadPkg for path, creating and queuing it if needed.
diff --git a/src/cmd/go/internal/modload/query.go b/src/cmd/go/internal/modload/query.go
index c69e49a..662e9cd 100644
--- a/src/cmd/go/internal/modload/query.go
+++ b/src/cmd/go/internal/modload/query.go
@@ -9,6 +9,7 @@
"cmd/go/internal/module"
"cmd/go/internal/semver"
"fmt"
+ pathpkg "path"
"strings"
)
@@ -187,3 +188,53 @@
func matchSemverPrefix(p, v string) bool {
return len(v) > len(p) && v[len(p)] == '.' && v[:len(p)] == p
}
+
+// QueryPackage looks up a revision of a module containing path.
+//
+// If multiple matching modules provide the requested package, QueryPackage
+// picks the one with the longest module path.
+//
+// If no module provides the given package but some module exists with a prefix
+// of path and a matching version, QueryPackage returns the info for that module
+// along with a non-nil error.
+func QueryPackage(path, query string, allowed func(module.Version) bool) (module.Version, *modfetch.RevInfo, error) {
+ var (
+ longest module.Version
+ longestInfo *modfetch.RevInfo
+ )
+ for p := path; p != "."; p = pathpkg.Dir(p) {
+ // We can't upgrade the main module.
+ // Note that this loop does consider upgrading other modules on the build list.
+ // If that's too aggressive we can skip all paths already on the build list,
+ // not just Target.Path, but for now let's try being aggressive.
+ if p == Target.Path {
+ if _, ok := dirInModule(path, Target.Path, ModRoot, true); ok {
+ if query == "latest" {
+ return Target, nil, nil
+ } else {
+ return module.Version{}, nil, fmt.Errorf("can't query for version %v of a package in the current module", query)
+ }
+ }
+ }
+
+ info, err := Query(p, query, allowed)
+ if err != nil {
+ continue
+ }
+ m := module.Version{Path: p, Version: info.Version}
+ root, isLocal, err := fetch(m)
+ if err != nil {
+ return module.Version{}, nil, err
+ }
+ _, ok := dirInModule(path, m.Path, root, isLocal)
+ if ok {
+ return m, info, nil
+ }
+ if longest.Path == "" {
+ longest = m
+ longestInfo = info
+ }
+ }
+
+ return longest, longestInfo, fmt.Errorf("no modules providing package %s for query %q", path, query)
+}
diff --git a/src/cmd/go/testdata/script/mod_bad_domain.txt b/src/cmd/go/testdata/script/mod_bad_domain.txt
index 236564e..ae1ad27 100644
--- a/src/cmd/go/testdata/script/mod_bad_domain.txt
+++ b/src/cmd/go/testdata/script/mod_bad_domain.txt
@@ -2,14 +2,14 @@
# explicit get should report errors about bad names
! go get appengine
-stderr 'cannot find module providing package appengine'
+stderr 'appengine is not a known package or module'
! go get x/y.z
-stderr 'cannot find module providing package x/y.z'
+stderr 'x/y.z is not a known package or module'
# build should skip over appengine imports
! go build
! stderr appengine
-stderr 'cannot find module providing package nonexistent.rsc.io'
+stderr 'nonexistent.rsc.io is not a known package or module'
-- go.mod --
module x
diff --git a/src/cmd/go/testdata/script/mod_get_indirect.txt b/src/cmd/go/testdata/script/mod_get_indirect.txt
index 8388ed1..b3acf73 100644
--- a/src/cmd/go/testdata/script/mod_get_indirect.txt
+++ b/src/cmd/go/testdata/script/mod_get_indirect.txt
@@ -11,7 +11,9 @@
stdout '^golang.org/x/text [v0-9a-f\.-]+ // indirect'
grep 'golang.org/x/text [v0-9a-f\.-]+ // indirect' go.mod
-# indirect tag should be removed upon seeing direct import
+# indirect tag should be removed upon seeing direct import,
+# even if the module doesn't actually provide source code for
+# the package itself.
cp $WORK/tmp/usetext.go x.go
go list
grep 'rsc.io/quote v1.5.2$' go.mod
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #2 to this change.
cmd/go: do not rely on Import to find the module for a new package
TODO: Further detail.
Fixes #26602.
DO NOT SUBMIT: 'go list' error-checking is currently broken.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/import.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/internal/modload/query.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
8 files changed, 94 insertions(+), 107 deletions(-)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Uploaded patch set 2: New patch set was added with same tree, parent, and commit message as Patch Set 1.
Bryan C. Mills (Gerrit)
5 comments:
File src/cmd/go/internal/modget/get.go:
Patch Set #2, Line 520:
if p.Error == nil {If we've got the right list of packages, the only errors here should be “no such package” errors for packages at the root of explicitly-requested modules, or for the implicitly-requested current directory.
File src/cmd/go/internal/modload/import.go:
Should validate that m.Path == path, or move the “tolerate valid modules” logic up a level and not return a module at all in case of error.
File src/cmd/go/internal/modload/load.go:
Not clear whether we need this call. Should probably leave it in.
Patch Set #2, Line 96:
// No such package. Leave the error for Import.Need to omit the Errorf call, I think.
File src/cmd/go/testdata/script/mod_get_indirect.txt:
Patch Set #2, Line 18:
go listThis command should probably require a `-e` flag: the code imports golang.org/x/text, but that isn't an actual package.
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Russ Cox (Gerrit)
I'm pretty uncomfortable with the amount of change in this CL. It's hard to believe this is the minimal fix necessary.
4 comments:
File src/cmd/go/internal/load/pkg.go:
Patch Set #2, Line 36:
ModLookup func(path string) (dir, realPath string, err error) // lookup effective meaning of importProbably this change should be in a separate CL. Seems unrelated.
File src/cmd/go/internal/modget/get.go:
Patch Set #2, Line 516:
// XXX: This is always true because search.CleanImportPaths returns "." for len(args)==0.If you run 'go get x@none' then len(install) == 0.
Patch Set #2, Line 520:
if p.Error == nil {
If we've got the right list of packages, the only errors here should be “no such package” errors for […]
What if we haven't got the right list of packages?
It seems very error-prone to assume that we want to discard all errors here for all future possible changes to the go command. There's one specific error we want to ignore - no Go files in the directory named by the import path. Any other error should certainly be reported (by keeping the import path in pkgs and then having work.InstallPackages do the reporting).
Patch Set #2, Line 561:
// Otherwise, try a package path.Doesn't this break:
go get golang.org/x/text/language@master
go get golang.org/x/tools/cmd/goimports@masteretc?
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #3 to this change.
cmd/go: do not rely on Import to find the module for a new package
TODO: Further detail.
Fixes #26602.
DO NOT SUBMIT: 'go list' error-checking is currently broken.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/list/list.go
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/import.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/internal/modload/query.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
9 files changed, 87 insertions(+), 105 deletions(-)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Uploaded patch set 3.
(6 comments)
6 comments:
Patch Set #2, Line 36:
ModLookup func(path string) (dir, realPath string, err error) // lookup effective meaning of importProbably this change should be in a separate CL. Seems unrelated.
Done
File src/cmd/go/internal/modget/get.go:
Patch Set #2, Line 516:
// All requested versions were explicitly @none.
If you run 'go get x@none' then len(install) == 0.
Thanks.
Patch Set #2, Line 520:
var pkgs []stringWhat if we haven't got the right list of packages? […]
Make the check more precise (and documented the rationale).
File src/cmd/go/internal/modload/import.go:
Should validate that m. […]
Done (moved up a level).
File src/cmd/go/internal/modload/load.go:
Not clear whether we need this call. Should probably leave it in.
Done
Patch Set #2, Line 96:
base.Errorf("go: directory %s outside available modules", base.ShortPath(dir))
Need to omit the Errorf call, I think.
Import won't necessarily be called: it's fine to report an error here because it's always from an explicit command-line argument.
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #4 to this change.
cmd/go: fix 'go get' for packages with modules whose roots are non-packages
Do not allow module roots that do not contain packages to be imported as
packages.
Report loading errors in modload.Lookup.
TODO: Split into more targeted changes.
Fixes #26602.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/import.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/internal/modload/query.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
M src/cmd/go/testdata/script/mod_list_bad_import.txt
M src/cmd/go/testdata/script/mod_readonly.txt
10 files changed, 93 insertions(+), 105 deletions(-)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Uploaded patch set 4.
Bryan C. Mills (Gerrit)
Patch Set 2:
(4 comments)
I'm pretty uncomfortable with the amount of change in this CL. It's hard to believe this is the minimal fix necessary.
I agree. Now that I've got it passing tests, I'm going to try pulling out smaller changes to find a minimal set for 1.11.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #5 to this change.
cmd/go/internal/modload: report errors explicitly from Lookup
Previously, we reported errors directly in (*loader).load via base.Errorf.
Unfortunately, (*loader).load can be called from contexts in which such errors
should not be considered fatal, such as by load.PackagesAndErrors.
Instead, we save the errors in pkg.err and modify Lookup to return that error.
This change is a bit awkward: we end up suppressing a "no Go files" error for
packages at the root of newly-imported modules, even if they really do contain
source files. I believe that that's due to a special-case lookup for modules in
the build list, which allows us to "validate" imports for modules in the build
list even though we haven't actually downloaded their sources (or verified that
they actually contain the requested package). The fix for that issue is in the
change that follows this one.
Updates #26602.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
A src/cmd/go/testdata/script/mod_list_bad_import.txt
M src/cmd/go/testdata/script/mod_readonly.txt
7 files changed, 100 insertions(+), 27 deletions(-)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Uploaded patch set 5: Run-TryBot+1.
Gobot Gobot (Gerrit)
Bryan C. Mills (Gerrit)
2 comments:
File src/cmd/go/internal/modget/get.go:
Doesn't this break: […]
That's exactly the case this stack of changes is intended to fix.
File src/cmd/go/testdata/script/mod_get_indirect.txt:
Patch Set #2, Line 18:
grep 'golang.org/x/text [v0-9a-f\.-]+' go.modThis command should probably require a `-e` flag: the code imports golang. […]
This matches 'go list' in GOPATH mode. See https://golang.org/cl/128016.
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Patch Set 4:
Patch Set 2:
(4 comments)
I'm pretty uncomfortable with the amount of change in this CL. It's hard to believe this is the minimal fix necessary.
I agree. Now that I've got it passing tests, I'm going to try pulling out smaller changes to find a minimal set for 1.11.
I was unable to find a smaller set of changes that can successfully 'go get' subpackages. However, I did find a reasonable midpoint at which to split the change.
Bryan C. Mills (Gerrit)
Uploaded patch set 6: Patch Set 5 was rebased.
Bryan C. Mills (Gerrit)
Uploaded patch set 7: Run-TryBot+1.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #7 to this change.
cmd/go/internal/modload: report errors explicitly from Lookup
Previously, we reported errors directly in (*loader).load via base.Errorf.
Unfortunately, (*loader).load can be called from contexts in which such errors
should not be considered fatal, such as by load.PackagesAndErrors.
Instead, we save the errors in pkg.err and modify Lookup to return that error.
This change is a bit awkward: we end up suppressing a "no Go files" error for
packages at the root of newly-imported modules, even if they really do contain
source files. I believe that that's due to a special-case lookup for modules in
the build list, which allows us to "validate" imports for modules in the build
list even though we haven't actually downloaded their sources (or verified that
they actually contain the requested package). The fix for that issue is in the
change that follows this one.
Updates #26602.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
A src/cmd/go/testdata/script/mod_list_bad_import.txt
M src/cmd/go/testdata/script/mod_readonly.txt
7 files changed, 100 insertions(+), 27 deletions(-)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 7:TryBot-Result +1
Bryan C. Mills (Gerrit)
Uploaded patch set 8: Patch Set 7 was rebased.
Russ Cox (Gerrit)
3 comments:
File src/cmd/go/testdata/script/mod_bad_domain.txt:
Patch Set #8, Line 12:
stderr '"appengine": no such package in active modules'You're changing the semantics here. The build is intended to skip over appengine and now it's not. I think that's a mistake. It will break vendoring.
File src/cmd/go/testdata/script/mod_list_bad_import.txt:
Patch Set #8, Line 1:
env GO111MODULE=onThere should be some comments here explaining what you want to test.
File src/cmd/go/testdata/script/mod_readonly.txt:
Patch Set #8, Line 7:
! go list all"go list all" is a lot more expensive than "go list". Why do all?
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #9 to this change.
cmd/go/internal/modload: report errors explicitly from Lookup
Previously, we reported errors directly in (*loader).load via base.Errorf.
Unfortunately, (*loader).load can be called from contexts in which such errors
should not be considered fatal, such as by load.PackagesAndErrors.
Instead, we save the errors in pkg.err and modify Lookup to return that error.
This change is a bit awkward: we end up suppressing a "no Go files" error for
packages at the root of newly-imported modules, even if they really do contain
source files. I believe that that's due to a special-case lookup for modules in
the build list, which allows us to "validate" imports for modules in the build
list even though we haven't actually downloaded their sources (or verified that
they actually contain the requested package). The fix for that issue is in the
change that follows this one.
Updates #26602.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
M src/cmd/go/testdata/script/mod_readonly.txt
M src/cmd/go/testdata/script/mod_vendor.txt
7 files changed, 70 insertions(+), 32 deletions(-)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Uploaded patch set 9: Run-TryBot+1.
(3 comments)
3 comments:
File src/cmd/go/testdata/script/mod_bad_domain.txt:
Patch Set #8, Line 12:
stderr 'cannot find package'
You're changing the semantics here. The build is intended to skip over appengine and now it's not.
The build should fail either way, and given that it's going to fail, it's arguably better to list all of the package errors rather than just the ones that happen to have valid domain prefixes.
> I think that's a mistake. It will break vendoring.
There is an explicit test for that, and it does not break:
https://github.com/golang/go/blob/9ef5ee911c2265cde86887032fc56ce4c335d580/src/cmd/go/testdata/script/mod_vendor.txt#L151-L157From what I understand, I'm actually breaking vendoring in the direction of being more permissive: the callers of LoadALL and LoadVendor don't actually end up checking pkg.err, so because (*loader).load no longer reports the errors directly, they ignore all unresolved imports — not just the ones that happen not to have valid module paths.
I think that's a good thing, so I've added a new input file to mod_vendor to lock it in.
File src/cmd/go/testdata/script/mod_list_bad_import.txt:
Patch Set #8, Line 1:
env GO111MODULE=onThere should be some comments here explaining what you want to test.
Moved this file to https://golang.org/cl/128016 and added comments.
File src/cmd/go/testdata/script/mod_readonly.txt:
Patch Set #8, Line 7:
env GOFLAGS=-mod=readonly"go list all" is a lot more expensive than "go list". […]
`go list` today doesn't fail due to missing imports (see https://golang.org/cl/128016), and we use the same code to drive `list` in GOPATH mode as in module mode.
Added a TODO.
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Gobot Gobot (Gerrit)
Build is still in progress...
This change failed on windows-386-2008:
See https://storage.googleapis.com/go-build-log/f6b0b8df/windows-386-2008_df94d216.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
Gobot Gobot (Gerrit)
2 of 19 TryBots failed:
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/f6b0b8df/windows-386-2008_df94d216.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/f6b0b8df/windows-amd64-2016_5b3037f0.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 9:TryBot-Result -1
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #10 to this change.
cmd/go/internal/modload: report errors explicitly from Lookup
Previously, we reported errors directly in (*loader).load via base.Errorf.
Unfortunately, (*loader).load can be called from contexts in which such errors
should not be considered fatal, such as by load.PackagesAndErrors.
Instead, we save the errors in pkg.err and modify Lookup to return that error.
This change is a bit awkward: we end up suppressing a "no Go files" error for
packages at the root of newly-imported modules, even if they really do contain
source files. I believe that that's due to a special-case lookup for modules in
the build list, which allows us to "validate" imports for modules in the build
list even though we haven't actually downloaded their sources (or verified that
they actually contain the requested package). The fix for that issue is in the
change that follows this one.
Updates #26602.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
M src/cmd/go/testdata/script/mod_list_bad_import.txt
M src/cmd/go/testdata/script/mod_readonly.txt
M src/cmd/go/testdata/script/mod_vendor.txt
8 files changed, 99 insertions(+), 50 deletions(-)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Uploaded patch set 10.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=a366a3f4
Bryan C. Mills (Gerrit)
I moved mod_list_bad_import to an earlier change to emphasize the differences: as you can see here, we end up swapping one set of bugs for another, but at least the bugs are now the same between GOPATH mode and module mode.
Gobot Gobot (Gerrit)
Build is still in progress...
This change failed on windows-386-2008:
See https://storage.googleapis.com/go-build-log/a366a3f4/windows-386-2008_57509fc5.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
Gobot Gobot (Gerrit)
2 of 19 TryBots failed:
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/a366a3f4/windows-386-2008_57509fc5.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/a366a3f4/windows-amd64-2016_8b655668.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 10:TryBot-Result -1
Bryan C. Mills (Gerrit)
Uploaded patch set 11.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #11 to this change.
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=99cfc115
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 11:TryBot-Result +1
Bryan C. Mills (Gerrit)
Uploaded patch set 12: Patch Set 11 was rebased.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=9cd0886f
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 12:TryBot-Result +1
Bryan C. Mills (Gerrit)
Uploaded patch set 13: Commit message was updated.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #13 to this change.
cmd/go/internal/modload: report errors explicitly from Lookup
Previously, we reported errors directly in (*loader).load via base.Errorf.
Unfortunately, (*loader).load can be called from contexts in which such errors
should not be considered fatal, such as by load.PackagesAndErrors.
Instead, we save the errors in pkg.err and modify Lookup to return that error.
This change is a bit awkward: we end up suppressing a "no Go files" error for
packages at the root of newly-imported modules, even if they really do contain
source files. I believe that that's due to a special-case lookup for modules in
the build list, which allows us to "validate" imports for modules in the build
list even though we haven't actually downloaded their sources (or verified that
they actually contain the requested package). The fix for that issue is in the
change that follows this one.
Fixes #26602.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
M src/cmd/go/testdata/script/mod_list_bad_import.txt
M src/cmd/go/testdata/script/mod_readonly.txt
M src/cmd/go/testdata/script/mod_vendor.txt
8 files changed, 99 insertions(+), 50 deletions(-)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
I did some more investigating, and it turns out that this change in isolation fixes at least the most egregious cases from #26602.
I think it would be fine to take this change for 1.11 and save the more invasive CL 128136 for 1.12.
Patch set 13:Run-TryBot +1
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=dacdb04c
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 13:TryBot-Result +1
Bryan C. Mills (Gerrit)
Uploaded patch set 14: Patch Set 13 was rebased.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=32305a38
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 14:TryBot-Result +1
Bryan C. Mills (Gerrit)
Uploaded patch set 15: Patch Set 14 was rebased.
Bryan C. Mills (Gerrit)
Uploaded patch set 16: Run-TryBot+1: Patch Set 15 was rebased.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=4355f961
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 16:TryBot-Result +1
Russ Cox (Gerrit)
2 comments:
File src/cmd/go/testdata/script/mod_bad_domain.txt:
Patch Set #16, Line 9:
# build should report all unsatisfied imports,These tests are fine but they skip over the fact that appengine should still be ignored in the global operations like 'go mod tidy'. Please add a test at the end:
# go mod vendor and go mod tidy should ignore appengine imports.
rm usenonexistent/x.go
go mod tidy
go mod vendor
File src/cmd/go/testdata/script/mod_list_bad_import.txt:
Patch Set #16, Line 7:
# Without -e, listing an otherwise-valid package with an unsatisfied direct import should fail.It really seems like if you have a fix for list that you should commit that instead of all these buggy-to-be-fixed-later tests. Is there a fix, or should I work on one?
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Russ Cox (Gerrit)
Code changes look good. I'm a little worried about the tests. Let me know about the list -e thing.
Bryan C. Mills (Gerrit)
1 comment:
Patch Set #16, Line 7:
# Without -e, listing an otherwise-valid package with an unsatisfied direct import should fail.
It really seems like if you have a fix for list that you should commit that instead of all these bug […]
I don't have a fix for it in the works — you're welcome to take it on, otherwise I'll come back to it for 1.12.
I'm adding these tests in part because it will remind me where to put them later, and in part because they make it easier to see which bugs are coverging. (Having two different sets of bugs between GOPATH and module mode implies that we probably have two different bugs to fix, whereas having the same bugs in both modes implies that we're at least making progress.)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #17 to this change.
cmd/go/internal/modload: report errors explicitly from Lookup
Previously, we reported errors directly in (*loader).load via base.Errorf.
Unfortunately, (*loader).load can be called from contexts in which such errors
should not be considered fatal, such as by load.PackagesAndErrors.
Instead, we save the errors in pkg.err and modify Lookup to return that error.
This change is a bit awkward: we end up suppressing a "no Go files" error for
packages at the root of newly-imported modules, even if they really do contain
source files. I believe that that's due to a special-case lookup for modules in
the build list, which allows us to "validate" imports for modules in the build
list even though we haven't actually downloaded their sources (or verified that
they actually contain the requested package). The fix for that issue is in the
change that follows this one.
Fixes #26602.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
---
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
M src/cmd/go/testdata/script/mod_list_bad_import.txt
M src/cmd/go/testdata/script/mod_readonly.txt
M src/cmd/go/testdata/script/mod_vendor.txt
8 files changed, 104 insertions(+), 50 deletions(-)
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Uploaded patch set 17.
(1 comment)
1 comment:
Patch Set #16, Line 9:
# build should report all unsatisfied imports,
These tests are fine but they skip over the fact that appengine should still be ignored in the globa […]
Done.
That introduces a bit of redundancy with mod_vendor.txt, but since there are no actual module dependencies it shouldn't waste much time.
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=56e14bd8
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 17:TryBot-Result +1
Bryan C. Mills (Gerrit)
Uploaded patch set 18: New patch set was added with same tree, parent, and commit message as Patch Set 17.
Bryan C. Mills (Gerrit)
Bryan C. Mills uploaded patch set #18 to this change.
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.
Russ Cox (Gerrit)
Everything is +2 now except mod_list_bad_import.txt.
Russ Cox (Gerrit)
Actually, I confused this CL with the one updating mod_internal_test. This one looks OK.
Patch set 18:Code-Review +2
Bryan C. Mills (Gerrit)
Bryan C. Mills merged this change.
cmd/go/internal/modload: report errors explicitly from Lookup
Previously, we reported errors directly in (*loader).load via base.Errorf.
Unfortunately, (*loader).load can be called from contexts in which such errors
should not be considered fatal, such as by load.PackagesAndErrors.
Instead, we save the errors in pkg.err and modify Lookup to return that error.
This change is a bit awkward: we end up suppressing a "no Go files" error for
packages at the root of newly-imported modules, even if they really do contain
source files. I believe that that's due to a special-case lookup for modules in
the build list, which allows us to "validate" imports for modules in the build
list even though we haven't actually downloaded their sources (or verified that
they actually contain the requested package). The fix for that issue is in the
change that follows this one.
Fixes #26602.
Change-Id: I16f00ceb143fbb797cfc3cb07fd08aeb6154575b
Reviewed-on: https://go-review.googlesource.com/127936
Run-TryBot: Bryan C. Mills <bcm...@google.com>
Reviewed-by: Russ Cox <r...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>
---
M src/cmd/go/internal/modget/get.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/testdata/script/mod_bad_domain.txt
M src/cmd/go/testdata/script/mod_get_commit.txt
M src/cmd/go/testdata/script/mod_get_indirect.txt
M src/cmd/go/testdata/script/mod_list_bad_import.txt
M src/cmd/go/testdata/script/mod_readonly.txt
M src/cmd/go/testdata/script/mod_vendor.txt
8 files changed, 104 insertions(+), 50 deletions(-)
diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index cf0c1ac..ee8ac8a 100644
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -515,13 +515,28 @@
}
if len(install) > 0 {
+ // All requested versions were explicitly @none.
+ // Note that 'go get -u' without any arguments results in len(install) == 1:
+ // search.CleanImportPaths returns "." for empty args.
work.BuildInit()
var pkgs []string
for _, p := range load.PackagesAndErrors(install) {
- if p.Error == nil || !strings.HasPrefix(p.Error.Err, "no Go files") {
- pkgs = append(pkgs, p.ImportPath)
+ // Ignore "no Go source files" errors for 'go get' operations on modules.
+ if p.Error != nil {
+ if len(args) == 0 && getU != "" && strings.HasPrefix(p.Error.Err, "no Go files") {
+ // Upgrading modules: skip the implicitly-requested package at the
+ // current directory, even if it is not tho module root.
+ continue
+ }
+ if strings.HasPrefix(p.Error.Err, "no Go files") && modload.ModuleInfo(p.ImportPath) != nil {
+ // Explicitly-requested module, but it doesn't contain a package at the
+ // module root.
+ continue
+ }
}
+ pkgs = append(pkgs, p.ImportPath)
}
+
// If -d was specified, we're done after the download: no build.
// (The load.PackagesAndErrors is what did the download
// of the named packages and their dependencies.)
@@ -564,7 +579,9 @@
// if found in the current source code.
// Then apply the version to that module.
m, _, err := modload.Import(path)
- if err != nil {
+ if e, ok := err.(*modload.ImportMissingError); ok && e.Module.Path != "" {
+ m = e.Module
+ } else if err != nil {
return module.Version{}, err
}
if m.Path == "" {
diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go
index 90f77ec..63a1725 100644
--- a/src/cmd/go/internal/modload/load.go
+++ b/src/cmd/go/internal/modload/load.go
@@ -158,6 +158,9 @@
have[path] = true
if path == "all" {
for _, pkg := range loaded.pkgs {
+ if e, ok := pkg.err.(*ImportMissingError); ok && e.Module.Path == "" {
+ continue // Package doesn't actually exist, so don't report it.
+ }
if !have[pkg.path] {
have[pkg.path] = true
final = append(final, pkg.path)
@@ -270,6 +273,9 @@
var paths []string
for _, pkg := range loaded.pkgs {
+ if e, ok := pkg.err.(*ImportMissingError); ok && e.Module.Path == "" {
+ continue // Package doesn't actually exist.
+ }
paths = append(paths, pkg.path)
}
return paths
@@ -337,21 +343,22 @@
return loaded.direct[path]
}
-// Lookup returns the source directory and import path for the package at path.
+// Lookup returns the source directory, import path, and any loading error for
+// the package at path.
// Lookup requires that one of the Load functions in this package has already
// been called.
func Lookup(path string) (dir, realPath string, err error) {
- realPath = ImportMap(path)
- if realPath == "" {
+ pkg, ok := loaded.pkgCache.Get(path).(*loadPkg)
+ if !ok {
if isStandardImportPath(path) {
dir := filepath.Join(cfg.GOROOT, "src", path)
if _, err := os.Stat(dir); err == nil {
return dir, path, nil
}
}
- return "", "", fmt.Errorf("no such package in module")
+ return "", "", errMissing
}
- return PackageDir(realPath), realPath, nil
+ return pkg.dir, pkg.path, pkg.err
}
// A loader manages the process of loading information about
@@ -459,9 +466,7 @@
}
continue
}
- if pkg.err != nil {
- base.Errorf("go: %s: %s", pkg.stackText(), pkg.err)
- }
+ // Leave other errors for Import or load.Packages to report.
}
base.ExitIfErrors()
if numAdded == 0 {
@@ -560,11 +565,6 @@
var err error
imports, testImports, err = scanDir(pkg.dir, ld.tags)
if err != nil {
- if strings.HasPrefix(err.Error(), "no Go ") {
- // Don't print about directories with no Go source files.
- // Let the eventual real package load do that.
- return
- }
pkg.err = err
return
}
diff --git a/src/cmd/go/testdata/script/mod_bad_domain.txt b/src/cmd/go/testdata/script/mod_bad_domain.txt
index 236564e..829c885 100644
--- a/src/cmd/go/testdata/script/mod_bad_domain.txt
+++ b/src/cmd/go/testdata/script/mod_bad_domain.txt
@@ -6,16 +6,24 @@
! go get x/y.z
stderr 'cannot find module providing package x/y.z'
-# build should skip over appengine imports
-! go build
-! stderr appengine
+# build should report all unsatisfied imports,
+# but should be more definitive about non-module import paths
+! go build ./useappengine
+stderr 'cannot find package'
+! go build ./usenonexistent
stderr 'cannot find module providing package nonexistent.rsc.io'
+# go mod vendor and go mod tidy should ignore appengine imports.
+rm usenonexistent/x.go
+go mod tidy
+go mod vendor
+
-- go.mod --
module x
--- x.go --
-package x
-
-import _ "appengine"
+-- useappengine/x.go --
+package useappengine
+import _ "appengine" // package does not exist
+-- usenonexistent/x.go --
+package usenonexistent
import _ "nonexistent.rsc.io" // domain does not exist
diff --git a/src/cmd/go/testdata/script/mod_get_commit.txt b/src/cmd/go/testdata/script/mod_get_commit.txt
index e96f097..2608397 100644
--- a/src/cmd/go/testdata/script/mod_get_commit.txt
+++ b/src/cmd/go/testdata/script/mod_get_commit.txt
@@ -2,12 +2,6 @@
# @commit should resolve
-# go get should skip build with no Go files in root
-go get golang.org/x/text@14c0d48
-
-# ... and go get should skip build with -m
-go get -m golang.org/x/text@14c0d48
-
# golang.org/x/text/language@commit should not resolve with -m,
# because that's not a module path.
! go get -m golang.org/x/text/language@14c0d48
@@ -17,6 +11,12 @@
go get -d -x golang.org/x/text/language@14c0d48
! stderr 'compile|cp|gccgo .*language\.a$'
+# go get should skip build with no Go files in root
+go get golang.org/x/text@14c0d48
+
+# ... and go get should skip build with -m
+go get -m golang.org/x/text@14c0d48
+
# dropping -d, we should see a build.
go get -x golang.org/x/text/language@14c0d48
stderr 'compile|cp|gccgo .*language\.a$'
diff --git a/src/cmd/go/testdata/script/mod_get_indirect.txt b/src/cmd/go/testdata/script/mod_get_indirect.txt
index 8388ed1..f567e97 100644
--- a/src/cmd/go/testdata/script/mod_get_indirect.txt
+++ b/src/cmd/go/testdata/script/mod_get_indirect.txt
@@ -11,8 +11,14 @@
stdout '^golang.org/x/text [v0-9a-f\.-]+ // indirect'
grep 'golang.org/x/text [v0-9a-f\.-]+ // indirect' go.mod
-# indirect tag should be removed upon seeing direct import
+# importing an empty module root as a package makes it direct.
+# TODO(bcmills): This doesn't seem correct. Fix is in the next change.
cp $WORK/tmp/usetext.go x.go
+go list -e
+grep 'golang.org/x/text [v0-9a-f\.-]+$' go.mod
+
+# indirect tag should be removed upon seeing direct import.
+cp $WORK/tmp/uselang.go x.go
go list
grep 'rsc.io/quote v1.5.2$' go.mod
grep 'golang.org/x/text [v0-9a-f\.-]+$' go.mod
@@ -24,7 +30,7 @@
grep 'golang.org/x/text [v0-9a-f\.-]+ // indirect' go.mod
# requirement should be dropped entirely if not needed
-cp $WORK/tmp/usetext.go x.go
+cp $WORK/tmp/uselang.go x.go
go mod tidy
! grep rsc.io/quote go.mod
grep 'golang.org/x/text [v0-9a-f\.-]+$' go.mod
@@ -37,6 +43,9 @@
-- $WORK/tmp/usetext.go --
package x
import _ "golang.org/x/text"
+-- $WORK/tmp/uselang.go --
+package x
+import _ "golang.org/x/text/language"
-- $WORK/tmp/usequote.go --
package x
import _ "rsc.io/quote"
diff --git a/src/cmd/go/testdata/script/mod_list_bad_import.txt b/src/cmd/go/testdata/script/mod_list_bad_import.txt
index c05fdea..b3cb0a4 100644
--- a/src/cmd/go/testdata/script/mod_list_bad_import.txt
+++ b/src/cmd/go/testdata/script/mod_list_bad_import.txt
@@ -4,25 +4,35 @@
env GO111MODULE=on
cd example.com
-# Listing an otherwise-valid package with an unsatisfied direct import should succeed,
-# but name that package in DepsErrors.
-! go list -f '{{if .Error}}error{{end}} {{if .Incomplete}}incomplete{{end}} {{range .DepsErrors}}bad dep: {{.Err}}{{end}}' example.com/direct
-stderr example.com[/\\]notfound
+# Without -e, listing an otherwise-valid package with an unsatisfied direct import should fail.
+# BUG: Today it succeeds.
+go list -f '{{if .Error}}error{{end}} {{if .Incomplete}}incomplete{{end}} {{range .DepsErrors}}bad dep: {{.Err}}{{end}}' example.com/direct
+! stdout ^error
+stdout 'incomplete'
+stdout 'bad dep: .*example.com/notfound'
# Listing with -deps should also fail.
-! go list -deps example.com/direct
-stderr example.com[/\\]notfound
+# BUG: Today, it does not.
+# ! go list -deps example.com/direct
+# stderr example.com/notfound
+go list -deps example.com/direct
+stdout example.com/notfound
# Listing an otherwise-valid package that imports some *other* package with an
-# unsatisfied import should also succeed.
-# NOTE: This behavior differs between GOPATH mode and module mode.
-! go list -f '{{if .Error}}error{{end}} {{if .Incomplete}}incomplete{{end}} {{range .DepsErrors}}bad dep: {{.Err}}{{end}}' example.com/indirect
-stderr example.com[/\\]notfound
+# unsatisfied import should also fail.
+# BUG: Today, it succeeds.
+go list -f '{{if .Error}}error{{end}} {{if .Incomplete}}incomplete{{end}} {{range .DepsErrors}}bad dep: {{.Err}}{{end}}' example.com/indirect
+! stdout ^error
+stdout incomplete
+stdout 'bad dep: .*example.com/notfound'
# Again, -deps should fail.
-! go list -deps example.com/indirect
-stderr example.com[/\\]notfound
+# BUG: Again, it does not.
+# ! go list -deps example.com/indirect
+# stderr example.com/notfound
+go list -deps example.com/indirect
+stdout example.com/notfound
# Listing the missing dependency directly should fail outright...
@@ -32,16 +42,17 @@
! stdout incomplete
# ...but listing with -e should succeed.
-# BUG: Today, it fails.
-! go list -e -f '{{if .Error}}error{{end}} {{if .Incomplete}}incomplete{{end}}' example.com/notfound
-stderr example.com[/\\]notfound
+go list -e -f '{{if .Error}}error{{end}} {{if .Incomplete}}incomplete{{end}}' example.com/notfound
+stdout error
+stdout incomplete
# The pattern "all" should match only packages that acutally exist,
# ignoring those whose existence is merely implied by imports.
-# BUG: Today, `go list -e` fails if there are any unresolved imports.
-! go list -e -f '{{.ImportPath}}' all
-stderr example.com[/\\]notfound
+go list -e -f '{{.ImportPath}}' all
+stdout example.com/direct
+stdout example.com/indirect
+! stdout example.com/notfound
-- example.com/go.mod --
diff --git a/src/cmd/go/testdata/script/mod_readonly.txt b/src/cmd/go/testdata/script/mod_readonly.txt
index 5ae74a4..1b5932e 100644
--- a/src/cmd/go/testdata/script/mod_readonly.txt
+++ b/src/cmd/go/testdata/script/mod_readonly.txt
@@ -1,10 +1,13 @@
env GO111MODULE=on
# -mod=readonly must not resolve missing modules nor update go.mod
+#
+# TODO(bcmills): 'go list' should suffice, but today it does not fail due to
+# unresolved imports. When that is fixed, use 'go list' instead of 'go list all'.
env GOFLAGS=-mod=readonly
go mod edit -fmt
cp go.mod go.mod.empty
-! go list
+! go list all
stderr 'import lookup disabled by -mod=readonly'
cmp go.mod go.mod.empty
diff --git a/src/cmd/go/testdata/script/mod_vendor.txt b/src/cmd/go/testdata/script/mod_vendor.txt
index 8915d15..b3769a8 100644
--- a/src/cmd/go/testdata/script/mod_vendor.txt
+++ b/src/cmd/go/testdata/script/mod_vendor.txt
@@ -155,6 +155,12 @@
import _ "appengine"
import _ "appengine/datastore"
+-- nonexistent.go --
+// +build alternatereality
+
+package m
+
+import _ "nonexistent.rsc.io"
-- mypkg/go.mod --
module me
-- mypkg/mydir/d.go --
To view, visit change 127936. To unsubscribe, or for help writing mail filters, visit settings.