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

Enforce the new fully-lowercase import path with Go 1.4 mecanism #859

Closed
wants to merge 1 commit into from

Conversation

dolmen
Copy link
Contributor

@dolmen dolmen commented Nov 9, 2018

The original import path of was github.com/Sirupsen/logrus but it has
changed to a fully lowercase path in July 2017.
References:

To help dependent project to detect the issue with mixing old and new
import path, let's use the Go 1.4 mecanism that enforces the import
path: https://golang.org/doc/go1.4#canonicalimports

This is a complement to go.mod for pre-Go-modules Go versions.

The original import path of was github.com/Sirupsen/logrus but it has
changed to a fully lowercase path in July 2017.
References:
- https://github.com/sirupsen/logrus/blob/7eeb7b7cbdeb0a582a5b7a1512d91f956f1674aa/README.md#case-sensitivity
- sirupsen#570 (comment)

To help dependent project to detect the issue with mixing old and new
import path, let's use the Go 1.4 mecanism that enforces the import
path: https://golang.org/doc/go1.4#canonicalimports

This is a complement to go.mod for pre-Go-modules Go versions.
@dgsb
Copy link
Collaborator

dgsb commented Nov 16, 2018

We can not merge that in v1 branch, it may break some build here and there.

@dolmen
Copy link
Contributor Author

dolmen commented Nov 21, 2018

The universe that depends on logrus is already broken because of the case change.
At least this change helps to diagnose those build failures.

@dgsb
Copy link
Collaborator

dgsb commented Nov 22, 2018

We did not receive any issue about the case change lastly, so I don't think there are so many broken build out there because of that.
Anyway, I kind of agree with the change, but as I said we can not merely merge that in a v1 version, I do not want to add more instability to any build using logrus.

@dolmen
Copy link
Contributor Author

dolmen commented Dec 17, 2018

The situation is so broken that in fact in my company we lived for a long time without noticing the case change. We have been bitten by the issue more than one year after the case change.

This patch enforces that everyone that upgrades will come in the new post-case-change world instead of staying in the ignorance. And it gives the information to the go tooling to help the user to fix the issue.

@dolmen
Copy link
Contributor Author

dolmen commented Feb 21, 2020

@dgsb #1041 (November 2019) is an example of how the import path change still affects dependent projects nowadays.

@dgsb
Copy link
Collaborator

dgsb commented May 28, 2020

the use of go.mod is the new standard for package management in go build. In this context the fix is not useful anymore, but it may harm users just building whatever packages are currently in the gopath.

@dgsb dgsb closed this May 28, 2020
@imunhatep
Copy link

$ go mod vendor
go: github.com/rancher/rke/pki imports
	github.com/docker/docker/pkg/archive imports
	github.com/Sirupsen/logrus: github.com/Sirupsen/[email protected]: parsing go.mod:
	module declares its path as: github.com/sirupsen/logrus
	        but was required as: github.com/Sirupsen/logrus

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

Successfully merging this pull request may close these issues.

3 participants