Skip to content

Align dependency handling with Bazel best practices#9165

Merged
acozzette merged 1 commit intoprotocolbuffers:3.19.xfrom
acozzette:bazel-fix
Oct 28, 2021
Merged

Align dependency handling with Bazel best practices#9165
acozzette merged 1 commit intoprotocolbuffers:3.19.xfrom
acozzette:bazel-fix

Conversation

@acozzette
Copy link
Copy Markdown

@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
Copy Markdown
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.

3 participants