From 9214677e7df1e6130249bc83d721130b00d829c4 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Wed, 9 Sep 2020 16:41:55 -0400 Subject: [PATCH] cmd/go: refactor modload.Import for better -mod=readonly errors When -mod=readonly is set, Import will now allow imports from replacements without explicit requirements. With -mod=mod, this would add a new requirement but does not trigger a module lookup, so it's determinisitic. Before reporting an error for an unknown import with -mod=readonly, check whether the import is valid. If there's a typo in the import, that's more relevant. For #40728 Change-Id: I05e138ff76ba3d0eb2e3010c15589fa363deb8d3 Reviewed-on: https://go-review.googlesource.com/c/go/+/253745 Run-TryBot: Jay Conrod TryBot-Result: Gobot Gobot Reviewed-by: Michael Matloob Reviewed-by: Bryan C. Mills --- src/cmd/go/internal/modload/import.go | 53 ++++++++++++++----- .../go/testdata/script/mod_build_info_err.txt | 2 +- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index c625184b8b..10b1e7f4b8 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -107,6 +107,25 @@ func (e *AmbiguousImportError) Error() string { var _ load.ImportPathError = &AmbiguousImportError{} +type invalidImportError struct { + importPath string + err error +} + +func (e *invalidImportError) ImportPath() string { + return e.importPath +} + +func (e *invalidImportError) Error() string { + return e.err.Error() +} + +func (e *invalidImportError) Unwrap() error { + return e.err +} + +var _ load.ImportPathError = &invalidImportError{} + // importFromBuildList finds the module and directory in the build list // containing the package with the given import path. The answer must be unique: // importFromBuildList returns an error if multiple modules attempt to provide @@ -207,17 +226,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 cfg.BuildMod == "readonly" { - var queryErr error - if !pathIsStd { - if cfg.BuildModReason == "" { - queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod) - } else { - queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason) - } - } - return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr} - } if modRoot == "" && !allowMissingModuleImports { return module.Version{}, &ImportMissingError{ Path: path, @@ -226,8 +234,9 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { } // Not on build list. - // To avoid spurious remote fetches, next try the latest replacement for each module. - // (golang.org/issue/26241) + // 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 { @@ -288,6 +297,11 @@ 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 @@ -299,6 +313,19 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { 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} + } + + // 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. fmt.Fprintf(os.Stderr, "go: finding module for package %s\n", path) candidates, err := QueryPackage(ctx, path, "latest", CheckAllowed) diff --git a/src/cmd/go/testdata/script/mod_build_info_err.txt b/src/cmd/go/testdata/script/mod_build_info_err.txt index 87a099b219..a6853b5c86 100644 --- a/src/cmd/go/testdata/script/mod_build_info_err.txt +++ b/src/cmd/go/testdata/script/mod_build_info_err.txt @@ -2,7 +2,7 @@ # Verifies golang.org/issue/34393. go list -e -deps -f '{{with .Error}}{{.Pos}}: {{.Err}}{{end}}' ./main -stdout 'bad[/\\]bad.go:3:8: malformed module path "🐧.example.com/string": invalid char ''🐧''' +stdout 'bad[/\\]bad.go:3:8: malformed import path "🐧.example.com/string": invalid char ''🐧''' -- go.mod -- module m