From ab2a5b48665eed6d670d719cdef5335bc3602359 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Tue, 11 Aug 2020 12:57:01 -0400 Subject: cmd/go: add basic support for overlays This CL adds basic support for listing packages with overlays. The new cmd/go/internal/fs package adds an abstraction for communicating with the file system that will open files according to their overlaid paths, and provides functions to override those in the build context to open overlaid files. There is also some support for executing builds on packages with overlays. In cmd/go/internal/work.(*Builder).build, paths are mapped to their overlaid paths before they are given as arguments to tools. For #39958 Change-Id: I5ec0eb9ebbca303e2f1e7dbe22ec32613bc1fd17 Reviewed-on: https://go-review.googlesource.com/c/go/+/253747 Trust: Michael Matloob Trust: Jay Conrod Run-TryBot: Michael Matloob TryBot-Result: Go Bot Reviewed-by: Jay Conrod Reviewed-by: Bryan C. Mills --- src/cmd/go/internal/modload/import.go | 51 ++--------------------------------- 1 file changed, 2 insertions(+), 49 deletions(-) (limited to 'src/cmd/go/internal/modload/import.go') diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index c36c8bd29b..3642de851a 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -17,6 +17,7 @@ import ( "time" "cmd/go/internal/cfg" + "cmd/go/internal/fsys" "cmd/go/internal/modfetch" "cmd/go/internal/par" "cmd/go/internal/search" @@ -438,57 +439,9 @@ func dirInModule(path, mpath, mdir string, isLocal bool) (dir string, haveGoFile // We don't care about build tags, not even "+build ignore". // We're just looking for a plausible directory. res := haveGoFilesCache.Do(dir, func() interface{} { - ok, err := isDirWithGoFiles(dir) + ok, err := fsys.IsDirWithGoFiles(dir) return goFilesEntry{haveGoFiles: ok, err: err} }).(goFilesEntry) return dir, res.haveGoFiles, res.err } - -func isDirWithGoFiles(dir string) (bool, error) { - f, err := os.Open(dir) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - defer f.Close() - - names, firstErr := f.Readdirnames(-1) - if firstErr != nil { - if fi, err := f.Stat(); err == nil && !fi.IsDir() { - return false, nil - } - - // Rewrite the error from ReadDirNames to include the path if not present. - // See https://golang.org/issue/38923. - var pe *os.PathError - if !errors.As(firstErr, &pe) { - firstErr = &os.PathError{Op: "readdir", Path: dir, Err: firstErr} - } - } - - for _, name := range names { - if strings.HasSuffix(name, ".go") { - info, err := os.Stat(filepath.Join(dir, name)) - if err == nil && info.Mode().IsRegular() { - // If any .go source file exists, the package exists regardless of - // errors for other source files. Leave further error reporting for - // later. - return true, nil - } - if firstErr == nil { - if os.IsNotExist(err) { - // If the file was concurrently deleted, or was a broken symlink, - // convert the error to an opaque error instead of one matching - // os.IsNotExist. - err = errors.New(err.Error()) - } - firstErr = err - } - } - } - - return false, firstErr -} -- cgit v1.2.1 From 3a65abfbdac7ab29f693d69bd1eb12b2148a11ae Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 29 Sep 2020 17:45:02 -0400 Subject: cmd/go: adjust ImportMissingError when module lookup is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, ImportMissingError said "cannot find module providing package …" even when we didn't even attempt to find such a module. Now, we write "no module requirement provides package …" when we did not attempt to identify a suitable module, and suggest either 'go mod tidy' or 'go get -d' as appropriate. Fixes #41576 Change-Id: I979bb999da4066828c54d99a310ea66bb31032ec Reviewed-on: https://go-review.googlesource.com/c/go/+/258298 Trust: Bryan C. Mills Trust: Jay Conrod Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Michael Matloob Reviewed-by: Jay Conrod --- src/cmd/go/internal/modload/import.go | 40 ++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 17 deletions(-) (limited to 'src/cmd/go/internal/modload/import.go') diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index 3642de851a..76fe6745d9 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -26,13 +26,15 @@ import ( "golang.org/x/mod/semver" ) -var errImportMissing = errors.New("import missing") - type ImportMissingError struct { Path string Module module.Version QueryErr error + // inAll indicates whether Path is in the "all" package pattern, + // and thus would be added by 'go mod tidy'. + inAll bool + // newMissingVersion is set to a newer version of Module if one is present // in the build list. When set, we can't automatically upgrade. newMissingVersion string @@ -46,7 +48,19 @@ func (e *ImportMissingError) Error() string { if e.QueryErr != nil { return fmt.Sprintf("cannot find module providing package %s: %v", e.Path, e.QueryErr) } - return "cannot find module providing package " + e.Path + if cfg.BuildMod == "mod" { + return "cannot find module providing package " + e.Path + } + + suggestion := "" + if !HasModRoot() { + suggestion = ": working directory is not part of a module" + } else if e.inAll { + suggestion = "; try 'go mod tidy' to add it" + } else { + suggestion = fmt.Sprintf("; try 'go get -d %s' to add it", e.Path) + } + return fmt.Sprintf("no required module provides package %s%s", e.Path, suggestion) } if e.newMissingVersion != "" { @@ -132,7 +146,7 @@ func (e *invalidImportError) Unwrap() error { // like "C" and "unsafe". // // If the package cannot be found in the current build list, -// importFromBuildList returns errImportMissing as the error. +// importFromBuildList returns an *ImportMissingError. func importFromBuildList(ctx context.Context, path string) (m module.Version, dir string, err error) { if strings.Contains(path, "@") { return module.Version{}, "", fmt.Errorf("import path should not have @version") @@ -144,6 +158,10 @@ func importFromBuildList(ctx context.Context, path string) (m module.Version, di // There's no directory for import "C" or import "unsafe". return module.Version{}, "", nil } + // Before any further lookup, check that the path is valid. + if err := module.CheckImportPath(path); err != nil { + return module.Version{}, "", &invalidImportError{importPath: path, err: err} + } // Is the package in the standard library? pathIsStd := search.IsStandardImportPath(path) @@ -212,7 +230,7 @@ func importFromBuildList(ctx context.Context, path string) (m module.Version, di return module.Version{}, "", &AmbiguousImportError{importPath: path, Dirs: dirs, Modules: mods} } - return module.Version{}, "", errImportMissing + return module.Version{}, "", &ImportMissingError{Path: path} } // queryImport attempts to locate a module that can be added to the current @@ -220,13 +238,6 @@ func importFromBuildList(ctx context.Context, path string) (m module.Version, di func queryImport(ctx context.Context, path string) (module.Version, error) { pathIsStd := search.IsStandardImportPath(path) - if modRoot == "" && !allowMissingModuleImports { - return module.Version{}, &ImportMissingError{ - Path: path, - QueryErr: errors.New("working directory is not part of a module"), - } - } - // Not on build list. // To avoid spurious remote fetches, next try the latest replacement for each // module (golang.org/issue/26241). This should give a useful message @@ -291,11 +302,6 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { } } - // Before any further lookup, check that the path is valid. - if err := module.CheckImportPath(path); err != nil { - return module.Version{}, &invalidImportError{importPath: path, err: err} - } - if pathIsStd { // This package isn't in the standard library, isn't in any module already // in the build list, and isn't in any other module that the user has -- cgit v1.2.1 From 8ee4d6e1bf1e76a4eac627d5fc9b0d0332d209e2 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Thu, 15 Oct 2020 15:53:09 -0400 Subject: cmd/go/internal/modload: move fetch to import.go From a comment in CL 262341. It makes more sense in import.go than in mvs.go. Change-Id: If4dfa1091077e110c5041bc849d99bc0be2bd8e2 Reviewed-on: https://go-review.googlesource.com/c/go/+/262780 Trust: Jay Conrod Run-TryBot: Jay Conrod TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills --- src/cmd/go/internal/modload/import.go | 39 +++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) (limited to 'src/cmd/go/internal/modload/import.go') diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index 76fe6745d9..8641cfec08 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -451,3 +451,42 @@ func dirInModule(path, mpath, mdir string, isLocal bool) (dir string, haveGoFile return dir, res.haveGoFiles, res.err } + +// fetch downloads the given module (or its replacement) +// and returns its location. +// +// The isLocal return value reports whether the replacement, +// if any, is local to the filesystem. +func fetch(ctx context.Context, mod module.Version) (dir string, isLocal bool, err error) { + if mod == Target { + return ModRoot(), true, nil + } + if r := Replacement(mod); r.Path != "" { + if r.Version == "" { + dir = r.Path + if !filepath.IsAbs(dir) { + dir = filepath.Join(ModRoot(), dir) + } + // Ensure that the replacement directory actually exists: + // dirInModule does not report errors for missing modules, + // so if we don't report the error now, later failures will be + // very mysterious. + if _, err := os.Stat(dir); err != nil { + if os.IsNotExist(err) { + // Semantically the module version itself “exists” — we just don't + // have its source code. Remove the equivalence to os.ErrNotExist, + // and make the message more concise while we're at it. + err = fmt.Errorf("replacement directory %s does not exist", r.Path) + } else { + err = fmt.Errorf("replacement directory %s: %w", r.Path, err) + } + return dir, true, module.VersionError(mod, err) + } + return dir, true, nil + } + mod = r + } + + dir, err = modfetch.Download(ctx, mod) + return dir, false, err +} -- cgit v1.2.1 From ff052737a946b5f9381dc054d61857ee4d500899 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 28 Sep 2020 20:59:47 -0400 Subject: cmd/go/internal/modload: allow 'go get' to use replaced versions 'go mod tidy' has been able to use replaced versions since CL 152739, but 'go get' failed for many of the same paths. Now that we are recommending 'go get' more aggressively due to #40728, we should make that work too. In the future, we might consider factoring out the new replacementRepo type so that 'go list' can report the new versions as well. For #41577 For #41416 For #37438 Updates #26241 Change-Id: I9140c556424b584fdd9bdd0a747842774664a7d8 Reviewed-on: https://go-review.googlesource.com/c/go/+/258220 Trust: Bryan C. Mills Trust: Jay Conrod Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Michael Matloob Reviewed-by: Jay Conrod --- src/cmd/go/internal/modload/import.go | 93 +++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 38 deletions(-) (limited to 'src/cmd/go/internal/modload/import.go') diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index 8641cfec08..1c572d5d6d 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -35,6 +35,12 @@ type ImportMissingError struct { // and thus would be added by 'go mod tidy'. inAll bool + // isStd indicates whether we would expect to find the package in the standard + // library. This is normally true for all dotless import paths, but replace + // directives can cause us to treat the replaced paths as also being in + // modules. + isStd bool + // newMissingVersion is set to a newer version of Module if one is present // in the build list. When set, we can't automatically upgrade. newMissingVersion string @@ -42,7 +48,7 @@ type ImportMissingError struct { func (e *ImportMissingError) Error() string { if e.Module.Path == "" { - if search.IsStandardImportPath(e.Path) { + if e.isStd { return fmt.Sprintf("package %s is not in GOROOT (%s)", e.Path, filepath.Join(cfg.GOROOT, "src", e.Path)) } if e.QueryErr != nil { @@ -230,46 +236,67 @@ func importFromBuildList(ctx context.Context, path string) (m module.Version, di return module.Version{}, "", &AmbiguousImportError{importPath: path, Dirs: dirs, Modules: mods} } - return module.Version{}, "", &ImportMissingError{Path: path} + return module.Version{}, "", &ImportMissingError{Path: path, isStd: pathIsStd} } // queryImport attempts to locate a module that can be added to the current // build list to provide the package with the given import path. +// +// Unlike QueryPattern, queryImport prefers to add a replaced version of a +// module *before* checking the proxies for a version to add. func queryImport(ctx context.Context, path string) (module.Version, error) { pathIsStd := search.IsStandardImportPath(path) - // Not on build list. - // To avoid spurious remote fetches, next try the latest replacement for each - // module (golang.org/issue/26241). This should give a useful message - // in -mod=readonly, and it will allow us to add a requirement with -mod=mod. - if modFile != nil { - latest := map[string]string{} // path -> version - for _, r := range modFile.Replace { - if maybeInModule(path, r.Old.Path) { - // Don't use semver.Max here; need to preserve +incompatible suffix. - v := latest[r.Old.Path] - if semver.Compare(r.Old.Version, v) > 0 { - v = r.Old.Version + if cfg.BuildMod == "readonly" { + if pathIsStd { + // If the package would be in the standard library and none of the + // available replacement modules could concievably provide it, report it + // as a missing standard-library package instead of complaining that + // module lookups are disabled. + maybeReplaced := false + if index != nil { + for p := range index.highestReplaced { + if maybeInModule(path, p) { + maybeReplaced = true + break + } } - latest[r.Old.Path] = v } + if !maybeReplaced { + return module.Version{}, &ImportMissingError{Path: path, isStd: true} + } + } + + var queryErr error + if cfg.BuildModExplicit { + queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod) + } else if cfg.BuildModReason != "" { + queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason) } + return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr} + } - mods := make([]module.Version, 0, len(latest)) - for p, v := range latest { - // If the replacement didn't specify a version, synthesize a - // pseudo-version with an appropriate major version and a timestamp below - // any real timestamp. That way, if the main module is used from within - // some other module, the user will be able to upgrade the requirement to - // any real version they choose. - if v == "" { - if _, pathMajor, ok := module.SplitPathVersion(p); ok && len(pathMajor) > 0 { - v = modfetch.PseudoVersion(pathMajor[1:], "", time.Time{}, "000000000000") + // To avoid spurious remote fetches, try the latest replacement for each + // module (golang.org/issue/26241). + if index != nil { + var mods []module.Version + for mp, mv := range index.highestReplaced { + if !maybeInModule(path, mp) { + continue + } + if mv == "" { + // The only replacement is a wildcard that doesn't specify a version, so + // synthesize a pseudo-version with an appropriate major version and a + // timestamp below any real timestamp. That way, if the main module is + // used from within some other module, the user will be able to upgrade + // the requirement to any real version they choose. + if _, pathMajor, ok := module.SplitPathVersion(mp); ok && len(pathMajor) > 0 { + mv = modfetch.PseudoVersion(pathMajor[1:], "", time.Time{}, "000000000000") } else { - v = modfetch.PseudoVersion("v0", "", time.Time{}, "000000000000") + mv = modfetch.PseudoVersion("v0", "", time.Time{}, "000000000000") } } - mods = append(mods, module.Version{Path: p, Version: v}) + mods = append(mods, module.Version{Path: mp, Version: mv}) } // Every module path in mods is a prefix of the import path. @@ -310,17 +337,7 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { // QueryPattern cannot possibly find a module containing this package. // // Instead of trying QueryPattern, report an ImportMissingError immediately. - return module.Version{}, &ImportMissingError{Path: path} - } - - if cfg.BuildMod == "readonly" { - var queryErr error - if cfg.BuildModExplicit { - queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod) - } else if cfg.BuildModReason != "" { - queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason) - } - return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr} + return module.Version{}, &ImportMissingError{Path: path, isStd: true} } // Look up module containing the package, for addition to the build list. -- cgit v1.2.1 From 41162be44a099803a870f9b6c147050594598d63 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 12 Oct 2020 15:32:49 -0400 Subject: cmd/go/internal/modload: avoid using the global build list in QueryPattern The Query function allows the caller to specify the current version of the requested module, but the QueryPattern function is missing that parameter: instead, it always assumes that the current version is the one selected from the global build list. This change removes that assumption, instead adding a callback function to determine the current version. (The callback is currently invoked once per candidate module, regardless of whether that module exists, but in a future change we can refactor it to invoke the callback only when needed.) For #36460 For #40775 Change-Id: I001a4a8ab24f5b4fcc66a670d9bd305b47e948ba Reviewed-on: https://go-review.googlesource.com/c/go/+/261640 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Jay Conrod Reviewed-by: Michael Matloob --- src/cmd/go/internal/modload/import.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/cmd/go/internal/modload/import.go') diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index 1c572d5d6d..6d0d8de944 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -345,7 +345,7 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { // and return m, dir, ImpportMissingError. fmt.Fprintf(os.Stderr, "go: finding module for package %s\n", path) - candidates, err := QueryPattern(ctx, path, "latest", CheckAllowed) + candidates, err := QueryPattern(ctx, path, "latest", Selected, CheckAllowed) if err != nil { if errors.Is(err, os.ErrNotExist) { // Return "cannot find module providing package […]" instead of whatever -- cgit v1.2.1 From 7bb721b9384bdd196befeaed593b185f7f2a5589 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 7 Jul 2020 13:49:21 -0400 Subject: all: update references to symbols moved from os to io/fs The old os references are still valid, but update our code to reflect best practices and get used to the new locations. Code compiled with the bootstrap toolchain (cmd/asm, cmd/dist, cmd/compile, debug/elf) must remain Go 1.4-compatible and is excluded. For #41190. Change-Id: I8f9526977867c10a221e2f392f78d7dec073f1bd Reviewed-on: https://go-review.googlesource.com/c/go/+/243907 Trust: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Go Bot Reviewed-by: Rob Pike --- src/cmd/go/internal/modload/import.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/cmd/go/internal/modload/import.go') diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index 6d0d8de944..bcbc9b0c3a 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -10,6 +10,7 @@ import ( "fmt" "go/build" "internal/goroot" + "io/fs" "os" "path/filepath" "sort" @@ -347,7 +348,7 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { candidates, err := QueryPattern(ctx, path, "latest", Selected, CheckAllowed) if err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, fs.ErrNotExist) { // Return "cannot find module providing package […]" instead of whatever // low-level error QueryPattern produced. return module.Version{}, &ImportMissingError{Path: path, QueryErr: err} -- cgit v1.2.1 From 5cd4390f3853b8d0d2d962f7acdac87c0eba3d77 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Tue, 13 Oct 2020 18:19:21 -0400 Subject: cmd/go: don't fetch files missing sums in readonly mode If the go command needs a .mod or .zip file in -mod=readonly mode (now the default), and that file doesn't have a hash in the main module's go.sum file, the go command will now report an error before fetching the file, rather than at the end when failing to update go.sum. The error says specifically which entry is missing. If this error is encountered when loading the build list, it will suggest 'go mod tidy'. If this error is encountered when loading a specific package (an import or command line argument), the error will mention that package and will suggest 'go mod tidy' or 'go get -d'. Fixes #41934 Fixes #41935 Change-Id: I96ec2ef9258bd4bade9915c43d47e6243c376a81 Reviewed-on: https://go-review.googlesource.com/c/go/+/262341 Reviewed-by: Bryan C. Mills Trust: Jay Conrod --- src/cmd/go/internal/modload/import.go | 68 +++++++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 6 deletions(-) (limited to 'src/cmd/go/internal/modload/import.go') diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index bcbc9b0c3a..ffe8733af6 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -124,6 +124,31 @@ func (e *AmbiguousImportError) Error() string { return buf.String() } +// ImportMissingSumError is reported in readonly mode when we need to check +// if a module in the build list contains a package, but we don't have a sum +// for its .zip file. +type ImportMissingSumError struct { + importPath string + found, inAll bool +} + +func (e *ImportMissingSumError) Error() string { + var message string + if e.found { + message = fmt.Sprintf("missing go.sum entry needed to verify package %s is provided by exactly one module", e.importPath) + } else { + message = fmt.Sprintf("missing go.sum entry for module providing package %s", e.importPath) + } + if e.inAll { + return message + "; try 'go mod tidy' to add it" + } + return message +} + +func (e *ImportMissingSumError) ImportPath() string { + return e.importPath +} + type invalidImportError struct { importPath string err error @@ -208,13 +233,23 @@ func importFromBuildList(ctx context.Context, path string) (m module.Version, di // Check each module on the build list. var dirs []string var mods []module.Version + haveSumErr := false for _, m := range buildList { if !maybeInModule(path, m.Path) { // Avoid possibly downloading irrelevant modules. continue } - root, isLocal, err := fetch(ctx, m) + needSum := true + root, isLocal, err := fetch(ctx, m, needSum) if err != nil { + if sumErr := (*sumMissingError)(nil); errors.As(err, &sumErr) { + // We are missing a sum needed to fetch a module in the build list. + // We can't verify that the package is unique, and we may not find + // the package at all. Keep checking other modules to decide which + // error to report. + haveSumErr = true + continue + } // Report fetch error. // Note that we don't know for sure this module is necessary, // but it certainly _could_ provide the package, and even if we @@ -230,12 +265,15 @@ func importFromBuildList(ctx context.Context, path string) (m module.Version, di dirs = append(dirs, dir) } } + if len(mods) > 1 { + return module.Version{}, "", &AmbiguousImportError{importPath: path, Dirs: dirs, Modules: mods} + } + if haveSumErr { + return module.Version{}, "", &ImportMissingSumError{importPath: path, found: len(mods) > 0} + } if len(mods) == 1 { return mods[0], dirs[0], nil } - if len(mods) > 0 { - return module.Version{}, "", &AmbiguousImportError{importPath: path, Dirs: dirs, Modules: mods} - } return module.Version{}, "", &ImportMissingError{Path: path, isStd: pathIsStd} } @@ -306,7 +344,8 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { return len(mods[i].Path) > len(mods[j].Path) }) for _, m := range mods { - root, isLocal, err := fetch(ctx, m) + needSum := true + root, isLocal, err := fetch(ctx, m, needSum) if err != nil { // Report fetch error as above. return module.Version{}, err @@ -473,9 +512,14 @@ func dirInModule(path, mpath, mdir string, isLocal bool) (dir string, haveGoFile // fetch downloads the given module (or its replacement) // and returns its location. // +// needSum indicates whether the module may be downloaded in readonly mode +// without a go.sum entry. It should only be false for modules fetched +// speculatively (for example, for incompatible version filtering). The sum +// will still be verified normally. +// // The isLocal return value reports whether the replacement, // if any, is local to the filesystem. -func fetch(ctx context.Context, mod module.Version) (dir string, isLocal bool, err error) { +func fetch(ctx context.Context, mod module.Version, needSum bool) (dir string, isLocal bool, err error) { if mod == Target { return ModRoot(), true, nil } @@ -505,6 +549,18 @@ func fetch(ctx context.Context, mod module.Version) (dir string, isLocal bool, e mod = r } + if cfg.BuildMod == "readonly" && needSum && !modfetch.HaveSum(mod) { + return "", false, module.VersionError(mod, &sumMissingError{}) + } + dir, err = modfetch.Download(ctx, mod) return dir, false, err } + +type sumMissingError struct { + suggestion string +} + +func (e *sumMissingError) Error() string { + return "missing go.sum entry" + e.suggestion +} -- cgit v1.2.1 From f24ff3856a629a6b5fefe28a1676638d5f103342 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Fri, 16 Oct 2020 14:02:16 -0400 Subject: cmd/go: change error message for missing import with unused replacement In readonly mode, if a package is not provided by any module in the build list, and there is an unused replacement that contains the package, we now recommend a 'go get' command to add a requirement on the highest replaced version. Fixes #41416 Change-Id: Iedf3539292c70ea6ba6857433fd184454d9325da Reviewed-on: https://go-review.googlesource.com/c/go/+/263146 Run-TryBot: Jay Conrod TryBot-Result: Go Bot Reviewed-by: Michael Matloob Reviewed-by: Bryan C. Mills Trust: Jay Conrod --- src/cmd/go/internal/modload/import.go | 72 +++++++++++++++++------------------ 1 file changed, 35 insertions(+), 37 deletions(-) (limited to 'src/cmd/go/internal/modload/import.go') diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index ffe8733af6..e959347020 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -15,7 +15,6 @@ import ( "path/filepath" "sort" "strings" - "time" "cmd/go/internal/cfg" "cmd/go/internal/fsys" @@ -42,6 +41,10 @@ type ImportMissingError struct { // modules. isStd bool + // replaced the highest replaced version of the module where the replacement + // contains the package. replaced is only set if the replacement is unused. + replaced module.Version + // newMissingVersion is set to a newer version of Module if one is present // in the build list. When set, we can't automatically upgrade. newMissingVersion string @@ -59,6 +62,14 @@ func (e *ImportMissingError) Error() string { return "cannot find module providing package " + e.Path } + if e.replaced.Path != "" { + suggestArg := e.replaced.Path + if !modfetch.IsZeroPseudoVersion(e.replaced.Version) { + suggestArg = e.replaced.String() + } + return fmt.Sprintf("module %s provides package %s and is replaced but not required; try 'go get -d %s' to add it", e.replaced.Path, e.Path, suggestArg) + } + suggestion := "" if !HasModRoot() { suggestion = ": working directory is not part of a module" @@ -284,37 +295,6 @@ func importFromBuildList(ctx context.Context, path string) (m module.Version, di // Unlike QueryPattern, queryImport prefers to add a replaced version of a // module *before* checking the proxies for a version to add. func queryImport(ctx context.Context, path string) (module.Version, error) { - pathIsStd := search.IsStandardImportPath(path) - - if cfg.BuildMod == "readonly" { - if pathIsStd { - // If the package would be in the standard library and none of the - // available replacement modules could concievably provide it, report it - // as a missing standard-library package instead of complaining that - // module lookups are disabled. - maybeReplaced := false - if index != nil { - for p := range index.highestReplaced { - if maybeInModule(path, p) { - maybeReplaced = true - break - } - } - } - if !maybeReplaced { - return module.Version{}, &ImportMissingError{Path: path, isStd: true} - } - } - - var queryErr error - if cfg.BuildModExplicit { - queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod) - } else if cfg.BuildModReason != "" { - queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason) - } - return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr} - } - // To avoid spurious remote fetches, try the latest replacement for each // module (golang.org/issue/26241). if index != nil { @@ -330,9 +310,9 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { // used from within some other module, the user will be able to upgrade // the requirement to any real version they choose. if _, pathMajor, ok := module.SplitPathVersion(mp); ok && len(pathMajor) > 0 { - mv = modfetch.PseudoVersion(pathMajor[1:], "", time.Time{}, "000000000000") + mv = modfetch.ZeroPseudoVersion(pathMajor[1:]) } else { - mv = modfetch.PseudoVersion("v0", "", time.Time{}, "000000000000") + mv = modfetch.ZeroPseudoVersion("v0") } } mods = append(mods, module.Version{Path: mp, Version: mv}) @@ -347,18 +327,23 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { needSum := true root, isLocal, err := fetch(ctx, m, needSum) if err != nil { - // Report fetch error as above. + if sumErr := (*sumMissingError)(nil); errors.As(err, &sumErr) { + return module.Version{}, &ImportMissingSumError{importPath: path} + } return module.Version{}, err } if _, ok, err := dirInModule(path, m.Path, root, isLocal); err != nil { return m, err } else if ok { + if cfg.BuildMod == "readonly" { + return module.Version{}, &ImportMissingError{Path: path, replaced: m} + } return m, nil } } if len(mods) > 0 && module.CheckPath(path) != nil { // The package path is not valid to fetch remotely, - // so it can only exist if in a replaced module, + // so it can only exist in a replaced module, // and we know from the above loop that it is not. return module.Version{}, &PackageNotInModuleError{ Mod: mods[0], @@ -369,7 +354,7 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { } } - if pathIsStd { + if search.IsStandardImportPath(path) { // This package isn't in the standard library, isn't in any module already // in the build list, and isn't in any other module that the user has // shimmed in via a "replace" directive. @@ -380,6 +365,19 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { return module.Version{}, &ImportMissingError{Path: path, isStd: true} } + if cfg.BuildMod == "readonly" { + // In readonly mode, we can't write go.mod, so we shouldn't try to look up + // the module. If readonly mode was enabled explicitly, include that in + // the error message. + var queryErr error + if cfg.BuildModExplicit { + queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod) + } else if cfg.BuildModReason != "" { + queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason) + } + return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr} + } + // Look up module containing the package, for addition to the build list. // Goal is to determine the module, download it to dir, // and return m, dir, ImpportMissingError. -- cgit v1.2.1