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

Rename com_google_protobuf to protobuf #28

Merged
merged 1 commit into from
Oct 29, 2021

Conversation

meteorcloudy
Copy link
Member

No description provided.

@Wyverald
Copy link
Member

Do we need to cherrypick your CL adding "protobuf" as a well-known module name into 5.0? Would people run into issues if they try to build Bazel with --experimental_enable_bzlmod?

@meteorcloudy
Copy link
Member Author

Good point, we should probably cherry-pick that.

@meteorcloudy
Copy link
Member Author

How does the cherry-pick work nowadays? Should I send a PR?

@Wyverald
Copy link
Member

Yeah, just send a PR to the release-5.0.0rc1 branch.

@meteorcloudy
Copy link
Member Author

Done at bazelbuild/bazel#14195

bazel-io pushed a commit to bazelbuild/bazel that referenced this pull request Oct 29, 2021
Context: bazelbuild/bazel-central-registry#28

RELNOTES: None
PiperOrigin-RevId: 406334617
@meteorcloudy
Copy link
Member Author

meteorcloudy commented Oct 29, 2021

Oh no, I'm getting the following error while building Bazel:

ERROR: /usr/local/google/home/pcloudy/.cache/bazel/_bazel_pcloudy/4c5ccabacef1ddac6600d71a1b8bfa58/external/com_google_protobuf/BUILD:812:36: every rule of type internal_gen_well_known_protos_java implicitly depends upon the target '@com_google_protobuf//:protoc', but this target could not be found because of: no such package '@com_google_protobuf//': The repository '@com_google_protobuf' could not be resolved: Repository '@com_google_protobuf' is not visible from repository '@com_google_protobuf'

This happens when the module name is different from the commonly used workspace name in the WORKSPACE file, because the repo itself will no longer be able to access targets of itself under the previous repo name...

Maybe we need to add an attribute repo_name to the module directive as well?

meteorcloudy added a commit to meteorcloudy/bazel that referenced this pull request Nov 4, 2021
Modules can specify the workspace_name attribute to allow accessing its
own targets under the repo name.

This helps make migration easier when the preferred workspace name is
different from the module name of the project.

Context: bazelbuild/bazel-central-registry#28 (comment)
Wyverald added a commit to Wyverald/protobuf that referenced this pull request Nov 4, 2021
The label `@com_google_protobuf//:foo` within the protobuf repo is often synonymous with just `//:foo`. We should prefer the latter as it allows us to use a shorter name for the module in the Bazel Central Registry (so just "protobuf" instead of "com_google_protobuf").

Note that the semantics can be subtle: in a macro, plain strings are anchored to the *calling* repo, so if we just use `//:foo` as the default value of a macro argument, it will be resolved to `@myrepo//:foo` if the macro is called from the repo `@myrepo`. In this case, it's necessary to directly call the `Label()` constructor to anchor the string label to the repo where the .bzl file lives.

See bazelbuild/bazel-central-registry#28 (comment) for a bit more context.
acozzette pushed a commit to protocolbuffers/protobuf that referenced this pull request Nov 4, 2021
* Use repo-relative labels wherever possible

The label `@com_google_protobuf//:foo` within the protobuf repo is often synonymous with just `//:foo`. We should prefer the latter as it allows us to use a shorter name for the module in the Bazel Central Registry (so just "protobuf" instead of "com_google_protobuf").

Note that the semantics can be subtle: in a macro, plain strings are anchored to the *calling* repo, so if we just use `//:foo` as the default value of a macro argument, it will be resolved to `@myrepo//:foo` if the macro is called from the repo `@myrepo`. In this case, it's necessary to directly call the `Label()` constructor to anchor the string label to the repo where the .bzl file lives.

See bazelbuild/bazel-central-registry#28 (comment) for a bit more context.

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

Successfully merging this pull request may close these issues.

2 participants