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

case-insensitive import collision #543

Closed
brunoksato opened this issue May 12, 2017 · 34 comments
Closed

case-insensitive import collision #543

brunoksato opened this issue May 12, 2017 · 34 comments

Comments

@brunoksato
Copy link

Hi,

My last build today I have this problem.

can't load package: package github.com/sirupsen/logrus/examples/hook: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"

can i help me ?

@sirupsen
Copy link
Owner

Why are you importing the example hook?

@jackric
Copy link

jackric commented May 13, 2017

It happens if you're trying to build the example:

$ go build ./examples/hook                                                                                             
can't load package: package github.com/sirupsen/logrus/examples/hook: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"

I have opened a pull request to fix logrus-airbrake-hook, which fixes building for me locally:
gemnasium/logrus-airbrake-hook#2

@domano
Copy link

domano commented May 18, 2017

I am also getting this. We are not using the example in our project. :(

@stweiz
Copy link

stweiz commented May 18, 2017

I have the same problem without using an example.

@sirupsen
Copy link
Owner

What errors are you getting?

@stweiz
Copy link

stweiz commented May 18, 2017

Sorry, I should post it in my comment! This is the error I am getting. It also occurs, if I fetch the needed libraries (go get)
main.go:11:2: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"

@domano
Copy link

domano commented May 18, 2017

Same here

@sirupsen
Copy link
Owner

Do you have a dependency that depends on upper-case, or lower-case, and then you're importing the opposite case?

@domano
Copy link

domano commented May 18, 2017

As far as i can tell they all use upper-case, except for your examples :)

@stweiz
Copy link

stweiz commented May 18, 2017

This fixes my problem: stweiz@0ae413d

I see, that you wrote in your README, that all import paths should be written in lower-case letters, but I recognized, that in the logrus project the first letter is either upper-case, or lower-case.

This problem should be fixed, if the import paths in our projects are all the same. So the first letter should be lower-case, right?

@stweiz
Copy link

stweiz commented May 18, 2017

So this should be the correct one: master...stweiz:issue_543_lower-case-imports

So everyone who uses the library, needs to set their imports lower-case.

@brunoksato
Copy link
Author

brunoksato commented May 19, 2017

I don't use example... continues here yet

@stweiz
Copy link

stweiz commented May 19, 2017

I have created a pull request: #546

@domano
Copy link

domano commented May 19, 2017

With this PR everything would work for us if we change it to lowercase everywhere (which is fine).
The only problem are some "transient" dependencies (specifically https://github.com/Shopify/toxiproxy) which use the upper-case variant. I opened an issue there.

@ypatent
Copy link

ypatent commented May 21, 2017

@sirupsen can we expect to see it merged to the master soon?

@sirupsen
Copy link
Owner

Are people getting this due to using the test package, and syslog? I created a null logger package that importa lower-case. We cannot just change the casing, as people depend on the upper-case import as well. This is a sticky situation, so we have to tread very carefully. It's possible we should instead take syslog out of core with a lower-case import, and deprecate the package in core.

@ypatent
Copy link

ypatent commented May 22, 2017

@sirupsen we get this from syslog, yeah

@sirupsen
Copy link
Owner

sirupsen commented Jun 2, 2017

@ypatent we'll have to copy that directory and provide symlinks, then reference logrus with lowercase. I don't want to reference lowercase in core just yet.

@bscott
Copy link

bscott commented Jun 4, 2017

I'm getting the same today, trying to use the elasticsearch hook

case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"

@genez
Copy link

genez commented Jun 6, 2017

the uppercase variant is still present in the syslog hook, causing build errors. I have manually edited the two files (syslog and test) and I can build correctly

@robks
Copy link

robks commented Jun 6, 2017

(Issue #553 is directly related to this). I am stating the obvious here, and firstly thank you for a great logging package, but when your project is widespread use (Congratulations!), or you hope it will be, global changes like these are a big no, no. The community values stability so we can run our production systems without worry. One reason for Go's widespread adoption is the 1.0 guarantee. In everything we do as open source developers, we should make stability a very high priority. Thanks again. So which way are we going, lower or upper 'Sirupsen'?

@sirupsen
Copy link
Owner

sirupsen commented Jun 6, 2017

Lower-case. This has been a mess since my handle was re-named, and the best long-term strategy is that everyone converts to lowercase. There's no turning back, as regretful as that is.

As mentioned in other issues, I failed to realize the implications this would have given Go's package management.

@sirupsen
Copy link
Owner

sirupsen commented Jun 6, 2017

I am terribly sorry for how messy this is. But I truly believe the best option at this point is biting the bullet and going full on lower-case at this point.

@n1koo
Copy link

n1koo commented Jun 6, 2017

For glide users theres a workaround in #553 (comment) . But remember to PR your dependencies not to rely on this hack for long :)

@shuLhan
Copy link
Contributor

shuLhan commented Jun 8, 2017

I think in the future we should have a legal age for creating account in Github or other remote repository.

@piyush-nimbalkar
Copy link

I am using glide up --strip-vendor which removes nested vendor directory, which results in the import collision
case-insensitive import collision: "github.com/portworx/torpedo/vendor/github.com/Sirupsen/logrus" and "github.com/portworx/torpedo/vendor/github.com/sirupsen/logrus"

Although the strange fact is, when I do a go build on the code it does not throw this error, it happens only on go test.
The packages that I import have a mix of lower and upper case Sirupsen/logrus imports. On glide up, it creates both vendor/Sirupsen and vendor/sirupsen directories. Is there a work around for this?

@cmlicata
Copy link

@piyush-nimbalkar, I'm actually having the same issue and it is very peculiar indeed.

115100 pushed a commit to 115100/rucksack that referenced this issue Nov 29, 2017
Solves case-sensitive imports according to the recommended solution in
sirupsen/logrus#543.
@X4mp
Copy link

X4mp commented Dec 13, 2017

I'm having the same case-insensitive collision with the latest moby/moby (docker/docker) release since they vendored Sirupsen and we are using sirupsen. Due to vendor flattening we now get the collision.

That rename caused sooo much issues -.-

cessien pushed a commit to cessien/mysqltools that referenced this issue Mar 20, 2018
mwhooker added a commit to hashicorp/packer that referenced this issue Apr 17, 2018
vahid-sohrabloo added a commit to vahid-sohrabloo/ytdl that referenced this issue May 4, 2018
This PR will fix the import collision issue with logrus
Please refer to sirupsen/logrus#543
@Dandi007
Copy link

I am also getting this. We are not using the example in our project. :(

same here

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

No branches or pull requests