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

Remove vendored tools for CI. #1220

Closed
wants to merge 3 commits into from
Closed

Remove vendored tools for CI. #1220

wants to merge 3 commits into from

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Jun 30, 2018

In preparation to vendor the dependencies of Trillian itself with dep,
this change removes the existing contents of the vendor directory, and
updates the .travis.yml to instead check out specific versions of the
commands that were in the vendor directory (etcd, etcdcli, and
protoc-gen-grpc-gateway) , matching the previously checked-in versions.

This is because vendoring with dep, which is approximately
representative of the direction Go as a whole is moving in, is
incompatible with vendoring arbitrary commands.

A subsequent change will introduce vendoring of Trillian's non-command
dependencies.

jsha added 3 commits June 30, 2018 00:41
In preparation to vendor the dependencies of Trillian itself with `dep`,
this change removes the existing contents of the vendor directory, and
updates the .travis.yml to instead check out specific versions of the
commands that were in the vendor directory (etcd, etcdcli, and
protoc-gen-grpc-gateway) , matching the previously checked-in versions.

This is because vendoring with `dep`, which is approximately
representative of the direction Go as a whole is moving in, is
incompatible with vendoring arbitrary commands.
@daviddrysdale
Copy link
Contributor

I also did a bit of experimenting with dep, and hit on a similar problem to what's covered in this PR.

Is it basically a dep limitation that it's not possible to manage binaries (here, etcd and etcdctl) that are related to code dependencies? So if you want to have consistent libraries and command-line tools, you have to have two versions of the dependency codebase and manually keep them in sync?

I hoped that setting required to include the command-line tools in Gopkg.toml (as per the docs), along with the [[prune.project]] flag unused-packages=false for the containing repo, might do the trick, but it didn't seem to -- the command-line tools showed up in vendor/, but their dependencies did not. But maybe I was missing something?

@jsha
Copy link
Contributor Author

jsha commented Jul 2, 2018

Is it basically a dep limitation that it's not possible to manage binaries (here, etcd and etcdctl) that are related to code dependencies?

I believe that's the case, though I haven't been able to find dep documentation explicitly ruling it in or out. But here's the logic: dep relies heavily on imports to figure out what it should vendor. You can use dep ensure -add PACKAGE to add the package without having anything import it, but you'll get:

"github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway" is not imported by your project, and has been temporarily added to Gopkg.lock and vendor/.
If you run "dep ensure" again before actually importing it, it will disappear from Gopkg.lock and vendor/.

If you try to trick dep by adding an unused import of the command, you'll get:

tooldeps.go:4:2: import "github.com/jsha/tooldeps/vendor/github.com/coreos/etcd" is a program, not an importable package

Personally, I think the approach of checking out a specific commit for etcd, etcdcli, and protoc-gen-grpc-gateway is likely to get us most of what we need, compatibility-wise. A more thorough solution would be to also request that each of those repositories vendor their own dependencies, so checking out a specific version would consistently get the same version of their dependencies.

It's also worth noting that the Go team is actively working on defining how dependencies should work, and seeking feedback. So if you think managing binaries alongside vendored dependencies is a valuable use caes, now would be a great time to get in touch!

@jsha
Copy link
Contributor Author

jsha commented Jul 2, 2018

I've been working on another branch that builds on this to vendor all the other dependencies. I'm running into the issue described at etcd-io/etcd#8692, which suggests that there may be a conflict between etcd's use of glide for dependency management, and dep. I'll dig a bit further but this may turn out to be a significant stumbling block.

@daviddrysdale
Copy link
Contributor

For the record: I think I found a way of dealing with the command-line tools problem with dep, via a couple of Gopkg.toml options:

  • For command-line tools that are Go binaries, adding their package to the required statement should work: "Use this for linters, generators, and other development tools that are needed by your project,
    aren't imported ... [and] you want to lock the version".
  • For repos that include non-Go tools and/or needed files (e.g. .proto files), set prune.project for the relevant repo to have unused-packages = false.

@daviddrysdale
Copy link
Contributor

However.... we're currently wondering whether we should leapfrog dep for Go modules in 1.11.

@daviddrysdale daviddrysdale removed their assignment May 3, 2019
@RJPercival
Copy link
Contributor

Is this obsolete, now that Go modules are the recommended way forward?

@daviddrysdale
Copy link
Contributor

Removing ./vendor/ would definitely be part of a move to Go modules, but other things need to happen as part of that transition too. #1400 probably has most recent discussion in it.

@daviddrysdale daviddrysdale removed their request for review May 30, 2019 09:03
@jsha jsha closed this Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants