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

Align dependency handling with Bazel best practices #9165

Merged
merged 1 commit into from Oct 28, 2021

Conversation

acozzette
Copy link
Member

@acozzette acozzette commented Oct 27, 2021

This commit removes the use of bind() since that function goes against
Bazel best practices:
https://docs.bazel.build/versions/main/external.html#repository-rules-1
The bind() function basically maps a dependency into //external:, but
there is no good reason to do this. By mapping dependencies into
//external: and relying on this in our own BUILD files, we're forcing
projects that depend on us to do the same. The one bind() call that I
did leave in place was //:python_headers. This one seems to be doing
something complicated I don't fully understand, and I don't want to risk
breaking it.

This change also moves our list of required Maven artifacts into
protobuf_deps.bzl. This way, projects that depend on us can refer to
this list when they invoke maven_install() and automatically pull in all
the necesary dependencies.

This fixes #9132.

@google-cla google-cla bot added the cla: yes label Oct 27, 2021
This commit removes the use of bind() since that function goes against
Bazel best practices:
https://docs.bazel.build/versions/main/external.html#repository-rules-1
The bind() function basically maps a dependency into //external, but
there is no good reason to do this. By mapping dependencies into
//external and relying on this in our own BUILD files, we're forcing
projects that depend on us to do the same. The one bind() call that I
did leave in place was //:python_headers. This one seems to be doing
something complicated I don't fully understand, and I don't want to risk
breaking it.

This change also moves our list of required Maven artifacts into a
constant in protobuf_deps.bzl. This way, projects that depend on us can
refer to this list when they invoke maven_install() and automatically
pull in all the necesary dependencies.

This fixes protocolbuffers#9132.
Copy link
Contributor

@perezd perezd left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants