From 68fea2d14232615dea2105ca56ceee7e4236573f Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Fri, 24 May 2024 17:50:46 +0100 Subject: [PATCH] cue/load: evaluate only those packages which are needed DO NOT REVIEW This change changes the cue/load logic to expand package wildcards before invoking the `modpkgload.LoadPackages` resolution logic. This involves moving the wildcard expansion code into a new place, independent of the `cue/load.loader` which is created after doing that. We use the `modimports.AllModuleFiles` logic to enumerate the packages matched by a wildcard. All tests pass apart from the ones involving `cue import`, which expose a significant flaw in the above approach: `modimports.AllModuleFiles` only looks for CUE files, but `cue import` relies on the fact that package patterns will currently match directories that do not contain any CUE files. Fixes #3155 Signed-off-by: Roger Peppe Change-Id: I9f4b210eb0588e21ae942a97d7cf34c9887363dc --- .../cmd/testdata/script/cmd_filetypes.txtar | 11 +- cmd/cue/cmd/testdata/script/fmt.txtar | 4 +- cmd/cue/cmd/testdata/script/issue174.txtar | 3 +- ...le_packages_no_matching_path_element.txtar | 10 +- ...e_packages_one_matching_path_element.txtar | 2 +- ...esolution_path_element_invalid_ident.txtar | 6 +- ...le_package_not_matching_path_element.txtar | 8 +- cue/load/config.go | 12 +- cue/load/import.go | 53 +-- cue/load/import_test.go | 24 +- cue/load/instances.go | 45 +-- cue/load/loader_common.go | 8 +- cue/load/loader_test.go | 34 +- cue/load/search.go | 333 ++++++++++++++++++ cue/load/testdata/testfetch/simple.txtar | 2 +- .../testmod-external/cue.mod/module.cue | 2 +- cue/load/testdata/testmod/other/main.cue | 4 +- .../testdata/testmod/toolonly/foo_tool.cue | 2 +- internal/mod/modimports/modimports.go | 120 +++++-- internal/mod/modimports/testdata/simple.txtar | 4 + .../testdata/tidy/canuseprerelease.txtar | 4 +- ...-major-version-with-explicit-default.txtar | 2 +- .../modload/testdata/tidy/preferstable.txtar | 2 +- internal/mod/modpkgload/pkgload.go | 16 +- .../mod/modpkgload/testdata/multidir.txtar | 38 ++ internal/tdtest/tdtest.go | 5 +- 26 files changed, 611 insertions(+), 143 deletions(-) create mode 100644 internal/mod/modpkgload/testdata/multidir.txtar diff --git a/cmd/cue/cmd/testdata/script/cmd_filetypes.txtar b/cmd/cue/cmd/testdata/script/cmd_filetypes.txtar index 65b7f3c32b2..2563233ba0b 100644 --- a/cmd/cue/cmd/testdata/script/cmd_filetypes.txtar +++ b/cmd/cue/cmd/testdata/script/cmd_filetypes.txtar @@ -29,11 +29,9 @@ exec cue export ./somepkg/ cmp stdout export.golden # TODO(mvdan): we should support loading packages from absolute directories. ! exec cue export ${WORK}${/}somepkg -[!windows] stderr 'no location found for package ".*somepkg"' -[windows] stderr 'no encoding specified for file ".*somepkg.*"' +stderr 'cannot use absolute directory ".*somepkg" as package path' ! exec cue export ${WORK}${/}somepkg${/} -[!windows] stderr 'no dependency found for package ".*somepkg"' -[windows] stderr 'no encoding specified for file ".*somepkg.*"' +stderr 'cannot use absolute directory ".*somepkg.*" as package path' # Now do some of the same filename tests, but for a file without any extension. cp somefile.cue somefile @@ -54,11 +52,10 @@ stderr 'standard library import path "somefile" cannot be imported as a CUE pack # TODO(mvdan): "cannot find package" is perhaps confusing to the user given that the file exists. ! exec cue export ./somefile -stderr 'cannot find package "\./somefile"' +stderr '\./somefile refers to a file not a package directory' # TODO(mvdan): we probably should not behave differently on Windows. ! exec cue export ${WORK}${/}somefile -[!windows] stderr 'no location found for package ".*somefile"' -[windows] stderr 'no encoding specified for file ".*somefile"' +stderr 'cannot use absolute directory ".*somefile" as package path' -- cue.mod/module.cue -- module: "mod.test/test" diff --git a/cmd/cue/cmd/testdata/script/fmt.txtar b/cmd/cue/cmd/testdata/script/fmt.txtar index d79f53539ca..78891c70d9c 100644 --- a/cmd/cue/cmd/testdata/script/fmt.txtar +++ b/cmd/cue/cmd/testdata/script/fmt.txtar @@ -10,7 +10,7 @@ cmp issue1744/invalid.cue issue1744/invalid.cue.orig -- expect-stderr -- expected 'STRING', found '.': - ./fmt/error.cue:1:9 + fmt/error.cue:1:9 -- fmt/error.cue -- import a.b "foo" @@ -21,4 +21,4 @@ bb: 3 ~ -- issue1744/stderr-golden -- illegal character U+007E '~': - ./issue1744/invalid.cue:1:1 \ No newline at end of file + ./issue1744/invalid.cue:1:1 diff --git a/cmd/cue/cmd/testdata/script/issue174.txtar b/cmd/cue/cmd/testdata/script/issue174.txtar index 9d42bee1ae4..086c6b95b5f 100644 --- a/cmd/cue/cmd/testdata/script/issue174.txtar +++ b/cmd/cue/cmd/testdata/script/issue174.txtar @@ -1,7 +1,8 @@ ! exec cue export ./issue174 cmp stderr expect-stderr -- expect-stderr -- -cannot enumerate all module imports: invalid import path "'foo'" in issue174/issue174.cue +build constraints exclude all CUE files in ./issue174: + issue174/issue174.cue: no package name -- cue.mod/module.cue -- module: "mod.test" language: version: "v0.9.0" diff --git a/cmd/cue/cmd/testdata/script/pkg_resolution_multiple_packages_no_matching_path_element.txtar b/cmd/cue/cmd/testdata/script/pkg_resolution_multiple_packages_no_matching_path_element.txtar index be602535a83..467957d7642 100644 --- a/cmd/cue/cmd/testdata/script/pkg_resolution_multiple_packages_no_matching_path_element.txtar +++ b/cmd/cue/cmd/testdata/script/pkg_resolution_multiple_packages_no_matching_path_element.txtar @@ -36,13 +36,7 @@ package z z: 5 -- import_stderr.golden -- -import failed: build constraints exclude all CUE files in mod.com/x: - root.cue: package is root, want x - x/y.cue: package is y, want x - x/z.cue: package is z, want x: +import failed: cannot find package "mod.com/x": no files in package directory with package name "x": ./root.cue:3:8 -- absolute_stderr.golden -- -build constraints exclude all CUE files in mod.com/x: - root.cue: package is root, want x - x/y.cue: package is y, want x - x/z.cue: package is z, want x +cannot find package "mod.com/x": no files in package directory with package name "x" diff --git a/cmd/cue/cmd/testdata/script/pkg_resolution_multiple_packages_one_matching_path_element.txtar b/cmd/cue/cmd/testdata/script/pkg_resolution_multiple_packages_one_matching_path_element.txtar index bb99b37f59c..6b3441b9980 100644 --- a/cmd/cue/cmd/testdata/script/pkg_resolution_multiple_packages_one_matching_path_element.txtar +++ b/cmd/cue/cmd/testdata/script/pkg_resolution_multiple_packages_one_matching_path_element.txtar @@ -10,7 +10,7 @@ cmp stdout stdout.golden # TODO the following command fails unexpectedly # although it should be consistent with the above. ! exec cue eval ./x -stderr 'found packages "x" \(x.cue\) and "y" \(y.cue\) in ".*script-pkg_resolution_multiple_packages_one_matching_path_element.*"' +stderr 'found packages "x" \(x.cue\) and "y" \(y.cue\) in ".*script-pkg_resolution_multiple_packages_one_matching_path_element.x"' -- stdout.golden -- x: 5 diff --git a/cmd/cue/cmd/testdata/script/pkg_resolution_path_element_invalid_ident.txtar b/cmd/cue/cmd/testdata/script/pkg_resolution_path_element_invalid_ident.txtar index dd95654de0b..1028a0e030c 100644 --- a/cmd/cue/cmd/testdata/script/pkg_resolution_path_element_invalid_ident.txtar +++ b/cmd/cue/cmd/testdata/script/pkg_resolution_path_element_invalid_ident.txtar @@ -47,9 +47,7 @@ x: 5 -- stdout.golden -- x: 5 -- test2-import-stderr.golden -- -import failed: build constraints exclude all CUE files in mod.com/1x: - 1x/x.cue: package is x, want 1x: +import failed: cannot find package "mod.com/1x": cannot determine package name from import path "mod.com/1x": ./test2/root.cue:3:8 -- test2-abs-stderr.golden -- -build constraints exclude all CUE files in mod.com/1x: - 1x/x.cue: package is x, want 1x +cannot find package "mod.com/1x": cannot determine package name from import path "mod.com/1x" diff --git a/cmd/cue/cmd/testdata/script/pkg_resolution_single_package_not_matching_path_element.txtar b/cmd/cue/cmd/testdata/script/pkg_resolution_single_package_not_matching_path_element.txtar index c9d739142e2..28a634ac1ef 100644 --- a/cmd/cue/cmd/testdata/script/pkg_resolution_single_package_not_matching_path_element.txtar +++ b/cmd/cue/cmd/testdata/script/pkg_resolution_single_package_not_matching_path_element.txtar @@ -31,11 +31,7 @@ package y y: 5 -- import_stderr.golden -- -import failed: build constraints exclude all CUE files in mod.com/x: - root.cue: package is root, want x - x/y.cue: package is y, want x: +import failed: cannot find package "mod.com/x": no files in package directory with package name "x": ./root.cue:3:8 -- absolute_stderr.golden -- -build constraints exclude all CUE files in mod.com/x: - root.cue: package is root, want x - x/y.cue: package is y, want x +cannot find package "mod.com/x": no files in package directory with package name "x" diff --git a/cue/load/config.go b/cue/load/config.go index 6ca33363a10..9db337f4703 100644 --- a/cue/load/config.go +++ b/cue/load/config.go @@ -38,6 +38,8 @@ const ( moduleFile = "module.cue" ) +type importPath string + // FromArgsUsage is a partial usage message that applications calling // FromArgs may wish to include in their -help output. // @@ -131,6 +133,7 @@ type Config struct { // A Module is a collection of packages and instances that are within the // directory hierarchy rooted at the module root. The module root can be // marked with a cue.mod file. + // If this is a relative path, it will be interpreted relative to [Config.Dir]. ModuleRoot string // Module specifies the module prefix. If not empty, this value must match @@ -300,17 +303,15 @@ func (c *Config) stdin() io.Reader { return c.Stdin } -type importPath string - type fsPath string -func addImportQualifier(pkg importPath, name string) (importPath, errors.Error) { +func addImportQualifier(pkg importPath, name string) (importPath, error) { if name == "" { return pkg, nil } ip := module.ParseImportPath(string(pkg)) if ip.ExplicitQualifier && ip.Qualifier != name { - return "", errors.Newf(token.NoPos, "non-matching package names (%s != %s)", ip.Qualifier, name) + return "", fmt.Errorf("non-matching package names (%s != %s)", ip.Qualifier, name) } ip.Qualifier = name return importPath(ip.String()), nil @@ -349,7 +350,7 @@ func (c Config) complete() (cfg *Config, err error) { if err := c.fileSystem.init(c.Dir, c.Overlay); err != nil { return nil, err } - + // log.Printf("complete with module root %q", c.ModuleRoot) // TODO: determine root on a package basis. Maybe we even need a // pkgname.cue.mod // Look to see if there is a cue.mod. @@ -389,6 +390,7 @@ func (c Config) complete() (cfg *Config, err error) { func (c *Config) loadModule() error { // TODO: also make this work if run from outside the module? mod := filepath.Join(c.ModuleRoot, modDir) + // log.Printf("loading module from %q", mod) info, cerr := c.fileSystem.stat(mod) if cerr != nil { return nil diff --git a/cue/load/import.go b/cue/load/import.go index c044eccba2b..8549fdf33eb 100644 --- a/cue/load/import.go +++ b/cue/load/import.go @@ -55,6 +55,7 @@ import ( // _ anonymous files (which may be marked with _) // * all packages func (l *loader) importPkg(pos token.Pos, p *build.Instance) []*build.Instance { + // log.Printf("importPkg %q", p.ImportPath) retErr := func(errs errors.Error) []*build.Instance { // XXX: move this loop to ReportError for _, err := range errors.Errors(errs) { @@ -181,7 +182,7 @@ func (l *loader) importPkg(pos token.Pos, p *build.Instance) []*build.Instance { impPath, err := addImportQualifier(importPath(p.ImportPath), p.PkgName) p.ImportPath = string(impPath) if err != nil { - p.ReportError(err) + p.ReportError(errors.Promote(err, "")) } all = append(all, p) @@ -300,10 +301,21 @@ func (l *loader) newRelInstance(pos token.Pos, path, pkgName string) *build.Inst } dir := filepath.Join(l.cfg.Dir, filepath.FromSlash(path)) - if importPath, e := l.importPathFromAbsDir(fsPath(dir), path); e != nil { + if pkgPath, e := importPathFromAbsDir(l.cfg, dir, path); e != nil { // Detect later to keep error messages consistent. } else { - p.ImportPath = string(importPath) + // Add package qualifier if the configuration requires it. + name := l.cfg.Package + switch name { + case "_", "*": + name = "" + } + pkgPath, e := addImportQualifier(pkgPath, name) + if e != nil { + // Detect later to keep error messages consistent. + } else { + p.ImportPath = string(pkgPath) + } } p.Dir = dir @@ -320,44 +332,33 @@ func (l *loader) newRelInstance(pos token.Pos, path, pkgName string) *build.Inst return p } -func (l *loader) importPathFromAbsDir(absDir fsPath, key string) (importPath, errors.Error) { - if l.cfg.ModuleRoot == "" { - return "", errors.Newf(token.NoPos, - "cannot determine import path for %q (root undefined)", key) +func importPathFromAbsDir(c *Config, absDir string, origPath string) (importPath, error) { + if c.ModuleRoot == "" { + return "", fmt.Errorf("cannot determine import path for %q (root undefined)", origPath) } - dir := filepath.Clean(string(absDir)) - if !strings.HasPrefix(dir, l.cfg.ModuleRoot) { - return "", errors.Newf(token.NoPos, - "cannot determine import path for %q (dir outside of root)", key) + dir := filepath.Clean(absDir) + if !strings.HasPrefix(dir, c.ModuleRoot) { + return "", fmt.Errorf("cannot determine import path for %q (dir outside of root)", origPath) } - pkg := filepath.ToSlash(dir[len(l.cfg.ModuleRoot):]) + pkg := filepath.ToSlash(dir[len(c.ModuleRoot):]) switch { case strings.HasPrefix(pkg, "/cue.mod/"): pkg = pkg[len("/cue.mod/"):] if pkg == "" { - return "", errors.Newf(token.NoPos, - "invalid package %q (root of %s)", key, modDir) + return "", fmt.Errorf("invalid package %q (root of %s)", origPath, modDir) } - case l.cfg.Module == "": - return "", errors.Newf(token.NoPos, - "cannot determine import path for %q (no module)", key) + case c.Module == "": + return "", fmt.Errorf("cannot determine import path for %q (no module)", origPath) default: - impPath := module.ParseImportPath(l.cfg.Module) + impPath := module.ParseImportPath(c.Module) impPath.Path += pkg impPath.Qualifier = "" pkg = impPath.String() } - - name := l.cfg.Package - switch name { - case "_", "*": - name = "" - } - - return addImportQualifier(importPath(pkg), name) + return importPath(pkg), nil } func (l *loader) newInstance(pos token.Pos, p importPath) *build.Instance { diff --git a/cue/load/import_test.go b/cue/load/import_test.go index 032eae65a31..9fe179b835c 100644 --- a/cue/load/import_test.go +++ b/cue/load/import_test.go @@ -16,6 +16,7 @@ package load import ( "fmt" + "os" "path/filepath" "reflect" "testing" @@ -27,12 +28,21 @@ func testdata(elems ...string) string { return filepath.Join(append([]string{"testdata"}, elems...)...) } +var pkgRootDir, _ = os.Getwd() + func getInst(pkg, cwd string) (*build.Instance, error) { - // Set ModuleRoot as well; otherwise we walk the parent directories - // all the way to the root of the git repository, causing Go's test caching - // to never kick in, as the .git directory almost always changes. - // Moreover, it's extra work that isn't useful to the tests. - insts := Instances([]string{pkg}, &Config{ModuleRoot: ".", Dir: cwd}) + insts := Instances([]string{pkg}, &Config{ + // Set ModuleRoot as well; otherwise we walk the parent directories + // all the way to the root of the git repository, causing Go's test caching + // to never kick in, as the .git directory almost always changes. + // Moreover, it's extra work that isn't useful to the tests. + // + // Note that we can't set ModuleRoot to cwd because if ModuleRoot is + // set, the logic will only look for a module file in that exact directory. + // So we set it to the module root actually used by all the callers of getInst: ./testdata/testmod. + ModuleRoot: filepath.Join(pkgRootDir, testdata("testmod")), + Dir: cwd, + }) if len(insts) != 1 { return nil, fmt.Errorf("expected one instance, got %d", len(insts)) } @@ -58,7 +68,7 @@ func TestEmptyFolderImport(t *testing.T) { path := testdata("testmod", "empty") _, err := getInst(".", path) if _, ok := err.(*NoFilesError); !ok { - t.Fatalf(`Import(%q) did not return NoCUEError.`, path) + t.Fatalf(`Import(%q) did not return NoCUEError, but instead %#v (%v)`, path, err, err) } } @@ -67,7 +77,7 @@ func TestMultiplePackageImport(t *testing.T) { _, err := getInst(".", path) mpe, ok := err.(*MultiplePackageError) if !ok { - t.Fatalf(`Import(%q) did not return MultiplePackageError.`, path) + t.Fatalf(`Import(%q) did not return MultiplePackageError, but instead %#v (%v)`, path, err, err) } mpe.Dir = "" want := &MultiplePackageError{ diff --git a/cue/load/instances.go b/cue/load/instances.go index 5e10ca10edb..f33307e3b9f 100644 --- a/cue/load/instances.go +++ b/cue/load/instances.go @@ -24,7 +24,6 @@ import ( "cuelang.org/go/cue/build" "cuelang.org/go/internal/cueexperiment" "cuelang.org/go/internal/filetypes" - "cuelang.org/go/internal/mod/modimports" "cuelang.org/go/internal/mod/modpkgload" "cuelang.org/go/internal/mod/modrequirements" "cuelang.org/go/mod/module" @@ -46,6 +45,7 @@ func Instances(args []string, c *Config) []*build.Instance { if c == nil { c = &Config{} } + // log.Printf("Instances with module root %q", c.ModuleRoot) // We want to consult the CUE_EXPERIMENT flag to see whether // consult external registries by default. if err := cueexperiment.Init(); err != nil { @@ -80,13 +80,22 @@ func Instances(args []string, c *Config) []*build.Instance { // Pass all arguments that look like packages to loadPackages // so that they'll be available when looking up the packages // that are specified on the command line. - // Relative import paths create a package with an associated - // error but it turns out that's actually OK because the cue/load - // logic resolves such paths without consulting pkgs. - pkgs, err := loadPackages(ctx, c, synCache, pkgArgs, otherFiles) + expandedPaths, err := expandPackageArgs(c, pkgArgs, c.Package) if err != nil { return []*build.Instance{c.newErrInstance(err)} } + pkgs, err := loadPackages(ctx, c, synCache, expandedPaths, otherFiles) + if err != nil { + return []*build.Instance{c.newErrInstance(err)} + } + // if pkgs != nil { + // log.Printf("got modpkgload.Packages") + // for _, p := range pkgs.All() { + // log.Printf("loaded pkg %v: %v", p.ImportPath(), p.Error()) + // } + // } else { + // log.Printf("no modpkgload.Packages") + // } tg := newTagger(c) l := newLoader(c, tg, synCache, pkgs) @@ -151,8 +160,9 @@ func Instances(args []string, c *Config) []*build.Instance { // loadPackages returns packages loaded from the given package list and also // including imports from the given build files. -func loadPackages(ctx context.Context, cfg *Config, synCache *syntaxCache, extraPkgs []string, otherFiles []*build.File) (*modpkgload.Packages, error) { +func loadPackages(ctx context.Context, cfg *Config, synCache *syntaxCache, pkgs []resolvedPackageArg, otherFiles []*build.File) (*modpkgload.Packages, error) { if cfg.Registry == nil || cfg.modFile == nil || cfg.modFile.Module == "" { + // log.Printf("no packages (registry %v; modFile %#v)", cfg.Registry != nil, cfg.modFile) return nil, nil } reqs := modrequirements.NewRequirements( @@ -167,27 +177,8 @@ func loadPackages(ctx context.Context, cfg *Config, synCache *syntaxCache, extra } pkgPaths := make(map[string]bool) // Add any packages specified directly on the command line. - for _, pkg := range extraPkgs { - pkgPaths[pkg] = true - } - if len(otherFiles) == 0 || len(extraPkgs) > 0 { - // Resolve all the imports in the current module. We specifically - // avoid doing this when files are specified on the command line - // but no packages because we don't want to fail because of - // invalid files in the module when we're just evaluating files. - // TODO this means that if files _and_ packages are specified, - // we can still error when there are problems with packages that - // aren't used by anything explicitly specified on the command line; - // a proper solution would involve pushing the pattern evaluation - // down into the module loading code, but that implies a significant - // refactoring of the modules code, so this will do for now. - modImports, err := modimports.AllImports(modimports.AllModuleFiles(mainModLoc.FS, mainModLoc.Dir)) - if err != nil { - return nil, fmt.Errorf("cannot enumerate all module imports: %v", err) - } - for _, pkg := range modImports { - pkgPaths[pkg] = true - } + for _, pkg := range pkgs { + pkgPaths[pkg.resolved] = true } // Add any imports found in other files. for _, f := range otherFiles { diff --git a/cue/load/loader_common.go b/cue/load/loader_common.go index e80c4fed7d2..2ccf9fcd116 100644 --- a/cue/load/loader_common.go +++ b/cue/load/loader_common.go @@ -450,8 +450,7 @@ func isLocalImport(path string) bool { func warnUnmatched(matches []*match) { for _, m := range matches { if len(m.Pkgs) == 0 { - m.Err = - errors.Newf(token.NoPos, "cue: %q matched no packages\n", m.Pattern) + m.Err = errors.Newf(token.NoPos, "cue: %q matched no packages", m.Pattern) } } } @@ -460,6 +459,9 @@ func warnUnmatched(matches []*match) { // command line. It canonicalizes the patterns but does not // evaluate any matches. func cleanPatterns(patterns []string) []string { + if len(patterns) == 0 { + return []string{"."} + } var out []string for _, a := range patterns { // Arguments are supposed to be import paths, but @@ -484,6 +486,8 @@ func cleanPatterns(patterns []string) []string { } // isMetaPackage checks if name is a reserved package name that expands to multiple packages. +// TODO none of these package names are actually recognized anywhere else +// and at least one (cmd) doesn't seem like it belongs in the CUE world. func isMetaPackage(name string) bool { return name == "std" || name == "cmd" || name == "all" } diff --git a/cue/load/loader_test.go b/cue/load/loader_test.go index 45dad5fbff7..3fa068f5c22 100644 --- a/cue/load/loader_test.go +++ b/cue/load/loader_test.go @@ -146,7 +146,7 @@ display:./anon`}, { args: []string{"./other"}, want: `err: import failed: relative import paths not allowed ("./file"): $CWD/testdata/testmod/other/main.cue:6:2 -path: mod.test/test/other@v0:main +path: mod.test/test/other@v0 module: mod.test/test@v0 root: $CWD/testdata/testmod dir: $CWD/testdata/testmod/other @@ -182,15 +182,13 @@ imports: name: "NoPackageName", cfg: dirCfg, args: []string{"mod.test/test/hello:nonexist"}, - want: `err: build constraints exclude all CUE files in mod.test/test/hello:nonexist: - anon.cue: no package name - test.cue: package is test, want nonexist - hello/test.cue: package is test, want nonexist + want: `err: cannot find package "mod.test/test/hello": no files in package directory with package name "nonexist" path: mod.test/test/hello:nonexist module: mod.test/test@v0 root: $CWD/testdata/testmod -dir: $CWD/testdata/testmod/hello -display:mod.test/test/hello:nonexist`}, { +dir: "" +display:mod.test/test/hello:nonexist`, + }, { name: "ExplicitNonPackageFiles", cfg: dirCfg, args: []string{"./anon.cue", "./other/anon.cue"}, @@ -280,7 +278,7 @@ imports: name: "OnlyToolFiles", cfg: dirCfg, args: []string{"./toolonly"}, - want: `path: mod.test/test/toolonly@v0:foo + want: `path: mod.test/test/toolonly@v0 module: mod.test/test@v0 root: $CWD/testdata/testmod dir: $CWD/testdata/testmod/toolonly @@ -294,9 +292,9 @@ files: args: []string{"./toolonly"}, want: `err: build constraints exclude all CUE files in ./toolonly: anon.cue: no package name - test.cue: package is test, want foo + test.cue: package is test, want toolonly toolonly/foo_tool.cue: _tool.cue files excluded in non-cmd mode -path: mod.test/test/toolonly@v0:foo +path: mod.test/test/toolonly@v0 module: mod.test/test@v0 root: $CWD/testdata/testmod dir: $CWD/testdata/testmod/toolonly @@ -447,7 +445,15 @@ func TestOverlays(t *testing.T) { return r } ctx := cuecontext.New() - insts, err := ctx.BuildInstances(Instances([]string{"./dir/..."}, c)) + buildInsts := Instances([]string{"./dir/..."}, c) + for i, inst := range buildInsts { + if inst.Err != nil { + t.Logf("got instance %d: %q: error %v", i, inst.ImportPath, inst.Err) + } else { + t.Logf("got instance %d: %q", i, inst.ImportPath) + } + } + insts, err := ctx.BuildInstances(buildInsts) if err != nil { t.Fatal(err) } @@ -461,6 +467,9 @@ func TestOverlays(t *testing.T) { t.Error(err) continue } + if i >= len(want) { + t.Fatalf("more instances returned from BuildInstances than expected (%d/%d); extras %q", len(insts), len(want), b) + } if got := string(bytes.Map(rmSpace, b)); got != want[i] { t.Errorf("%s: got %s; want %s", inst.BuildInstance().Dir, got, want[i]) } @@ -473,6 +482,7 @@ func TestLoadOrder(t *testing.T) { Package: "*", Dir: testDataDir, }) + t.Logf("got %d instances; error %v", len(insts), insts[0].Err) var actualFiles = []string{} for _, inst := range insts { @@ -494,7 +504,7 @@ func TestLoadInstancesConcurrent(t *testing.T) { // if there's an underlying race condition. // See https://cuelang.org/issue/1746 race(t, func() error { - _, err := getInst(".", testdata("testmod", "hello")) + _, err := getInst(".:test", testdata("testmod", "hello")) return err }) } diff --git a/cue/load/search.go b/cue/load/search.go index c5cc95427a0..1b8e352bfbc 100644 --- a/cue/load/search.go +++ b/cue/load/search.go @@ -15,14 +15,18 @@ package load import ( + "fmt" "io/fs" "path" + pathpkg "path" "path/filepath" "strings" "cuelang.org/go/cue/build" "cuelang.org/go/cue/errors" "cuelang.org/go/cue/token" + "cuelang.org/go/internal/mod/modimports" + "cuelang.org/go/mod/module" ) // TODO: should be matched from module file only. @@ -265,3 +269,332 @@ func (l *loader) importPathsQuiet(patterns []string) []*match { } return out } + +type resolvedPackageArg struct { + original string + resolved string +} + +func expandPackageArgs(c *Config, pkgArgs []string, pkgQual string) ([]resolvedPackageArg, error) { + expanded := make([]resolvedPackageArg, 0, len(pkgArgs)) + for _, p := range pkgArgs { + var err error + expanded, err = appendExpandedPackageArg(c, expanded, p, pkgQual) + if err != nil { + return nil, err + } + } + return expanded, nil +} + +// appendExpandedPackageArg appends all the package paths matched by p to pkgPaths +// and returns the result. It also cleans the paths and makes them absolute. +// +// pkgQual is used to determine which packages to match when wildcards are expanded. +// Its semantics follow those of [Config.Package]. +func appendExpandedPackageArg(c *Config, pkgPaths []resolvedPackageArg, p string, pkgQual string) ([]resolvedPackageArg, error) { + origp := p + if filepath.IsAbs(p) { + return nil, fmt.Errorf("cannot use absolute directory %q as package path", p) + } + // Arguments are supposed to be import paths, but + // as a courtesy to Windows developers, rewrite \ to / + // in command-line arguments. Handles .\... and so on. + p = filepath.ToSlash(p) + + ip := module.ParseImportPath(p) + + isRel := strings.HasPrefix(ip.Path, "./") + // Put argument in canonical form. + ip.Path = pathpkg.Clean(ip.Path) + if isRel && ip.Path != "." { + // Preserve leading "./". + ip.Path = "./" + ip.Path + } + isLocal := isLocalImport(ip.Path) + // Note that when c.Module is empty, c.ModuleRoot is sometimes, + // but not always, the same as c.Dir. Specifically it might point + // to the directory containing a cue.mod directory even if that + // directory doesn't actually contain a module.cue file. + moduleRoot := c.ModuleRoot + if isLocal { + if c.Module != "" { + // Make local import paths into absolute paths inside + // the module root. + absPath := pathpkg.Join(c.Dir, ip.Path) + pkgPath, err := importPathFromAbsDir(c, absPath, origp) + if err != nil { + return nil, err + } + ip1 := module.ParseImportPath(string(pkgPath)) + // Leave ip.Qualifier and ip.ExplicitQualifier intact. + ip.Path = ip1.Path + ip.Version = ip1.Version + } else { + // There's no module, so we can't make + // the import path absolute. + moduleRoot = c.Dir + } + } + if !strings.Contains(ip.Path, "...") { + if isLocal && !ip.ExplicitQualifier { + // A package qualifier has not been explicitly specified for a local + // import path so we need to walk the package directory to find the + // packages in it. We have a special rule for local imports because it's + // inconvenient always to have to specify a package qualifier when + // there's only one package in the current directory but the last + // component of its package path does not match its name. + return appendExpandedUnqualifiedPackagePath(pkgPaths, origp, ip, pkgQual, module.SourceLoc{ + FS: c.fileSystem.ioFS(moduleRoot), + Dir: ".", + }, c.Module) + } + return append(pkgPaths, resolvedPackageArg{origp, ip.String()}), nil + } + // Strip the module prefix, leaving only the directory relative + // to the module root. + ip, ok := cutModulePrefix(ip, c.Module) + if !ok { + return nil, fmt.Errorf("pattern not allowed in external package path %q", origp) + } + return appendExpandedWildcardPackagePath(pkgPaths, ip, pkgQual, module.SourceLoc{ + FS: c.fileSystem.ioFS(moduleRoot), + Dir: ".", + }, c.Module) +} + +// appendExpandedUnqualifiedPackagePath expands the given import path, +// which is relative to the root of the module, into its resolved and +// qualified package paths according to the following rules (the first rule +// that applies is used) +// +// 1. if pkgQual is "*", it chooses all the packages present in the +// package directory. +// 2. if pkgQual is "_", it looks for a package file with no package name. +// 3. if there's a package named after ip.Qualifier it chooses that +// 4. if there's exactly one package in the directory it will choose that. +// 5. if there's more than one package in the directory, it returns a MultiplePackageError. +// 6. if there are no package files in the directory, it just appends the import path as is, leaving it +// to later logic to produce an error in this case. +func appendExpandedUnqualifiedPackagePath(pkgPaths []resolvedPackageArg, origp string, ip module.ImportPath, pkgQual string, mainModRoot module.SourceLoc, mainModPath string) (_ []resolvedPackageArg, _err error) { + ipRel, ok := cutModulePrefix(ip, mainModPath) + if !ok { + // Should never happen. + return nil, fmt.Errorf("internal error: local import path %q in module %q has resulted in non-internal package %q", origp, mainModPath, ip) + } + dir := pathpkg.Join(mainModRoot.Dir, ipRel.Path) + info, err := fs.Stat(mainModRoot.FS, dir) + if err != nil { + // The package directory doesn't exist. Treat it like an + // empty directory and let later logic deal with it. + return append(pkgPaths, resolvedPackageArg{origp, ip.String()}), nil + } + if !info.IsDir() { + return nil, fmt.Errorf("%s refers to a file not a package directory", origp) + } + iter := modimports.PackageFiles(mainModRoot.FS, dir, "*") + + // 1. if pkgQual is "*", it appends all the packages present in the + // package directory. + if pkgQual == "*" { + wasAdded := make(map[string]bool) + iter(func(f modimports.ModuleFile, err error) bool { + if err != nil { + _err = err + return false + } + pkgName := f.Syntax.PackageName() + if wasAdded[pkgName] { + return true + } + wasAdded[pkgName] = true + ip := ip + ip.Qualifier = pkgName + p := ip.String() + pkgPaths = append(pkgPaths, resolvedPackageArg{p, p}) + return true + }) + if _err != nil { + return nil, _err + } + return pkgPaths, nil + } + var files []modimports.ModuleFile + foundQualifier := false + // TODO(rog) for f, err := range iter { + iter(func(f modimports.ModuleFile, err error) bool { + if err != nil { + _err = err + return false + } + pkgName := f.Syntax.PackageName() + // 2. if pkgQual is "_", it looks for a package file with no package name. + // 3. if there's a package named after ip.Qualifier it chooses that + if pkgName == ip.Qualifier || (pkgQual == "_" && pkgName == "") { + foundQualifier = true + return false + } + if pkgName != "" { + files = append(files, f) + } + return true + }) + if _err != nil { + return nil, _err + } + if foundQualifier { + // We found the actual package that was implied + // by the import path (or pkgQual == "_"). This takes precedence over + // anything else. + return append(pkgPaths, resolvedPackageArg{origp, ip.String()}), nil + } + if len(files) == 0 { + // 6. if there are no package files in the directory, it just appends the import path as is, leaving it + // to later logic to produce an error in this case. + return append(pkgPaths, resolvedPackageArg{origp, ip.String()}), nil + } + pkgName := files[0].Syntax.PackageName() + for _, f := range files[1:] { + // 5. if there's more than one package in the directory, it returns a MultiplePackageError. + if pkgName1 := f.Syntax.PackageName(); pkgName1 != pkgName { + return nil, &MultiplePackageError{ + Dir: dir, + Packages: []string{pkgName, pkgName1}, + Files: []string{ + pathpkg.Base(files[0].FilePath), + pathpkg.Base(f.FilePath), + }, + } + } + } + // 4. if there's exactly one package in the directory it will choose that. + ip.Qualifier = pkgName + return append(pkgPaths, resolvedPackageArg{origp, ip.String()}), nil +} + +// appendExpandedWildcardPackagePath expands the given pattern into any packages that it matches +// and appends the results to pkgPaths. +// It returns an error if the pattern matching nothing. +// Note: +// +// We know that pattern contains ... +// We know that pattern is relative to the module root +func appendExpandedWildcardPackagePath(pkgPaths []resolvedPackageArg, pattern module.ImportPath, pkgQual string, mainModRoot module.SourceLoc, mainModPath string) (_ []resolvedPackageArg, _err error) { + modIpath := module.ParseImportPath(mainModPath) + // Find directory to begin the scan. + // Could be smarter but this one optimization + // is enough for now, since ... is usually at the + // end of a path. + // TODO strip package qualifier + i := strings.Index(pattern.Path, "...") + dir, _ := path.Split(pattern.Path[:i]) + dir = pathpkg.Join(mainModRoot.Dir, dir) + var isSelected func(string) bool + switch pkgQual { + case "_": + isSelected = func(pkgName string) bool { + return pkgName == "" + } + case "*": + isSelected = func(pkgName string) bool { + return true + } + case "": + isSelected = func(pkgName string) bool { + // The package ambiguity logic will be triggered if there's more than one + // package in the same directory. + return pkgName != "" + } + default: + isSelected = func(pkgName string) bool { + return pkgName == pkgQual + } + } + + var prevFile modimports.ModuleFile + var prevImportPath module.ImportPath + iter := modimports.AllModuleFiles(mainModRoot.FS, dir) + iter(func(f modimports.ModuleFile, err error) bool { + if err != nil { + return false + } + pkgName := f.Syntax.PackageName() + if !isSelected(pkgName) { + return true + } + if pkgName == "" { + pkgName = "_" + } + ip := module.ImportPath{ + Path: path.Join(modIpath.Path, path.Dir(f.FilePath)), + Qualifier: pkgName, + Version: modIpath.Version, + } + if modIpath.Path == "" { + // There's no module, so make sure that the path + // still looks like a relative import path. + if !strings.HasPrefix(ip.Path, "../") { + ip.Path = "./" + ip.Path + } + } + if ip == prevImportPath { + // TODO(rog) this isn't sufficient for full deduplication: we can get an alternation of different + // package names within the same directory. We'll need to maintain a map. + return true + } + if pkgQual == "" { + // Note: we can look at the previous item only rather than maintaining a map + // because modimports.AllModuleFiles guarantees that files in the same + // package are always adjacent. + if prevFile.FilePath != "" && prevImportPath.Path == ip.Path && ip.Qualifier != prevImportPath.Qualifier { + // A wildcard isn't currently allowed to match multiple packages + // in a single directory. + _err = &MultiplePackageError{ + Dir: path.Dir(f.FilePath), + Packages: []string{prevImportPath.Qualifier, ip.Qualifier}, + Files: []string{ + pathpkg.Base(prevFile.FilePath), + pathpkg.Base(f.FilePath), + }, + } + return false + } + } + pkgPaths = append(pkgPaths, resolvedPackageArg{ip.String(), ip.String()}) + prevFile, prevImportPath = f, ip + return true + }) + return pkgPaths, _err +} + +// cutModulePrefix strips the given module path from p and reports +// whether p is inside mod. It returns a relative package +// path within m. +// +// If p does not contain a major version suffix but otherwise +// matches mod, it counts as a match. +func cutModulePrefix(p module.ImportPath, mod string) (module.ImportPath, bool) { + if mod == "" { + return p, true + } + modPath, modVers, ok := module.SplitPathVersion(mod) + if !ok { + modPath = mod + } + if !strings.HasPrefix(p.Path, modPath) { + return module.ImportPath{}, false + } + if p.Path == modPath { + p.Path = "." + return p, true + } + if p.Path[len(modPath)] != '/' { + return module.ImportPath{}, false + } + if p.Version != "" && modVers != "" && p.Version != modVers { + return module.ImportPath{}, false + } + p.Path = "." + p.Path[len(modPath):] + p.Version = "" + return p, true +} diff --git a/cue/load/testdata/testfetch/simple.txtar b/cue/load/testdata/testfetch/simple.txtar index c8f3a3aa471..8c4a694181d 100644 --- a/cue/load/testdata/testfetch/simple.txtar +++ b/cue/load/testdata/testfetch/simple.txtar @@ -7,7 +7,7 @@ "example.com@v0": "v0.0.1" } -- cue.mod/module.cue -- -module: "main.org" +module: "test.example/main" language: version: "v0.8.0" deps: { "example.com@v0": v: "v0.0.1" diff --git a/cue/load/testdata/testmod-external/cue.mod/module.cue b/cue/load/testdata/testmod-external/cue.mod/module.cue index f315ef0ce25..379aaba740b 100644 --- a/cue/load/testdata/testmod-external/cue.mod/module.cue +++ b/cue/load/testdata/testmod-external/cue.mod/module.cue @@ -1,3 +1,3 @@ -module: "main.example@v0" +module: "test.example/main@v0" language: version: "v0.8.0" deps: "foo.example@v0": v: "v0.0.1" diff --git a/cue/load/testdata/testmod/other/main.cue b/cue/load/testdata/testmod/other/main.cue index 923e89a1912..8e2a1e42f0c 100644 --- a/cue/load/testdata/testmod/other/main.cue +++ b/cue/load/testdata/testmod/other/main.cue @@ -1,9 +1,9 @@ // Test data - not compiled. -package main +package other import ( "./file" ) -{} \ No newline at end of file +{} diff --git a/cue/load/testdata/testmod/toolonly/foo_tool.cue b/cue/load/testdata/testmod/toolonly/foo_tool.cue index 4a782e77799..fb9e4afe4b4 100644 --- a/cue/load/testdata/testmod/toolonly/foo_tool.cue +++ b/cue/load/testdata/testmod/toolonly/foo_tool.cue @@ -1,4 +1,4 @@ -package foo +package toolonly import "tool/cli" diff --git a/internal/mod/modimports/modimports.go b/internal/mod/modimports/modimports.go index 20e68d26015..9a6edbbf60d 100644 --- a/internal/mod/modimports/modimports.go +++ b/internal/mod/modimports/modimports.go @@ -68,23 +68,78 @@ func AllImports(modFilesIter func(func(ModuleFile, error) bool)) (_ []string, re // If pkgQualifier is "*", files from all packages in the directory will be produced. func PackageFiles(fsys fs.FS, dir string, pkgQualifier string) func(func(ModuleFile, error) bool) { return func(yield func(ModuleFile, error) bool) { - entries, err := fs.ReadDir(fsys, dir) - if err != nil { - yield(ModuleFile{ - FilePath: dir, - }, err) - return + // Start at the target directory, but also include package files + // from packages with the same name(s) in parent directories. + // Stop the iteration when we find a cue.mod entry, signifying + // the module root. If the location is inside a `cue.mod` directory + // already, do not look at parent directories - this mimics historic + // behavior. + selectPackage := func(pkg string) bool { + if pkgQualifier == "*" { + return true + } + return pkg == pkgQualifier + } + inCUEMod := false + if before, after, ok := strings.Cut(dir, "cue.mod"); ok { + // We're underneath a cue.mod directory if some parent + // element is cue.mod. + inCUEMod = + (before == "" || strings.HasSuffix(before, "/")) && + (after == "" || strings.HasPrefix(after, "/")) } - for _, e := range entries { - if !yieldPackageFile(fsys, path.Join(dir, e.Name()), pkgQualifier, yield) { + var matchedPackages map[string]bool + for { + entries, err := fs.ReadDir(fsys, dir) + if err != nil { + yield(ModuleFile{ + FilePath: dir, + }, err) + return + } + inModRoot := false + for _, e := range entries { + if e.Name() == "cue.mod" { + inModRoot = true + } + pkgName, cont := yieldPackageFile(fsys, path.Join(dir, e.Name()), selectPackage, yield) + if !cont { + return + } + if pkgName != "" { + if matchedPackages == nil { + matchedPackages = make(map[string]bool) + } + matchedPackages[pkgName] = true + } + } + if inModRoot || inCUEMod { + // We're at the module root or we're inside the cue.mod + // directory. Don't go any further up the hierarchy. return } + if matchedPackages == nil { + // No packages possible in parent directories if there are + // no matching package files in the package directory itself. + return + } + selectPackage = func(pkgName string) bool { + return matchedPackages[pkgName] + } + parent := path.Dir(dir) + if len(parent) >= len(dir) { + // No more parent directories. + return + } + dir = parent } } } // AllModuleFiles returns an iterator that produces all the CUE files inside the // module at the given root. +// +// The caller may assume that files from the same package are always adjacent. func AllModuleFiles(fsys fs.FS, root string) func(func(ModuleFile, error) bool) { return func(yield func(ModuleFile, error) bool) { yieldAllModFiles(fsys, root, true, yield) @@ -115,33 +170,50 @@ func yieldAllModFiles(fsys fs.FS, fpath string, topDir bool, yield func(ModuleFi } } } + // Generate all entries for the package before moving onto packages + // in subdirectories. + for _, entry := range entries { + if entry.IsDir() { + continue + } + fpath := path.Join(fpath, entry.Name()) + if _, ok := yieldPackageFile(fsys, fpath, func(string) bool { return true }, yield); !ok { + return false + } + } + for _, entry := range entries { name := entry.Name() + if !entry.IsDir() { + continue + } + if name == "cue.mod" || strings.HasPrefix(name, ".") || strings.HasPrefix(name, "_") { + continue + } fpath := path.Join(fpath, name) - if entry.IsDir() { - if name == "cue.mod" || strings.HasPrefix(name, ".") || strings.HasPrefix(name, "_") { - continue - } - if !yieldAllModFiles(fsys, fpath, false, yield) { - return false - } - } else if !yieldPackageFile(fsys, fpath, "*", yield) { + if !yieldAllModFiles(fsys, fpath, false, yield) { return false } } return true } -func yieldPackageFile(fsys fs.FS, fpath, pkgQualifier string, yield func(ModuleFile, error) bool) bool { +// yieldPackageFile invokes yield with the contents of the package file +// at the given path if selectPackage returns true for the file's +// package name. +// +// It returns the yielded package name (if any) and reports whether +// the iteration should continue. +func yieldPackageFile(fsys fs.FS, fpath string, selectPackage func(pkgName string) bool, yield func(ModuleFile, error) bool) (pkgName string, cont bool) { if !strings.HasSuffix(fpath, ".cue") { - return true + return "", true } pf := ModuleFile{ FilePath: fpath, } f, err := fsys.Open(fpath) if err != nil { - return yield(pf, err) + return "", yield(pf, err) } defer f.Close() @@ -152,16 +224,16 @@ func yieldPackageFile(fsys fs.FS, fpath, pkgQualifier string, yield func(ModuleF // on a reader in a streaming manner. data, err := cueimports.Read(f) if err != nil { - return yield(pf, err) + return "", yield(pf, err) } syntax, err := parser.ParseFile(fpath, data, parser.ImportsOnly) if err != nil { - return yield(pf, err) + return "", yield(pf, err) } - if pkgQualifier != "*" && syntax.PackageName() != pkgQualifier { - return true + if !selectPackage(syntax.PackageName()) { + return "", true } pf.Syntax = syntax - return yield(pf, nil) + return syntax.PackageName(), yield(pf, nil) } diff --git a/internal/mod/modimports/testdata/simple.txtar b/internal/mod/modimports/testdata/simple.txtar index ecf651e5191..0fa6a522520 100644 --- a/internal/mod/modimports/testdata/simple.txtar +++ b/internal/mod/modimports/testdata/simple.txtar @@ -1,5 +1,6 @@ -- want -- x.cue "dep1" "dep2:a" "dep2:b" "dep3" +z.cue y/y1.cue "dep3" "dep4" y/y2.cue y/z1.cue @@ -40,6 +41,9 @@ package z -- y/z2.cue -- package z +-- z.cue -- +package example + -- _omitted1/foo.cue -- not even looked at diff --git a/internal/mod/modload/testdata/tidy/canuseprerelease.txtar b/internal/mod/modload/testdata/tidy/canuseprerelease.txtar index 3ab167e8113..98b2d1b77a3 100644 --- a/internal/mod/modload/testdata/tidy/canuseprerelease.txtar +++ b/internal/mod/modload/testdata/tidy/canuseprerelease.txtar @@ -2,7 +2,7 @@ # no stable releases available. -- tidy-check-error -- -module is not tidy: cannot find module providing package example.com@v0 +module is not tidy: cannot find module providing package example.com@v0:main -- want -- module: "main.org@v0" language: { @@ -20,7 +20,7 @@ language: version: "v0.8.0" -- main.cue -- package main -import "example.com@v0" +import "example.com@v0:main" -- _registry/example.com_v0.0.1-foo/cue.mod/module.cue -- module: "example.com@v0" diff --git a/internal/mod/modload/testdata/tidy/implied-major-version-with-explicit-default.txtar b/internal/mod/modload/testdata/tidy/implied-major-version-with-explicit-default.txtar index b8f903fa4ad..2cba107b8d7 100644 --- a/internal/mod/modload/testdata/tidy/implied-major-version-with-explicit-default.txtar +++ b/internal/mod/modload/testdata/tidy/implied-major-version-with-explicit-default.txtar @@ -26,7 +26,7 @@ deps: { -- main.cue -- package main -import "example.com" +import "example.com:main" -- _registry/example.com_v0.0.1/cue.mod/module.cue -- module: "example.com@v0" diff --git a/internal/mod/modload/testdata/tidy/preferstable.txtar b/internal/mod/modload/testdata/tidy/preferstable.txtar index 752f39ee1e0..418ca21919b 100644 --- a/internal/mod/modload/testdata/tidy/preferstable.txtar +++ b/internal/mod/modload/testdata/tidy/preferstable.txtar @@ -22,7 +22,7 @@ language: { -- main.cue -- package main -import "example.com@v0" +import "example.com@v0:main" -- _registry/example.com_v0.0.1/cue.mod/module.cue -- module: "example.com@v0" diff --git a/internal/mod/modpkgload/pkgload.go b/internal/mod/modpkgload/pkgload.go index 68f0831f6f6..b5ae11b33e6 100644 --- a/internal/mod/modpkgload/pkgload.go +++ b/internal/mod/modpkgload/pkgload.go @@ -245,9 +245,19 @@ func (pkgs *Packages) load(ctx context.Context, pkg *Package) { pkgs.applyPkgFlags(ctx, pkg, PkgInAll) } pkgQual := module.ParseImportPath(pkg.path).Qualifier + if pkgQual == "" { + pkg.err = fmt.Errorf("cannot determine package name from import path %q", pkg.path) + return + } importsMap := make(map[string]bool) + foundPackageFile := false for _, loc := range pkg.locs { - imports, err := modimports.AllImports(modimports.PackageFiles(loc.FS, loc.Dir, pkgQual)) + pkgFileIter := modimports.PackageFiles(loc.FS, loc.Dir, pkgQual) + pkgFileIter(func(_ modimports.ModuleFile, err error) bool { + foundPackageFile = err == nil + return false + }) + imports, err := modimports.AllImports(pkgFileIter) if err != nil { pkg.err = fmt.Errorf("cannot get imports: %v", err) return @@ -256,6 +266,10 @@ func (pkgs *Packages) load(ctx context.Context, pkg *Package) { importsMap[imp] = true } } + if !foundPackageFile { + pkg.err = fmt.Errorf("no files in package directory with package name %q", pkgQual) + return + } imports := make([]string, 0, len(importsMap)) for imp := range importsMap { imports = append(imports, imp) diff --git a/internal/mod/modpkgload/testdata/multidir.txtar b/internal/mod/modpkgload/testdata/multidir.txtar new file mode 100644 index 00000000000..45f21041560 --- /dev/null +++ b/internal/mod/modpkgload/testdata/multidir.txtar @@ -0,0 +1,38 @@ +-- test0/initial-requirements -- +main.test@v0 +example.com@v0.0.1 +-- test0/root-packages -- +main.test@v0:main +-- test0/default-major-versions -- +-- test0/want -- +main.test@v0:main + flags: inAll,isRoot,fromRoot,importsLoaded + mod: main.test@v0 + location: . + imports: + example.com/blah@v0 +example.com/blah@v0 + flags: inAll,isRoot,fromRoot,importsLoaded + mod: example.com@v0.0.1 + location: _registry/example.com_v0.0.1/blah + imports: + foo.com/bar/hello/goodbye@v0 +foo.com/bar/hello/goodbye@v0 + flags: inAll,isRoot,fromRoot + error: cannot find module providing package foo.com/bar/hello/goodbye@v0 + missing: true +-- main.cue -- +package main +import "example.com/blah@v0" + +-- _registry/example.com_v0.0.1/cue.mod/module.cue -- +module: "example.com@v0" +language: version: "v0.8.0" +-- _registry/example.com_v0.0.1/blah/blah.cue -- +package blah +-- _registry/example.com_v0.0.1/x.cue -- +package blah +import _ "foo.com/bar/hello/goodbye@v0" +-- _registry/example.com_v0.0.1/y.cue -- +package other +import _ "foo.com/bar/somethingelse@v0" diff --git a/internal/tdtest/tdtest.go b/internal/tdtest/tdtest.go index e3850eb7bea..e249cd67bde 100644 --- a/internal/tdtest/tdtest.go +++ b/internal/tdtest/tdtest.go @@ -30,6 +30,8 @@ import ( "runtime" "strings" "testing" + + "github.com/google/go-cmp/cmp" ) // TODO: @@ -133,7 +135,8 @@ func (t *T) Equal(actual, field any, msgAndArgs ...any) { t.updateField(info, ci, actual) case len(msgAndArgs) == 0: _, ci := t.getCallInfo() - t.Errorf("unexpected value for field %s:\ngot: %v;\nwant: %v", ci.fieldName, actual, field) + //t.Errorf("unexpected value for field %s:\nGOT:\n%v\nWANT:\n%v", ci.fieldName, actual, field) + t.Errorf("unexpected value for field %s:\ndiff (-want, +got):\n%s", ci.fieldName, cmp.Diff(field, actual)) default: format := msgAndArgs[0].(string) + ":\ngot: %v;\nwant: %v" args := append(msgAndArgs[1:], actual, field)