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/go: mod init reject module names ending in /v0 and /v1 #44052

Open
seankhliao opened this issue Feb 1, 2021 · 13 comments
Open

cmd/go: mod init reject module names ending in /v0 and /v1 #44052

seankhliao opened this issue Feb 1, 2021 · 13 comments

Comments

@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Feb 1, 2021

What version of Go are you using (go version)?

$ go version
go version devel +6ac91e4 Mon Feb 1 05:14:58 2021 +0000 linux/amd64

What did you do?

go mod init example.com/foo/v1

What did you expect to see?

A descriptive error saying v0 or /v1 isn't allowed, ex with gorelease:

main » gorelease
gorelease: module path "example.com/foo/v1" has major version suffix "v1".
A major version suffix is only allowed for v2 or later.

What did you see instead?

go.mod created without error

main » go mod init example.com/foo/v1
go: creating new go.mod: module example.com/foo/v1
@seankhliao seankhliao changed the title cmd/go: reject module names ending in /v1 cmd/go: mod init reject module names ending in /v1 Feb 1, 2021
@Xe
Copy link

@Xe Xe commented Feb 1, 2021

Please also include /v0 in this list

@seankhliao seankhliao changed the title cmd/go: mod init reject module names ending in /v1 cmd/go: mod init reject module names ending in /v0 and /v1 Feb 1, 2021
@jayconrod jayconrod added this to the Go1.17 milestone Feb 1, 2021
@tpaschalis
Copy link
Contributor

@tpaschalis tpaschalis commented Feb 1, 2021

I'll take a stab if there's no objection; I'll try to find a way which minimizes the changes needed to CreateModFile and introduce a test.

Do you think there's any other 'special' rules we should take care of, or just disallow the /v0 and /v1 suffixes?

@seankhliao
Copy link
Contributor Author

@seankhliao seankhliao commented Feb 1, 2021

The rules are https://golang.org/ref/mod#go-mod-file-ident

It currently also doesn't reject leading or trailing ., you'll have to check the others

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Feb 1, 2021

Let's limit a fix to just the major version suffix.

There are lots of local modules that just have a single element path or don't have a dot in the first element. These modules can't be fetched normally, but they can appear on the left side of a replace directive. That should keep working.

Technically, we could say this is working as intended, but that seems user hostile. Someone who names their module m won't expect that to work with go get. But someone who names their module github.com/foo/bar/v1 might expect that to work without a replacement.

cc @bcmills @matloob

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 2, 2021

Change https://golang.org/cl/288712 mentions this issue: cmd/go: make mod init disallow invalid major version suffixes

@tpaschalis
Copy link
Contributor

@tpaschalis tpaschalis commented Feb 2, 2021

I've opened a CL which utilizes module.SplitPathVersion to enforce the rules in place for the major version suffix.
I hope I got this right, let me know if I missed anything obvious.

One more point, and I'm not sure I'm reading this correctly is that the 'Module paths and versions' documentation could be improved.

Right now, I think it's a little contradicting on whether dots are allowed in /vN-like suffixes (first mentions that can be used and then that cannot). But I should probably open a new issue for that one.

For a final path element of the form /vN where N looks numeric (ASCII digits and dots), N must not begin with a leading zero, must not be /v1, and must not contain any dots.

@inet56
Copy link

@inet56 inet56 commented Feb 3, 2021

@tpaschalis Why gopkg.in is in the CL?

@inet56
Copy link

@inet56 inet56 commented Feb 3, 2021

@seankhliao Are negative numbers allowed?

@tpaschalis
Copy link
Contributor

@tpaschalis tpaschalis commented Feb 3, 2021

Hello @inet56, thanks for reaching out. The documentation defines the valid major version suffixes, for gopkg.in and non-gopkg.in module paths. These checks are implemented in modules.SplitPathVersion. The CL provides a helpful error message for both cases. Do you think it makes sense?

Also, I don't think that a negative version would make sense in a versioning system.

@inet56
Copy link

@inet56 inet56 commented Feb 3, 2021

Why is gopkg.in treated specially?

@inet56
Copy link

@inet56 inet56 commented Feb 3, 2021

Also, I don't think that a negative version would make sense in a versioning system.

What will happen if someone run

go mod init example.com/foo/v-1
@tpaschalis
Copy link
Contributor

@tpaschalis tpaschalis commented Feb 3, 2021

I think the reason why gopkg.in is treated in a special way is mainly compatibility. The major version suffixes and the import compatibility rule were already in place at gopkg.in before Go modules came along. I feel this goes well in hand with Go's compatibility promise, and not breaking users' workflow.

Regarding the go mod init example.com/foo/v-1 example, I think it should create a module named example.com/foo/v-1 with no version suffix.

My 2c from the docs and gut feeling that v-1000 is a valid module path element (eg. a username, repo name, or URL path), and is not easily mistaken for a version suffix so that it's explicitly disallowed, but someone more experienced could chime in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants