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

Add go.mod file #945

Merged
merged 3 commits into from
Jan 11, 2020
Merged

Add go.mod file #945

merged 3 commits into from
Jan 11, 2020

Conversation

ChrisHines
Copy link
Member

The go.mod file was generated with the following sequence of commands:

go mod init
go mod tidy

The first try resulted in two problems.

  • One problem was fixed by updating the import path of nats-server to
    use semantic import v2 in transport/nats/subscriber_test.go.
  • The other problem was fixed by changing the required version of
    go.etcd.io/etcd to v0.0.0-20191023171146-3cf2f69b5738 which
    corresponds to v3.4.3. We cannot use the v3.4.3 tag directly because
    of problems within the etcd repo itself described in
    go get error for 3.4.0 etcd-io/etcd#11154

After those two changes a second go mod tidy succeeds and produces the
go.mod file included in this commit.

The go.mod file was generated with the following sequence of commands:

go mod init
go mod tidy

The first try resulted in two problems.

- One problem was fixed by updating the import path of nats-server to
  use semantic import v2 in transport/nats/subscriber_test.go.
- The other problem was fixed by changing the required version of
  go.etcd.io/etcd to v0.0.0-20191023171146-3cf2f69b5738 which
  corresponds to v3.4.3. We cannot use the v3.4.3 tag directly because
  of problems within the etcd repo itself described in
  etcd-io/etcd#11154

After those two changes a second go mod tidy succeeds and produces the
go.mod file included in this commit.
Performed by deleteding all lines marked // indirect in the go.mod file
and running go mod tidy to restore the required entries.

The entries ultimately removed were probably a remnant of a prior state
of the go.mod file in this sequence.
@sagikazarmark
Copy link
Contributor

It's probably worth noting that the /v2 imports won't work bellow Go 1.11. It's not necessarily an issue, but should probably be explicitly mentioned somewhere.

@ChrisHines
Copy link
Member Author

ChrisHines commented Jan 10, 2020

It's probably worth noting that the /v2 imports won't work bellow Go 1.11. It's not necessarily an issue, but should probably be explicitly mentioned somewhere.

I think the correct statement is that the /v2 imports won't work below Go 1.9.7 or Go 1.10.3 for those versions of Go. I will claim some credit here, as I raised this issue with the Go project during the module proposal phase; see golang/go#24301 (comment).

Russ Cox responded to my concern with a proposal to allow older versions of Go to still work with module aware code in the presence of major versions >= v2; see golang/go#24301 (comment).

That idea was implemented in golang/go@d4e2128, backported to Go 1.10.x in golang/go@28ae826, and Go 1.9.x in golang/go@05604d7.

There is a risk that despite those efforts other changes to the modules system prevent it from working as intended, so I installed Go 1.9.7 just now and was able to compile most of the code and it's dependencies on this branch. But a few dependencies use strings.Builder which was added in Go 1.10, so it doesn't matter if modules work with Go 1.9.7 anyway.

Using Go 1.10.8 I was able to get a run of go build ./... to succeed. Even the v2 packages such as github.com/casbin/casbin built successfully.

I was not brave enough to try running tests.

That said, I don't think it is a strong goal for go-kit to support older release of Go than the Go team does. At the moment that means Go 1.13 and 1.12. I believe the CI scripts only test with the latest release and tip. So if Go 1.10 and 1.11 still work, great, but I doubt we would block making changes if they didn't.

@ChrisHines
Copy link
Member Author

@peterbourgon I know that maintaining the go.mod file is a concern. We could use github.com/psampaz/go-mod-outdated to ease that burden. For example, running that tool just now identifies a few updates since I created the go.mod on this branch.

$ go list -json -m -u all | go-mod-outdated -direct -update
[snip]
+---------------------------+------------------------------------+------------------------------------+--------+------------------+
|          MODULE           |              VERSION               |            NEW VERSION             | DIRECT | VALID TIMESTAMPS |
+---------------------------+------------------------------------+------------------------------------+--------+------------------+
| github.com/aws/aws-sdk-go | v1.27.0                            | v1.27.4                            | true   | true             |
| github.com/streadway/amqp | v0.0.0-20190827072141-edfb9018d271 | v0.0.0-20200108173154-1c71cc93ed71 | true   | true             |
| go.etcd.io/etcd           | v0.0.0-20191023171146-3cf2f69b5738 | v3.3.18+incompatible               | true   | true             |
| golang.org/x/tools        | v0.0.0-20200103221440-774c71fcf114 | v0.0.0-20200110004031-563860d11da6 | true   | true             |
+---------------------------+------------------------------------+------------------------------------+--------+------------------+

The etcd entry is wrong for the same reason I had to manually patch its entry in the go.mod as described above. But otherwise the tool does a nice job of filtering out all the noise and nicely presenting possible updates.

We would likely only need to worry about this if we add some code that needs a new feature of some dependency or as we prepare a new Go kit release.

Or we could add a CI step that tests against the newest releases of all the direct dependencies as an early warning about maintenance needed on Go kit itself. That might require finding, or writing a tool to help do exactly what we want, but I think it is clear that the output of go list -m -json all provides sufficient data to solve the problem without too much fuss.

@sagikazarmark
Copy link
Contributor

@ChrisHines thanks a lot for detailed response, much appreciated.

@peterbourgon
Copy link
Member

Given that adding a go.mod to this repo seems unavoidable, I'm fine with this PR.

We would likely only need to worry about [updating the go.mod file] if we add some code that needs a new feature of some dependency or as we prepare a new Go kit release.

Yes, I guess one consequence of modules and version affinity is that we only need to care about updating dependencies when we tag a new version ourselves.

@basvanbeek
Copy link
Member

I'm good with this too. Thanks @ChrisHines for the effort

@ChrisHines ChrisHines marked this pull request as ready for review January 11, 2020 01:51
@ChrisHines ChrisHines changed the title Add initial go.mod file Add go.mod file Jan 11, 2020
@ChrisHines ChrisHines merged commit 9f139f1 into master Jan 11, 2020
@ChrisHines ChrisHines deleted the go-mod branch January 11, 2020 01:56
@ChrisHines
Copy link
Member Author

OK. It's merged. I don't believe it will have any effect for most users until we tag a new release. I will defer that decision to @peterbourgon.

@sagikazarmark sagikazarmark mentioned this pull request Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants