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: check casing of github user names in import paths #22291

Closed
dlsniper opened this issue Oct 16, 2017 · 14 comments
Closed

cmd/go: check casing of github user names in import paths #22291

dlsniper opened this issue Oct 16, 2017 · 14 comments

Comments

@dlsniper
Copy link
Contributor

Hi,

Because the current specifications do not limit or prevent people to name their packages in any way, situations like sirupsen/logrus#553 happen when a project can be imported via github.com/sirupsen/logrus or via github.com/Sirupsen/logrus. There are many other instances of organization names or project names that have these issues, for example Netflix with https://github.com/netflix/chaosmonkey instead of the correct https://github.com/Netflix/chaosmonkey

Granted this is as much of a @github issue as is a Go tooling issue but it's quicker to fix Go than to break the Internet by getting Github to change their matching algorithm.

However I believe that specifying that packages should always be lowercase and if they are not they are converted automatically by the tooling that creates / downloads them from third party systems.

To make this as backwards compatible change, this could start be softly applied, e.g. go get and other tooling will always perform the lowercase transformation when downloading packages locally but the Go compiler or other existing tool is not changed until Go 2.0. Tools such as go vet or golint could meanwhile start to flag this as an issue so that people can gradually fix this.

What do you think?

@gopherbot gopherbot added this to the Proposal milestone Oct 16, 2017
@mvdan
Copy link
Member

mvdan commented Oct 16, 2017

I think you want package import paths, not package names - in the example, the package name is logrus and the path is github.com/sirupsen/logrus.

I'm not sure if I'm behind this proposal, as this would (eventually) force certain users like @shurcooL to update all of their import paths. He also uses uppercase in some projects like https://github.com/shurcooL/Go-Package-Store.

I realise that you proposed a soft version of this for Go1, but I'm still not convinced this is worth the trouble in Go1. Also note that go vet will be integrated with go test soon, so you would likely end up breaking people's package tests.

@dlsniper
Copy link
Contributor Author

I think you want package import paths, not package names

As far as I can understand from the specifications, there is no concept of import path, just package. Also I always think of github.com/dlsniper/demo as the package named demo that is contained by the package dlsniper that is contained by the package github.com. But yes, I would like both the path on the storage medium after the $GOPATH/ to be lowercased as well as the package itself.

@mvdan
Copy link
Member

mvdan commented Oct 16, 2017

My bad - I keep forgetting that interpreting package names as paths is not an explicit part of the spec. Ignore the first sentence of my comment above.

@cznic
Copy link
Contributor

cznic commented Oct 16, 2017

Also I always think of github.com/dlsniper/demo as the package named demo that is contained by the package dlsniper that is contained by the package github.com.

Packages have no parent/child/contains relationship. The import path above refers to a package named often, but not necessarily 'demo'.

@ianlancetaylor ianlancetaylor changed the title proposal: specify and enforce package names to be all lowercase proposal: specify and enforce package paths to be all lowercase Oct 16, 2017
@ianlancetaylor
Copy link
Contributor

I've changed the proposal topic to say "package paths" rather than "package names", since that seems to be what you are discussing. go get works on package paths, not package names.

I don't think we can reasonably restrict package paths to be all lowercase. That simply doesn't make sense on systems like Windows that routinely use uppercase letters (and spaces) in directory names.

@dmitshur
Copy link
Contributor

dmitshur commented Oct 16, 2017

From what I understand, only a very small number of Go packages in the wild have problems with import path case inconsistency, is that right? What do you think of taking the approach of directly fixing those specific Go packages that are broken? The import path comment (aka canonical import path) is specifically helpful for clarifying (and enforcing) the exact import path of a package, and broken packages can use make use of it.

I think that'd better, because it'd be less overall work/changes required to resolve the problem, and doesn't cause unnecessary changes in many packages that are not broken.

As @mvdan mentioned, my GitHub username happens to have a capital letter in it. I have over 197 Go packages with a capital letter in the import path, and all of them work without issues. I would be open to the possibility of changing them if were needed, but it doesn't seem to be needed.

If the Go documentation doesn't make it very clear that import paths are case sensitive, perhaps that can be improved. (I think it does, but I haven't checked recently.)

@dmitshur
Copy link
Contributor

My bad - I keep forgetting that interpreting package names as paths is not an explicit part of the spec. Ignore the first sentence of my comment above.

Both "package name" and "import path" are an explicit part of the spec, see https://golang.org/ref/spec#PackageName and https://golang.org/ref/spec#Import_declarations. There's no interpreting of the import path, but an import path as a concept is well defined. It's a string literal. There are some restrictions on its contents:

The interpretation of the ImportPath is implementation-dependent but it is typically a substring of the full file name of the compiled package and may be relative to a repository of installed packages.

Implementation restriction: A compiler may restrict ImportPaths to non-empty strings using only characters belonging to Unicode's L, M, N, P, and S general categories (the Graphic characters without spaces) and may also exclude the characters !"#$%&'()*,:;<=>?[]^`{|} and the Unicode replacement character U+FFFD.

@dmitshur
Copy link
Contributor

dmitshur commented Oct 16, 2017

Because the current specifications do not limit or prevent people to name their packages in any way

This statement is untrue, there are multiple restrictions on the import paths in the current Go implementations:

Implementation restriction: A compiler may restrict ImportPaths to non-empty strings using only characters belonging to Unicode's L, M, N, P, and S general categories (the Graphic characters without spaces) and may also exclude the characters !"#$%&'()*,:;<=>?[]^`{|} and the Unicode replacement character U+FFFD.

See also #20210; it's a related proposal about considering additional restrictions on the import path.

@bcmills
Copy link
Contributor

bcmills commented Oct 18, 2017

Isn't that pretty much exactly the problem that canonical import paths were added to address? It seems like it would be much easier to migrate to enforcing canonical paths than to enforcing lower-case paths.

@rsc
Copy link
Contributor

rsc commented Jan 23, 2018

We're not going to require import paths to be lower case. That ship has sailed. However, the go command probably should verify that the spelling in the import path matches the spelling of the github user name. Maybe for Go 1.11.

@rsc rsc changed the title proposal: specify and enforce package paths to be all lowercase cmd/go: check casing of github user names in import paths Jan 23, 2018
@rsc rsc modified the milestones: Proposal, Go1.11 Jan 23, 2018
@dlsniper
Copy link
Contributor Author

dlsniper commented Jan 24, 2018

We're not going to require import paths to be lower case. That ship has sailed.

I would rather want to see this in Go 2, and given some of the other requests, I believe this would be doable. There are some implications in the eco-system, but this can be done gradually and assisted by tools, go get could always write lowecased import paths and the go compiler could always use the paths in lowercases. Furthermore, we can have this in go vet / golint and other tools could also report / assist users to migrate to lowercased paths.

However, the go command probably should verify that the spelling in the import path matches the spelling of the github user name.

What about non-github.com repositories, or github enterprise users, and so on? I don't think making a special case for github.com alone would be good enough.

As such, I please ask you to reconsider such a check, and if the proposal is still accepted, then accept it for the original proposal.

Thank you.

Sorry, I've hit submit too quick.

@dmitshur
Copy link
Contributor

@dlsniper Did you see the question @bcmills posted above? Can you answer it please?

@SCKelemen
Copy link
Contributor

I also have capital letters in my GitHub username, and on my computers and servers, I created the go path and directories with all lowercase directory names. Currently, this works fine. If we were to implement this change, wouldn't I need to again go get or clone these repositories again with the proper casing?

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jul 6, 2018
@bcmills bcmills modified the milestones: Go1.12, Go1.13 Oct 25, 2018
@bcmills
Copy link
Contributor

bcmills commented Oct 25, 2018

If the repo has a go.mod file, we already verify that the casing of the import path matches the declared module path. This should resolve on its own as package owners add go.mod files.

@bcmills bcmills closed this as completed Oct 25, 2018
@golang golang locked and limited conversation to collaborators Oct 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants