cloudflare / goflow

The high-scalability sFlow/NetFlow/IPFIX collector used internally at Cloudflare.
BSD 3-Clause "New" or "Revised" License
859 stars 172 forks source link

Fix v3 module update #59

Closed mmlb closed 4 years ago

mmlb commented 4 years ago

Without this change go get in module mode (and go mod commands) fail since go is fetching/importing the latest v1 release. Its nice that v1 fails to build because of import path requirement that brings this issue to light.

$ GO111MODULE=on go get github.com/cloudflare/goflow
go: github.com/cloudflare/goflow 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

$ GO111MODULE=on go get github.com/cloudflare/goflow@v3.2.0
go: finding github.com/cloudflare v3.2.0
go: finding github.com v3.2.0
go: finding github.com/cloudflare/goflow v3.2.0
go: finding github.com/cloudflare/goflow v3.2.0
go get github.com/cloudflare/goflow@v3.2.0: github.com/cloudflare/goflow@v3.2.0: invalid version: module contains a go.mod file, so major version must be compatible: should be v0 or v1, not v3

Which is the case in v1.1.0 tag.

lspgn commented 4 years ago

Thanks for the PR @mmlb!

I can get rid of the tag v1 but cannot find much doc on the behavior of go get which would fetch directly v1. This seems to describe the go.mod with vXX.

Will take a look why the build fails.

mmlb commented 4 years ago

So I think it might be because all of the goflow code also imports itself @v1? I think the bit about sed ... illustrates that. I'm not a fan of the vN subdirs and thought that might not be required, but might be wrong.

mmlb commented 4 years ago

@lspgn I think I've figured out the test failures. Without the import path re-write like shown in the page you linked the repo was in a weird mixed version mode I think. When I ran make race-test locally it failed just like in travis. With the import path re-written to have the version in it, tests now pass locally. Lets see if its true in travis too.

mmlb commented 4 years ago

Tests are green :tada:

mmlb commented 4 years ago

I can get rid of the tag v1 but cannot find much doc on the behavior of go get which would fetch directly v1.

You don't want to do that, that could break other users.

mmlb commented 4 years ago

ping @lspgn tests are green now, anything else before merge?