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

Migrate to Go Modules #24023

Merged
merged 69 commits into from
Nov 22, 2021
Merged

Migrate to Go Modules #24023

merged 69 commits into from
Nov 22, 2021

Conversation

marceloneil
Copy link
Contributor

@marceloneil marceloneil commented Apr 30, 2020

What does this mean?

Go Modules is Go's new dependency management system, and has been built into the go command since Go 1.11. Transitioning to Go Modules is the logical next step for the Keybase client, and comes with a few improvements too. Also, the govendor project that was used by the Keybase client is now archived in favour of Go Modules: "Please don't use this tool anymore. Please use Go modules."

The main difference for Keybase developers is that the client no longer needs to be under your $GOPATH, and the process for adding, removing and upgrading dependencies is a bit different (how to do it is outlined in these notes). There is no longer a go/vendor/ directory.

You can build/run/test/vet as you normally would. Go will pull any missing dependencies required by the go.mod file to the $GOPATH/pkg/mod directory. These two commands are identical when you are in the root of the repository, although it should be noted you need to be somewhere within the go/ folder of the repository for this to work:

go install -tags production github.com/keybase/client/go/keybase
go install -tags production ./keybase
cd keybase; go install -tags production .

Progress

vendor.json -> go.mod migration

Code

  • Fix import of forked package (alias mvdan.cc/xurls/v2 -> github.com/keybase/xurls )

Pre-commit

CI

  • CircleCI
    • Run tests/build outside of the $GOPATH if possible to ensure that there aren't any dependencies on the $GOPATH
    • Use build tools that are pinned by Go Modules
    • Update Android NDK in CircleCI base image (needed for gomobile version bump)
      • Create PR
      • Merge PR
      • Push Image
  • Jenkins
    • Run tests/build outside of the $GOPATH if possible to ensure that there aren't any dependencies on the $GOPATH
    • Use build tools that are pinned by Go Modules

Mobile

Protocol

Tools

Tests

  • Remove dependencies on $GOPATH and go/vendor/ from tests
  • Pass all tests

Packaging

  • Remove dependencies on go/vendor/ from packaging scripts
  • Test packaging/releases

What is changing to make this happen?

It might be useful to use the following command to view changes instead of using GitHub:

git diff --no-renames master marcel/HOTPOT-2576-go-modules -- . ':!go/vendor' ':!go/protocol'

Here are the files that have changed in this PR (ignoring protocol rebuild & vendor being removed):

$ git diff --stat=80 --no-renames master marcel/HOTPOT-2576-go-modules -- . ':!go/vendor' ':!go/protocol'
 .circleci/config.yml                       |   6 +-
 .pre-commit-config.yaml                    |   7 +-
 Jenkinsfile                                |  75 +++--
 git-hooks/pre-push                         |   4 +-
 go/Makefile                                |   8 +-
 go/buildtools/go.mod                       |  19 ++
 go/buildtools/go.sum                       | 433 +++++++++++++++++++++++++
 go/buildtools/tools.go                     |  12 +
 go/chat/attachments/gallery.go             |   2 +-
 go/chat/unfurl/extractor.go                |   2 +-
 go/chat/utils/utils.go                     |   2 +-
 go/copyright.sh                            |   2 +-
 go/gen_deps.pl                             |  10 +-
 go/go.mod                                  | 153 +++++++++
 go/go.sum                                  | 497 +++++++++++++++++++++++++++++
 go/kbfs/Makefile                           |   1 -
 go/kbfs/libkbfs/libkbfs_mocks_test.go      |   6 +-
 go/notes/modules.md                        |  51 +++
 go/notes/vendor.md                         |  63 ----
 go/teams/loader_chain_test.go              |  15 +-
 go/test/run_tests.sh                       |   2 +-
 packaging/desktop/package_darwin.sh        |   2 +-
 packaging/github/kbfs.sh                   |   2 +-
 packaging/github/keybase.sh                |   2 +-
 packaging/goinstall.sh                     |  21 --
 packaging/licenses/README                  |   6 +-
 packaging/linux/build_and_push_packages.sh |   2 +-
 packaging/prerelease/build_app.sh          |   2 +-
 packaging/prerelease/report.sh             |   2 +-
 packaging/prerelease/s3_index.sh           |   2 +-
 protocol/Makefile                          |  28 +-
 protocol/package.json                      |   2 +-
 protocol/yarn.lock                         |   8 +-
 shared/package.json                        |   2 +-
 shared/react-native/gobuild.sh             |  42 +--
 35 files changed, 1301 insertions(+), 192 deletions(-)

I will update this list frequently while this draft PR is in progress.

I've tried to make minimal changes to the codebase and dependencies in order for this to work. There are a few sections of the codebase that rely on vendored dependencies and the project being in the $GOPATH as well as some quirks with Go Modules that I've had to deal with.

vendor.json -> go.mod migration

  • The go/vendor/ directory has been entirely removed in favour of a go.mod file in the go/ folder. It should be noted that build/run/test/vet can only be run somewhere inside the go/ directory, the go.mod file acts much like a package.json file.
  • All of the dependencies and their pinned versions have been migrated from vendor.json to the go.mod file, with a few issues described in the "Notes about dependencies" section of this PR description.
  • Some notes on how to use Go Modules have been added here.

Code

  • Imports of github.com/keybase/xurls have been replaced with imports of mvdan.cc/xurls/v2, although they still ultimately reference github.com/keybase/xurls due to a replace directive in the go.mod (look at the "Forked Dependencies" section of this PR description to learn more). This is required because github.com/keybase/xurls declares itself as mvdan.cc/xurls/v2.

Pre-commit

  • The go-vet pre-commit hook didn't play well with Go Modules, so it was modified to support Go Modules.
  • The pre-commit config has also been modified to vet tuxbot and the client go files separately (as they have different go.mod files).

CI

  • CircleCI and Jenkins no longer checkout the client code into the $GOPATH. This is to ensure that no tests or build processes depend on the client code being inside the $GOPATH.
  • CircleCi and Jenkins both install build and test tools using the versions found in go/buildtools/go.mod.
  • The CircleCI docker image has been updated with a newer version of the android NDK required by gomobile.
  • Jenkins and gen_deps.pl have been modified to support Go Modules rather than depend on changes in the now defunct go/vendor/ directory. It should selectively run tests on a package when it depends on a client package that has been modified, or when it depends on a go module that has been added/upgraded/downgraded.

Mobile

  • gomobile has been updated to support Go Modules (x/mobile: build failing when using go modules golang/go#27234).
    • I've updated gomobile to its most recent version.
    • The new version of gomobile requires a higher version of the android NDK to be used (at least r19c). This is seemingly unavoidable as this requirement was added before gomobile began supporting Go Modules. However, the change in NDK version is not extremely large (only about a year), and only drops support for the ancient Android Ice Cream Sandwich (which I doubt Keybase worked on before).
  • gobuild.sh has been modified slightly for Go Modules and the new version of gomobile.
  • The build dependency on gobind and gomobile has been specified in go/buildtools/tools.go, and the gomobile module version has been pinned in the go/buildtools/go.mod file.

Protocol

  • The AVDL compiler has been updated to support Go Modules, and the protocol Makefile has been updated to make use of this new functionality. The protocol can be built in any directory and no longer depends on the $GOPATH.

Tools

  • The build tools required are specified in the go/buildtools/tools.go file and pinned in the go/buildtools/go.mod file.
  • Build tools can be added or updated using Go Modules in the go/buildtools/ directory following the instructions found in these notes.

Tests

  • References to $GOPATH and vendor/go/ have been removed.
    • Some of the teamchain loader tests originally depended on keybase-test-vectors being in the go/vendor/ directory. Now they use go list -json -m github.com/keybase/keybase-test-vectors to get the required json files from the $GOPATH.

Packaging

  • Removed goinstall.sh in favour of pinning required build tools in go/buildtools/go.mod.
  • References to vendor/go/ have been removed as it is no longer necessary with Go Modules.
  • References to the $GOPATH have not been removed from the packaging scripts. Go Modules still works even when the project is within the $GOPATH, these packaging scripts can be modified if necessary in the future.

Notes about dependencies

Semver

Go Modules supports Semantic Versioning which is great, since govendor didn't support it. Whenever possible, dependencies have been pinned in go.mod by a tag rather than by a commit hash. For example, github.com/PuerkitoBio/goquery@7f42915 is now pinned as github.com/PuerkitoBio/goquery@v1.0.0. Dependencies that were vendored by a hash that didn't correspond with a tag still use a semver. For example, github.com/golang/mock@e698a2e is now pinned as github.com/golang/mock@v1.2.1-0.20181221212429-e698a2ea17fe.

Version consolidation

Go Modules only lets you have one version per required module, so each package in the module must share the same version. However, govendor did not have this restriction, you could have different versions of packages from the same module. In the cases where the client vendored inconsistent versions of packages within a module (which Go Modules doesn't support), I opted to use the most recent version of the required module in the go.mod.

For example, the client vendored the package github.com/akavel/rsrc/binutil@59b2acb and the package github.com/akavel/rsrc/coff@ba14da1. However Go Modules requires there to be only one version per required module (in this case the required module is github.com/akavel/rsrc), so I pinned the modulegithub.com/akavel/rsrc@ba14da1 since ba14da1 was the most recent version of that module.

Here is a list of all of the modules that had this issue and their revised version:

github.com/akavel/rsrc v0.2.1-0.20151103204339-ba14da1f8271
github.com/keybase/go-billy v3.1.1-0.20180828145748-b5a7b7bc2074+incompatibl
github.com/keybase/go-framed-msgpack-rpc v0.0.0-20200311211234-26e5d1ace9c8
github.com/keybase/go-keychain v0.0.0-20200325143049-65d7292bc904
github.com/keybase/go-ps v0.0.0-20190827175125-91aafc93ba19
github.com/keybase/keybase-test-vectors v1.0.12-0.20200309162119-ea1e58fecd5d
github.com/keybase/saltpack v0.0.0-20200228190633-d75baa96bffb
github.com/keybase/stellar-org v0.0.0-20191010205648-0fc3bfe3dfa7
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
golang.org/x/sys v0.0.0-20190909082730-f460065e899a

Indirect dependencies

Go Modules brings in a new concept of "indirect" dependencies. These are defined by go help mod:

As part of maintaining the require statements in go.mod, the go command tracks which ones provide packages imported directly by the current module and which ones provide packages only used indirectly by other module dependencies. Requirements needed only for indirect uses are marked with a "// indirect" comment in the go.mod file. Indirect requirements are automatically removed from the go.mod file once they are implied by other direct requirements. Indirect requirements only arise when using modules that fail to state some of their own dependencies or when explicitly upgrading a module's dependencies ahead of its own stated requirements.

TLDR indirect dependencies are sub-dependencies not pinned by the requiring dependency

There are quite a few indirect dependencies listed, and that is because many of the dependency versions required by the client are quite old, and before the dependencies adopted Go Modules themselves. There are quite a few sub-dependencies listed as indirect, and we are responsible for pinning their versions. Hopefully as we update more of our dependencies, these indirect dependencies will decrease in number and will be pinned by the dependencies themselves instead of by us.

New indirect dependencies

Some of the indirect dependencies that are in the go.mod weren't originally listed in the vendor.json. However, they are required to be listed in the go.mod now. My guess is because they are dependencies of a module that we use, but weren't dependencies of the exact packages that were vendored (this seems to be the case for a lot of them). Either way, Go Modules adds them to the go.mod, pinning them at their latest version (which I didn't touch). You can find out which dependency requires an indirect dependency using the go mod graph and go mod why commands. Here is a list of these new indirect dependencies:

github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7 // indirect
github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 // indirect
github.com/blevesearch/blevex v0.0.0-20190916190636-152f0fe5c040 // indirect
github.com/cznic/b v0.0.0-20181122101859-a26611c4d92d // indirect
github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548 // indirect
github.com/cznic/strutil v0.0.0-20181122101858-275e90344537 // indirect
github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 // indirect
github.com/gliderlabs/ssh v0.3.0 // indirect
github.com/google/go-cmp v0.4.0 // indirect
github.com/jmhodges/levigo v1.0.0 // indirect
github.com/keybase/vcr v0.0.0-20191017153547-a32d93056205 // indirect
github.com/reiver/go-oi v1.0.0 // indirect
github.com/reiver/go-telnet v0.0.0-20180421082511-9ff0b2ab096e // indirect
github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0 // indirect
github.com/simplereach/timeutils v1.2.0 // indirect
github.com/strib/gomounts v0.0.0-20180215003523-d9ea4eaa52ca // indirect
github.com/tecbot/gorocksdb v0.0.0-20191217155057-f0fad39f321c // indirect
gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 // indirect

"Removed" sub-dependencies

There are some sub-dependencies that were originally being pinned in vendor.json that haven't been added to the go.mod. This is usually because one or more of the dependencies that required this sub-dependency uses Go Modules and requires a higher version or equal than the one that was originally pinned in vendor.json. The sub-dependency version is still pinned by one of the dependencies, it just isn't explicitly pinned in the go.mod file. For example, vendor.json originally pinned github.com/BurntSushi/toml@a368813, however one of the dependencies (github.com/spf13/cobra@v0.0.5) requires the newer version github.com/BurntSushi/toml@v0.3.1. Another reason that could have been removed from the go.mod is that only one dependency required said sub-dependency, and pinned it itself using Go Modules. Here is a list of modules that were originally pinned in vendor.json but are no longer pinned explicitly in go.mod. You can view their relationships with other modules within client by using the go mod graph command.

github.com/BurntSushi/toml
github.com/asaskevich/govalidator
github.com/glycerine/go-unsnap-stream
github.com/golang/protobuf
github.com/golang/snappy
github.com/lib/pq
github.com/manucorporat/sse
github.com/mitchellh/go-homedir
github.com/mschoch/smat
github.com/philhofer/fwd
github.com/pmezard/go-difflib
github.com/stellar/go-xdr
github.com/tinylib/msgp
google.golang.org/appengine

Upgraded dependencies

Some dependencies have been bumped because they are pinned as higher by another dependency. Again, you can investigate these relationships using go mod graph. Here is a list of dependencies that have been bumped:

github.com/davecgh/go-spew v1.1.1
github.com/hashicorp/golang-lru v0.5.0
github.com/mattn/go-isatty v0.0.9
github.com/pkg/errors v0.9.1
github.com/stretchr/testify v1.5.1
golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59
golang.org/x/image v0.0.0-20190802002840-cff245a6509b
golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297
golang.org/x/text v0.3.2

Forked dependencies

Some dependencies in the client are imported by their original name, but are expected to be forked. For example, the client requires bazil.org/fuse but should actually be using the keybase maintained fork github.com/keybase/fuse. In the past, govendor has supported this using the origin directive. Go Modules also supports this with the replace directive. Instructions on how to add a new forked dependency can be found here.

@marceloneil marceloneil force-pushed the marcel/HOTPOT-2576-go-modules branch 3 times, most recently from 65ff034 to fd8c4b5 Compare May 4, 2020 19:59
@marceloneil marceloneil force-pushed the marcel/HOTPOT-2576-go-modules branch 12 times, most recently from c5233b8 to 520400d Compare May 8, 2020 01:01
.circleci/config.yml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@joshblum joshblum requested a review from jzila May 8, 2020 21:22
@marceloneil marceloneil force-pushed the marcel/HOTPOT-2576-go-modules branch 5 times, most recently from 313f12a to 7a29a54 Compare May 9, 2020 00:58
@marceloneil marceloneil marked this pull request as ready for review May 9, 2020 02:31
@marceloneil marceloneil requested a review from mmaxim May 9, 2020 02:32
@chrisnojima
Copy link
Contributor

  • after release

@marceloneil marceloneil force-pushed the marcel/HOTPOT-2576-go-modules branch 2 times, most recently from 27ffe5a to a3acae3 Compare May 11, 2020 21:26
@joshblum joshblum merged commit a66861b into master Nov 22, 2021
@joshblum joshblum deleted the marcel/HOTPOT-2576-go-modules branch November 22, 2021 17:07
@joshblum joshblum mentioned this pull request Dec 7, 2021
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.

None yet

5 participants