Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mod/modfile: allow earlier language versions #3145

Open
rogpeppe opened this issue May 14, 2024 · 7 comments
Open

mod/modfile: allow earlier language versions #3145

rogpeppe opened this issue May 14, 2024 · 7 comments
Labels
modules Issues related to CUE modules and the experimental implementation NeedsInvestigation

Comments

@rogpeppe
Copy link
Member

Currently when parsing in non-legacy mode, mod/modfile rejects any language version earlier
than v0.8.0, as that's when the language version field started to be supported and the
module syntax became closed, disallowing unrecognized fields.

@mvdan points out that it's useful for users to be able to specify a language version that's
earlier than that, as information about the actual language version that some code was
evaluated against.

We could see that the language version is before v0.8.0 and allow (but discard) any field
other than module, mirroring the fact that all fields were allowed before then.

@rogpeppe rogpeppe added NeedsInvestigation modules Issues related to CUE modules and the experimental implementation labels May 14, 2024
@mvdan
Copy link
Member

mvdan commented May 14, 2024

For some more clarity: particularly in terms of regression testing or Unity, it is a lot easier to understand the original writer's intended effect when knowing what version of CUE they were using. For example, even if the language spec didn't have any significant changes between v0.5.0 and v0.8.0, the evaluator still evolved behind the scenes, so it's very possible that their CUE code started behaving differently in some way - perhaps some errors are different, perhaps the exported result is different, or perhaps some errors appeared or disappeared.

I think it's fine if commands like cue mod tidy or cue mod get refuse to do anything until the user runs cue mod fix or manually updates the language version to one with a formal schema. We could say that module.cue files with old versions (before v0.8.0) are a form of legacy mode as well. But I also think it would be fine for us to apply the v0.8.0 schema to older versions, because it's very rare for downstreams to have extra fields other than precursors for language.version like cue.lang or cue.

@mvdan
Copy link
Member

mvdan commented May 16, 2024

A passing thought: right now we are coupling the language version with the module.cue schema version. It's convenient to do this for us and for most users, because there is one less version to keep track of. But we don't need to tie the two together; someone might want to use the latest schema version, for example to be able to cue mod publish with a source.kind, but declare that the language spec version they are using is still v0.6.0. @rustamabd2 brought up this issue as one of our repositories had test module.cue files like:

module: "github.com/user_publisher/public_repo/mod/subdir@v0"
language: {
    version: "v0.8.0-alpha.3"
}
source: kind: "self"

which cmd/cue at master started rejecting as designed:

invalid module.cue file: source field is not allowed at this language version; need at least v0.9.0-alpha.0

The fix was to bump language.version, but in a way, it feels unrelated to update to a newer language spec version to be able to cue mod upload with a source (which is now mandatory, no matter what language version is declared).

Go has started splitting up the go version a bit; first with toolchain, and now with godebug: https://go.dev/issue/65573

I'm not saying we definitely want to split up language.version as well; I'm just saying we can, and Go has started doing so already. If we find that tying the two versions together is a pain for us or for users, we should consider it.

@mvdan
Copy link
Member

mvdan commented May 16, 2024

Another example: right now we have some CUE users on ancient versions like v0.4.3 due to regressions or breakage in later versions of the evaluator. Once they do eventually update to v0.9.0 or v0.10.0, the current design would force them to set language.version to the latest version to be able to cue mod publish, whereas in general language spec jumps should be done very carefully and are unrelated to any modules features.

In this particular case, there aren't any breaking language spec changes between v0.4.3 and v0.9.0, but we do intend to make breaking changes in upcoming language versions: see for instance #2237 or #2932. If a large project makes use of language features that we are removing, a user should be able to stick to the older language.version for a bit longer until they are ready to make the jump.

@mvdan
Copy link
Member

mvdan commented May 16, 2024

I also note that there are two possible solutions to the problem here:

  1. Allow using ancient language.version strings like v0.4.3 by using a very lax module.cue schema for those.
  2. Allow decoupling the module.cue schema version from language.version, by e.g. adding a new language.schemaVersion field which defaults to language.version, but can be higher than language.version when set.

Either of those will fix the immediate concerns, but longer term, 2 might be the better approach. If e.g. v0.11.0 ships with a new module.cue schema for cue mod publish features, but also breaking language spec changes, it would be unfortunate to force a user into updating both or neither at the same time.

Option 2 could also name the field something like "target version", which wouldn't be a maximum CUE version that can be used, but it would signal for example: even though I use the language spec at version v0.6.0, and that is the oldest version I support for evaluation, our developers use v0.9.0 for maintaining and publishing this module, and we want to make use of new backwards-compatible features like source.kind.

@rustamabd2
Copy link

Another thing is, trying to publish a module looking like this

module: "github.com/user_publisher/public_repo/mod/subdir@v0"
language: {
    version: "v0.8.0-alpha.3"
}

using cue tip version (v0.9.0-alpha.4.0.20240515162813-45755585913b) results in an error:

no source field found in cue.mod/module.cue

This seems wrong, as language version 0.8.0 didn't use the source field. Adding the source field changes the error to the one mentioned above and cue mod fix doesn't help with resolving this, either.

@mvdan mvdan assigned mvdan and unassigned rogpeppe May 30, 2024
@mvdan
Copy link
Member

mvdan commented May 30, 2024

The original blocking aspect of this for v0.9.0 was the fact that any modules created very early during the CUE modules experiment, such as one of mine with v0.8.0-alpha.3, started failing like:

cannot find schema suitable for reading module file with language version "v0.8.0-alpha.3"

This immediate issue got fixed by https://review.gerrithub.io/c/cue-lang/cue/+/1194685, where the oldest schema version was lowered from v0.8.0 to v0.8.0-alpha.0.

The other blocking bug I see here is what @rustamabd2 brings up in #3145 (comment); existing users of cue mod publish with a version like language: version: v0.8.0-alpha.3 might be very confused by this transition, as they are told to add a field that results in an error once added.

Those users on v0.8 were able to publish with v0.8 versions of the software, and now they cannot, unless they bump language.version to v0.9 and add source.kind. This seems like a hard regression, unless we are intentionally breaking those users. My understanding is that we should be backwards compatible and allow publishing those older modules as if they declared source: kind: "self", mimicking the old logic that wasn't aware of VCS at all. I will do such a change.

https://github.com/cue-labs/services/issues/141 seems to agree with my understanding by saying:

When language.version is at least v0.9.0-alpha.2, we know that the cue mod publish command fails if the source field isn't present, so we can do that same check in the registry too, predicated on language.version.

cueckoo pushed a commit that referenced this issue May 30, 2024
See the added comments to the testscript; this shows the current logic.
The following commit will tweak the logic to fix the TODO.

For #3145.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Ib50b1f6fb0e685ffe7af687fe962ea02d9ef6056
cueckoo pushed a commit that referenced this issue May 30, 2024
Any users who had created CUE modules with CUE_EXPERIMENT=modules
using one of the v0.8 releases could publish to a registry, such as:

    module: "github.com/example/repo@v0"
    language: {
        version: "v0.8.0-alpha.3"
    }

Suddenly, the latest v0.9 alphas started failing on those:

    no source field found in cue.mod/module.cue

This error is a breaking change for the users which isn't necessary.
The old logic was basically equivalent to source.kind="self";
we should only require the explicit field on newer language versions.

Moreover, the error was confusing: just adding a source.kind field,
without bumping language.version to one with the newer schema, failed:

    invalid module.cue file: source field is not allowed at this language version; need at least v0.9.0-alpha.0

Updates #3145.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I6752e4d6f2e687e2cd568f4a807b76b64e1e057e
@mvdan
Copy link
Member

mvdan commented May 30, 2024

https://review.gerrithub.io/c/cue-lang/cue/+/1195526 should fix this last issue brought up by Rustam.

We should leave this issue open for v0.10 for the sake of deciding if we want to allow setting very old language.version values, such as v0.4.3, which is important for Unity or other cases where an end user wants to declare "I wrote this code using ancient language spec semantics".

Earlier in this thread we also discussed decoupling the language version from the module.cue schema version. That's also a possibility, although not as much of an immediate need or issue as the others.

@mvdan mvdan removed their assignment May 30, 2024
cueckoo pushed a commit that referenced this issue May 30, 2024
See the added comments to the testscript; this shows the current logic.
The following commit will tweak the logic to fix the TODO.

For #3145.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Ib50b1f6fb0e685ffe7af687fe962ea02d9ef6056
cueckoo pushed a commit that referenced this issue May 30, 2024
Any users who had created CUE modules with CUE_EXPERIMENT=modules
using one of the v0.8 releases could publish to a registry, such as:

    module: "github.com/example/repo@v0"
    language: {
        version: "v0.8.0-alpha.3"
    }

Suddenly, the latest v0.9 alphas started failing on those:

    no source field found in cue.mod/module.cue

This error is a breaking change for the users which isn't necessary.
The old logic was basically equivalent to source.kind="self";
we should only require the explicit field on newer language versions.

Moreover, the error was confusing: just adding a source.kind field,
without bumping language.version to one with the newer schema, failed:

    invalid module.cue file: source field is not allowed at this language version; need at least v0.9.0-alpha.0

Updates #3145.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I6752e4d6f2e687e2cd568f4a807b76b64e1e057e
cueckoo pushed a commit that referenced this issue May 31, 2024
See the added comments to the testscript; this shows the current logic.
The following commit will tweak the logic to fix the TODO.

For #3145.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Ib50b1f6fb0e685ffe7af687fe962ea02d9ef6056
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1195525
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.io>
cueckoo pushed a commit that referenced this issue May 31, 2024
Any users who had created CUE modules with CUE_EXPERIMENT=modules
using one of the v0.8 releases could publish to a registry, such as:

    module: "github.com/example/repo@v0"
    language: {
        version: "v0.8.0-alpha.3"
    }

Suddenly, the latest v0.9 alphas started failing on those:

    no source field found in cue.mod/module.cue

This error is a breaking change for the users which isn't necessary.
The old logic was basically equivalent to source.kind="self";
we should only require the explicit field on newer language versions.

Moreover, the error was confusing: just adding a source.kind field,
without bumping language.version to one with the newer schema, failed:

    invalid module.cue file: source field is not allowed at this language version; need at least v0.9.0-alpha.0

Updates #3145.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I6752e4d6f2e687e2cd568f4a807b76b64e1e057e
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1195526
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Paul Jolly <paul@myitcv.io>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules Issues related to CUE modules and the experimental implementation NeedsInvestigation
Projects
Status: Backlog
Status: Backlog
Development

No branches or pull requests

3 participants