Skip to content

Commit cd91ab5

Browse files
author
Bryan C. Mills
committed
cmd/go/internal/modload: fix spurious import resolution error
Due to a bug in CL 173017, if QueryPackages found multiple candidates for the given package and *at least* one of those candidates was not available to add, we would reject *all* such candidates — even those that were still viable. Now, we return the first viable candidate, and only return an error if *no* candidate is viable given the current build list. Fixes #41113 Change-Id: Idb2e77244be7c0f5dd511efb142c3059925d7336 Reviewed-on: https://go-review.googlesource.com/c/go/+/251446 Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jay Conrod <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
1 parent 2c8d2a0 commit cd91ab5

5 files changed

+76
-12
lines changed

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

+14-12
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,10 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
312312
}
313313
}
314314

315-
m := candidates[0].Mod
316-
newMissingVersion := ""
317-
for _, c := range candidates {
315+
candidate0MissingVersion := ""
316+
for i, c := range candidates {
318317
cm := c.Mod
318+
canAdd := true
319319
for _, bm := range buildList {
320320
if bm.Path == cm.Path && semver.Compare(bm.Version, cm.Version) > 0 {
321321
// QueryPackage proposed that we add module cm to provide the package,
@@ -326,20 +326,22 @@ func queryImport(ctx context.Context, path string) (module.Version, error) {
326326
// version (e.g., v1.0.0) of a module, but we have a newer version
327327
// of the same module in the build list (e.g., v1.0.1-beta), and
328328
// the package is not present there.
329-
//
330-
// TODO(#41113): This is probably incorrect when there are multiple
331-
// candidates, such as when a nested module is split out but only one
332-
// half of the split is tagged.
333-
m = cm
334-
newMissingVersion = bm.Version
329+
canAdd = false
330+
if i == 0 {
331+
candidate0MissingVersion = bm.Version
332+
}
335333
break
336334
}
337335
}
336+
if canAdd {
337+
return cm, nil
338+
}
338339
}
339-
if newMissingVersion != "" {
340-
return m, &ImportMissingError{Path: path, Module: m, newMissingVersion: newMissingVersion}
340+
return module.Version{}, &ImportMissingError{
341+
Path: path,
342+
Module: candidates[0].Mod,
343+
newMissingVersion: candidate0MissingVersion,
341344
}
342-
return m, nil
343345
}
344346

345347
// maybeInModule reports whether, syntactically,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
Written by hand.
2+
Test case for getting a package that has been moved to a nested module,
3+
with a +incompatible verison (and thus no go.mod file) at the root module.
4+
5+
-- .mod --
6+
module example.com/split-incompatible/subpkg
7+
-- .info --
8+
{"Version": "v0.1.0"}
9+
-- go.mod --
10+
module example.com/split-incompatible/subpkg
11+
12+
go 1.16
13+
-- subpkg.go --
14+
package subpkg
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Written by hand.
2+
Test case for getting a package that has been moved to a nested module,
3+
with a +incompatible verison (and thus no go.mod file) at the root module.
4+
5+
-- .mod --
6+
module example.com/split-incompatible
7+
-- .info --
8+
{"Version": "v2.0.0+incompatible"}
9+
-- subpkg/subpkg.go --
10+
package subpkg
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Written by hand.
2+
Test case for getting a package that has been moved to a nested module,
3+
with a +incompatible verison (and thus no go.mod file) at the root module.
4+
5+
-- .mod --
6+
module example.com/split-incompatible
7+
-- .info --
8+
{"Version": "v2.1.0-pre+incompatible"}
9+
-- README.txt --
10+
subpkg has moved to module example.com/split-incompatible/subpkg
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# Regression test for https://golang.org/issue/41113.
2+
#
3+
# When resolving a missing import path, the inability to add the package from
4+
# one module path should not interfere with adding a nested path.
5+
6+
# Initially, our module depends on split-incompatible v2.1.0-pre+incompatible,
7+
# from which an imported package has been removed (and relocated to the nested
8+
# split-incompatible/subpkg module). modload.QueryPackage will suggest
9+
# split-incompatible v2.0.0+incompatible, which we cannot use (because it would
10+
# be an implicit downgrade), and split-incompatible/subpkg v0.1.0, which we
11+
# *should* use.
12+
13+
go mod tidy
14+
15+
go list -m all
16+
stdout '^example.com/split-incompatible/subpkg v0\.1\.0$'
17+
! stdout '^example.com/split-incompatible .*'
18+
19+
-- go.mod --
20+
module golang.org/issue/41113
21+
22+
go 1.16
23+
24+
require example.com/split-incompatible v2.1.0-pre+incompatible
25+
-- x.go --
26+
package issue41113
27+
28+
import _ "example.com/split-incompatible/subpkg"

0 commit comments

Comments
 (0)