Skip to content

Commit

Permalink
cue/load: use modpkgload to resolve dependencies
Browse files Browse the repository at this point in the history
This uses exactly the same logic as `cue mod tidy` to resolve all
the dependencies for `cue/load`. It's not hugely efficient as we
end up doing a redundant scan through all the imports, but fixing
that would require a much more significant refactor of `cue/load`
which we'd like to avoid for now.

It does expose some deficiencies in the current test data: the
dependency resolution will now fail if a dependency isn't explicitly
mentioned in the main module - something that's easily remedied by
running `cue mod tidy`.

We also implement a rule for defaulting the major version of the
main module:
- if there's no major version in the module path in the module.cue
file, we assume @v0.
- the default major version for the main module is always that
specified in the `module:` declaration (and we reject a module file
that tried to specify default major version for a different major
version of the main module)

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I8ce8719d834ad1449097c17846f35445991bf36b
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1175995
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
  • Loading branch information
rogpeppe committed Jan 25, 2024
1 parent 8889d6a commit fa12218
Show file tree
Hide file tree
Showing 17 changed files with 184 additions and 80 deletions.
3 changes: 1 addition & 2 deletions cmd/cue/cmd/testdata/script/registry_auth.txtar
Expand Up @@ -10,8 +10,7 @@ env CUE_MODCACHE=$WORK/.tmp/different-cache
env-fill dockerconfig/badpassword.json
cp dockerconfig/badpassword.json dockerconfig/config.json
! exec cue export .
stderr 'instance: cannot resolve dependencies: example.com/e@v0.0.1: module example.com/e@v0.0.1: error response: 401 Unauthorized; body: "invalid credentials'

stderr 'import failed: cannot find package "example.com/e": cannot fetch example.com/e@v0.0.1: module example.com/e@v0.0.1: error response: 401 Unauthorized; body: "invalid credentials'
-- dockerconfig/config.json --
{
"auths": {
Expand Down
3 changes: 1 addition & 2 deletions cmd/cue/cmd/testdata/script/registry_lazy_config.txtar
Expand Up @@ -12,8 +12,7 @@ cmp stdout expect-stdout
cp OTHER/main.cue main.cue
cp OTHER/cue.mod/module.cue cue.mod/module.cue
! exec cue export .
stderr 'instance: cannot resolve dependencies: example.com/e@v0.0.1: module example.com/e@v0.0.1: cannot make client: cannot load OCI auth configuration: invalid config file ".*config.json": decode failed: .*'

stderr '^import failed: cannot find package "example.com/e": cannot fetch example.com/e@v0.0.1: module example.com/e@v0.0.1: cannot make client: cannot load OCI auth configuration: invalid config file ".*config.json": decode failed: .*'
-- dockerconfig/config.json --
should be JSON but isn't
-- expect-stdout --
Expand Down
5 changes: 3 additions & 2 deletions cmd/cue/cmd/testdata/script/registry_module_not_found.txtar
Expand Up @@ -5,7 +5,8 @@
cmp stderr expect-stderr

-- expect-stderr --
instance: cannot resolve dependencies: example.com/e@v0.0.2: module example.com/e@v0.0.2: module not found
import failed: cannot find package "example.com/e": cannot fetch example.com/e@v0.0.2: module example.com/e@v0.0.2: module not found:
./main.cue:2:8
-- main.cue --
package main
import "example.com/e"
Expand All @@ -18,5 +19,5 @@ deps: "example.com/e": v: "v0.0.2"
-- _registry/example.com_e_v0.0.1/cue.mod/module.cue --
module: "example.com/e@v0"

-- main.cue --
-- _registry/example.com_e_v0.0.1/main.cue --
package e
20 changes: 16 additions & 4 deletions cmd/cue/cmd/testdata/script/registry_mux.txtar
Expand Up @@ -8,10 +8,22 @@ main: "main"
"baz.org@v0": "v0.10.1 in registry2"
"example.com@v0": "v0.0.1"
-- cue.mod/module.cue --
module: "main.org"

deps: "example.com@v0": v: "v0.0.1"

module: "main.org@v0"
deps: {
"bar.com@v0": {
v: "v0.5.0"
}
"baz.org@v0": {
v: "v0.10.1"
}
"example.com@v0": {
v: "v0.0.1"
}
"foo.com/bar/hello@v0": {
v: "v0.2.3"
default: true
}
}
-- main.cue --
package main
import "example.com@v0:main"
Expand Down
20 changes: 16 additions & 4 deletions cmd/cue/cmd/testdata/script/registry_mux_auth.txtar
Expand Up @@ -26,10 +26,22 @@ main: "main"
}
}
-- cue.mod/module.cue --
module: "main.org"

deps: "example.com@v0": v: "v0.0.1"

module: "main.org@v0"
deps: {
"bar.com@v0": {
v: "v0.5.0"
}
"baz.org@v0": {
v: "v0.10.1"
}
"example.com@v0": {
v: "v0.0.1"
}
"foo.com/bar/hello@v0": {
v: "v0.2.3"
default: true
}
}
-- main.cue --
package main
import "example.com@v0:main"
Expand Down
16 changes: 12 additions & 4 deletions cmd/cue/cmd/testdata/script/registry_publish.txtar
Expand Up @@ -24,9 +24,16 @@ main: "main"
"baz.org@v0": "v0.10.1"
"example.com@v0": "v0.0.1"
-- main/cue.mod/module.cue --
module: "main.org"

deps: "example.com@v0": v: "v0.0.1"
module: "main.org@v0"
deps: {
"bar.com@v0": v: "v0.5.0"
"baz.org@v0": v: "v0.10.1"
"example.com@v0": v: "v0.0.1"
"foo.com/bar/hello@v0": {
v: "v0.2.3"
default: true
}
}

-- main/main.cue --
package main
Expand All @@ -37,8 +44,9 @@ main
-- example/cue.mod/module.cue --
module: "example.com@v0"
deps: {
"foo.com/bar/hello@v0": v: "v0.2.3"
"bar.com@v0": v: "v0.5.0"
"baz.org@v0": v: "v0.10.1"
"foo.com/bar/hello@v0": v: "v0.2.3"
}

-- example/top.cue --
Expand Down
21 changes: 17 additions & 4 deletions cmd/cue/cmd/testdata/script/registry_simple.txtar
@@ -1,3 +1,4 @@
exec cat cue.mod/module.cue
exec cue eval .
cmp stdout expect-stdout
-- expect-stdout --
Expand All @@ -7,10 +8,22 @@ main: "main"
"baz.org@v0": "v0.10.1"
"example.com@v0": "v0.0.1"
-- cue.mod/module.cue --
module: "main.org"

deps: "example.com@v0": v: "v0.0.1"

module: "main.org@v0"
deps: {
"bar.com@v0": {
v: "v0.5.0"
}
"baz.org@v0": {
v: "v0.10.1"
}
"example.com@v0": {
v: "v0.0.1"
}
"foo.com/bar/hello@v0": {
v: "v0.2.3"
default: true
}
}
-- main.cue --
package main
import "example.com@v0:main"
Expand Down
9 changes: 4 additions & 5 deletions cmd/cue/cmd/testdata/script/registry_submodule.txtar
@@ -1,13 +1,12 @@
# 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.
! exec cue eval .
stderr 'import failed: cannot get directory for external module "example.com/foo@v0": more than one default module for import path "example.com/foo@v0"'
# cmp stdout expect-stdout
exec cue eval .
cmp stdout expect-stdout
-- expect-stdout --
a: {
e: true
f: true
e: true
f: true
}
-- cue.mod/module.cue --
module: "main.org"
Expand Down
7 changes: 5 additions & 2 deletions cmd/cue/cmd/testdata/script/registry_wrong_prefix.txtar
Expand Up @@ -3,7 +3,8 @@ env CUE_REGISTRY=$DEBUG_REGISTRY_HOST/something+insecure
cmp stderr expect-stderr

-- expect-stderr --
instance: cannot resolve dependencies: example.com/e@v0.0.1: module example.com/e@v0.0.1: 404 Not Found: name unknown: repository name not known to registry
import failed: cannot find package "example.com/e": cannot fetch example.com/e@v0.0.1: module example.com/e@v0.0.1: 404 Not Found: name unknown: repository name not known to registry:
./main.cue:2:8
-- main.cue --
package main
import "example.com/e"
Expand All @@ -16,5 +17,7 @@ deps: "example.com/e": v: "v0.0.1"
-- _registry/example.com_e_v0.0.1/cue.mod/module.cue --
module: "example.com/e@v0"

-- main.cue --
-- _registry/example.com_e_v0.0.1/main.cue --
package e

foo: "blah"
57 changes: 25 additions & 32 deletions cue/load/import.go
Expand Up @@ -15,7 +15,6 @@
package load

import (
"context"
"fmt"
"os"
pathpkg "path"
Expand All @@ -29,6 +28,7 @@ import (
"cuelang.org/go/cue/token"
"cuelang.org/go/internal/filetypes"
"cuelang.org/go/internal/mod/modcache"
"cuelang.org/go/internal/mod/modpkgload"
"cuelang.org/go/internal/mod/module"
)

Expand Down Expand Up @@ -338,7 +338,6 @@ func (l *loader) absDirFromImportPath(pos token.Pos, p importPath) (absDir, name
parts := module.ParseImportPath(string(p))
name = parts.Qualifier
p = importPath(parts.Unqualified().String())
// TODO: fully test that name is a valid identifier.
if name == "" {
err = errors.Newf(pos, "empty package name in import path %q", p)
} else if strings.IndexByte(name, '.') >= 0 {
Expand All @@ -348,8 +347,24 @@ func (l *loader) absDirFromImportPath(pos token.Pos, p importPath) (absDir, name
err = errors.Newf(pos,
"implied package identifier %q from import path %q is not valid", name, p)
}
if l.cfg.Registry != nil {
// TODO predicate registry-aware lookup on module.cue-declared CUE version?
pkg := l.pkgs.Pkg(parts.Canonical().String())
if pkg == nil {
return "", name, errors.Newf(pos, "no dependency found for package %q", p)
}
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)
}

// Determine the directory.
return absDir1, name, nil
}

// Determine the directory without using the registry.

sub := filepath.FromSlash(string(p))
switch hasPrefix := strings.HasPrefix(string(p), l.cfg.Module); {
Expand All @@ -360,41 +375,19 @@ func (l *loader) absDirFromImportPath(pos token.Pos, p importPath) (absDir, name
absDir = filepath.Join(l.cfg.ModuleRoot, sub[len(l.cfg.Module)+1:])

default:
// TODO predicate registry-aware lookup on module.cue-declared CUE version?
if l.cfg.Registry != nil {
var err error
absDir, err = l.externalPackageDir(p)
if err != nil {
// TODO why can't we use %w ?
return "", name, errors.Newf(token.NoPos, "cannot get directory for external module %q: %v", p, err)
}
} else {
absDir = filepath.Join(GenPath(l.cfg.ModuleRoot), sub)
}
absDir = filepath.Join(GenPath(l.cfg.ModuleRoot), sub)
}

return absDir, name, err
}

func (l *loader) externalPackageDir(p importPath) (dir string, err error) {
if l.deps == nil {
return "", fmt.Errorf("no dependency found for import path %q (no dependencies at all)", p)
}
m, subPath, err := l.deps.lookup(p)
if err != nil {
return "", err
}
loc, err := l.cfg.Registry.Fetch(context.TODO(), m)
if err != nil {
return "", fmt.Errorf("cannot get contents for %v: %v", m, err)
}
func absPathForSourceLoc(loc modpkgload.SourceLoc) (string, error) {
osfs, ok := loc.FS.(modcache.OSRootFS)
if !ok {
return "", fmt.Errorf("cannot get root for downloaded module in FS of type %T", loc.FS)
return "", fmt.Errorf("cannot get absolute path for FS of type %T", loc.FS)
}
fsRoot := osfs.OSRoot()
if fsRoot == "" {
return "", fmt.Errorf("cannot get root for downloaded module in FS of type %T", loc.FS)
osPath := osfs.OSRoot()
if osPath == "" {
return "", fmt.Errorf("cannot get absolute path for FS of type %T", loc.FS)
}
return filepath.Join(fsRoot, filepath.FromSlash(loc.Dir), filepath.FromSlash(subPath)), nil
return filepath.Join(osPath, loc.Dir), nil
}
41 changes: 35 additions & 6 deletions cue/load/instances.go
Expand Up @@ -20,11 +20,15 @@ package load
// - go/build

import (
"context"
"fmt"

"cuelang.org/go/cue/ast"
"cuelang.org/go/cue/build"
"cuelang.org/go/internal/filetypes"
"cuelang.org/go/internal/mod/modimports"
"cuelang.org/go/internal/mod/modpkgload"
"cuelang.org/go/internal/mod/modrequirements"

// Trigger the unconditional loading of all core builtin packages if load
// is used. This was deemed the simplest way to avoid having to import
Expand All @@ -39,6 +43,7 @@ import (
// instance, but errors that occur loading dependencies are recorded in these
// dependencies.
func Instances(args []string, c *Config) []*build.Instance {
ctx := context.TODO()
if c == nil {
c = &Config{}
}
Expand All @@ -47,18 +52,17 @@ func Instances(args []string, c *Config) []*build.Instance {
return []*build.Instance{c.newErrInstance(err)}
}
c = newC

// TODO use predictable location
var deps *dependencies
var pkgs *modpkgload.Packages
if c.Registry != nil {
deps1, err := resolveDependencies(c.modFile, c.Registry)
pkgs, err = loadPackages(ctx, c)
if err != nil {
return []*build.Instance{c.newErrInstance(fmt.Errorf("cannot resolve dependencies: %v", err))}
return []*build.Instance{c.newErrInstance(err)}
}
deps = deps1

}
tg := newTagger(c)
l := newLoader(c, tg, deps)
l := newLoader(c, tg, pkgs)

if c.Context == nil {
c.Context = build.NewContext(
Expand Down Expand Up @@ -129,3 +133,28 @@ func Instances(args []string, c *Config) []*build.Instance {

return a
}

func loadPackages(ctx context.Context, cfg *Config) (*modpkgload.Packages, error) {
reqs := modrequirements.NewRequirements(
cfg.modFile.Module,
cfg.Registry,
cfg.modFile.DepVersions(),
cfg.modFile.DefaultMajorVersions(),
)
mainModLoc := modpkgload.SourceLoc{
FS: cfg.fileSystem.ioFS(cfg.ModuleRoot),
Dir: ".",
}
allImports, err := modimports.AllImports(modimports.AllModuleFiles(mainModLoc.FS, mainModLoc.Dir))
if err != nil {
return nil, fmt.Errorf("cannot enumerate all module imports: %v", err)
}
return modpkgload.LoadPackages(
ctx,
cfg.Module,
mainModLoc,
reqs,
cfg.Registry,
allImports,
), nil
}
7 changes: 4 additions & 3 deletions cue/load/loader.go
Expand Up @@ -26,6 +26,7 @@ import (
"cuelang.org/go/cue/errors"
"cuelang.org/go/cue/token"
"cuelang.org/go/internal/encoding"
"cuelang.org/go/internal/mod/modpkgload"

// Trigger the unconditional loading of all core builtin packages if load
// is used. This was deemed the simplest way to avoid having to import
Expand All @@ -39,14 +40,14 @@ type loader struct {
tagger *tagger
stk importStack
loadFunc build.LoadFunc
deps *dependencies
pkgs *modpkgload.Packages
}

func newLoader(c *Config, tg *tagger, deps *dependencies) *loader {
func newLoader(c *Config, tg *tagger, pkgs *modpkgload.Packages) *loader {
l := &loader{
cfg: c,
tagger: tg,
deps: deps,
pkgs: pkgs,
}
l.loadFunc = l._loadFunc
return l
Expand Down

0 comments on commit fa12218

Please sign in to comment.