From: Bryan C. Mills Date: Sat, 29 Aug 2020 01:32:05 +0000 (-0400) Subject: cmd/go/internal/modload: fix spurious import resolution error X-Git-Tag: go1.16beta1~1098 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=cd91ab5d9601c975286f1ac83cd289e34aa117f8;p=gostls13.git 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 TryBot-Result: Gobot Gobot Reviewed-by: Jay Conrod Reviewed-by: Michael Matloob --- diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go index e04d66c5b1..c625184b8b 100644 --- a/src/cmd/go/internal/modload/import.go +++ b/src/cmd/go/internal/modload/import.go @@ -312,10 +312,10 @@ func queryImport(ctx context.Context, path string) (module.Version, error) { } } - m := candidates[0].Mod - newMissingVersion := "" - for _, c := range candidates { + candidate0MissingVersion := "" + for i, c := range candidates { cm := c.Mod + canAdd := true for _, bm := range buildList { if bm.Path == cm.Path && semver.Compare(bm.Version, cm.Version) > 0 { // 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) { // version (e.g., v1.0.0) of a module, but we have a newer version // of the same module in the build list (e.g., v1.0.1-beta), and // the package is not present there. - // - // TODO(#41113): This is probably incorrect when there are multiple - // candidates, such as when a nested module is split out but only one - // half of the split is tagged. - m = cm - newMissingVersion = bm.Version + canAdd = false + if i == 0 { + candidate0MissingVersion = bm.Version + } break } } + if canAdd { + return cm, nil + } } - if newMissingVersion != "" { - return m, &ImportMissingError{Path: path, Module: m, newMissingVersion: newMissingVersion} + return module.Version{}, &ImportMissingError{ + Path: path, + Module: candidates[0].Mod, + newMissingVersion: candidate0MissingVersion, } - return m, nil } // maybeInModule reports whether, syntactically, diff --git a/src/cmd/go/testdata/mod/example.com_split-incompatible_subpkg_v0.1.0.txt b/src/cmd/go/testdata/mod/example.com_split-incompatible_subpkg_v0.1.0.txt new file mode 100644 index 0000000000..8f9e49176c --- /dev/null +++ b/src/cmd/go/testdata/mod/example.com_split-incompatible_subpkg_v0.1.0.txt @@ -0,0 +1,14 @@ +Written by hand. +Test case for getting a package that has been moved to a nested module, +with a +incompatible verison (and thus no go.mod file) at the root module. + +-- .mod -- +module example.com/split-incompatible/subpkg +-- .info -- +{"Version": "v0.1.0"} +-- go.mod -- +module example.com/split-incompatible/subpkg + +go 1.16 +-- subpkg.go -- +package subpkg diff --git a/src/cmd/go/testdata/mod/example.com_split-incompatible_v2.0.0+incompatible.txt b/src/cmd/go/testdata/mod/example.com_split-incompatible_v2.0.0+incompatible.txt new file mode 100644 index 0000000000..35c3f27710 --- /dev/null +++ b/src/cmd/go/testdata/mod/example.com_split-incompatible_v2.0.0+incompatible.txt @@ -0,0 +1,10 @@ +Written by hand. +Test case for getting a package that has been moved to a nested module, +with a +incompatible verison (and thus no go.mod file) at the root module. + +-- .mod -- +module example.com/split-incompatible +-- .info -- +{"Version": "v2.0.0+incompatible"} +-- subpkg/subpkg.go -- +package subpkg diff --git a/src/cmd/go/testdata/mod/example.com_split-incompatible_v2.1.0-pre+incompatible.txt b/src/cmd/go/testdata/mod/example.com_split-incompatible_v2.1.0-pre+incompatible.txt new file mode 100644 index 0000000000..917fc0f559 --- /dev/null +++ b/src/cmd/go/testdata/mod/example.com_split-incompatible_v2.1.0-pre+incompatible.txt @@ -0,0 +1,10 @@ +Written by hand. +Test case for getting a package that has been moved to a nested module, +with a +incompatible verison (and thus no go.mod file) at the root module. + +-- .mod -- +module example.com/split-incompatible +-- .info -- +{"Version": "v2.1.0-pre+incompatible"} +-- README.txt -- +subpkg has moved to module example.com/split-incompatible/subpkg diff --git a/src/cmd/go/testdata/script/mod_import_issue41113.txt b/src/cmd/go/testdata/script/mod_import_issue41113.txt new file mode 100644 index 0000000000..e98ac63d48 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_import_issue41113.txt @@ -0,0 +1,28 @@ +# Regression test for https://golang.org/issue/41113. +# +# When resolving a missing import path, the inability to add the package from +# one module path should not interfere with adding a nested path. + +# Initially, our module depends on split-incompatible v2.1.0-pre+incompatible, +# from which an imported package has been removed (and relocated to the nested +# split-incompatible/subpkg module). modload.QueryPackage will suggest +# split-incompatible v2.0.0+incompatible, which we cannot use (because it would +# be an implicit downgrade), and split-incompatible/subpkg v0.1.0, which we +# *should* use. + +go mod tidy + +go list -m all +stdout '^example.com/split-incompatible/subpkg v0\.1\.0$' +! stdout '^example.com/split-incompatible .*' + +-- go.mod -- +module golang.org/issue/41113 + +go 1.16 + +require example.com/split-incompatible v2.1.0-pre+incompatible +-- x.go -- +package issue41113 + +import _ "example.com/split-incompatible/subpkg"