Skip to content

Commit

Permalink
cue/load: add Config.AcceptLegacyModules
Browse files Browse the repository at this point in the history
In some cases, we want to be able to accept legacy module files (module
files lacking a major version) because there's no easy way to fix them
by running `cue mod fix`. When running the cue command, `cue mod fix`
should always be a possibility - the problem exists principally when
using `cue/load` directly.

We don't want to make this the default, because interpreting module
files in this way is problematic, as it will discard fields that are
legitimately part of the non-legacy module schema.

So we add a boolean field that explicitly enables this behavior.

Fixes #3219.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I24d66eb680be3267adbe0a972705cc3414936362
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1196156
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1196230
Reviewed-by: Paul Jolly <paul@myitcv.io>
  • Loading branch information
rogpeppe authored and myitcv committed Jun 12, 2024
1 parent f14a31c commit aa83471
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 1 deletion.
11 changes: 11 additions & 0 deletions cue/load/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ type Config struct {
// the module field of an existing cue.mod file.
Module string

// AcceptLegacyModules causes the module resolution code
// to accept module files that lack a language.version field.
AcceptLegacyModules bool

// modFile holds the contents of the module file, or nil
// if no module file was present. If non-nil, then
// after calling Config.complete, modFile.Module will be
Expand Down Expand Up @@ -414,6 +418,13 @@ func (c *Config) loadModule() error {
parseModFile := modfile.ParseNonStrict
if c.Registry == nil {
parseModFile = modfile.ParseLegacy
} else if c.AcceptLegacyModules {
// Note: technically this does not support all legacy module
// files because some old module files might contain non-concrete
// data, but that seems like an OK restriction for now at least,
// given that no actual instances of non-concrete data in
// module files have been discovered in the wild.
parseModFile = modfile.FixLegacy
}
mf, err := parseModFile(data, mod)
if err != nil {
Expand Down
70 changes: 69 additions & 1 deletion cue/load/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,75 @@ root: $CWD/testdata/testmod
dir: $CWD/testdata/testmod/cycle
display:./cycle
files:
$CWD/testdata/testmod/cycle/cycle.cue`}}
$CWD/testdata/testmod/cycle/cycle.cue`}, {
name: "AcceptLegacyModuleWithLegacyModule",
cfg: &Config{
Dir: testdata("testmod_legacy"),
AcceptLegacyModules: true,
},
want: `path: test.example/foo@v0
module: test.example/foo@v0
root: $CWD/testdata/testmod_legacy
dir: $CWD/testdata/testmod_legacy
display:.
files:
$CWD/testdata/testmod_legacy/foo.cue`}, {
name: "AcceptLegacyModuleWithNonLegacyModule",
cfg: &Config{
Dir: testdataDir,
Tools: true,
AcceptLegacyModules: true,
},
args: []string{"./imports"},
want: `path: mod.test/test/imports@v0
module: mod.test/test@v0
root: $CWD/testdata/testmod
dir: $CWD/testdata/testmod/imports
display:./imports
files:
$CWD/testdata/testmod/imports/imports.cue
imports:
mod.test/catch: $CWD/testdata/testmod/cue.mod/pkg/mod.test/catch/catch.cue
mod.test/helper:helper1: $CWD/testdata/testmod/cue.mod/pkg/mod.test/helper/helper1.cue`}, {
name: "MismatchedModulePathInConfig",
cfg: &Config{
Dir: testdataDir,
Tools: true,
Module: "wrong.test@v0",
},
args: []string{"./imports"},
want: `err: inconsistent modules: got "mod.test/test@v0", want "wrong.test@v0"
path: ""
module: wrong.test@v0
root: ""
dir: ""
display:""`}, {
name: "ModulePathInConfigWithoutMajorVersion",
cfg: &Config{
Dir: testdataDir,
Tools: true,
Module: "mod.test/test",
},
args: []string{"./imports"},
want: `err: inconsistent modules: got "mod.test/test@v0", want "mod.test/test"
path: ""
module: mod.test/test
root: ""
dir: ""
display:""`}, {
name: "ModulePathInConfigWithoutMajorVersionAndMismatchedPath",
cfg: &Config{
Dir: testdataDir,
Tools: true,
Module: "mod.test/wrong",
},
args: []string{"./imports"},
want: `err: inconsistent modules: got "mod.test/test@v0", want "mod.test/wrong"
path: ""
module: mod.test/wrong
root: ""
dir: ""
display:""`}}
tdtest.Run(t, testCases, func(t *tdtest.T, tc *loadTest) {
pkgs := Instances(tc.args, tc.cfg)

Expand Down
7 changes: 7 additions & 0 deletions cue/load/testdata/testmod_legacy/cue.mod/module.cue
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Note: no major version suffix.
module: "test.example/foo"

// Note: no language.version field present.

// Note: extra field not allowed by later strict module versions.
extraField: 124
2 changes: 2 additions & 0 deletions cue/load/testdata/testmod_legacy/foo.cue
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
package foo

0 comments on commit aa83471

Please sign in to comment.