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

Update protobuf with relative repo names patch #29

Merged
merged 2 commits into from
Nov 4, 2021
Merged

Update protobuf with relative repo names patch #29

merged 2 commits into from
Nov 4, 2021

Conversation

Wyverald
Copy link
Member

@Wyverald Wyverald commented Nov 4, 2021

Essentially a "cherrypick" of protocolbuffers/protobuf#9187

@keith
Copy link
Member

keith commented Nov 4, 2021

Won't changing the name here break all existing users of proto? If they want to use bazel mod

@Wyverald
Copy link
Member Author

Wyverald commented Nov 4, 2021

No, as long as they specify the repo_name attribute:

bazel_dep(name="protobuf", version="3.19.0", repo_name="com_google_protobuf")

They wouldn't need to update their references.

@keith
Copy link
Member

keith commented Nov 4, 2021

So is the plan as repos add themselves to this to change their names to not use the previous conventions?

@alexeagle
Copy link
Contributor

This seems scary in general. There isn't an agreement on what to name repositories in general (three core rulesets disagree on io_bazel_stardoc for example). bzlmod picking new names just adds a footgun for users. I don't think this is the layer where you can do vanity renames.

@Wyverald
Copy link
Member Author

Wyverald commented Nov 4, 2021

So is the plan as repos add themselves to this to change their names to not use the previous conventions?

By "add themselves to this", do you mean projects submitting themselves to the central registry? They yes, I think it's the most sensible to use something immediately recognizable, instead of the reverse domain convention.

There isn't an agreement on what to name repositories in general (three core rulesets disagree on io_bazel_stardoc for example).

This is exactly what the central registry aims to resolve. The modules in the registry have a clear name; the module "foo" will become the repo @foo in your workspace by default.

bzlmod picking new names just adds a footgun for users.

Could you clarify your concern? Why is this a footgun? I would expect, as a user, that adding a dependency on a module called "foo" should give me @foo, and the fact that I can rename it to @bar in my repo by specifying repo_name="bar" seems straightforward enough.

@Wyverald Wyverald merged commit 92ef914 into main Nov 4, 2021
@Wyverald Wyverald deleted the protobuf branch November 4, 2021 16:50
@alexeagle
Copy link
Contributor

alexeagle commented Nov 5, 2021

earlier you said

as long as they specify the repo_name attribute

and I don't think users are going to know they have to do that. You're giving them an easy way to make the repo be named "@protobuf" which is breaking but not an easy way to make it be named "@com_google_protobuf" which will be required for interop with all other rulesets.

I'm fairly confident this will be a major obstacle to bzlmod adoption.

@meteorcloudy
Copy link
Member

@alexeagle Let me clarify a bit how repo_name works.
repo_name is essentially the local repo name for a dependency that the module can see.

For example, if A depends on B and C, B depends on C. In A's MODULE.bazel file, you can define:

bazel_dep(name = "B", version = "1.0")
bazel_dep(name = "C", version = "1.0", repo_name = "com_foo_bar_c")

Then A can refer to targets of C via repo name "@com_foo_bar_c".
But if in B's MODULE.bazel file, no repo_name is specified for C:

bazel_dep(name = "C", version = "1.0")

then B still sees targets of module C via "@C".

So, basically different modules can depend on the same module with different repo names. We implemented this with the repo mapping feature. Every module has a canonical repo name (<module_name>.), that's used to create the <output_base>/external/<canonical repo name> directory. And each module has its own repo mappings, which is a map of its dependencies' local repo names to the canonical repo names.

Here's a little example: https://github.com/meteorcloudy/bzlmod-examples/blob/main/examples/09-repo-name/MODULE.bazel

@Wyverald
Copy link
Member Author

Wyverald commented Nov 8, 2021

What Yun said; to be more explicit, this part

make it be named "@com_google_protobuf" which will be required for interop with all other rulesets.

is not true. You can use any repo_name for your protobuf bazel_dep and it would not affect any other ruleset.

@alexeagle
Copy link
Contributor

thanks @meteorcloudy and @Wyverald I think I understand, any ruleset that depends on com_google_protobuf today will have to include an explicit repo_name to preserve that naming, but end users won't.

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.

4 participants