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

Should I use lowercase or uppercase for the package name? #553

Closed
kitgary opened this issue Jun 3, 2017 · 31 comments
Closed

Should I use lowercase or uppercase for the package name? #553

kitgary opened this issue Jun 3, 2017 · 31 comments

Comments

@kitgary
Copy link

kitgary commented Jun 3, 2017

Hi,

I am using two hooks, https://github.com/rogierlommers/logrus-redis-hook and https://github.com/bshuster-repo/logrus-logstash-hook, one is using upper case while the other one is using lowercase. Therefore I got this annoying error
can't load package: package github.com/sirupsen/logrus/examples/hook: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"

How can I solve this issue?
As there is a thread mentioned that the change has been reverted, so should I use uppercase or lowercase package name?

I am very confused!

Thanks

@robks
Copy link

robks commented Jun 5, 2017

Having similar trouble with Glide, I'm using logrus with the Postgres hook, and glide can't load the repo. Here's my vote for leaving it uppercase, so all dependencies don't break.

@montanaflynn
Copy link

Also hitting this, we've switched to lowercase, filed issues in upstream packages but it's a bit of a mess if they have dependencies that use logrus as well...

@sirupsen
Copy link
Owner

sirupsen commented Jun 6, 2017

We have to converge to lower-case because reverting would break things for people too. Unfortunately, this is messy either way, caused by the author renaming to lower-case. We have to convert everything to lowercase, as regretful as it is.

@n1koo
Copy link

n1koo commented Jun 6, 2017

So I ended up working around this by levering glide aliasing support. Eg in your glide.yaml do :

- package: github.com/sirupsen/logrus
  version: master
- package: github.com/Sirupsen/logrus
  repo: [email protected]:/sirupsen/logrus
  vcs: git
  version: master

the version: master is optional but I wanted to make sure I get #547 in

@rohanthewiz
Copy link

rohanthewiz commented Jun 6, 2017

As a convenience, posting this here for Glide:
Glide has a cache that needs to be busted, so once you update imports to 'sirupsen' do this:
glide install --force
FYI: The glide cache is located at ~/.glide/cache

@sagikazarmark
Copy link

I understand that renaming the account was incredibly important (clearly, the difference matters), but since you realized the consequences, can you please tag a new release so that we can stop using the latest master (as you advised us not to do it)?

@iheitlager
Copy link

iheitlager commented Jun 8, 2017

Be sure to clear your glide cache as @rohanthewiz is telling us. Renaming is a b!@#$, but yah .....

Also we had issues with renaming on osx in the previous renaming, if so follow this:

  1. delete vendor/github.com/Sirupsen folder, commit
  2. (if you want to be sure remove references in glide.yml/glide.lock
  3. rename references in you project, commit
  4. clear cache
  5. glide update

@mattfarina
Copy link

@sdboyer FYI.... when thinking of dep.

@sagikazarmark
Copy link

@iheitlager clearing the cache does not help this:

https://github.com/sirupsen/logrus/blob/v0.11.5/hooks/test/test.go#L6

@iheitlager
Copy link

well you need to remove the version pinning, i can see master is updated to lower case

https://github.com/sirupsen/logrus/blob/master/hooks/test/test.go

@sagikazarmark
Copy link

well you need to remove the version pinning

Well, that's a terrible idea. There are reasons why we version software.

@iheitlager
Copy link

don't use version 0.11.5 I would say, that one is not compatible

@sagikazarmark
Copy link

So which (preferably newer) version should I use than?

@rohanthewiz
Copy link

rohanthewiz commented Jun 8, 2017

I've been fine on this one:

# glide.yaml
- package: github.com/sirupsen/logrus
  version: 33a34430d1c3dc71777fdd677864928e5b5812bc

Hopefully we'll get a semantic release soon.

@montanaflynn
Copy link

We hit this and were then forced to vendor dependency packages and manually modify them to work with our own packages using logrus. It was easy to make a PR changing glide for one of our deps but maybe that actually would be worse for people already using it.

Once dependencies dependencies start mixing things may grow out of hand. Maybe Golang itself needs an option to ignore capitalization with it's toolchain since this seems like it will affect a lot of people and could easily be remedied.

@sirupsen
Copy link
Owner

sirupsen commented Jun 8, 2017

v1.0.0 has been released, which is the official release that performs the rename.

@sagikazarmark please consider your tone. I understand the frustration, but it doesn't help anything.

I am completely aware this was a mistake, but it's not as simple as "the author renamed because he wanted to cause havoc in the Go community and refuses to revert". Originally, this was attempting to address problems on systems where the upper-case of Logrus was causing issues, the maintainers made the incorrect assumption that most users of Logrus would be vendored and it wouldn't cause major issues—but it did. It then turned out that people had been importing in lower-case in various places anyway because it worked with both after the rename. This was realized late in the process, and reverting, either way, would be painful.

@sagikazarmark
Copy link

@sirupsen sorry for the harsh words, indeed I was quite frustrated about the situation.

Thanks for the release.

As others pointed out, a postmortem blog post might be a good idea, partly to communicate to the world why half of go applications broke down, also to provide a clear picture to Golang maintainers.

surendarchandra pushed a commit to surendarchandra/logrus_firehose that referenced this issue Jun 9, 2017
- logrus changed its name from upper case to lower case - fix it.
  sirupsen/logrus#553

- NewWithAWSConfig was not passing on the name parameter.

- Minor fix in readme to use the Config value.
abduld pushed a commit to rai-project/vipertags that referenced this issue Jun 9, 2017
abduld pushed a commit to rai-project/database that referenced this issue Jun 9, 2017
abduld pushed a commit to rai-project/tracer that referenced this issue Jun 9, 2017
abduld pushed a commit to rai-project/inle that referenced this issue Jun 9, 2017
abduld pushed a commit to rai-project/ratelimit that referenced this issue Jun 9, 2017
abduld pushed a commit to rai-project/grpc that referenced this issue Jun 9, 2017
abduld pushed a commit to rai-project/logger that referenced this issue Jun 9, 2017
abduld pushed a commit to rai-project/config that referenced this issue Jun 9, 2017
abduld pushed a commit to rai-project/cudainfo that referenced this issue Jun 9, 2017
wking added a commit to wking/go-mtree that referenced this issue Nov 3, 2017
With:

  $ git mv vendor/github.com/{S,s}irupsen
  $ sed -i 's/Sirupsen/sirupsen/g' $(git grep -l Sirupsen)

catching up with the upstream lowercasing [1,2,3,4].  Note the
compatibility issues discussed in [3], some consumers may prefer to
use the old uppercase version until they have time to update their
other Logrus consumers to the new lowercase form.

[1]: https://github.com/sirupsen/logrus/blame/v1.0.3/README.md#L6
[2]: sirupsen/logrus#384
[3]: sirupsen/logrus#570 (comment)
[4]: sirupsen/logrus#553
wking added a commit to wking/go-mtree that referenced this issue Nov 3, 2017
With:

  $ git mv vendor/github.com/{S,s}irupsen
  $ sed -i 's/Sirupsen/sirupsen/g' $(git grep -l Sirupsen)

catching up with the upstream lowercasing [1,2,3,4].  Because of the
compatibility issues discussed in [3], some consumers may prefer to
use the old uppercase version until they have time to update their
other Logrus consumers to the new lowercase form.

[1]: https://github.com/sirupsen/logrus/blame/v1.0.3/README.md#L6
[2]: sirupsen/logrus#384
[3]: sirupsen/logrus#570 (comment)
[4]: sirupsen/logrus#553
@rws-github
Copy link

I have added the glide solution earlier on the thread but I get the following:

[ERROR]	Update failed for github.com/Sirupsen/logrus: Unable to get repository: Cloning into '/home/travis/.glide/cache/src/git-github.com--sirupsen-logrus'...
Warning: Permanently added the RSA host key for IP address '192.30.253.112' to the list of known hosts.
Permission denied (publickey).
fatal: Could not read from remote repository.

Anyone know the solution?

@dgsb
Copy link
Collaborator

dgsb commented Jul 21, 2018

@rws-github I don't have the whole history. But today the package should be named all lowercase and if an importing package use some uppercase it should be fixed.

@pdrum
Copy link

pdrum commented Sep 1, 2018

This thread helped me fix my problem Masterminds/glide#210

yasker added a commit to yasker/longhorn-engine that referenced this issue Mar 7, 2019
yasker added a commit to yasker/longhorn-manager that referenced this issue Mar 7, 2019
yasker added a commit to longhorn/longhorn-manager that referenced this issue Mar 8, 2019
yasker added a commit to yasker/longhorn-engine that referenced this issue Mar 8, 2019
yasker added a commit to longhorn/longhorn-engine that referenced this issue Mar 8, 2019
johanot pushed a commit to DBCDK/shelob that referenced this issue Jul 17, 2019
This required bump of some external dependencies, in order
to get rid of old versions of logrus as derived depedency.

see issues:
- sirupsen/logrus#553
- golang/go#28489
johanot pushed a commit to DBCDK/shelob that referenced this issue Jul 17, 2019
This required bump of some external dependencies, in order
to get rid of old versions of logrus as derived dependency.

see issues:
- sirupsen/logrus#553
- golang/go#28489
johanot pushed a commit to DBCDK/shelob that referenced this issue Jul 17, 2019
This required bump of some external dependencies, in order
to get rid of old versions of logrus as derived dependency.

see issues:
- sirupsen/logrus#553
- golang/go#28489
@shuaichang
Copy link

shuaichang commented Oct 8, 2020

So I ended up working around this by levering glide aliasing support. Eg in your glide.yaml do :

- package: github.com/sirupsen/logrus
  version: master
- package: github.com/Sirupsen/logrus
  repo: [email protected]:/sirupsen/logrus
  vcs: git
  version: master

the version: master is optional but I wanted to make sure I get #547 in

For anyone who've tried this solution in MacOS: I believe this fix only works for running glide in Linux with case sensitive filesystem. On Mac OS this will only create either sirupsen or Sirupsen in vendor. So if you've built a docker image that can run on Mac OS but the same image will fail running in Linux.

Reference: Masterminds/glide#868

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