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

Rename back to Sirupsen/logrus #570

Closed
mcandre opened this issue Jul 6, 2017 · 14 comments
Closed

Rename back to Sirupsen/logrus #570

mcandre opened this issue Jul 6, 2017 · 14 comments

Comments

@mcandre
Copy link
Contributor

@mcandre mcandre commented Jul 6, 2017

Ay, a lot of projects are now broken after changing the casing of this project's git repository name. Docker/Moby, for example, can no longer be installed via glide, due to the Sirupsen/logrus 404.

@vearutop
Copy link

@vearutop vearutop commented Jul 7, 2017

Yes, original rename was a perfect failure.

@elaijuh
Copy link

@elaijuh elaijuh commented Jul 7, 2017

Any ETA? I was just gliding again and again and again

@vearutop
Copy link

@vearutop vearutop commented Jul 7, 2017

@elaijuh adding lowercase package and repo to your glide.yaml may fix the problem

- package: github.com/sirupsen/logrus
  version: master
- package: github.com/Sirupsen/logrus
  repo: https://github.com/sirupsen/logrus.git
  version: 10f801ebc38b33738c9d17d50860f484a0988ff5

@elaijuh
Copy link

@elaijuh elaijuh commented Jul 7, 2017

i was seeing this

[ERROR] Update failed for github.com/sirupsen/logrus: The Remote does not match the VCS endpoint

oh that was glide cache problem though

now glide helps to resolve the dep like this

- package: github.com/sirupsen/logrus
  version: master
- package: github.com/Sirupsen/logrus
  repo: https://github.com/sirupsen/logrus.git
  version: master

@dobegor
Copy link

@dobegor dobegor commented Jul 7, 2017

@vearutop it won't help if you are running OS with case-insensitive filesystem (macOS by default and Windows are).

@sirupsen
Copy link
Owner

@sirupsen sirupsen commented Jul 9, 2017

See answer here #566 (comment) as well.

I feel powerless, guilty, and deeply ashamed. I have not worked enough with Go in the past few years, and thus this decision was not taken with enough care. This is the root of why this has gone awry. At this point, I am fairly convinced a revert will make things worse. Keep in mind this is only breaking for people with mixed imports. Upper-case continues to work in projects that don't mix case. Every library should use lower-case to avoid this problem.

I am actively engaging now with prospective maintainers who can provide better stewardship of Logrus than I have been able to provide. Before the rename, many people used Logrus with a lower-case import, because it happened to also work. Internally and in the projects I've been engaged with that use Logrus, lower-case imports have dominated. I was swayed by this availability bias. Before making this decision, data should've been gathered for the final direction (force lower-case, or force upper-case) and the other method actively deprecated.

I do know many struggled with the uppercase variant, but it is possible the better course of action would've been to work at workarounds there and kill the lower-case altogether. Of course, those benefitting from the change will not be active here (but I am fairly convinced fewer people benefit from the change, than it negatively affects, thus the regret).

For the record, the original proposal was in this PR: #384.

Note also that it's not the rename of my handle from Sirupsen to sirupsen that's causing issues, but rather the conflicting imports mixing upper- and lower-case. Github sets up a proper redirect. You can still use upper-case imports, you just can't mix them. That's why I originally didn't think this would cause as large issues as it's proved to.

Only people who are importing projects that also use Logrus, where both mix cases are causing issues. This is the vast minority, but those dealing with it—I understand it's frustrating.

I hope, at least, this will serve a greater lesson to the Go community and never happen again. This was, unfortunately, bound to happen at some point that someone would be careless enough to do what was done here.

I am confident that this project will come out better because of it:

  1. There is a clear path forward for which case to use.
  2. This was the nail in the coffin that caused me to take action based on how large Logrus has grown, and that we need a more competent team of maintainers who are actively working with the library. This project is at the point where it needs to be driven by a committee, not an individual. If you'd like to assist in maintaining this project, please reach out to me by email.

An abbreviated timeline of this issue:

  1. #384 proposed
  2. #384 reverted, due to conflicts in community. Handle stayed lower-case, but that's not an issue as upper-case continues to work.
  3. Internal projects were all using lower-case, and I was being pushed to make a decision both by the Logrus community and internally as for casing. I did not gather the appropriate information to make the right call, and simply made the call to proceed with lower-case. I wish that this decision would've been taken at a point in time where I'd had the surplus energy to put into investigating this that it deserved.
  4. Lower-case is adopted in the community, as the decision is made. Due to neglect on my part (due to many other obligations), this proceeded and at this point undoing the change would cause more bad than good. I do believe that Logrus should've been lower-case to begin with, but the ROI on renaming was simply not high enough—as pointd out by many members of the community.

@VojtechVitek
Copy link

@VojtechVitek VojtechVitek commented Jul 11, 2017

Ugh, we have major issues even when using https://github.com/golang/dep

error while exporting github.com/sirupsen/logrus: /var/folders/hr/5zb8r0yx4sv4_1dc0rlccflm0000gn/T/dep105205437/github.com/sirupsen/logrus/.gitignore already exists, no checkout
...
etc.

I'm on MacOS, which has case-insensitive file system. Well, even git itself has problems renaming the repo :D


EDIT:

I figured it out. For anyone interested (and using https://github.com/golang/dep), I had to:

  • Remove all instances of the github.com/Sirupsen/logrus from our codebase
  • Update/Remove all the dependencies that imported the github.com/Sirupsen/logrus - this includes all the transitive dependencies!
  • Also, I had to rm -rf $GOPATH/pkg/dep cache
    and rm -rf $REPO/vendor/github.com/Sirupsen/logrus

VojtechVitek added a commit to pressly/go-emailacid that referenced this issue Jul 11, 2017
VojtechVitek added a commit to pressly/go-emailacid that referenced this issue Jul 11, 2017
mattbostock added a commit to mattbostock/timbala that referenced this issue Jul 23, 2017
I ran `govendor status` to check the integrity of vendored packages and
found that the vendored copy of the logrus library did not match the
version pinned in `vendor.json`.

govendor gave the following error:

    $ govendor status
    The following packages are missing or modified locally:
            github.com/Sirupsen/logrus
    Error: status failed for 1 package(s)

Additionally, remove the unused lowercase variant of logrus from
`vendor.json`. The URL for this library changed upstream to use
lowercase `sirupsen` instead of `Sirupsen`. The best way forward for now
is to continue to use the uppercase version as that's what's used by the
Prometheus libraries; mixed-case imports result in an import collision:

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

See this comment for more details:
sirupsen/logrus#570 (comment)
mattbostock added a commit to mattbostock/timbala that referenced this issue Jul 23, 2017
I ran `govendor status` to check the integrity of vendored packages and
found that the vendored copy of the logrus library did not match the
version pinned in `vendor.json`.

govendor gave the following error:

    $ govendor status
    The following packages are missing or modified locally:
            github.com/Sirupsen/logrus
    Error: status failed for 1 package(s)

Additionally, remove the unused lowercase variant of logrus from
`vendor.json`. The URL for this library changed upstream to use
lowercase `sirupsen` instead of `Sirupsen`. The best way forward for now
is to continue to use the uppercase version as that's what's used by the
Prometheus libraries; mixed-case imports result in an import collision:

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

See this comment for more details:
sirupsen/logrus#570 (comment)
gesanderson added a commit to mathpl/go-tsdmetrics that referenced this issue Jul 24, 2017
This also fixes the bug on case-sensitive package naming. See these
links:

sirupsen/logrus#553 (comment)
sirupsen/logrus#570 (comment)
werberson pushed a commit to werberson/bindman-dns-webhook that referenced this issue Dec 18, 2018
LeBlue pushed a commit to LeBlue/mynewt-newtmgr that referenced this issue May 13, 2019
LeBlue pushed a commit to LeBlue/mynewt-newtmgr that referenced this issue May 13, 2019
LeBlue pushed a commit to LeBlue/mynewt-newtmgr that referenced this issue May 13, 2019
LeBlue pushed a commit to LeBlue/mynewt-newtmgr that referenced this issue May 13, 2019
LeBlue pushed a commit to LeBlue/mynewt-newtmgr that referenced this issue May 13, 2019
eladishay pushed a commit to alcideio/moby that referenced this issue Jul 2, 2019
eladishay pushed a commit to alcideio/moby that referenced this issue Jul 2, 2019
abilioesteves pushed a commit to labbsr0x/bindman-dns-webhook that referenced this issue Sep 13, 2019
abilioesteves pushed a commit to labbsr0x/bindman-dns-webhook that referenced this issue Sep 13, 2019
@richard-mauri
Copy link

@richard-mauri richard-mauri commented Oct 25, 2019

I too am dealing with fallout from this issue.
I am trying to go (1.13.3) build gitlab.com/msvechla/vaultbeat with GO111MODULE=on
This project does not yet have a go.mod (I am trying to push this frontier)
It's unclear how to fix this.

go: gitlab.com/msvechla/vaultbeat imports
github.com/elastic/beats/libbeat/cmd/instance imports
github.com/elastic/beats/libbeat/autodiscover/providers/docker imports
github.com/elastic/beats/libbeat/common/docker imports
github.com/docker/docker/api imports
github.com/Sirupsen/logrus: github.com/Sirupsen/logrus@v1.4.2: parsing go.mod:
module declares its path as: github.com/sirupsen/logrus
but was required as: github.com/Sirupsen/logrus

maci3jka added a commit to maci3jka/junk that referenced this issue Feb 18, 2020
wGEric added a commit to wGEric/sumologic that referenced this issue May 19, 2020
tamalsaha pushed a commit to kmodules/shared-informer that referenced this issue Aug 13, 2020
Automatic merge from submit-queue (batch tested with PRs 51186, 50350, 51751, 51645, 51837)

Update Cadvisor Dependency

Fixes: kubernetes/kubernetes#51832
This is the worst dependency update ever...
The root of the problem is the [name change of Sirupsen -> sirupsen](sirupsen/logrus#570 (comment)).  This means that in order to update cadvisor, which venders the lowercase, we need to update all dependencies to use the lower-cased version.  With that being said, this PR updates the following packages:

`github.com/docker/docker`
- `github.com/docker/distribution`
  - `github.com/opencontainers/go-digest`
  - `github.com/opencontainers/image-spec`
  - `github.com/opencontainers/runtime-spec`
  - `github.com/opencontainers/selinux`
  - `github.com/opencontainers/runc`
    - `github.com/mrunalp/fileutils`
  - `golang.org/x/crypto`
    - `golang.org/x/sys`
- `github.com/docker/go-connections`
- `github.com/docker/go-units`
- `github.com/docker/libnetwork`
- `github.com/docker/libtrust`
- `github.com/sirupsen/logrus`
- `github.com/vishvananda/netlink`

`github.com/google/cadvisor`
- `github.com/euank/go-kmsg-parser`

`github.com/json-iterator/go`

Fixed kubernetes/kubernetes#51832

```release-note
Fix journalctl leak on kubelet restart
Fix container memory rss
Add hugepages monitoring support
Fix incorrect CPU usage metrics with 4.7 kernel
Add tmpfs monitoring support
```

Kubernetes-commit: 99aa992ce845fe947a406ac4d3f99d2208f0416b
tamalsaha pushed a commit to gomodules/jsonpath that referenced this issue Apr 21, 2021
Automatic merge from submit-queue (batch tested with PRs 51186, 50350, 51751, 51645, 51837)

Update Cadvisor Dependency

Fixes: kubernetes/kubernetes#51832
This is the worst dependency update ever...
The root of the problem is the [name change of Sirupsen -> sirupsen](sirupsen/logrus#570 (comment)).  This means that in order to update cadvisor, which venders the lowercase, we need to update all dependencies to use the lower-cased version.  With that being said, this PR updates the following packages:

`github.com/docker/docker`
- `github.com/docker/distribution`
  - `github.com/opencontainers/go-digest`
  - `github.com/opencontainers/image-spec`
  - `github.com/opencontainers/runtime-spec`
  - `github.com/opencontainers/selinux`
  - `github.com/opencontainers/runc`
    - `github.com/mrunalp/fileutils`
  - `golang.org/x/crypto`
    - `golang.org/x/sys`
- `github.com/docker/go-connections`
- `github.com/docker/go-units`
- `github.com/docker/libnetwork`
- `github.com/docker/libtrust`
- `github.com/sirupsen/logrus`
- `github.com/vishvananda/netlink`

`github.com/google/cadvisor`
- `github.com/euank/go-kmsg-parser`

`github.com/json-iterator/go`

Fixed kubernetes/kubernetes#51832

```release-note
Fix journalctl leak on kubelet restart
Fix container memory rss
Add hugepages monitoring support
Fix incorrect CPU usage metrics with 4.7 kernel
Add tmpfs monitoring support
```

Kubernetes-commit: 99aa992ce845fe947a406ac4d3f99d2208f0416b
tamalsaha pushed a commit to gomodules/jsonpath that referenced this issue Apr 21, 2021
Automatic merge from submit-queue (batch tested with PRs 51186, 50350, 51751, 51645, 51837)

Update Cadvisor Dependency

Fixes: kubernetes/kubernetes#51832
This is the worst dependency update ever...
The root of the problem is the [name change of Sirupsen -> sirupsen](sirupsen/logrus#570 (comment)).  This means that in order to update cadvisor, which venders the lowercase, we need to update all dependencies to use the lower-cased version.  With that being said, this PR updates the following packages:

`github.com/docker/docker`
- `github.com/docker/distribution`
  - `github.com/opencontainers/go-digest`
  - `github.com/opencontainers/image-spec`
  - `github.com/opencontainers/runtime-spec`
  - `github.com/opencontainers/selinux`
  - `github.com/opencontainers/runc`
    - `github.com/mrunalp/fileutils`
  - `golang.org/x/crypto`
    - `golang.org/x/sys`
- `github.com/docker/go-connections`
- `github.com/docker/go-units`
- `github.com/docker/libnetwork`
- `github.com/docker/libtrust`
- `github.com/sirupsen/logrus`
- `github.com/vishvananda/netlink`

`github.com/google/cadvisor`
- `github.com/euank/go-kmsg-parser`

`github.com/json-iterator/go`

Fixed kubernetes/kubernetes#51832

```release-note
Fix journalctl leak on kubelet restart
Fix container memory rss
Add hugepages monitoring support
Fix incorrect CPU usage metrics with 4.7 kernel
Add tmpfs monitoring support
```

Kubernetes-commit: 99aa992ce845fe947a406ac4d3f99d2208f0416b
tamalsaha pushed a commit to kmodules/airgap that referenced this issue Jun 24, 2021
Automatic merge from submit-queue (batch tested with PRs 51186, 50350, 51751, 51645, 51837)

Update Cadvisor Dependency

Fixes: kubernetes/kubernetes#51832
This is the worst dependency update ever...
The root of the problem is the [name change of Sirupsen -> sirupsen](sirupsen/logrus#570 (comment)).  This means that in order to update cadvisor, which venders the lowercase, we need to update all dependencies to use the lower-cased version.  With that being said, this PR updates the following packages:

`github.com/docker/docker`
- `github.com/docker/distribution`
  - `github.com/opencontainers/go-digest`
  - `github.com/opencontainers/image-spec`
  - `github.com/opencontainers/runtime-spec`
  - `github.com/opencontainers/selinux`
  - `github.com/opencontainers/runc`
    - `github.com/mrunalp/fileutils`
  - `golang.org/x/crypto`
    - `golang.org/x/sys`
- `github.com/docker/go-connections`
- `github.com/docker/go-units`
- `github.com/docker/libnetwork`
- `github.com/docker/libtrust`
- `github.com/sirupsen/logrus`
- `github.com/vishvananda/netlink`

`github.com/google/cadvisor`
- `github.com/euank/go-kmsg-parser`

`github.com/json-iterator/go`

Fixed kubernetes/kubernetes#51832

```release-note
Fix journalctl leak on kubelet restart
Fix container memory rss
Add hugepages monitoring support
Fix incorrect CPU usage metrics with 4.7 kernel
Add tmpfs monitoring support
```

Kubernetes-commit: 99aa992ce845fe947a406ac4d3f99d2208f0416b
tamalsaha pushed a commit to kmodules/airgap that referenced this issue Jun 24, 2021
Automatic merge from submit-queue (batch tested with PRs 51186, 50350, 51751, 51645, 51837)

Update Cadvisor Dependency

Fixes: kubernetes/kubernetes#51832
This is the worst dependency update ever...
The root of the problem is the [name change of Sirupsen -> sirupsen](sirupsen/logrus#570 (comment)).  This means that in order to update cadvisor, which venders the lowercase, we need to update all dependencies to use the lower-cased version.  With that being said, this PR updates the following packages:

`github.com/docker/docker`
- `github.com/docker/distribution`
  - `github.com/opencontainers/go-digest`
  - `github.com/opencontainers/image-spec`
  - `github.com/opencontainers/runtime-spec`
  - `github.com/opencontainers/selinux`
  - `github.com/opencontainers/runc`
    - `github.com/mrunalp/fileutils`
  - `golang.org/x/crypto`
    - `golang.org/x/sys`
- `github.com/docker/go-connections`
- `github.com/docker/go-units`
- `github.com/docker/libnetwork`
- `github.com/docker/libtrust`
- `github.com/sirupsen/logrus`
- `github.com/vishvananda/netlink`

`github.com/google/cadvisor`
- `github.com/euank/go-kmsg-parser`

`github.com/json-iterator/go`

Fixed kubernetes/kubernetes#51832

```release-note
Fix journalctl leak on kubelet restart
Fix container memory rss
Add hugepages monitoring support
Fix incorrect CPU usage metrics with 4.7 kernel
Add tmpfs monitoring support
```

Kubernetes-commit: 99aa992ce845fe947a406ac4d3f99d2208f0416b
tamalsaha pushed a commit to kmodules/airgap that referenced this issue Jun 24, 2021
Automatic merge from submit-queue (batch tested with PRs 51186, 50350, 51751, 51645, 51837)

Update Cadvisor Dependency

Fixes: kubernetes/kubernetes#51832
This is the worst dependency update ever...
The root of the problem is the [name change of Sirupsen -> sirupsen](sirupsen/logrus#570 (comment)).  This means that in order to update cadvisor, which venders the lowercase, we need to update all dependencies to use the lower-cased version.  With that being said, this PR updates the following packages:

`github.com/docker/docker`
- `github.com/docker/distribution`
  - `github.com/opencontainers/go-digest`
  - `github.com/opencontainers/image-spec`
  - `github.com/opencontainers/runtime-spec`
  - `github.com/opencontainers/selinux`
  - `github.com/opencontainers/runc`
    - `github.com/mrunalp/fileutils`
  - `golang.org/x/crypto`
    - `golang.org/x/sys`
- `github.com/docker/go-connections`
- `github.com/docker/go-units`
- `github.com/docker/libnetwork`
- `github.com/docker/libtrust`
- `github.com/sirupsen/logrus`
- `github.com/vishvananda/netlink`

`github.com/google/cadvisor`
- `github.com/euank/go-kmsg-parser`

`github.com/json-iterator/go`

Fixed kubernetes/kubernetes#51832

```release-note
Fix journalctl leak on kubelet restart
Fix container memory rss
Add hugepages monitoring support
Fix incorrect CPU usage metrics with 4.7 kernel
Add tmpfs monitoring support
```

Kubernetes-commit: 99aa992ce845fe947a406ac4d3f99d2208f0416b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet