Skip to content

Commit f24ff38

Browse files
author
Jay Conrod
committed
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 <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Michael Matloob <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> Trust: Jay Conrod <[email protected]>
1 parent 4fdb98d commit f24ff38

File tree

4 files changed

+99
-63
lines changed

4 files changed

+99
-63
lines changed

src/cmd/go/internal/modfetch/pseudo.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ func PseudoVersion(major, older string, t time.Time, rev string) string {
7676
return v + incDecimal(patch) + "-0." + segment + build
7777
}
7878

79+
// ZeroPseudoVersion returns a pseudo-version with a zero timestamp and
80+
// revision, which may be used as a placeholder.
81+
func ZeroPseudoVersion(major string) string {
82+
return PseudoVersion(major, "", time.Time{}, "000000000000")
83+
}
84+
7985
// incDecimal returns the decimal string incremented by 1.
8086
func incDecimal(decimal string) string {
8187
// Scan right to left turning 9s to 0s until you find a digit to increment.
@@ -120,6 +126,12 @@ func IsPseudoVersion(v string) bool {
120126
return strings.Count(v, "-") >= 2 && semver.IsValid(v) && pseudoVersionRE.MatchString(v)
121127
}
122128

129+
// IsZeroPseudoVersion returns whether v is a pseudo-version with a zero base,
130+
// timestamp, and revision, as returned by ZeroPseudoVersion.
131+
func IsZeroPseudoVersion(v string) bool {
132+
return v == ZeroPseudoVersion(semver.Major(v))
133+
}
134+
123135
// PseudoVersionTime returns the time stamp of the pseudo-version v.
124136
// It returns an error if v is not a pseudo-version or if the time stamp
125137
// embedded in the pseudo-version is not a valid time.

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

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"path/filepath"
1616
"sort"
1717
"strings"
18-
"time"
1918

2019
"cmd/go/internal/cfg"
2120
"cmd/go/internal/fsys"
@@ -42,6 +41,10 @@ type ImportMissingError struct {
4241
// modules.
4342
isStd bool
4443

44+
// replaced the highest replaced version of the module where the replacement
45+
// contains the package. replaced is only set if the replacement is unused.
46+
replaced module.Version
47+
4548
// newMissingVersion is set to a newer version of Module if one is present
4649
// in the build list. When set, we can't automatically upgrade.
4750
newMissingVersion string
@@ -59,6 +62,14 @@ func (e *ImportMissingError) Error() string {
5962
return "cannot find module providing package " + e.Path
6063
}
6164

65+
if e.replaced.Path != "" {
66+
suggestArg := e.replaced.Path
67+
if !modfetch.IsZeroPseudoVersion(e.replaced.Version) {
68+
suggestArg = e.replaced.String()
69+
}
70+
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)
71+
}
72+
6273
suggestion := ""
6374
if !HasModRoot() {
6475
suggestion = ": working directory is not part of a module"
@@ -284,37 +295,6 @@ func importFromBuildList(ctx context.Context, path string) (m module.Version, di
284295
// Unlike QueryPattern, queryImport prefers to add a replaced version of a
285296
// module *before* checking the proxies for a version to add.
286297
func queryImport(ctx context.Context, path string) (module.Version, error) {
287-
pathIsStd := search.IsStandardImportPath(path)
288-
289-
if cfg.BuildMod == "readonly" {
290-
if pathIsStd {
291-
// If the package would be in the standard library and none of the
292-
// available replacement modules could concievably provide it, report it
293-
// as a missing standard-library package instead of complaining that
294-
// module lookups are disabled.
295-
maybeReplaced := false
296-
if index != nil {
297-
for p := range index.highestReplaced {
298-
if maybeInModule(path, p) {
299-
maybeReplaced = true
300-
break
301-
}
302-
}
303-
}
304-
if !maybeReplaced {
305-
return module.Version{}, &ImportMissingError{Path: path, isStd: true}
306-
}
307-
}
308-
309-
var queryErr error
310-
if cfg.BuildModExplicit {
311-
queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod)
312-
} else if cfg.BuildModReason != "" {
313-
queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason)
314-
}
315-
return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr}
316-
}
317-
318298
// To avoid spurious remote fetches, try the latest replacement for each
319299
// module (golang.org/issue/26241).
320300
if index != nil {
@@ -330,9 +310,9 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
330310
// used from within some other module, the user will be able to upgrade
331311
// the requirement to any real version they choose.
332312
if _, pathMajor, ok := module.SplitPathVersion(mp); ok && len(pathMajor) > 0 {
333-
mv = modfetch.PseudoVersion(pathMajor[1:], "", time.Time{}, "000000000000")
313+
mv = modfetch.ZeroPseudoVersion(pathMajor[1:])
334314
} else {
335-
mv = modfetch.PseudoVersion("v0", "", time.Time{}, "000000000000")
315+
mv = modfetch.ZeroPseudoVersion("v0")
336316
}
337317
}
338318
mods = append(mods, module.Version{Path: mp, Version: mv})
@@ -347,18 +327,23 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
347327
needSum := true
348328
root, isLocal, err := fetch(ctx, m, needSum)
349329
if err != nil {
350-
// Report fetch error as above.
330+
if sumErr := (*sumMissingError)(nil); errors.As(err, &sumErr) {
331+
return module.Version{}, &ImportMissingSumError{importPath: path}
332+
}
351333
return module.Version{}, err
352334
}
353335
if _, ok, err := dirInModule(path, m.Path, root, isLocal); err != nil {
354336
return m, err
355337
} else if ok {
338+
if cfg.BuildMod == "readonly" {
339+
return module.Version{}, &ImportMissingError{Path: path, replaced: m}
340+
}
356341
return m, nil
357342
}
358343
}
359344
if len(mods) > 0 && module.CheckPath(path) != nil {
360345
// The package path is not valid to fetch remotely,
361-
// so it can only exist if in a replaced module,
346+
// so it can only exist in a replaced module,
362347
// and we know from the above loop that it is not.
363348
return module.Version{}, &PackageNotInModuleError{
364349
Mod: mods[0],
@@ -369,7 +354,7 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
369354
}
370355
}
371356

372-
if pathIsStd {
357+
if search.IsStandardImportPath(path) {
373358
// This package isn't in the standard library, isn't in any module already
374359
// in the build list, and isn't in any other module that the user has
375360
// shimmed in via a "replace" directive.
@@ -380,6 +365,19 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
380365
return module.Version{}, &ImportMissingError{Path: path, isStd: true}
381366
}
382367

368+
if cfg.BuildMod == "readonly" {
369+
// In readonly mode, we can't write go.mod, so we shouldn't try to look up
370+
// the module. If readonly mode was enabled explicitly, include that in
371+
// the error message.
372+
var queryErr error
373+
if cfg.BuildModExplicit {
374+
queryErr = fmt.Errorf("import lookup disabled by -mod=%s", cfg.BuildMod)
375+
} else if cfg.BuildModReason != "" {
376+
queryErr = fmt.Errorf("import lookup disabled by -mod=%s\n\t(%s)", cfg.BuildMod, cfg.BuildModReason)
377+
}
378+
return module.Version{}, &ImportMissingError{Path: path, QueryErr: queryErr}
379+
}
380+
383381
// Look up module containing the package, for addition to the build list.
384382
// Goal is to determine the module, download it to dir,
385383
// and return m, dir, ImpportMissingError.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ stderr '^go get example: package example is not in GOROOT \(.*\)$'
8787
go mod edit -replace [email protected]=./example
8888

8989
! go list example
90-
stderr '^no required module provides package example; try ''go get -d example'' to add it$'
90+
stderr '^module example provides package example and is replaced but not required; try ''go get -d example@v0.1.0'' to add it$'
9191

9292
go get -d example
9393
go list -m example
Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,62 @@
1-
# Regression test for https://golang.org/issue/41577:
2-
# 'go list -mod=readonly' should not resolve missing packages from
3-
# available replacements.
1+
# Check that with -mod=readonly, when we load a package in a module that is
2+
# replaced but not required, we emit an error with the command to add the
3+
# requirement.
4+
# Verifies golang.org/issue/41416, golang.org/issue/41577.
5+
cp go.mod go.mod.orig
6+
7+
# Replace all versions of a module without requiring it.
8+
# With -mod=mod, we'd add a requirement for a "zero" pseudo-version, but we
9+
# can't in readonly mode, since its go.mod may alter the build list.
10+
go mod edit -replace rsc.io/quote=./quote
11+
! go list rsc.io/quote
12+
stderr '^module rsc.io/quote provides package rsc.io/quote and is replaced but not required; try ''go get -d rsc.io/quote'' to add it$'
13+
go get -d rsc.io/quote
14+
cmp go.mod go.mod.latest
15+
go list rsc.io/quote
16+
cp go.mod.orig go.mod
17+
18+
# Same test with a specific version.
19+
go mod edit -replace rsc.io/[email protected]=./quote
20+
! go list rsc.io/quote
21+
stderr '^module rsc.io/quote provides package rsc.io/quote and is replaced but not required; try ''go get -d rsc.io/[email protected]'' to add it$'
22+
go get -d rsc.io/[email protected]
23+
cmp go.mod go.mod.specific
24+
go list rsc.io/quote
25+
cp go.mod.orig go.mod
26+
27+
# If there are multiple versions, the highest is suggested.
28+
go mod edit -replace rsc.io/[email protected]=./quote
29+
go mod edit -replace rsc.io/[email protected]=./quote
30+
! go list rsc.io/quote
31+
stderr '^module rsc.io/quote provides package rsc.io/quote and is replaced but not required; try ''go get -d rsc.io/[email protected]'' to add it$'
432

5-
# Control case: when there is no replacement, 'go list' of a missing package
6-
# fails due to defaulting to '-mod=readonly'.
33+
-- go.mod --
34+
module m
735

8-
! go list example.com/x
9-
stderr '^no required module provides package example.com/x; try ''go get -d example.com/x'' to add it$'
36+
go 1.16
37+
-- go.mod.latest --
38+
module m
1039

11-
# When an unused replacement is added, 'go list' should still fail in the same way.
12-
# (Previously, it would resolve the missing import despite -mod=readonly.)
40+
go 1.16
1341

14-
go mod edit -replace=example.com/[email protected]=./x
15-
go mod edit -replace=example.com/[email protected]=./x
16-
! go list example.com/x
17-
stderr '^no required module provides package example.com/x; try ''go get -d example.com/x'' to add it$'
42+
replace rsc.io/quote => ./quote
1843

19-
# The command suggested by 'go list' should successfully resolve using the replacement.
44+
require rsc.io/quote v1.5.2 // indirect
45+
-- go.mod.specific --
46+
module m
2047

21-
go get -d example.com/x
22-
go list example.com/x
23-
go list -m example.com/x
24-
stdout '^example.com/x v0.2.0 '
48+
go 1.16
2549

50+
replace rsc.io/quote v1.0.0-doesnotexist => ./quote
2651

27-
-- go.mod --
28-
module example.com
52+
require rsc.io/quote v1.0.0-doesnotexist // indirect
53+
-- use.go --
54+
package use
2955

30-
go 1.16
31-
-- x/go.mod --
32-
module example.com/x
56+
import _ "rsc.io/quote"
57+
-- quote/go.mod --
58+
module rsc.io/quote
3359

3460
go 1.16
35-
-- x/x.go --
36-
package x
61+
-- quote/quote.go --
62+
package quote

0 commit comments

Comments
 (0)