internal/mod/modpkgload: support gen directories

This change adds support for gen directories in the main module.
If there's a package in a gen (or pkg or usr) directory, then it
will be used unless there's another such package already present
in the module graph, in which case we'll get an "ambiguous import"
error.

In order to fit this into the existing algorithm, we invent a new
special module, "local" which notionally holds all packages
that are inside `cue.mod`.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I1b74bf76653301098f525298005b8ac19d831156
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1176005
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>

cmd/cue/cmd/testdata/script/registry_submodule.txtar

@@ -1,6 +1,5 @@
-# Note: this test exposes an error in the current dependency resolution code.
-# this `cue eval` call should not fail, because there is no actual ambiguity here:
-# example.com/foo is only present in a single module.
+# This test tests that submodules work OK.
+
 exec cue eval .
 cmp stdout expect-stdout
 -- expect-stdout --

cmd/cue/cmd/testdata/script/registry_with_pkg.txtar

@@ -1,38 +1,78 @@
 # When CUE_REGISTRY is correctly set, packages in the
-# pkg directory are ignored.
+# pkg, usr and gen directories will be used when they don't conflict with
+# other module dependencies.
+
 exec cue export .
 cmp stdout expect-stdout
 
 -- expect-stdout --
-"registry source"
+{
+    "e": {
+        "a": {
+            "remote": true
+        }
+    },
+    "local_pkg": {
+        "a": {
+            "pkg": true
+        }
+    },
+    "local_pkg_usr": {
+        "a": {
+            "pkg": true,
+            "usr": true
+        }
+    },
+    "local_gen_usr": {
+        "a": {
+            "gen": true,
+            "usr": true
+        }
+    }
+}
 -- main.cue --
 package main
-import "example.com/e"
+import (
+	"example.com/e"
+	local_pkg "example.com/e/local_pkg:x"
+	local_pkg_usr "example.com/e/local_pkg_usr:x"
+	local_gen_usr "example.com/e/local_gen_usr:x"
+)
 
-e.foo
+"e": e
+"local_pkg": local_pkg
+"local_pkg_usr": local_pkg_usr
+"local_gen_usr": local_gen_usr
 
 -- cue.mod/module.cue --
 module: "test.org"
 deps: "example.com/e": v: "v0.0.1"
 
--- cue.mod/pkg/example.com/e/cue.mod/module.cue --
-module: "example.com/e"
+-- cue.mod/pkg/example.com/e/local_pkg/x.cue --
+package x
+a: pkg: true
 
--- cue.mod/pkg/example.com/e/main.cue --
-package e
-foo: "cue.mod/pkg source"
+-- cue.mod/pkg/example.com/e/local_pkg_usr/x.cue --
+package x
+a: pkg: true
 
--- cue.mod/pkg/example.com/e/cue.mod/module.cue --
-module: "example.com/e"
+-- cue.mod/usr/example.com/e/local_pkg_usr/x.cue --
+package x
+a: usr: true
 
--- cue.mod/pkg/example.com/e/main.cue --
-package e
-foo: "cue.mod/pkg source"
+-- cue.mod/usr/example.com/e/local_gen_usr/x.cue --
+package x
+a: usr: true
+
+-- cue.mod/gen/example.com/e/local_gen_usr/x.cue --
+package x
+a: gen: true
 
 -- _registry/example.com_e_v0.0.1/cue.mod/module.cue --
 module: "example.com/e@v0"
 
 -- _registry/example.com_e_v0.0.1/main.cue --
 package e
 
-foo: "registry source"
+a: remote: true
+

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

@@ -0,0 +1,36 @@
+# When CUE_REGISTRY is correctly set, packages in the
+# pkg directory will result in an error when they conflict with actual
+# dependencies.
+
+! exec cue export .
+cmp stderr expect-stderr
+
+-- expect-stderr --
+import failed: cannot find package "example.com/e": ambiguous import: found package example.com/e in multiple modules:
+	example.com/e@v0 v0.0.1 (.)
+	local (cue.mod/pkg/example.com/e):
+    ./main.cue:2:8
+-- main.cue --
+package main
+import "example.com/e"
+
+e.foo
+
+-- cue.mod/module.cue --
+module: "test.org"
+deps: "example.com/e": v: "v0.0.1"
+
+-- cue.mod/pkg/example.com/e/cue.mod/module.cue --
+module: "example.com/e"
+
+-- cue.mod/pkg/example.com/e/main.cue --
+package e
+foo: "cue.mod/pkg source"
+
+-- _registry/example.com_e_v0.0.1/cue.mod/module.cue --
+module: "example.com/e@v0"
+
+-- _registry/example.com_e_v0.0.1/main.cue --
+package e
+
+foo: "registry source"

cmd/cue/cmd/testdata/script/registry_with_remote_gen.txtar

@@ -0,0 +1,43 @@
+# This tests what happens when there's a remote module that includes
+# a gen directory. It's not usable unless a local gen directory is
+# created too.
+
+! exec cue export .
+stderr 'import failed: import failed: cannot find package "other.com/p": cannot find module providing package other.com/p:'
+mkdir cue.mod/gen/other.com/p
+cp _registry/example.com_e_v0.0.1/gen/other.com/p/p.cue cue.mod/gen/other.com/p
+exec cue export .
+cmp stdout expect-stdout
+-- expect-stdout --
+{
+    "e": {
+        "a": {
+            "a": {
+                "p": true
+            }
+        }
+    }
+}
+-- main.cue --
+package main
+import "example.com/e"
+
+"e": e
+
+-- cue.mod/module.cue --
+module: "test.org"
+deps: "example.com/e": v: "v0.0.1"
+
+-- _registry/example.com_e_v0.0.1/cue.mod/module.cue --
+module: "example.com/e@v0"
+
+-- _registry/example.com_e_v0.0.1/main.cue --
+package e
+import "other.com/p"
+
+a: p
+
+-- _registry/example.com_e_v0.0.1/gen/other.com/p/p.cue --
+package p
+a: p: true
+

cue/load/import.go

@@ -356,12 +356,24 @@ func (l *loader) absDirFromImportPath(pos token.Pos, p importPath) (absDir, name
 		if err := pkg.Error(); err != nil {
 			return "", name, errors.Newf(pos, "cannot find package %q: %v", p, err)
 		}
-		absDir1, err1 := absPathForSourceLoc(pkg.Location())
-		if err != nil {
-			return "", name, errors.Newf(pos, "cannot determine source directory for package %q: %v", p, err1)
+		if mv := pkg.Mod(); mv.Path() == "local" {
+			// It's a local package that's present inside one or both of the gen, usr or pkg
+			// directories. Even though modpkgload tells us exactly what those directories
+			// are, the rest of the cue/load logic expects only a single directory for now,
+			// so just use that.
+			absDir = filepath.Join(GenPath(l.cfg.ModuleRoot), parts.Path)
+		} else {
+			locs := pkg.Locations()
+			if len(locs) > 1 {
+				return "", "", errors.Newf(pos, "package %q unexpectedly found in multiple locations", p)
+			}
+			var err error
+			absDir, err = absPathForSourceLoc(locs[0])
+			if err != nil {
+				return "", name, errors.Newf(pos, "cannot determine source directory for package %q: %v", p, err)
+			}
 		}
-
-		return absDir1, name, nil
+		return absDir, name, nil
 	}
 
 	// Determine the directory without using the registry.

internal/mod/modpkgload/import.go

@@ -29,11 +29,11 @@ import (
 // If the package is present in exactly one module, importFromModules will
 // return the module, its root directory, and a list of other modules that
 // lexically could have provided the package but did not.
-func (pkgs *Packages) importFromModules(ctx context.Context, pkgPath string) (m module.Version, loc SourceLoc, altMods []module.Version, err error) {
-	fail := func(err error) (module.Version, SourceLoc, []module.Version, error) {
-		return module.Version{}, SourceLoc{}, nil, err
+func (pkgs *Packages) importFromModules(ctx context.Context, pkgPath string) (m module.Version, pkgLocs []SourceLoc, altMods []module.Version, err error) {
+	fail := func(err error) (module.Version, []SourceLoc, []module.Version, error) {
+		return module.Version{}, []SourceLoc(nil), nil, err
 	}
-	failf := func(format string, args ...interface{}) (module.Version, SourceLoc, []module.Version, error) {
+	failf := func(format string, args ...interface{}) (module.Version, []SourceLoc, []module.Version, error) {
 		return fail(fmt.Errorf(format, args...))
 	}
 	// Note: we don't care about the package qualifier at this point
@@ -54,9 +54,17 @@ func (pkgs *Packages) importFromModules(ctx context.Context, pkgPath string) (m
 	}
 
 	// Check each module on the build list.
-	var locs []SourceLoc
+	var locs [][]SourceLoc
 	var mods []module.Version
 	var mg *modrequirements.ModuleGraph
+	localPkgLocs, err := pkgs.findLocalPackage(pkgPathOnly)
+	if err != nil {
+		return fail(err)
+	}
+	if len(localPkgLocs) > 0 {
+		mods = append(mods, module.MustNewVersion("local", ""))
+		locs = append(locs, localPkgLocs)
+	}
 
 	// Iterate over possible modules for the path, not all selected modules.
 	// Iterating over selected modules would make the overall loading time
@@ -115,7 +123,7 @@ func (pkgs *Packages) importFromModules(ctx context.Context, pkgPath string) (m
 				return fail(fmt.Errorf("cannot find package: %v", err))
 			} else if ok {
 				mods = append(mods, m)
-				locs = append(locs, loc)
+				locs = append(locs, []SourceLoc{loc})
 			} else {
 				altMods = append(altMods, m)
 			}
@@ -210,6 +218,24 @@ func locInModule(pkgPath, mpath string, mloc SourceLoc, isLocal bool) (loc Sourc
 	return loc, haveCUEFiles, err
 }
 
+var localPkgDirs = []string{"cue.mod/gen", "cue.mod/usr", "cue.mod/pkg"}
+
+func (pkgs *Packages) findLocalPackage(pkgPath string) ([]SourceLoc, error) {
+	var locs []SourceLoc
+	for _, d := range localPkgDirs {
+		loc := pkgs.mainModuleLoc
+		loc.Dir = path.Join(loc.Dir, d, pkgPath)
+		ok, err := isDirWithCUEFiles(loc)
+		if err != nil {
+			return nil, err
+		}
+		if ok {
+			locs = append(locs, loc)
+		}
+	}
+	return locs, nil
+}
+
 func isDirWithCUEFiles(loc SourceLoc) (bool, error) {
 	// It would be nice if we could inspect the error returned from
 	// ReadDir to see if it's failing because it's not a directory,
@@ -256,7 +282,7 @@ func (pkgs *Packages) fetch(ctx context.Context, mod module.Version) (loc Source
 // directory.
 type AmbiguousImportError struct {
 	ImportPath string
-	Locations  []SourceLoc
+	Locations  [][]SourceLoc
 	Modules    []module.Version // Either empty or 1:1 with Dirs.
 }
 
@@ -278,9 +304,9 @@ func (e *AmbiguousImportError) Error() string {
 				fmt.Fprintf(&buf, " %s", m.Version())
 			}
 			// TODO work out how to present source locations in error messages.
-			fmt.Fprintf(&buf, " (%s)", loc.Dir)
+			fmt.Fprintf(&buf, " (%s)", loc[0].Dir)
 		} else {
-			buf.WriteString(loc.Dir)
+			buf.WriteString(loc[0].Dir)
 		}
 	}
 

internal/mod/modpkgload/pkgload.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/fs"
 	"runtime"
+	"sort"
 	"strings"
 	"sync/atomic"
 
@@ -95,7 +96,7 @@ type Package struct {
 
 	// Populated by [loader.load].
 	mod          module.Version // module providing package
-	loc          SourceLoc      // location of source code
+	locs         []SourceLoc    // location of source code directories
 	err          error          // error loading package
 	imports      []*Package     // packages imported by this one
 	inStd        bool
@@ -122,8 +123,8 @@ func (pkg *Package) FromExternalModule() bool {
 	return pkg.fromExternal
 }
 
-func (pkg *Package) Location() SourceLoc {
-	return pkg.loc
+func (pkg *Package) Locations() []SourceLoc {
+	return pkg.locs
 }
 
 func (pkg *Package) Error() error {
@@ -245,19 +246,30 @@ func (pkgs *Packages) load(ctx context.Context, pkg *Package) {
 		return
 	}
 	pkg.fromExternal = pkg.mod != pkgs.mainModuleVersion
-	pkg.mod, pkg.loc, pkg.altMods, pkg.err = pkgs.importFromModules(ctx, pkg.path)
+	pkg.mod, pkg.locs, pkg.altMods, pkg.err = pkgs.importFromModules(ctx, pkg.path)
 	if pkg.err != nil {
 		return
 	}
 	if pkgs.mainModuleVersion.Path() == pkg.mod.Path() {
 		pkgs.applyPkgFlags(ctx, pkg, PkgInAll)
 	}
 	pkgQual := module.ParseImportPath(pkg.path).Qualifier
-	imports, err := modimports.AllImports(modimports.PackageFiles(pkg.loc.FS, pkg.loc.Dir, pkgQual))
-	if err != nil {
-		pkg.err = fmt.Errorf("cannot get imports: %v", err)
-		return
+	importsMap := make(map[string]bool)
+	for _, loc := range pkg.locs {
+		imports, err := modimports.AllImports(modimports.PackageFiles(loc.FS, loc.Dir, pkgQual))
+		if err != nil {
+			pkg.err = fmt.Errorf("cannot get imports: %v", err)
+			return
+		}
+		for _, imp := range imports {
+			importsMap[imp] = true
+		}
+	}
+	imports := make([]string, 0, len(importsMap))
+	for imp := range importsMap {
+		imports = append(imports, imp)
 	}
+	sort.Strings(imports) // Make the algorithm deterministic for tests.
 
 	pkg.imports = make([]*Package, 0, len(imports))
 	var importFlags Flags

internal/mod/modpkgload/pkgload_test.go

@@ -65,7 +65,9 @@ func TestLoadPackages(t *testing.T) {
 						printf("\tmissing: %v\n", errors.As(pkg.Error(), new(*ImportMissingError)))
 					} else {
 						printf("\tmod: %v\n", pkg.Mod())
-						printf("\tlocation: %v\n", pkg.Location().Dir)
+						for _, loc := range pkg.Locations() {
+							printf("\tlocation: %v\n", loc.Dir)
+						}
 						if imps := pkg.Imports(); len(imps) > 0 {
 							printf("\timports:\n")
 							for _, imp := range imps {