TOOLS-2801: Migrate from dep to Go modules and update README #344

edobranov · 2021-02-05T20:34:57Z

Jira: https://jira.mongodb.org/browse/TOOLS-2801

This replaces Gopkg.toml / Gopkg.lock with go.mod / go.sum. I did this by running go mod init followed by go mod tidy and manually removing the Gopkg files. I also updated the README with some relevant info.

I'll revendor with go mod vendor later and push those changes separately, since the size of those changes exceed Evergreen's limits for patch builds.

edobranov · 2021-02-05T20:39:42Z

README.md

-$GOPATH.
+We currently build the tools with Go version 1.15. Other Go versions may work but they are untested.
+
+Using `go get` will not work. To build the tools, you need to first clone this repository into your Go workspace inside your $GOPATH.


I originally thought that switching to Go modules will allow others to download and build the tools with go get, but that's not the case.

If you do go get github.com/mongodb/mongo-tools, that'll download the tools just fine, but it'll also run go install which won't work since we have multiple binaries to build. So users still have to use the make script or run build.go themselves.

That's too bad. Do you know what specific go install command it runs? When you say it "won't work" do you mean it does nothing or does it encounter an error?

Also, on a different note: since we've updated mongo-tools to modules, do we need to strictly clone the repo inside the $GOPATH?

Specifically for building the tools — we haven't made the tools "go gettable" yet, but judging from the docs, I believe go get github.com/mongodb/mongo-tools will download the repo's contents to $GOPATH/src and then run a plain go install from the repo's directory:

With no package arguments and also without -u, 'go get' is not much more than 'go install'

I tried doing a plain go install / go get from the mongo-tools root directory and found that both basically make the build file into an executable named mongo-tools, and put it in $GOPATH/bin like so:

$ $GOPATH/bin/mongo-tools USAGE: [tasks ...] [options ...] TASKS: build(tools) build the tools --------------------------------------------------------------------- test -> [test:unit test:integration test:kerberos] test:integration(tools, ssl, auth, kerberos, topology) runs integration tests ... $ $GOPATH/bin/mongo-tools build START | build | rm: bin/bsondump | exec: '/usr/local/bin/go run release/release.go get-version' ...

To summarize our options, I think these will all be equivalent ways to build the tools:

git clone github.com/mongodb/mongo-tools + ./make build from the cloned directory

go get github.com/mongodb/mongo-tools + $GOPATH/bin/mongo-tools build from the $GOPATH/src/github.com/mongodb/mongo-tools directory

go get -d github.com/mongodb/mongo-tools + ./make build from the $GOPATH/src/github.com/mongodb/mongo-tools directory

So go get will technically work, but it seems kind of impractical to me to use it as a cloning mechanism for building the tools. It might be better suited to use in projects that need mongo-tools as a dependency.

If this all sounds right to you, do you think it's worth mentioning in the README?

And to your second q — you're right that we won't need to clone inside the $GOPATH anymore. One of the many upsides of Go modules :)

I'll remove that phrase from the README.

If this all sounds right to you, do you think it's worth mentioning in the README?

Nah, not worth it! Thanks for the detailed explanation.

And to your second q — you're right that we won't need to clone inside the $GOPATH anymore. One of the many upsides of Go modules :)

I'll remove that phrase from the README.

Great

mattChiaravalloti · 2021-02-08T16:44:47Z

README.md

-$GOPATH.
+We currently build the tools with Go version 1.15. Other Go versions may work but they are untested.
+
+Using `go get` will not work. To build the tools, you need to first clone this repository into your Go workspace inside your $GOPATH.


That's too bad. Do you know what specific go install command it runs? When you say it "won't work" do you mean it does nothing or does it encounter an error?

Also, on a different note: since we've updated mongo-tools to modules, do we need to strictly clone the repo inside the $GOPATH?

mattChiaravalloti · 2021-02-08T16:46:55Z

README.md

+
+Then run `go mod vendor -v` to reconstruct the `vendor` directory to match the changed `go.mod` file.
+
+Optionally, run `go mod tidy -v` to ensure that the `go.mod` file matches the vendored code.


Is that really what tidy does? I thought go mod tidy removed unused dependencies. Saying it ensures the go.mod file matches the vendored code seems misleading.

Ah, I pulled the phrasing from the Go docs entry for go mod (first sentence) but I misinterpreted it. Thanks for catching this.

It does remove unused dependencies, but it could also add new ones. Would phrasing it like this seem alright? Feel free to suggest something else too.

Optionally, run go mod tidy -v to ensure that the go.mod file matches the mongo-tools source code.

Yes that is better phrasing.

rychipman

what did you end up deciding re: how you wanted to handle the vendoring for this PR?

rychipman · 2021-02-09T02:23:16Z

go.mod

@@ -0,0 +1,29 @@
+module github.com/mongodb/mongo-tools
+
+go 1.14


The README.md says go 1.15, but this says go 1.14. Which one is correct?

It's go 1.15, thanks for catching that. I thought I was using that version locally but I have go 1.14, which is why the go.mod file defaulted to that.

rychipman · 2021-02-09T02:24:57Z

go.mod

+	github.com/youmark/pkcs8 v0.0.0-20181117223130-1be2e3e5546d // indirect
+	go.mongodb.org/mongo-driver v1.4.2
+	golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527 // indirect
+	gopkg.in/mgo.v2 v2.0.0-00010101000000-000000000000


it looks like it might be an accident that we're still depending on mgo. can you file a ticket to get rid of it?

It looks like there was only one mgo import in mongostat/mongostat_test.go. Switching that to the mongo-driver and running go mod tidy removes the entry for mgo.

I think we can just include the change here if that sounds ok to you.

edobranov · 2021-02-09T19:22:19Z

linter.go

+
+package main
+
+import _ "github.com/3rf/mongo-lint/golint"


I think this file is necessary to retain a vendored third-party tool that we use in Evergreen. Without this file, go mod tidy will remove it.

Some more explanation here on why it seems to be the best way to do it.

Seems reasonable since it's following the golang convention.

edobranov · 2021-02-09T19:25:16Z

common.yml

@@ -815,6 +815,8 @@ functions:
          set -v
          set -e
          mkdir -p bin
+          ${_set_shell_env}
+          go mod vendor -v


what did you end up deciding re: how you wanted to handle the vendoring for this PR?

@rychipman, I took @huan-Mongo's suggestion to temporarily revendor on Evergreen to ensure that static checks and compilation pass.

I'll remove these two lines before merging and subsequently push the vendored changes directly to master.

edobranov

I made some iterative changes, though there's one issue we can't really fix at the moment.

Since go mod prunes all files that aren't directly relevant to the mongo-tools Go code, it'll also prune any PEM files in mtc. This makes it impossible for mtc's testutil package to function properly, so most (all?) of the mongo-tools tests will fail for the moment.

We can fix this by making an adjusted branch in mtc, or by making mtc a subpackage of tools. @rychipman suggested directly doing the latter and letting some of the tools tests fail temporarily.

mattChiaravalloti

The core changes here look good.

Letting some tests fail for now sounds good to me since you talked it out with Ryan 👍

rychipman

LGTM

TOOLS-2801: Migrate from dep to Go modules and update README

e2152ad

edobranov requested review from rychipman and mattChiaravalloti February 5, 2021 20:34

edobranov commented Feb 5, 2021

View reviewed changes

mattChiaravalloti reviewed Feb 8, 2021

View reviewed changes

TOOLS-2801: Temporarily revendor on evergreen

a060b30

rychipman requested changes Feb 9, 2021

View reviewed changes

edobranov added 5 commits February 9, 2021 10:48

TOOLS-2801: Add build constraint & blank identifier for lint tool

cbac0cb

TOOLS-2801: Remove mention of mgo

62417eb

TOOLS-2801: Update go.mod to go 1.15

1dbe43e

TOOLS-2801: Move mtc pem files to testdata dir

6533d13

TOOLS-2801: Update README per Matt's comments

29d6b78

edobranov commented Feb 9, 2021

View reviewed changes

mattChiaravalloti approved these changes Feb 9, 2021

View reviewed changes

rychipman approved these changes Feb 9, 2021

View reviewed changes

edobranov merged commit 2ed4bb1 into mongodb:master Feb 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TOOLS-2801: Migrate from dep to Go modules and update README #344

TOOLS-2801: Migrate from dep to Go modules and update README #344

edobranov commented Feb 5, 2021

edobranov Feb 5, 2021

mattChiaravalloti Feb 8, 2021

edobranov Feb 8, 2021

edobranov Feb 8, 2021

mattChiaravalloti Feb 9, 2021

mattChiaravalloti Feb 8, 2021

mattChiaravalloti Feb 8, 2021

edobranov Feb 8, 2021

mattChiaravalloti Feb 9, 2021

rychipman left a comment

rychipman Feb 9, 2021

edobranov Feb 9, 2021

rychipman Feb 9, 2021

edobranov Feb 9, 2021 •

edited

edobranov Feb 9, 2021

mattChiaravalloti Feb 9, 2021

edobranov Feb 9, 2021

edobranov left a comment •

edited

mattChiaravalloti left a comment

rychipman left a comment


		Then run `go mod vendor -v` to reconstruct the `vendor` directory to match the changed `go.mod` file.

		Optionally, run `go mod tidy -v` to ensure that the `go.mod` file matches the vendored code.

		@@ -0,0 +1,29 @@
		module github.com/mongodb/mongo-tools

		go 1.14

TOOLS-2801: Migrate from dep to Go modules and update README #344

TOOLS-2801: Migrate from dep to Go modules and update README #344

Conversation

edobranov commented Feb 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rychipman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edobranov Feb 9, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edobranov left a comment • edited

Choose a reason for hiding this comment

mattChiaravalloti left a comment

Choose a reason for hiding this comment

rychipman left a comment

Choose a reason for hiding this comment

edobranov Feb 9, 2021 •

edited

edobranov left a comment •

edited