logrus: Case change breaks builds?

Fixed in master. Let me know if this works for you. Thanks for providing adequate context.

I’ve added a public comment about this to the README should anyone come across this.

I apologize for this. I underestimated the impact this had. This was not acceptable, and not a justified thing to break backwards incompatibility for. If we wish to provide a lower-case name-change, we’ll do so under a different name (e.g. under a logrus organization).

Sorry for the long reply…

Conventions are great - I’m a big believer in solid conventions and following them - but following conventions should be done before creating a dependency tree.

Changes like this have a significant impact downstream, especially if those projects use third-party libs/plugins that havent updated their import paths so the only solutions are to:

1. Wait for these third party Devs to update their code (or accept your PR - which still leaves you in the lurch)
2. Internalise everything
3. Work off of pinned revisions (a very unpalatable option)

We use 3 or 4 hooks, some provided by this repo and others external. And they all disagree as to what to reference, this change broke the build for about 5 of our projects and about 3 dependent internal libs.

Personally I prefer to not vendor because it means that stable, non-breaking updates make it into your build organically, and breaking changes are usually documented by library maintainers well enough to make for a painless transition - non-breaking backwards compatibility is pretty core to Go AFAIK, and it’s a pattern to be followed, because it encourages responsibility on bahalf of the library maintainer.

But because logrus is a pluggable lib, it has an ecosystem that is dependent on its stability, and like all ecosystems those extensions only update at the behest of their maintainers - which is never a given in OSS.

That means changing the core dependency so it breaks the ecosystem should be done with great care, because now after this PR, the logrus ecosystem is broken and unusable until all hook developers, and formatters, and plugin builders update their code.

We’ve mitigated this upstream risk by forking and modifying all dependent projects -including logrus - because we can’t wait for the ecosystem to self-correct, that was effort we could have expended elsewhere.

(Yes these should all be pull requests to the existing maintainers, and they will be - but to get to production waiting for a PR in an automated environment is not an option)

This is a great library, so please keep making it better and extending it, but please consider introducing major breaking changes more carefully.

This is confusing, what’s the right way to do this now? In the readme it says:

Seeing weird case-sensitive problems? See this issue. This change has been reverted. I apologize for causing this. I greatly underestimated the impact this would have. Logrus strives for stability and backwards compatibility and failed to provide that.

but it also says:

The organization’s name was changed to lower-case–and this will not be changed back. If you are getting import conflicts due to case sensitivity, please use the lower-case import: github.com/sirupsen/logrus. - Source

I side with @sirupsen’s suggestion here https://github.com/sirupsen/logrus/pull/384#issuecomment-264333311, in were he suggests this to be continued in logrus/logrus.

This ensures support for those vendoring, or go get’ing, either upper or lower cases.

However - I feel a repo change shouldn’t be purely on a naming and conventional basis, but should have a larger driving factor.

@stevvooe there was plenty of ways to resolve it, but only one way that resolved itself. I’m not sure what your point is about “go get” and production, or “gobgp” production code being built with “go get”. This isn’t a production issue, no one here was claiming they had systems down.

The complaint was a needless interruption in everyones workflow. Maybe you don’t, but I along with many other developers manage their many (or one) GOPATHS for local development using go get. I don’t vendor every single command, binary, project, etc that I use day-to-day. Many of those projects don’t vendor their own libraries.

With how fast the Go ecosystem moves and interesting new libraries are popping up I don’t see how someone could ever discourage the use of go get, then to justify breaking many peoples workflows over a capital letter on their file system system… just seems unusual to me.

@s2ugimot I think vendoring may be a good idea given the scale of GoBGP and the fact the binary commands (gobgp gobgpd) are widely used ad-hoc with go-get.

This still hasn’t been rolled back, @sirupsen are you keeping this change? Every single persons build who uses your library is broken right now… over a capital letter. If you truly felt it was imperative to have a all lowercase package name and the project is your #1 priority- why not move it into it’s own organization with a new name all together to give people a chance to migrate… github.com/logrus/logrus or something.

I think this should be reverted and reevaluated, there is not even a fix for this. I would need to hunt down every single dependency of every library… and have them update in a single transaction across multiple organizations and repos … all to change the casing of a letter to … what? Keep your name inside the url? Seems petty… just my opinion, it’s your repo, name and code.

just wanted to let folks who’re monitoring this issue know that golang/dep will soon have some more sophisticated reporting around this problem: golang/dep#1079. we can’t actually solve it in dep - doing so requires rewriting import statements, which we strictly do not do - but we can guide the user to where the problem is, and to know which projects need issues filed against them to change from S -> s.

We didn’t have any build failures, since we use vendoring, but I was able to reproduce this earlier today with gobgp.

On my local box (OS X), where I use go get extensively, I updated by moving github.com/Sirupsen to githhub.com/sirupsen. You can try to clear out $GOPATH/pkg in case there are mixed case packages lingering. This works fine for shallow imports.

With larger projects, more is required. This is the build error I received with gobgp:

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

Nothing a little perl can’t solve:

$ find ~/go/src/github.com/osrg/gobgp -type f -name '*.go' | xargs perl -i -pne 's|Sirupsen|sirupsen|'

After running that, the build works fine.

This is probably not a “production” solution, but neither is “go get”.

For any Glide users affected by this change, the GitHub name change also poses a problem. See a workaround here: https://github.com/sirupsen/logrus/pull/384#issuecomment-264035988

The solution we went with for now was locking to an old Logrus version and making the repo URL explicit.