Skip to content

Commit 32b845b

Browse files
rogpepperogpeppe
committed
internal/mod/modpkgload: add FindPackageLocations
This factors out some of the logic that was previously internal to Packages.importFromModules into a place where it can be reused to find a module for an absolute import path. Also define a `PackageLoc` type to make it more convenient to return a slice of package locations. While we're about here, remove the unused `altMods` field. For #3618. Signed-off-by: Roger Peppe <[email protected]> Change-Id: Ia15d6447a75a6f979a02edb313da57d1fedb0962 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1211886 Reviewed-by: Daniel Martí <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Unity-Result: CUE porcuepine <[email protected]>
1 parent d9aaf0a commit 32b845b

File tree

6 files changed

+234
-99
lines changed

6 files changed

+234
-99
lines changed

‎cmd/cue/cmd/testdata/script/modtidy_ambiguous_import.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
cmp stderr want-stderr
77

88
-- want-stderr --
9-
failed to resolve "example.com/foo/bar@v0": ambiguous import: found package example.com/foo/bar@v0 in multiple modules:
9+
failed to resolve "example.com/foo/bar@v0": ambiguous import: found package example.com/foo/bar@v0 in multiple locations:
1010
example.com@v0 v0.0.1 (foo/bar)
1111
example.com/foo@v0 v0.1.0 (bar)
1212
-- cue.mod/module.cue --

‎cmd/cue/cmd/testdata/script/registry_with_pkg_conflict.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
cmp stderr expect-stderr
77

88
-- expect-stderr --
9-
test.org@v0: import failed: cannot find package "example.com/e": ambiguous import: found package example.com/e in multiple modules:
9+
test.org@v0: import failed: cannot find package "example.com/e": ambiguous import: found package example.com/e in multiple locations:
1010
example.com/e@v0 v0.0.1 (.)
1111
local (cue.mod/pkg/example.com/e):
1212
./main.cue:2:8

‎internal/mod/modpkgload/import.go

+152-93
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"io/fs"
8+
"iter"
89
"path"
910
"path/filepath"
1011
"slices"
@@ -30,11 +31,11 @@ import (
3031
// If the package is present in exactly one module, importFromModules will
3132
// return the module, its root directory, and a list of other modules that
3233
// lexically could have provided the package but did not.
33-
func (pkgs *Packages) importFromModules(ctx context.Context, pkgPath string) (m module.Version, pkgLocs []module.SourceLoc, altMods []module.Version, err error) {
34-
fail := func(err error) (module.Version, []module.SourceLoc, []module.Version, error) {
35-
return module.Version{}, []module.SourceLoc(nil), nil, err
34+
func (pkgs *Packages) importFromModules(ctx context.Context, pkgPath string) (m module.Version, pkgLocs []module.SourceLoc, err error) {
35+
fail := func(err error) (module.Version, []module.SourceLoc, error) {
36+
return module.Version{}, nil, err
3637
}
37-
failf := func(format string, args ...interface{}) (module.Version, []module.SourceLoc, []module.Version, error) {
38+
failf := func(format string, args ...interface{}) (module.Version, []module.SourceLoc, error) {
3839
return fail(fmt.Errorf(format, args...))
3940
}
4041
// Note: we don't care about the package qualifier at this point
@@ -55,16 +56,47 @@ func (pkgs *Packages) importFromModules(ctx context.Context, pkgPath string) (m
5556
}
5657

5758
// Check each module on the build list.
58-
var locs [][]module.SourceLoc
59-
var mods []module.Version
59+
var locs []PackageLoc
6060
var mg *modrequirements.ModuleGraph
61+
versionForModule := func(ctx context.Context, prefix string) (module.Version, error) {
62+
var (
63+
v string
64+
ok bool
65+
)
66+
pkgVersion := pathParts.Version
67+
if pkgVersion == "" {
68+
if pkgVersion, _ = pkgs.requirements.DefaultMajorVersion(prefix); pkgVersion == "" {
69+
return module.Version{}, nil
70+
}
71+
}
72+
prefixPath := prefix + "@" + pkgVersion
73+
// Note: mg is nil the first time around the loop.
74+
if mg == nil {
75+
v, ok = pkgs.requirements.RootSelected(prefixPath)
76+
} else {
77+
v, ok = mg.Selected(prefixPath), true
78+
}
79+
if !ok || v == "none" {
80+
// No possible module
81+
return module.Version{}, nil
82+
}
83+
m, err := module.NewVersion(prefixPath, v)
84+
if err != nil {
85+
// Not all package paths are valid module versions,
86+
// but a parent might be.
87+
return module.Version{}, nil
88+
}
89+
return m, nil
90+
}
6191
localPkgLocs, err := pkgs.findLocalPackage(pkgPathOnly)
6292
if err != nil {
6393
return fail(err)
6494
}
6595
if len(localPkgLocs) > 0 {
66-
mods = append(mods, module.MustNewVersion("local", ""))
67-
locs = append(locs, localPkgLocs)
96+
locs = append(locs, PackageLoc{
97+
Module: module.MustNewVersion("local", ""),
98+
Locs: localPkgLocs,
99+
})
68100
}
69101

70102
// Iterate over possible modules for the path, not all selected modules.
@@ -81,67 +113,27 @@ func (pkgs *Packages) importFromModules(ctx context.Context, pkgPath string) (m
81113
// to load the package using the full
82114
// requirements in mg.
83115
for {
84-
var altMods []module.Version
85-
// TODO we could probably do this loop concurrently.
86-
87-
for prefix := pkgPathOnly; prefix != "."; prefix = path.Dir(prefix) {
88-
var (
89-
v string
90-
ok bool
91-
)
92-
pkgVersion := pathParts.Version
93-
if pkgVersion == "" {
94-
if pkgVersion, _ = pkgs.requirements.DefaultMajorVersion(prefix); pkgVersion == "" {
95-
continue
96-
}
97-
}
98-
prefixPath := prefix + "@" + pkgVersion
99-
if mg == nil {
100-
v, ok = pkgs.requirements.RootSelected(prefixPath)
101-
} else {
102-
v, ok = mg.Selected(prefixPath), true
103-
}
104-
if !ok || v == "none" {
105-
continue
106-
}
107-
m, err := module.NewVersion(prefixPath, v)
108-
if err != nil {
109-
// Not all package paths are valid module versions,
110-
// but a parent might be.
111-
continue
112-
}
113-
mloc, isLocal, err := pkgs.fetch(ctx, m)
114-
if err != nil {
115-
// Report fetch error.
116-
// Note that we don't know for sure this module is necessary,
117-
// but it certainly _could_ provide the package, and even if we
118-
// continue the loop and find the package in some other module,
119-
// we need to look at this module to make sure the import is
120-
// not ambiguous.
121-
return fail(fmt.Errorf("cannot fetch %v: %v", m, err))
122-
}
123-
if loc, ok, err := locInModule(pkgPathOnly, prefix, mloc, isLocal); err != nil {
124-
return fail(fmt.Errorf("cannot find package: %v", err))
125-
} else if ok {
126-
mods = append(mods, m)
127-
locs = append(locs, []module.SourceLoc{loc})
128-
} else {
129-
altMods = append(altMods, m)
130-
}
116+
// Note: if fetch fails, we return an error:
117+
// we don't know for sure this module is necessary,
118+
// but it certainly _could_ provide the package, and even if we
119+
// continue the loop and find the package in some other module,
120+
// we need to look at this module to make sure the import is
121+
// not ambiguous.
122+
plocs, err := FindPackageLocations(ctx, pkgPath, versionForModule, pkgs.fetch)
123+
if err != nil {
124+
return fail(err)
131125
}
132-
133-
if len(mods) > 1 {
126+
locs = append(locs, plocs...)
127+
if len(locs) > 1 {
134128
// We produce the list of directories from longest to shortest candidate
135129
// module path, but the AmbiguousImportError should report them from
136130
// shortest to longest. Reverse them now.
137-
slices.Reverse(mods)
138131
slices.Reverse(locs)
139-
return fail(&AmbiguousImportError{ImportPath: pkgPath, Locations: locs, Modules: mods})
132+
return fail(&AmbiguousImportError{ImportPath: pkgPath, Locations: locs})
140133
}
141-
142-
if len(mods) == 1 {
134+
if len(locs) == 1 {
143135
// We've found the unique module containing the package.
144-
return mods[0], locs[0], altMods, nil
136+
return locs[0].Module, locs[0].Locs, nil
145137
}
146138

147139
if mg != nil {
@@ -163,18 +155,81 @@ func (pkgs *Packages) importFromModules(ctx context.Context, pkgPath string) (m
163155
}
164156
}
165157

166-
// locInModule returns the location that would hold the package named by the given path,
167-
// if it were in the module with module path mpath and root location mloc.
168-
// If pkgPath is syntactically not within mpath,
169-
// or if mdir is a local file tree (isLocal == true) and the directory
170-
// that would hold path is in a sub-module (covered by a go.mod below mdir),
158+
// PackageLoc holds a module version and a location of a package
159+
// within that module.
160+
type PackageLoc struct {
161+
Module module.Version
162+
// Locs holds the source locations of the package. There is always
163+
// at least one element; there can be more than one when the
164+
// module path is "local" (for exampe packages inside cue.mod/pkg).
165+
Locs []module.SourceLoc
166+
}
167+
168+
// FindPackageLocations finds possible module candidates for a given import path.
169+
//
170+
// It tries each parent of the import path as a possible module location,
171+
// using versionForModule to determine a version for that module
172+
// and fetch to fetch the location for a given module version.
173+
//
174+
// versionForModule may indicate that there is no possible module
175+
// for a given path by returning the zero version and a nil error.
176+
//
177+
// The fetch function also reports whether the location is "local"
178+
// to the current module, allowing some checks to be skipped when false.
179+
//
180+
// It returns possible locations for the package. Each location may or may
181+
// not contain the package itself, although it will hold some CUE files.
182+
func FindPackageLocations(
183+
ctx context.Context,
184+
importPath string,
185+
versionForModule func(ctx context.Context, prefixPath string) (module.Version, error),
186+
fetch func(ctx context.Context, m module.Version) (loc module.SourceLoc, isLocal bool, err error),
187+
) ([]PackageLoc, error) {
188+
ip := ast.ParseImportPath(importPath)
189+
var locs []PackageLoc
190+
for prefix := range pathAncestors(ip.Path) {
191+
v, err := versionForModule(ctx, prefix)
192+
if err != nil {
193+
return nil, err
194+
}
195+
if !v.IsValid() {
196+
continue
197+
}
198+
mloc, isLocal, err := fetch(ctx, v)
199+
if err != nil {
200+
return nil, fmt.Errorf("cannot fetch %v: %w", v, err)
201+
}
202+
if mloc.FS == nil {
203+
// Not found but not an error.
204+
continue
205+
}
206+
loc, ok, err := locInModule(ip.Path, prefix, mloc, isLocal)
207+
if err != nil {
208+
return nil, fmt.Errorf("cannot find package: %v", err)
209+
}
210+
if ok {
211+
locs = append(locs, PackageLoc{
212+
Module: v,
213+
Locs: []module.SourceLoc{loc},
214+
})
215+
}
216+
}
217+
return locs, nil
218+
}
219+
220+
// locInModule returns the location that would hold the package named by
221+
// the given path, if it were in the module with module path mpath and
222+
// root location mloc. If pkgPath is syntactically not within mpath, or
223+
// if mdir is a local file tree (isLocal == true) and the directory that
224+
// would hold path is in a sub-module (covered by a cue.mod below mdir),
171225
// locInModule returns "", false, nil.
172226
//
173-
// Otherwise, locInModule returns the name of the directory where
174-
// CUE source files would be expected, along with a boolean indicating
175-
// whether there are in fact CUE source files in that directory.
176-
// A non-nil error indicates that the existence of the directory and/or
177-
// source files could not be determined, for example due to a permission error.
227+
// Otherwise, locInModule returns the name of the directory where CUE
228+
// source files would be expected, along with a boolean indicating
229+
// whether there are in fact CUE source files in that directory. A
230+
// non-nil error indicates that the existence of the directory and/or
231+
// source files could not be determined, for example due to a permission
232+
// error.
178233
func locInModule(pkgPath, mpath string, mloc module.SourceLoc, isLocal bool) (loc module.SourceLoc, haveCUEFiles bool, err error) {
179234
loc.FS = mloc.FS
180235

@@ -279,44 +334,48 @@ func (pkgs *Packages) fetch(ctx context.Context, mod module.Version) (loc module
279334
if mod == pkgs.mainModuleVersion {
280335
return pkgs.mainModuleLoc, true, nil
281336
}
282-
283337
loc, err = pkgs.registry.Fetch(ctx, mod)
284338
return loc, false, err
285339
}
286340

341+
// pathAncestors returns an iterator over all the ancestors
342+
// of p, including p itself.
343+
func pathAncestors(p string) iter.Seq[string] {
344+
return func(yield func(s string) bool) {
345+
for {
346+
if !yield(p) {
347+
return
348+
}
349+
prev := p
350+
p = path.Dir(p)
351+
if p == "." || p == prev {
352+
return
353+
}
354+
}
355+
}
356+
}
357+
287358
// An AmbiguousImportError indicates an import of a package found in multiple
288359
// modules in the build list, or found in both the main module and its vendor
289360
// directory.
290361
type AmbiguousImportError struct {
291362
ImportPath string
292-
Locations [][]module.SourceLoc
293-
Modules []module.Version // Either empty or 1:1 with Dirs.
363+
Locations []PackageLoc
294364
}
295365

296366
func (e *AmbiguousImportError) Error() string {
297-
locType := "modules"
298-
if len(e.Modules) == 0 {
299-
locType = "locations"
300-
}
301-
302367
var buf strings.Builder
303-
fmt.Fprintf(&buf, "ambiguous import: found package %s in multiple %s:", e.ImportPath, locType)
368+
fmt.Fprintf(&buf, "ambiguous import: found package %s in multiple locations:", e.ImportPath)
304369

305-
for i, loc := range e.Locations {
370+
for _, loc := range e.Locations {
306371
buf.WriteString("\n\t")
307-
if i < len(e.Modules) {
308-
m := e.Modules[i]
309-
buf.WriteString(m.Path())
310-
if m.Version() != "" {
311-
fmt.Fprintf(&buf, " %s", m.Version())
312-
}
313-
// TODO work out how to present source locations in error messages.
314-
fmt.Fprintf(&buf, " (%s)", loc[0].Dir)
315-
} else {
316-
buf.WriteString(loc[0].Dir)
372+
buf.WriteString(loc.Module.Path())
373+
if v := loc.Module.Version(); v != "" {
374+
fmt.Fprintf(&buf, " %s", v)
317375
}
376+
// TODO work out how to present source locations in error messages.
377+
fmt.Fprintf(&buf, " (%s)", loc.Locs[0].Dir)
318378
}
319-
320379
return buf.String()
321380
}
322381

‎internal/mod/modpkgload/pkgload.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ type Package struct {
115115
imports []*Package // packages imported by this one
116116
inStd bool
117117
fromExternal bool
118-
altMods []module.Version // modules that could have contained the package but did not
119118

120119
// Populated by postprocessing in [Packages.buildStacks]:
121120
stack *Package // package importing this one in minimal import stack for this pkg
@@ -263,7 +262,7 @@ func (pkgs *Packages) load(ctx context.Context, pkg *Package) {
263262
return
264263
}
265264
pkg.fromExternal = pkg.mod != pkgs.mainModuleVersion
266-
pkg.mod, pkg.locs, pkg.altMods, pkg.err = pkgs.importFromModules(ctx, pkg.path)
265+
pkg.mod, pkg.locs, pkg.err = pkgs.importFromModules(ctx, pkg.path)
267266
if pkg.err != nil {
268267
return
269268
}

0 commit comments

Comments
 (0)