cue/load: consistently use canonical import paths to load packages

Otherwise trying to load an import path with an unnecessary qualifier,
such as "mod.com/x:x", would be added to the map of known packages
under its canonical form "mod.com/x", but we would do the lookup
based on the original non-canonical import path, not finding a match.

Add reminders to the relevant places about the paths being canonical,
and use a separate variable for unqualified import paths for clarity.
The use of unqualified paths seems wrong to me too, so add a TODO.

Fixes #3162.

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

cmd/cue/cmd/testdata/script/pkg_resolution_single_package_matching_path_element.txtar

@@ -9,6 +9,15 @@ cmp stdout output.golden
 exec cue eval ./x
 cmp stdout output.golden
 
+# Also test that the above all works when using an explicit qualifier.
+
+exec cue eval root_qualifier.cue
+cmp stdout output.golden
+exec cue eval mod.com/x:x
+cmp stdout output.golden
+exec cue eval ./x:x
+cmp stdout output.golden
+
 -- output.golden --
 x: 5
 -- cue.mod/module.cue --
@@ -19,6 +28,12 @@ package root
 
 import "mod.com/x"
 
+x
+-- root_qualifier.cue --
+package root
+
+import "mod.com/x:x"
+
 x
 -- x/y.cue --
 package x

cue/load/import.go

@@ -407,25 +407,27 @@ func (l *loader) absDirFromImportPath1(pos token.Pos, p importPath) (absDir stri
 	if isStdlibPackage(string(p)) {
 		return "", fmt.Errorf("standard library import path %q cannot be imported as a CUE package", p)
 	}
-	origp := p
 	// Extract the package name.
 	parts := module.ParseImportPath(string(p))
-	p = importPath(parts.Unqualified().String())
+	unqualified := parts.Unqualified().String()
 	if l.cfg.Registry != nil {
 		if l.pkgs == nil {
 			return "", fmt.Errorf("imports are unavailable because there is no cue.mod/module.cue file")
 		}
 		// TODO predicate registry-aware lookup on module.cue-declared CUE version?
 
-		// Note: use the original form of the import path because
-		// that's the form passed to modpkgload.LoadPackages
+		// Note: use the canonical form of the import path because
+		// that's the form passed to [modpkgload.LoadPackages]
 		// and hence it's available by that name via Pkg.
-		pkg := l.pkgs.Pkg(string(origp))
+		pkg := l.pkgs.Pkg(parts.Canonical().String())
+		// TODO(mvdan): using "unqualified" for the errors below doesn't seem right,
+		// should we not be using either the original path or the canonical path?
+		// The unqualified import path should only be used for filepath.FromSlash further below.
 		if pkg == nil {
-			return "", fmt.Errorf("no dependency found for package %q", p)
+			return "", fmt.Errorf("no dependency found for package %q", unqualified)
 		}
 		if err := pkg.Error(); err != nil {
-			return "", fmt.Errorf("cannot find package %q: %v", p, err)
+			return "", fmt.Errorf("cannot find package %q: %v", unqualified, err)
 		}
 		if mv := pkg.Mod(); mv.IsLocal() {
 			// It's a local package that's present inside one or both of the gen, usr or pkg
@@ -436,28 +438,28 @@ func (l *loader) absDirFromImportPath1(pos token.Pos, p importPath) (absDir stri
 		} else {
 			locs := pkg.Locations()
 			if len(locs) > 1 {
-				return "", fmt.Errorf("package %q unexpectedly found in multiple locations", p)
+				return "", fmt.Errorf("package %q unexpectedly found in multiple locations", unqualified)
 			}
 			if len(locs) == 0 {
-				return "", fmt.Errorf("no location found for package %q", p)
+				return "", fmt.Errorf("no location found for package %q", unqualified)
 			}
 			var err error
 			absDir, err = absPathForSourceLoc(locs[0])
 			if err != nil {
-				return "", fmt.Errorf("cannot determine source directory for package %q: %v", p, err)
+				return "", fmt.Errorf("cannot determine source directory for package %q: %v", unqualified, err)
 			}
 		}
 		return absDir, nil
 	}
 
 	// Determine the directory without using the registry.
 
-	sub := filepath.FromSlash(string(p))
-	switch hasPrefix := strings.HasPrefix(string(p), l.cfg.Module); {
+	sub := filepath.FromSlash(unqualified)
+	switch hasPrefix := strings.HasPrefix(unqualified, l.cfg.Module); {
 	case hasPrefix && len(sub) == len(l.cfg.Module):
 		absDir = l.cfg.ModuleRoot
 
-	case hasPrefix && p[len(l.cfg.Module)] == '/':
+	case hasPrefix && unqualified[len(l.cfg.Module)] == '/':
 		absDir = filepath.Join(l.cfg.ModuleRoot, sub[len(l.cfg.Module)+1:])
 
 	default:

cue/load/instances.go

@@ -168,7 +168,7 @@ func loadPackages(ctx context.Context, cfg *Config, synCache *syntaxCache, pkgs
 	pkgPaths := make(map[string]bool)
 	// Add any packages specified directly on the command line.
 	for _, pkg := range pkgs {
-		pkgPaths[pkg.resolved] = true
+		pkgPaths[pkg.resolvedCanonical] = true
 	}
 	// Add any imports found in other files.
 	for _, f := range otherFiles {

cue/load/search.go

@@ -272,8 +272,8 @@ func (l *loader) importPathsQuiet(patterns []string) []*match {
 type resolvedPackageArg struct {
 	// The original field may be needed once we want to replace the original
 	// package pattern matching code, as it is necessary to populate Instance.DisplayPath.
-	original string
-	resolved string
+	original          string
+	resolvedCanonical string
 }
 
 func expandPackageArgs(c *Config, pkgArgs []string, pkgQual string, tg *tagger) ([]resolvedPackageArg, error) {
@@ -350,7 +350,7 @@ func appendExpandedPackageArg(c *Config, pkgPaths []resolvedPackageArg, p string
 				Dir: ".",
 			}, c.Module, tg)
 		}
-		return append(pkgPaths, resolvedPackageArg{origp, ip.String()}), nil
+		return append(pkgPaths, resolvedPackageArg{origp, ip.Canonical().String()}), nil
 	}
 	// Strip the module prefix, leaving only the directory relative
 	// to the module root.

internal/mod/modpkgload/pkgload.go

@@ -146,6 +146,8 @@ func (pkg *Package) Mod() module.Version {
 // packages they import, recursively, using modules from the given
 // requirements to determine which modules they might be obtained from,
 // and reg to download module contents.
+//
+// rootPkgPaths should only contain canonical import paths.
 func LoadPackages(
 	ctx context.Context,
 	mainModulePath string,
@@ -210,8 +212,9 @@ func (pkgs *Packages) All() []*Package {
 	return slices.Clip(pkgs.pkgs)
 }
 
-func (pkgs *Packages) Pkg(pkgPath string) *Package {
-	pkg, _ := pkgs.pkgCache.Get(pkgPath)
+// Pkg obtains a given package given its canonical import path.
+func (pkgs *Packages) Pkg(canonicalPkgPath string) *Package {
+	pkg, _ := pkgs.pkgCache.Get(canonicalPkgPath)
 	return pkg
 }