Skip to content

remove inappropriate transitive dependencies by splitting conformance into a separate module #806

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

Closed
bcmills opened this issue Feb 28, 2019 · 5 comments

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 28, 2019

#804 reports an error with go get -u for this module.

The problem arises through a large transitive dependency graph, introduced via a dependency on google.golang.org/genproto (and eventually google.golang.org/grpc and its wide net of dependencies) in the conformance/internal/conformance_proto package:

~/src/github.com/golang/protobuf$ go mod why -m google.golang.org/genproto
# google.golang.org/genproto
github.com/golang/protobuf/conformance/internal/conformance_proto
google.golang.org/genproto/protobuf/field_mask

However, the conformance subtree doesn't even build using cmd/go: it has its own Makefile and .sh test driver. It should not be included in the main module, and then the inappropriate grpc dependency will be removed.

That will also help to address golang/lint#436.

CC @dmitshur @thepudds

@dsnet
Copy link
Member

dsnet commented Feb 28, 2019

We should just delete the conformance test. It hasn't been run in forever, and most of the new shiny stuff is in v2 anyways (along with a eventually better written conformance test).

@dsnet
Copy link
Member

dsnet commented Feb 28, 2019

Back-tracking a bit, it seems that the real issue is that field_mask is in genproto. It's entirely reasonable that this repo needs to depend on field_mask again in the future since it's a well-known type with special semantic meaning.

Thus, we should really move field_mask over to this repo.

@dsnet
Copy link
Member

dsnet commented Feb 28, 2019

Related issues with field_mask are #745 and #398 and #218.

@bcmills
Copy link
Contributor Author

bcmills commented Feb 28, 2019

That's fair, but I think for now let's stop the bleeding by cutting out the conformance module. Then we can do the rest of the cleanup as time permits.

@dsnet
Copy link
Member

dsnet commented Feb 28, 2019

Fixed by #808.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants