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

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

Closed
myitcv opened this issue Jun 12, 2024 · 5 comments
Closed

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

myitcv opened this issue Jun 12, 2024 · 5 comments
Labels
modules Issues related to CUE modules and the experimental implementation NeedsInvestigation

Comments

@myitcv
Copy link
Member

myitcv commented Jun 12, 2024

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.

@myitcv myitcv added NeedsInvestigation modules Issues related to CUE modules and the experimental implementation labels Jun 12, 2024
@rogpeppe
Copy link
Member

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
Copy link
Member Author

myitcv commented Jun 13, 2024

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
Copy link
Member

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 rogpeppe moved this from Backlog to v0.10.0-alpha.2 in Release v0.10 Jun 26, 2024
@rogpeppe
Copy link
Member

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
Copy link
Member Author

myitcv commented Jul 16, 2024

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?

@github-project-automation github-project-automation bot moved this from Backlog to Done in Modules Roadmap Jul 19, 2024
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
Archived in project
Status: v0.10.0-alpha.2
Development

No branches or pull requests

2 participants