Skip to content

Commit 9214677

Browse files
author
Jay Conrod
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Michael Matloob <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]>
1 parent 6e3df74 commit 9214677

File tree

2 files changed

+41
-14
lines changed

2 files changed

+41
-14
lines changed

src/cmd/go/internal/modload/import.go

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,25 @@ func (e *AmbiguousImportError) Error() string {
107107

108108
var _ load.ImportPathError = &AmbiguousImportError{}
109109

110+
type invalidImportError struct {
111+
importPath string
112+
err error
113+
}
114+
115+
func (e *invalidImportError) ImportPath() string {
116+
return e.importPath
117+
}
118+
119+
func (e *invalidImportError) Error() string {
120+
return e.err.Error()
121+
}
122+
123+
func (e *invalidImportError) Unwrap() error {
124+
return e.err
125+
}
126+
127+
var _ load.ImportPathError = &invalidImportError{}
128+
110129
// importFromBuildList finds the module and directory in the build list
111130
// containing the package with the given import path. The answer must be unique:
112131
// 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
207226
func queryImport(ctx context.Context, path string) (module.Version, error) {
208227
pathIsStd := search.IsStandardImportPath(path)
209228

210-
if cfg.BuildMod == "readonly" {
211-
var queryErr error
212-
if !pathIsStd {
213-
if cfg.BuildModReason == "" {
214-
queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod)
215-
} else {
216-
queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason)
217-
}
218-
}
219-
return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr}
220-
}
221229
if modRoot == "" && !allowMissingModuleImports {
222230
return module.Version{}, &ImportMissingError{
223231
Path: path,
@@ -226,8 +234,9 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
226234
}
227235

228236
// Not on build list.
229-
// To avoid spurious remote fetches, next try the latest replacement for each module.
230-
// (golang.org/issue/26241)
237+
// To avoid spurious remote fetches, next try the latest replacement for each
238+
// module (golang.org/issue/26241). This should give a useful message
239+
// in -mod=readonly, and it will allow us to add a requirement with -mod=mod.
231240
if modFile != nil {
232241
latest := map[string]string{} // path -> version
233242
for _, r := range modFile.Replace {
@@ -288,6 +297,11 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
288297
}
289298
}
290299

300+
// Before any further lookup, check that the path is valid.
301+
if err := module.CheckImportPath(path); err != nil {
302+
return module.Version{}, &invalidImportError{importPath: path, err: err}
303+
}
304+
291305
if pathIsStd {
292306
// This package isn't in the standard library, isn't in any module already
293307
// 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) {
299313
return module.Version{}, &ImportMissingError{Path: path}
300314
}
301315

316+
if cfg.BuildMod == "readonly" {
317+
var queryErr error
318+
if cfg.BuildModExplicit {
319+
queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod)
320+
} else if cfg.BuildModReason != "" {
321+
queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason)
322+
}
323+
return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr}
324+
}
325+
326+
// Look up module containing the package, for addition to the build list.
327+
// Goal is to determine the module, download it to dir,
328+
// and return m, dir, ImpportMissingError.
302329
fmt.Fprintf(os.Stderr, "go: finding module for package %s\n", path)
303330

304331
candidates, err := QueryPackage(ctx, path, "latest", CheckAllowed)

src/cmd/go/testdata/script/mod_build_info_err.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# Verifies golang.org/issue/34393.
33

44
go list -e -deps -f '{{with .Error}}{{.Pos}}: {{.Err}}{{end}}' ./main
5-
stdout 'bad[/\\]bad.go:3:8: malformed module path "🐧.example.com/string": invalid char ''🐧'''
5+
stdout 'bad[/\\]bad.go:3:8: malformed import path "🐧.example.com/string": invalid char ''🐧'''
66

77
-- go.mod --
88
module m

0 commit comments

Comments
 (0)