-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Migrate to Go Modules #24023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
65ff034
to
fd8c4b5
Compare
c5233b8
to
520400d
Compare
joshblum
reviewed
May 8, 2020
313f12a
to
7a29a54
Compare
|
27ffe5a
to
a3acae3
Compare
mmaxim
approved these changes
Nov 19, 2021
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 ago/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 thego/
folder of the repository for this to work:Progress
vendor.json
->go.mod
migrationgo.mod
vendor.json
replace
directivesgo get
error due to module rename (go get error with bbolt blevesearch/bleve#1360)vendor.json
, and list any discrepanciesgo/vendor/
or$GOPATH
in documentationCode
mvdan.cc/xurls/v2
->github.com/keybase/xurls
)Pre-commit
CI
$GOPATH
if possible to ensure that there aren't any dependencies on the$GOPATH
gomobile
version bump)$GOPATH
if possible to ensure that there aren't any dependencies on the$GOPATH
Mobile
gomobile
in order to support Go Modules (x/mobile: build failing when using go modules golang/go#27234)gobuild.sh
in order to support Go Modulesgomobile
and the android NDKgomobile
Protocol
avdl-compiler
protocol/Makefile
to use the new AVDL compiler Go Module flagTools
gomobile
mockgen
golint
golangci-lint
Tests
$GOPATH
andgo/vendor/
from testsPackaging
go/vendor/
from packaging scriptsWhat is changing to make this happen?
It might be useful to use the following command to view changes instead of using GitHub:
Here are the files that have changed in this PR (ignoring protocol rebuild & vendor being removed):
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
migrationgo/vendor/
directory has been entirely removed in favour of ago.mod
file in thego/
folder. It should be noted that build/run/test/vet can only be run somewhere inside thego/
directory, thego.mod
file acts much like apackage.json
file.vendor.json
to thego.mod
file, with a few issues described in the "Notes about dependencies" section of this PR description.Code
github.com/keybase/xurls
have been replaced with imports ofmvdan.cc/xurls/v2
, although they still ultimately referencegithub.com/keybase/xurls
due to areplace
directive in thego.mod
(look at the "Forked Dependencies" section of this PR description to learn more). This is required becausegithub.com/keybase/xurls
declares itself asmvdan.cc/xurls/v2
.Pre-commit
go-vet
pre-commit hook didn't play well with Go Modules, so it was modified to support Go Modules.go.mod
files).CI
$GOPATH
. This is to ensure that no tests or build processes depend on the client code being inside the$GOPATH
.go/buildtools/go.mod
.gomobile
.gen_deps.pl
have been modified to support Go Modules rather than depend on changes in the now defunctgo/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).gomobile
to its most recent version.gomobile
requires a higher version of the android NDK to be used (at leastr19c
). This is seemingly unavoidable as this requirement was added beforegomobile
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 ofgomobile
.gobind
andgomobile
has been specified ingo/buildtools/tools.go
, and thegomobile
module version has been pinned in thego/buildtools/go.mod
file.Protocol
$GOPATH
.Tools
go/buildtools/tools.go
file and pinned in thego/buildtools/go.mod
file.go/buildtools/
directory following the instructions found in these notes.Tests
$GOPATH
andvendor/go/
have been removed.keybase-test-vectors
being in thego/vendor/
directory. Now they usego list -json -m github.com/keybase/keybase-test-vectors
to get the required json files from the$GOPATH
.Packaging
goinstall.sh
in favour of pinning required build tools ingo/buildtools/go.mod
.vendor/go/
have been removed as it is no longer necessary with Go Modules.$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 asgithub.com/PuerkitoBio/[email protected]
. 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 asgithub.com/golang/[email protected]
.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 packagegithub.com/akavel/rsrc/coff@ba14da1
. However Go Modules requires there to be only one version per required module (in this case the required module isgithub.com/akavel/rsrc
), so I pinned the modulegithub.com/akavel/rsrc@ba14da1
sinceba14da1
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:
Indirect dependencies
Go Modules brings in a new concept of "indirect" dependencies. These are defined by
go help mod
: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 thevendor.json
. However, they are required to be listed in thego.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 thego.mod
, pinning them at their latest version (which I didn't touch). You can find out which dependency requires an indirect dependency using thego mod graph
andgo mod why
commands. Here is a list of these new indirect dependencies:"Removed" sub-dependencies
There are some sub-dependencies that were originally being pinned in
vendor.json
that haven't been added to thego.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 invendor.json
. The sub-dependency version is still pinned by one of the dependencies, it just isn't explicitly pinned in thego.mod
file. For example,vendor.json
originally pinnedgithub.com/BurntSushi/toml@a368813
, however one of the dependencies (github.com/spf13/[email protected]
) requires the newer versiongithub.com/BurntSushi/[email protected]
. Another reason that could have been removed from thego.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 invendor.json
but are no longer pinned explicitly ingo.mod
. You can view their relationships with other modules within client by using thego mod graph
command.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: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 forkgithub.com/keybase/fuse
. In the past, govendor has supported this using theorigin
directive. Go Modules also supports this with thereplace
directive. Instructions on how to add a new forked dependency can be found here.