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 init's DX of "non-conforming path ..." is confusing #3022

Closed
jpluscplusm opened this issue Apr 15, 2024 · 1 comment
Closed
Labels
modules Issues related to CUE modules and the experimental implementation NeedsInvestigation

Comments

@jpluscplusm
Copy link
Collaborator

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

$ cue version
cue version v0.8.1

go version go1.22.1
      -buildmode exe
       -compiler gc
       -trimpath true
     CGO_ENABLED 0
          GOARCH amd64
            GOOS linux
         GOAMD64 v1

Does this issue reproduce with the latest stable release?

Yes, 0.8.1.

What did you do?

I opened #3021 because I didn't know that module paths now need to be lower case, and nothing outside the experimentally-scoped documentation at https://cuelang.org/docs/reference/modules/ mentions this.

When attempting to initialise a module with a path that contains upper case letters, this is the current DX:

$ cue mod init github.com/MyUsername/MyRepoName
invalid module name "github.com/MyUsername/MyRepoName": non-conforming path "github.com/MyUsername/MyRepoName/

This is confusing, especially as the only guidance I can recall us giving around module names is that they should generally reflect domains under the user's control or (on shared domains) paths under the user's control. The user "MyUserName", above, doesn't explicitly control the namespace github.com/myusername/. In this error message we should give them clear guidance about what's wrong with their proposed module name; and in the case of it being a case issue we should link to guidance about when it's safe to fold case. I.e. there will be services out there where user MyUserName does not control service.example/myusername/, and we should be cautious in not providing blanket "just lower case the path" guidance.

What did you expect to see?

  • An indication of the explicit problem with the module name chosen
  • Guidance about how to move forward with a correct module name

What did you see instead?

$ cue mod init github.com/MyUsername/MyRepoName
invalid module name "github.com/MyUsername/MyRepoName": non-conforming path "github.com/MyUsername/MyRepoName/
@jpluscplusm jpluscplusm added NeedsInvestigation Triage Requires triage/attention labels Apr 15, 2024
@mvdan mvdan moved this to Planned in Encodings Roadmap Apr 16, 2024
@mvdan mvdan added modules Issues related to CUE modules and the experimental implementation and removed Triage Requires triage/attention labels May 3, 2024
@myitcv myitcv moved this from Backlog to v0.9.0-rc.1 in Release v0.9 May 8, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Release v0.9 May 8, 2024
@myitcv myitcv moved this from Backlog to Planned in Modules Roadmap May 8, 2024
@myitcv myitcv moved this from v0.9.0-alpha.5 to v0.9.0-rc.1 in Release v0.9 May 15, 2024
@myitcv
Copy link
Member

myitcv commented May 28, 2024

  • An indication of the explicit problem with the module name chosen
  • Guidance about how to move forward with a correct module name

Agreed - we can do better here, perhaps indeed pointing people towards https://cuelang.org/docs/reference/modules/#module-path

@mvdan mvdan moved this from v0.9.0-rc.1 to v0.9.0 in Release v0.9 May 31, 2024
@mvdan mvdan removed this from Release v0.9 Jun 5, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Release v0.10 Jun 5, 2024
@rogpeppe rogpeppe moved this from Backlog to v0.10.0-alpha.2 in Release v0.10 Jun 26, 2024
cueckoo pushed a commit that referenced this issue Jul 22, 2024
This adds some tests for some errors that will be improved in
the next CL in this chain.

We also add tests for some of the module path checking functions
that were not properly unit tested previously so we can be sure
that their behavior does not change (an earlier iteration _did_
unexpectedly change the behavior of `CheckImportPath`).

As the test table is shared between several test functions (useful,
as it's nice to see the results for each check function across all
the input data), we needed to update the `tdtest` package to support
tables defined at the top level. We include this change in this CL
because then it's clear that it actually works.

For #3022

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I7cd5da04858c7610dcadfa6a28dff8a0f2e6ea77
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198135
Reviewed-by: Daniel Martí <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
@github-project-automation github-project-automation bot moved this from Planned to Done in Modules Roadmap Jul 22, 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

4 participants