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

Vendorize dependencies using govend #11

Closed
wants to merge 3 commits into from
Closed

Vendorize dependencies using govend #11

wants to merge 3 commits into from

Conversation

cce
Copy link
Contributor

@cce cce commented Jan 16, 2018

Use vendored dependencies for protobuf, grpc, pkg/errors, and bson by running:

govend -v -l github.com/golang/protobuf/proto github.com/pkg/errors google.golang.org/grpc google.golang.org/grpc/credentials gopkg.in/mgo.v2/bson

@cce
Copy link
Contributor Author

cce commented Jan 16, 2018

Manually verified using a fresh, empty Go workspace that this PR successfully removes library dependency requirements from the sample app:

⇢ ~/freshgo/src/github.com/appoptics/appoptics-apm-go/examples/sample_app$ git co master
Already on 'master'
Your branch is up-to-date with 'origin/master'.

~/freshgo/src/github.com/appoptics/appoptics-apm-go/examples/sample_app$ go build
../../v1/ao/internal/reporter/collector/collector.pb.go:21:8: cannot find package "github.com/golang/protobuf/proto" in any of:
	/usr/local/go/src/github.com/golang/protobuf/proto (from $GOROOT)
	/Users/cce/freshgo/src/github.com/golang/protobuf/proto (from $GOPATH)
../../v1/ao/internal/hdrhist/decode.go:10:2: cannot find package "github.com/pkg/errors" in any of:
	/usr/local/go/src/github.com/pkg/errors (from $GOROOT)
	/Users/cce/freshgo/src/github.com/pkg/errors (from $GOPATH)
../../v1/ao/internal/reporter/collector/collector.pb.go:26:2: cannot find package "golang.org/x/net/context" in any of:
	/usr/local/go/src/golang.org/x/net/context (from $GOROOT)
	/Users/cce/freshgo/src/golang.org/x/net/context (from $GOPATH)
../../v1/ao/internal/reporter/collector/collector.pb.go:27:2: cannot find package "google.golang.org/grpc" in any of:
	/usr/local/go/src/google.golang.org/grpc (from $GOROOT)
	/Users/cce/freshgo/src/google.golang.org/grpc (from $GOPATH)
../../v1/ao/internal/reporter/reporter_grpc.go:21:2: cannot find package "google.golang.org/grpc/credentials" in any of:
	/usr/local/go/src/google.golang.org/grpc/credentials (from $GOROOT)
	/Users/cce/freshgo/src/google.golang.org/grpc/credentials (from $GOPATH)
../../v1/ao/internal/reporter/utils.go:18:2: cannot find package "gopkg.in/mgo.v2/bson" in any of:
	/usr/local/go/src/gopkg.in/mgo.v2/bson (from $GOROOT)
	/Users/cce/freshgo/src/gopkg.in/mgo.v2/bson (from $GOPATH)

~/freshgo/src/github.com/appoptics/appoptics-apm-go/examples/sample_app$ git co vendorize
Switched to branch 'vendorize'
Your branch is up-to-date with 'origin/vendorize'.

~/freshgo/src/github.com/appoptics/appoptics-apm-go/examples/sample_app$ go build
~/freshgo/src/github.com/appoptics/appoptics-apm-go/examples/sample_app$ ls
Dockerfile  main.go  sample_app*
~/freshgo/src/github.com/appoptics/appoptics-apm-go/examples/sample_app$

@cce cce requested a review from dschnabel January 16, 2018 02:47
Copy link
Contributor

@dschnabel dschnabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must admit I didn't look through all 1,044,092 lines ;) but the structure looks good. This means though we have pinned to certain vendor versions. Will we be updating the vendor code regularly and how would we do that?

@tillberg
Copy link
Contributor

@cce I hit grpc/grpc-go#566 when I tried this branch, and I think anyone else that already imports their own copy of google.golang.org/grpc also will. The underlying issue is actually in golang.org/x/net/trace, which is incompatible with being vendored and imported twice, though I can't find an issue already filed for it in the main golang issue tracker.

The workaround isn't too tricky, though it's a little messy: users that hit that issue could just delete this vendored copy of grpc and use their own.

@cce
Copy link
Contributor Author

cce commented Jan 22, 2018

That's too bad — I was hoping to avoid issues with customers that have grpc.SupportPackageIsVersion3 set in their own gRPC and we are using grpc.SupportPackageIsVersion4. What if we just removed golang.org/x/net/trace from our vendor?

@codecov-io
Copy link

codecov-io commented Jan 23, 2018

Codecov Report

Merging #11 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #11   +/-   ##
=======================================
  Coverage   76.01%   76.01%           
=======================================
  Files          21       21           
  Lines        2289     2289           
=======================================
  Hits         1740     1740           
  Misses        499      499           
  Partials       50       50

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcc9135...1343682. Read the comment docs.

@cce
Copy link
Contributor Author

cce commented Jul 16, 2018

I no longer think this is a good idea — closing.

@cce cce closed this Jul 16, 2018
@jiwen624 jiwen624 deleted the vendorize branch September 12, 2019 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants