From f740041b2ec137acb1c080ca43bc28ec25247790 Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Wed, 15 May 2024 16:33:48 +0100 Subject: [PATCH] internal/cueexperiment: flip modules experiment flag default 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 Change-Id: I7c384c785eac6203966460703283ecfa2d9f0447 --- cmd/cue/cmd/script_test.go | 10 +-- .../cmd/testdata/script/cmd_filetypes.txtar | 6 +- cmd/cue/cmd/testdata/script/issue174.txtar | 3 +- .../script/modinit_nomajorversion.txtar | 1 + .../testdata/script/modinit_withsource.txtar | 4 +- .../modpublish_registry_not_enabled.txtar | 1 + .../script/modtidy_registry_not_enabled.txtar | 9 +-- .../script/registry_experiment_not_set.txtar | 4 +- .../registry_no_experiment_warning.txtar | 2 +- cue/interpreter/wasm/exe_test.go | 1 + cue/load/import.go | 6 ++ cue/load/loader_test.go | 62 +++++++++++-------- internal/cueexperiment/exp.go | 2 +- internal/cueexperiment/exp_test.go | 2 +- 14 files changed, 64 insertions(+), 49 deletions(-) diff --git a/cmd/cue/cmd/script_test.go b/cmd/cue/cmd/script_test.go index 8924e22e660..e293126cc05 100644 --- a/cmd/cue/cmd/script_test.go +++ b/cmd/cue/cmd/script_test.go @@ -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 @@ -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 { @@ -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, diff --git a/cmd/cue/cmd/testdata/script/cmd_filetypes.txtar b/cmd/cue/cmd/testdata/script/cmd_filetypes.txtar index 1c54dba5833..65b7f3c32b2 100644 --- a/cmd/cue/cmd/testdata/script/cmd_filetypes.txtar +++ b/cmd/cue/cmd/testdata/script/cmd_filetypes.txtar @@ -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. @@ -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 -- diff --git a/cmd/cue/cmd/testdata/script/issue174.txtar b/cmd/cue/cmd/testdata/script/issue174.txtar index 086c6b95b5f..9d42bee1ae4 100644 --- a/cmd/cue/cmd/testdata/script/issue174.txtar +++ b/cmd/cue/cmd/testdata/script/issue174.txtar @@ -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" diff --git a/cmd/cue/cmd/testdata/script/modinit_nomajorversion.txtar b/cmd/cue/cmd/testdata/script/modinit_nomajorversion.txtar index 311f2b46f7f..ae649a95d11 100644 --- a/cmd/cue/cmd/testdata/script/modinit_nomajorversion.txtar +++ b/cmd/cue/cmd/testdata/script/modinit_nomajorversion.txtar @@ -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 diff --git a/cmd/cue/cmd/testdata/script/modinit_withsource.txtar b/cmd/cue/cmd/testdata/script/modinit_withsource.txtar index 50a12602b71..dba45226e3b 100644 --- a/cmd/cue/cmd/testdata/script/modinit_withsource.txtar +++ b/cmd/cue/cmd/testdata/script/modinit_withsource.txtar @@ -19,7 +19,7 @@ cd $WORK/test2 cmp stderr $WORK/want-stderr -- want-module.cue-0 -- -module: "test.example" +module: "test.example@v0" language: { version: "$CUE_LANGUAGE_VERSION" } @@ -27,7 +27,7 @@ source: { kind: "self" } -- want-module.cue-1 -- -module: "test.example" +module: "test.example@v0" language: { version: "$CUE_LANGUAGE_VERSION" } diff --git a/cmd/cue/cmd/testdata/script/modpublish_registry_not_enabled.txtar b/cmd/cue/cmd/testdata/script/modpublish_registry_not_enabled.txtar index af68db59f7f..d75d45a6c62 100644 --- a/cmd/cue/cmd/testdata/script/modpublish_registry_not_enabled.txtar +++ b/cmd/cue/cmd/testdata/script/modpublish_registry_not_enabled.txtar @@ -1,3 +1,4 @@ +env CUE_EXPERIMENT=modules=false ! exec cue mod publish v1.0.0 cmp stderr want-stderr -- want-stderr -- diff --git a/cmd/cue/cmd/testdata/script/modtidy_registry_not_enabled.txtar b/cmd/cue/cmd/testdata/script/modtidy_registry_not_enabled.txtar index 73b34e3bb2e..97c82708766 100644 --- a/cmd/cue/cmd/testdata/script/modtidy_registry_not_enabled.txtar +++ b/cmd/cue/cmd/testdata/script/modtidy_registry_not_enabled.txtar @@ -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" diff --git a/cmd/cue/cmd/testdata/script/registry_experiment_not_set.txtar b/cmd/cue/cmd/testdata/script/registry_experiment_not_set.txtar index 61028280f77..1db0c773fd3 100644 --- a/cmd/cue/cmd/testdata/script/registry_experiment_not_set.txtar +++ b/cmd/cue/cmd/testdata/script/registry_experiment_not_set.txtar @@ -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 diff --git a/cmd/cue/cmd/testdata/script/registry_no_experiment_warning.txtar b/cmd/cue/cmd/testdata/script/registry_no_experiment_warning.txtar index b1d165920b5..07d8b037999 100644 --- a/cmd/cue/cmd/testdata/script/registry_no_experiment_warning.txtar +++ b/cmd/cue/cmd/testdata/script/registry_no_experiment_warning.txtar @@ -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 diff --git a/cue/interpreter/wasm/exe_test.go b/cue/interpreter/wasm/exe_test.go index 869a7f3a020..f68912d50ea 100644 --- a/cue/interpreter/wasm/exe_test.go +++ b/cue/interpreter/wasm/exe_test.go @@ -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, diff --git a/cue/load/import.go b/cue/load/import.go index 33c09720911..799fb1a8715 100644 --- a/cue/load/import.go +++ b/cue/load/import.go @@ -257,6 +257,12 @@ func setFileSource(cfg *Config, f *build.File) error { } else { f.Source = fi.contents } + } else { + b, err := os.ReadFile(fullPath) + if err != nil { + return err + } + f.Source = b } return nil } diff --git a/cue/load/loader_test.go b/cue/load/loader_test.go index 8ef4542c78f..5d12aaad29b 100644 --- a/cue/load/loader_test.go +++ b/cue/load/loader_test.go @@ -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: "" @@ -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/..."}, @@ -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"}, @@ -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 what is this actually testing? cfg: dirCfg, // Absolute file is normalized. @@ -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", diff --git a/internal/cueexperiment/exp.go b/internal/cueexperiment/exp.go index 63f5f98b8d3..c808e024d8d 100644 --- a/internal/cueexperiment/exp.go +++ b/internal/cueexperiment/exp.go @@ -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. diff --git a/internal/cueexperiment/exp_test.go b/internal/cueexperiment/exp_test.go index 1054a44d21c..ff4e390709b 100644 --- a/internal/cueexperiment/exp_test.go +++ b/internal/cueexperiment/exp_test.go @@ -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.