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

Fix logrus dependency #10791

Closed
wants to merge 1 commit into from
Closed

Fix logrus dependency #10791

wants to merge 1 commit into from

Conversation

jbltx
Copy link

@jbltx jbltx commented Feb 17, 2019

This will fix the issue #4642

As mentionned, the uppercase letter creates unexpected module path error using go mod command.

As we can see from logrus repository sirupsen/logrus#570, others libraries have also fixed the issue at their side.

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@sayden
Copy link
Contributor

sayden commented Feb 18, 2019

Hi @jbltx

I have just done a quick check and it seems that we are not using logrus anymore and #4642 was an outdated issue.

@jsoriano can you confirm this?

@jsoriano
Copy link
Member

@sayden we are not using logrus directly, but it is a transitive dependency of some other packages we use, we should probably upgrade it if it has been fixed upstream.

@andrewkroh
Copy link
Member

I think we should fix this so that we can unblock the usage of go mod. In addition we should update logrus to get the renamed version. So

govendor remove github.com/Sirupsen/logrus
govendor fetch github.com/sirupsen/logrus
make notice

But there are two other projects that would need to be updated because they depend on the Sirupsen version.

./vendor/github.com/docker/docker/api/common.go:14:	"github.com/Sirupsen/logrus"
./vendor/github.com/docker/docker/pkg/system/syscall_windows.go:7:	"github.com/Sirupsen/logrus"
./vendor/github.com/docker/docker/pkg/fileutils/fileutils_unix.go:10:	"github.com/Sirupsen/logrus"
./vendor/github.com/docker/docker/pkg/fileutils/fileutils.go:13:	"github.com/Sirupsen/logrus"
./vendor/github.com/docker/go-connections/tlsconfig/config.go:16:	"github.com/Sirupsen/logrus"
./vendor/github.com/docker/go-connections/tlsconfig/certpool_other.go:8:	"github.com/Sirupsen/logrus"
./vendor/github.com/docker/go-connections/tlsconfig/certpool_go17.go:9:	"github.com/Sirupsen/logrus"

@exekias @jsoriano I vaguely recall some reason that updating the docker packages could cause problems? Do either of you know more? If going to the head of docker/docker is problematic maybe we could at least move forward to moby/moby@1009e6a.

@exekias
Copy link
Contributor

exekias commented Apr 15, 2019

I think we are using a patched version with this commit? exekias/moby@83d94aa. I don't know about the status upstream

@jsoriano
Copy link
Member

@andrewkroh would the changes done by @kvch in #11330 fix this?

@kvch
Copy link
Contributor

kvch commented Apr 15, 2019

My PR is fixing this issue, as I have completely removed Sirupsen/logrus from our dependencies.
I also updated the dependencies which need this package. So @andrewkroh I am not sure how you have found those lines, but when I run git grep Sirupsen in the root of our repo, it does not return anything.

@andrewkroh
Copy link
Member

@kvch I'm glad you fixed this. I was looking at an older tag and I didn't realize this was fixed on master.

With that being said I'm still getting an error while attempting to use go mod with a community beat. At some point I'll try to investigate this further.

go: github.com/Sirupsen/logrus@v1.4.1: parsing go.mod: unexpected module path "github.com/sirupsen/logrus"

@andrewkroh
Copy link
Member

Closing logrus was fixed by #11330.

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

Successfully merging this pull request may close these issues.

7 participants