quasilyte / go-consistent

Source code analyzer that helps you to make your Go programs more consistent.
MIT License
336 stars 16 forks source link

Module rename has broken indirectly golangci-lint #34

Open XoseRamon opened 5 years ago

XoseRamon commented 5 years ago

Your latest commit s/Quasilyte/quasilyte might have broken projects that reference indirectly (maybe even directly, not sure) go-consistent since go mod is case-sensitive and checks the paths (see this issue).

I am not sure if this is the right place to report it, but since it is the source of the error, I just wanted to warn you in case it is an unexpected side effect. The dependency chain is golangci-lint -> go-critic -> go-consistent.

To reproduce:

GO111MODULE=on go get -u github.com/golangci/golangci-lint/cmd/golangci-lint@master              
go: finding github.com/golangci/golangci-lint/cmd/golangci-lint master                                                                      
go: finding github.com/golangci/golangci-lint/cmd master                                                                                    
go: finding github.com/golangci/golangci-lint master                                                                                        
go: finding github.com/golangci/goconst latest
...
 go: finding github.com/golangci/unconvert latest
go: github.com/Quasilyte/go-consistent@v0.0.0-20190521200055-c6f3937de18c: parsing go.mod: unexpected module path "github.com/quasilyte/go-consistent"
go: finding github.com/logrusorgru/aurora latest
go: finding gopkg.in/tomb.v1 latest
go: finding github.com/StackExchange/wmi latest
go get: error loading module requirements     
quasilyte commented 5 years ago

Hmm, I think go-consistent is only used in ci for go-critic. It shouldn't be a first-class dependency. I'll take a look, thanks.

mec07 commented 5 years ago

I'm having the same problem... đŸ˜¢

mec07 commented 5 years ago

I've found a solution, it's not super satisfactory, but seems to work in circleci: go get github.com/golangci/golangci-lint/cmd/golangci-lint@de1d1ad It came from this: https://github.com/golang/go/issues/31148

quasilyte commented 5 years ago

See the referenced commit. Should help to resolve the problem.

mec07 commented 5 years ago

Thanks for this. I tried it out again to see if the issue was resolved by your commit, but it's still happening...

go: github.com/Quasilyte/go-consistent@v0.0.0-20190521200055-c6f3937de18c: parsing go.mod: unexpected module path "github.com/quasilyte/go-consistent"

So I'm sticking with the suboptimal solution for now:

go get github.com/golangci/golangci-lint/cmd/golangci-lint@de1d1ad
thepudds commented 5 years ago

@quasilyte When modules are enabled, the go tool enforces that the module name declared in the module statement on the first line of the go.mod matches the import path used by consumers.

Is it necessary to change the case for the import path for github.com/Quasilyte/go-consistent?

Changes like that are breaking changes. They can end up having wide repercussions across the ecosystem.

For example, Sirupsen/logrus vs. sirupsen/logrus has caused a large amount of churn (see for example https://github.com/golang/go/issues/28489). A similar but slightly different example is golang.org/x/lint vs. github.com/golang/lint (e.g., golang/lint#436 and https://github.com/golang/go/issues/30831), where the ecosystem was using two different import paths and introducing a canonical name in the go.mod caused problems for many consumers.

golangci-lint happens to be having other issues now (including due to a change in x/tools/go/packages), but would be good if this rename here could be avoided?

CC @dmitshur @matloob

quasilyte commented 5 years ago

go-consistent is not a library and the reason it end up in a dep tree is https://github.com/go-critic/go-critic/blob/master/tools.go. I don't like that a tool that is used during ci tests of go-critic is a dependency of golangci-lint. This is the problem, not the import path, it shouldn't be a dependency on the first place.

quasilyte commented 5 years ago

And now import path should match. I've done the renaming in go-critic.

thepudds commented 5 years ago

Hi @quasilyte, With the change you made in go-critic, do upgrades end up working for you?

For example, if I do:

cd $(mktemp -d)
go mod init tempmod
go get -m github.com/go-critic/go-critic@f6f702b31734df2
go get -u

It fails during the upgrade step with:

go: finding github.com/Quasilyte/go-consistent latest
go: github.com/Quasilyte/go-consistent@v0.0.0-20190521200055-c6f3937de18c: 
parsing go.mod: unexpected module path "github.com/quasilyte/go-consistent"
go: finding github.com/logrusorgru/aurora latest
go: finding golang.org/x/tools latest
go get: error loading module requirements
thepudds commented 5 years ago

That was a synthetic example intended to show the problem more clearly, but here is closer to a real example:

cd $(mktemp -d)
go mod init tempmod
go get github.com/golangci/golangci-lint@master
go get -u

which fails during the upgrade step with:

...
go: github.com/Quasilyte/go-consistent@v0.0.0-20190521200055-c6f3937de18c: 
parsing go.mod: unexpected module path "github.com/quasilyte/go-consistent"
go get: error loading module requirements
quasilyte commented 5 years ago

go get -m github.com/go-critic/go-critic@f6f702b31734df2

Do you get the revision prior to the fix intentionally?

golangci-lint probably uses gocritic version prior to the fix as well. So it needs to be updated. But all that is an issue only if you're using unreleased versions, I think.

quasilyte commented 5 years ago

Changes like that are breaking changes. They can end up having wide repercussions across the ecosystem

I believe it's mostly true for libraries.

For example, Sirupsen/logrus vs. sirupsen/logrus has caused a large amount of churn (see for example golang/go#28489). A similar but slightly different example is golang.org/x/lint vs. github.com/golang/lint (e.g., golang/lint#436 and golang/go#30831), where the ecosystem was using two different import paths and introducing a canonical name in the go.mod caused problems for many consumers.

I hear you, but again, those are released libraries intended for public use. Go-consistent is a linter, not something that others can/should depend on inside their modules. The problem is that it made it's way into dependencies tree of golangci-lint. We should probably concentrate on that. This is my point of view.

thepudds commented 5 years ago

FWIW, the new v1.17.0 release of golangci-lint seems to fail when doing a go get -u:

cd $(mktemp -d)
go mod init tempmod
go get -u github.com/golangci/golangci-lint@v1.17.0

which fails with:

go: github.com/Quasilyte/go-consistent@v0.0.0-20190521200055-c6f3937de18c: parsing go.mod: unexpected module path "github.com/quasilyte/go-consistent"

Some related discussion in https://github.com/golangci/golangci-lint/issues/479.

jirfag commented 5 years ago

try go get -v -u github.com/golangci/golangci-lint@v1.17.1, please

cristaloleg commented 4 years ago

Close?

quasilyte commented 2 years ago

Should be fixed by https://github.com/go-critic/go-critic/pull/1162 If anyone can confirm that it is, we can close this issue.