cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
4.99k stars 283 forks source link

cmd/cue: mod publish adds major version suffix to error message #3225

Closed myitcv closed 1 month ago

myitcv commented 2 months ago

What version of CUE are you using (cue version)?

$ cue version
cue version v0.0.0-20240612144709-b44f02fb7cd4

go version go1.22.3
      -buildmode exe
       -compiler gc
  DefaultGODEBUG httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1
     CGO_ENABLED 1
          GOARCH arm64
            GOOS linux
             vcs git
    vcs.revision b44f02fb7cd417e9280bd770c4b4bc348a9de5d8
        vcs.time 2024-06-12T14:47:09Z
    vcs.modified false
cue.lang.version v0.9.0

Does this issue reproduce with the latest release?

Yes

What did you do?

exec cue mod registry localhost:4132 &
env CUE_REGISTRY=localhost:4132

! exec cue mod publish v1.0.0
cmp stderr stderr.golden

-- blah.cue --
package blah

blah: 42
-- cue.mod/module.cue --
module: "mod.example/blah"
language: {
    version: "v0.9.0"
}
source: {
    kind: "self"
}
-- stderr.golden --
cannot form module version: mismatched major version suffix in "mod.example/blah" (version v1.0.0)

What did you expect to see?

A passing test (modulo #3224)

What did you see instead?

> exec cue mod registry localhost:4132 &
> env CUE_REGISTRY=localhost:4132
> ! exec cue mod publish v1.0.0
[stderr]
cannot form module version: mismatched major version suffix in "mod.example/blah@v0" (version v1.0.0)
[exit status 1]
> cmp stderr stderr.golden
diff stderr stderr.golden
--- stderr
+++ stderr.golden
@@ -1,1 +1,1 @@
-cannot form module version: mismatched major version suffix in "mod.example/blah@v0" (version v1.0.0)
+cannot form module version: mismatched major version suffix in "mod.example/blah" (version v1.0.0)

FAIL: /tmp/testscript2764044822/repro.txtar/script.txtar:5: stderr and stderr.golden differ

Notice that the module.cue file does not contain a major version suffix. And yet the error message suggests there is one.

This lack of consistency from the user's perspective is confusing: the user will be confused.

rogpeppe commented 2 months ago

Would you think it's OK if the error message didn't explicitly mention the word "suffix"; something like this?

cannot form module version: major version of "mod.example/blah@v0" does not match major version of v1.0.0

The reasoning being that @v0 is always implied if there's no major version suffix. Given that module paths are canonicalized in many places in the code, it might be hard to avoid mentioning @v0 in all error messages. I guess another possible change might just be to disallow @v0 everywhere, but that's a much bigger change and I'm not sure it's a good idea.

myitcv commented 2 months ago

Given that module paths are canonicalized in many places in the code, it might be hard to avoid mentioning @v0 in all error messages.

Presumably we don't just drop what the actual declared module path is.

I'm therefore unclear why there is that much effort involved.

rogpeppe commented 2 months ago

Presumably we don't just drop what the actual declared module path is.

In many code paths, we are working with just dependencies and their versions in canonicalized form, and we don't necessarily have access to the original module path at that point. From that point of view, the original path is "dropped" in such a code path, even though it might be available elsewhere in the calling stack.

Note: that's not necessarily the case for this particular error message though, just that I think that it might take a reasonable amount of work to thread this information through all the code in the aid of fixing all error messages that might mention the full module path.

rogpeppe commented 1 month ago

I've been investigating this, and I'm not entirely sure that the "expected" error message is good. A path with no major version suffix does not always imply @v0 (for example, as an import path the major version is implied from the version in the module.cue file), so cannot form module version: mismatched major version suffix in "mod.example/blah" (version v1.0.0) might also be confusing for a user that isn't aware that @v0 is implied in this case.

One possibility for a fix would be to change the type of a module path in the code to be a struct rather than a string, so we could pass around both the original version and the qualified version, but this will be a highly invasive change and involve changing many APIs. I'm not entirely sure that the effort involved would be worthwhile.

For now, I'm inclined to change the error message to something like this:

cannot form module version from "mod.example/blah": mismatched major version suffix in "mod.example/blah@v0" (version v1.0.0)

WDYT @myitcv ?

myitcv commented 1 month ago

I should perhaps have been "softer" in my original description: "for some variation of the error message".

The main thing I am suggesting we solve for here is that we report an error message in terms of what the user knows.

In that regard:

cannot form module version from "mod.example/blah": mismatched major version suffix in "mod.example/blah@v0" (version v1.0.0)

I'm not clear that a (first-time) user of modules will be clear what to do in this instance, are you?