Skip to content

Commit

Permalink
internal/cueexperiment: flip modules experiment flag default
Browse files Browse the repository at this point in the history
DO NOT REVIEW: some fixes still needed

This change enables the modules experiment by default.
This will cause CUE code to fail if there is no language
version in the module.cue file, amongst other things.

The experiment can be disabled by setting

        CUE_EXPERIMENT=modules=0

Many of the unit tests need to be changed in order to
add the now-mandatory language.version field.

Some error messages have now changed.

Some logic in the `cue fmt` command that relies on seeing
a `load.PackageError` required the respective code inside
`cue/load` to produce that error, which also changes some
error messages.

The code which adds `"user"` to the path stack when evaluating
command-line-provided files has been removed as being
confusing (ISTM that the name "user" is not something that
is likely to make any sense to an end user in that context).

The protobuf code is fixed to work in the presence
of major version module suffixes.

The `cue/load` code is fixed to construct import paths correctly
in the presence of major version suffixes. That also addresses
an existing TODO to use `module.ParseImportPath`.

An upcoming `cue mod fix` command will be added to make
it more straightforward to migrate.

Fixes #3127.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I7c384c785eac6203966460703283ecfa2d9f0447
  • Loading branch information
rogpeppe committed May 16, 2024
1 parent 45f9aba commit 1080e28
Show file tree
Hide file tree
Showing 17 changed files with 64 additions and 55 deletions.
10 changes: 2 additions & 8 deletions cmd/cue/cmd/script_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ func TestScript(t *testing.T) {
if err != nil {
return fmt.Errorf("cannot read workdir: %v", err)
}
hasRegistry := false
// As modules are enabled by default, we always want a cache directory.
e.Vars = append(e.Vars, "CUE_CACHE_DIR="+filepath.Join(e.WorkDir, ".tmp/cache"))
for _, entry := range entries {
if !entry.IsDir() {
continue
Expand All @@ -250,7 +251,6 @@ func TestScript(t *testing.T) {
}
// There's a _registry directory. Start a fake registry server to serve
// the modules in it.
hasRegistry = true
registryDir := filepath.Join(e.WorkDir, entry.Name())
prefix := ""
if data, err := os.ReadFile(filepath.Join(e.WorkDir, "_registry"+regID+"_prefix")); err == nil {
Expand All @@ -271,15 +271,9 @@ func TestScript(t *testing.T) {
// Some tests execute cue commands that need to write cache files.
// Since os.UserCacheDir relies on OS-specific env vars that we don't set,
// explicitly set up the cache directory somewhere predictable.
"CUE_CACHE_DIR="+filepath.Join(e.WorkDir, ".tmp/cache"),
)
e.Defer(reg.Close)
}
if hasRegistry {
e.Vars = append(e.Vars,
"CUE_EXPERIMENT=modules",
)
}
return nil
},
Condition: cuetest.Condition,
Expand Down
6 changes: 3 additions & 3 deletions cmd/cue/cmd/testdata/script/cmd_filetypes.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ 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 'cannot find package ".*somepkg"'
[!windows] stderr 'no location found for package ".*somepkg"'
[windows] stderr 'no encoding specified for file ".*somepkg.*"'
! exec cue export ${WORK}${/}somepkg${/}
[!windows] stderr 'cannot find package ".*somepkg"'
[!windows] stderr 'no dependency found for package ".*somepkg"'
[windows] stderr 'no encoding specified for file ".*somepkg.*"'

# Now do some of the same filename tests, but for a file without any extension.
Expand All @@ -57,7 +57,7 @@ stderr 'standard library import path "somefile" cannot be imported as a CUE pack
stderr 'cannot find package "\./somefile"'
# TODO(mvdan): we probably should not behave differently on Windows.
! exec cue export ${WORK}${/}somefile
[!windows] stderr 'cannot find package ".*somefile"'
[!windows] stderr 'no location found for package ".*somefile"'
[windows] stderr 'no encoding specified for file ".*somefile"'

-- cue.mod/module.cue --
Expand Down
3 changes: 1 addition & 2 deletions cmd/cue/cmd/testdata/script/issue174.txtar
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
! exec cue export ./issue174
cmp stderr expect-stderr
-- expect-stderr --
build constraints exclude all CUE files in ./issue174:
issue174/issue174.cue: no package name
cannot enumerate all module imports: invalid import path "'foo'" in issue174/issue174.cue
-- cue.mod/module.cue --
module: "mod.test"
language: version: "v0.9.0"
Expand Down
1 change: 1 addition & 0 deletions cmd/cue/cmd/testdata/script/modinit_nomajorversion.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ env-fill want-module.cue
env-fill want-module-experiment.cue

# Without the experiment, we use the module path as-is.
env CUE_EXPERIMENT=modules=false
exec cue mod init foo.com/bar
cmp cue.mod/module.cue want-module.cue
exists cue.mod/usr
Expand Down
4 changes: 2 additions & 2 deletions cmd/cue/cmd/testdata/script/modinit_withsource.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ cd $WORK/test2
cmp stderr $WORK/want-stderr

-- want-module.cue-0 --
module: "test.example"
module: "test.example@v0"
language: {
version: "$CUE_LANGUAGE_VERSION"
}
source: {
kind: "self"
}
-- want-module.cue-1 --
module: "test.example"
module: "test.example@v0"
language: {
version: "$CUE_LANGUAGE_VERSION"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
env CUE_EXPERIMENT=modules=false
! exec cue mod publish v1.0.0
cmp stderr want-stderr
-- want-stderr --
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
! exec cue mod tidy
cmp stderr want-stderr
-- want-stderr --
modules experiment not enabled (enable with CUE_EXPERIMENT=modules)
# Check that modules are enabled by default.
exec cue mod tidy
-- cue.mod/module.cue --
module: "test.example"
language: version: "v0.9.0"
4 changes: 2 additions & 2 deletions cmd/cue/cmd/testdata/script/registry_experiment_not_set.txtar
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# When CUE_EXPERIMENT is empty, the CUE_REGISTRY
# When CUE_EXPERIMENT is disabled, the CUE_REGISTRY
# variable is ignored, as are the new fields in module.cue

env CUE_EXPERIMENT=
env CUE_EXPERIMENT=modules=false
exec cue export .
cmp stdout expect-stdout

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
env CUE_EXPERIMENT=
env CUE_EXPERIMENT=modules=false
env CUE_REGISTRY=ignored.example.com
exec cue export .
cmp stdout expect-stdout
Expand Down
1 change: 1 addition & 0 deletions cue/interpreter/wasm/exe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func TestExe(t *testing.T) {
RequireUniqueNames: true,
Setup: func(e *testscript.Env) error {
copyWasmFiles(t, e.WorkDir, wasmFiles)
e.Vars = append(e.Vars, "CUE_CACHE_DIR="+filepath.Join(e.WorkDir, ".tmp/cache"))
return nil
},
Condition: cuetest.Condition,
Expand Down
62 changes: 37 additions & 25 deletions cue/load/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,14 @@ func TestLoad(t *testing.T) {
name: "BadModuleFile",
cfg: badModCfg,
args: []string{"."},
want: `err: module: cannot use value 123 (type int) as string:
want: `err: module: 2 errors in empty disjunction:
module: conflicting values 123 and "" (mismatched types int and string):
$CWD/testdata/badmod/cue.mod/module.cue:2:9
cuelang.org/go/mod/modfile/schema.cue:56:22
module: conflicting values 123 and string (mismatched types int and string):
$CWD/testdata/badmod/cue.mod/module.cue:2:9
cuelang.org/go/mod/modfile/schema.cue:56:12
cuelang.org/go/mod/modfile/schema.cue:101:12
path: ""
module: ""
root: ""
Expand All @@ -91,30 +97,30 @@ display:""`,
// package of this directory.
cfg: dirCfg,
args: nil,
want: `err: import failed: cannot find package "mod.test/test/sub":
$CWD/testdata/testmod/test.cue:3:8
path: mod.test/test@v0
want: `path: mod.test/test@v0
module: mod.test/test@v0
root: $CWD/testdata/testmod
dir: $CWD/testdata/testmod
display:.
files:
$CWD/testdata/testmod/test.cue`}, {
$CWD/testdata/testmod/test.cue
imports:
mod.test/test/sub: $CWD/testdata/testmod/sub/sub.cue`}, {
name: "DefaultPackageWithExplicitDotArgument",
// Even though the directory is called testdata, the last path in
// the module is test. So "package test" is correctly the default
// package of this directory.
cfg: dirCfg,
args: []string{"."},
want: `err: import failed: cannot find package "mod.test/test/sub":
$CWD/testdata/testmod/test.cue:3:8
path: mod.test/test@v0
want: `path: mod.test/test@v0
module: mod.test/test@v0
root: $CWD/testdata/testmod
dir: $CWD/testdata/testmod
display:.
files:
$CWD/testdata/testmod/test.cue`}, {
$CWD/testdata/testmod/test.cue
imports:
mod.test/test/sub: $CWD/testdata/testmod/sub/sub.cue`}, {
name: "RelativeImportPathWildcard",
cfg: dirCfg,
args: []string{"./other/..."},
Expand Down Expand Up @@ -150,36 +156,41 @@ files:
name: "RelativePathSuccess",
cfg: dirCfg,
args: []string{"./hello"},
want: `err: import failed: cannot find package "mod.test/test/sub":
$CWD/testdata/testmod/test.cue:3:8
path: mod.test/test/hello@v0:test
want: `path: mod.test/test/hello@v0:test
module: mod.test/test@v0
root: $CWD/testdata/testmod
dir: $CWD/testdata/testmod/hello
display:./hello
files:
$CWD/testdata/testmod/test.cue
$CWD/testdata/testmod/hello/test.cue`}, {
$CWD/testdata/testmod/hello/test.cue
imports:
mod.test/test/sub: $CWD/testdata/testmod/sub/sub.cue`}, {
name: "ExplicitPackageIdentifier",
cfg: dirCfg,
args: []string{"mod.test/test/hello:test"},
want: `err: cannot find package "mod.test/test/hello:test"
path: mod.test/test/hello:test
want: `path: mod.test/test/hello:test
module: mod.test/test@v0
root: $CWD/testdata/testmod
dir: $CWD/testdata/testmod/cue.mod/gen/mod.test/test/hello
display:mod.test/test/hello:test`,
}, {
dir: $CWD/testdata/testmod/hello
display:mod.test/test/hello:test
files:
$CWD/testdata/testmod/test.cue
$CWD/testdata/testmod/hello/test.cue
imports:
mod.test/test/sub: $CWD/testdata/testmod/sub/sub.cue`}, {
name: "NoPackageName",
cfg: dirCfg,
args: []string{"mod.test/test/hello:nonexist"},
want: `err: cannot find package "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
path: mod.test/test/hello:nonexist
module: mod.test/test@v0
root: $CWD/testdata/testmod
dir: $CWD/testdata/testmod/cue.mod/gen/mod.test/test/hello
display:mod.test/test/hello:nonexist`,
}, {
dir: $CWD/testdata/testmod/hello
display:mod.test/test/hello:nonexist`}, {
name: "ExplicitNonPackageFiles",
cfg: dirCfg,
args: []string{"./anon.cue", "./other/anon.cue"},
Expand All @@ -190,7 +201,8 @@ dir: $CWD/testdata/testmod
display:command-line-arguments
files:
$CWD/testdata/testmod/anon.cue
$CWD/testdata/testmod/other/anon.cue`}, {
$CWD/testdata/testmod/other/anon.cue`,
}, {
name: "AbsoluteFileIsNormalized", // TODO(rogpeppe) what is this actually testing?
cfg: dirCfg,
// Absolute file is normalized.
Expand All @@ -215,11 +227,11 @@ files:
name: "BadIdentifier",
cfg: dirCfg,
args: []string{"foo.com/bad-identifier"},
want: `err: implied package identifier "bad-identifier" from import path "foo.com/bad-identifier" is not valid
want: `err: cannot find package "foo.com/bad-identifier": cannot find module providing package foo.com/bad-identifier
path: foo.com/bad-identifier
module: mod.test/test@v0
root: $CWD/testdata/testmod
dir: $CWD/testdata/testmod/cue.mod/gen/foo.com/bad-identifier
dir: ""
display:foo.com/bad-identifier`,
}, {
name: "NonexistentStdlibImport",
Expand Down
2 changes: 1 addition & 1 deletion encoding/gocode/testdata/cue.mod/module.cue
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
module: "cuelang.org/go/encoding/gocode/testdata"
module: "cuelang.org/go/encoding/gocode/testdata@v0"
language: version: "v0.9.0"
2 changes: 1 addition & 1 deletion encoding/gocode/testdata/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func main() {
inst := cue.Build(load.Instances([]string{pkg}, &load.Config{
Dir: dir,
ModuleRoot: dir,
Module: "cuelang.org/go/encoding/gocode/testdata",
Module: "cuelang.org/go/encoding/gocode/testdata@v0",
}))[0]
if err := inst.Err; err != nil {
log.Fatal(err)
Expand Down
4 changes: 2 additions & 2 deletions encoding/gocode/testdata/pkg1/cue_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions encoding/gocode/testdata/pkg2/cue_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/cueexperiment/exp.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
// Flags holds the set of CUE_EXPERIMENT flags. It is initialized
// by Init.
var Flags struct {
Modules bool
Modules bool `envflag:"default:true"`

// YAMLV3Decoder swaps the old internal/third_party/yaml decoder with the new
// decoder implemented in internal/encoding/yaml on top of yaml.v3.
Expand Down
2 changes: 1 addition & 1 deletion internal/cueexperiment/exp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func TestInit(t *testing.T) {
t.Setenv("CUE_EXPERIMENT", "")
err := initAlways()
qt.Assert(t, qt.IsNil(err))
qt.Assert(t, qt.IsFalse(Flags.Modules))
qt.Assert(t, qt.IsTrue(Flags.Modules))
qt.Assert(t, qt.IsTrue(Flags.YAMLV3Decoder))

// Check that we can enable all experiments.
Expand Down

0 comments on commit 1080e28

Please sign in to comment.