Bzlmod: allow repos to be visible to other module extensions

[tyler-french]

Description of the feature request:

In certain situations, there exists a need to have a module extension be able to have visibility of repositories that are created by other module extensions.

Let's take go_sdk module extension for example in rules_go.

I have some repo declared locally in the main module, say for example @go_sdk_linux_amd64 is the name of the repo.

Now, I want to have the go_sdk module extension load this with something that is similar to the WORKSPACE repository rule go_wrap_sdk: (https://github.com/bazelbuild/rules_go/blob/master/go/private/sdk.bzl#L372-L391)

The issue here, is that if I make a tag (for example go_sdk.wrap(name = "@go_sdk_linux_amd64"), the build will fail because the module extension go_deps doesn't see the repo @go_sdk_linux_amd64

ERROR: An error occurred during the fetch of repository 'rules_go~override~go_sdk~go_sdk':
   Traceback (most recent call last):
        File "/home/user/.cache/bazel/_bazel_tfrench/b97476d719d716accead0f2d5b93104f/external/rules_go~override/go/private/sdk.bzl", line 366, column 26, in _go_wrap_sdk_impl
                goroot = str(ctx.path(root_file).dirname)
Error in path: Unable to load package for @go_sdk_linux_amd64//:README.md: The repository '@go_sdk_linux_amd64' could not be resolved: Repository '@go_sdk_linux_amd64' is not defined
ERROR: <builtin>: fetching go_wrap_sdk_rule rule //:rules_go~override~go_sdk~go_sdk: Traceback (most recent call last):
        File "/home/user/.cache/bazel/_bazel_tfrench/b97476d719d716accead0f2d5b93104f/external/rules_go~override/go/private/sdk.bzl", line 366, column 26, in _go_wrap_sdk_impl
                goroot = str(ctx.path(root_file).dirname)
Error in path: Unable to load package for @go_sdk_linux_amd64//:README.md: The repository '@go_sdk_linux_amd64' could not be resolved: Repository '@go_sdk_linux_amd64' is not defined
...
FAILED: Build did NOT complete successfully (77 packages loaded, 786 targets configured)

Which category does this issue belong to?

No response

What underlying problem are you trying to solve with this feature?

The ability to use a repo that is declared in the main module in a call to a module extension that is externally declared.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

release 6.3.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

N/A non-public code

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

[shahms]

FWIW, I encountered a similar situation when attempting to modularize Kythe. We depend on https://github.com/google/brotli from both C++ and Go. The Go module also depends on the C++ library, but that library can't be handled directly by Go. In order to accomplish this, I had to patch the BUILD file to use the canonical repo path to the non-modular C++ repository. Were that repository to be a modular dependency, this gets really fiddly and flaky.

Similarly, any kind of shared dependency that needs to bridge external package managers becomes challenging without someway of making that dependency known and available to the language-specific module extension.

[fmeum]

While this doesn't help in the general case, in the Go case you can just add the Go module with patches to the BCR as a Bazel module and go_deps will pick it up as a Go dependency automatically.

But I fully agree we need something more flexible and generally applicable to add the repo mapping entries for patches.

[Wyverald]

The issue here, is that if I make a tag (for example go_sdk.wrap(name = "@go_sdk_linux_amd64"), the build will fail because the module extension go_deps doesn't see the repo @go_sdk_linux_amd64

This can work as long as the name attribute is of type label, not string. If the MODULE.bazel in which you're writing that tag has visibility to a repo with the apparent name "go_sdk_linux_amd64", that label would be properly translated into one with a canonical repo name which the go_sdk extension can freely use. I recommend (re-)reading https://bazel.build/external/overview#concepts (especially the two items about repo names) to understand more fully how this works.

Similar for @shahms, as long as you make sure the tag attribute has type "label", you should be able to pass that label around (in a capability-passing fashion, almost).

[fmeum]

Similar for @shahms, as long as you make sure the tag attribute has type "label", you should be able to pass that label around (in a capability-passing fashion, almost).

@shahms use case, which is similar to bazelbuild/rules_python#48 (comment), also requires a way to reference this label from (patched) BUILD files.

I can think of three ways to make this work:

1. The brotli dependency is patched into gazelle as a bazel_dep so that the repositories generated by go_deps can reference it under a stable, friendly name. This requires patching both gazelle as well as the go_deps-generated repo. If brotli were fetched by some other module extension, the required bazel_dep and extension usage for that repo would also need to be patched into gazelle. Compared to WORKSPACE, this requires patching an additional repository.
2. go_deps needs to be extended with some kind of a label_keyed_string_dict attribute that matches canonical repo names to user-provided stable identifiers and textually replaces the latter with the former in all patch files (e.g. replace @brotli with @@<brotli's canonical name>). This would make existing patches work with Bzlmod, but textually replacing labels in patch files may turn out to be brittle.
3. Instead of textually replacing apparent with canonical labels, use the label_keyed_string_dict to generate a .bzl file with a patch_label function that is visible to all go_deps-generated repos and that patches can wrap their apparent labels into to get a canonical label string. Compared to WORKSPACE, this would make patches longer as they would also need to patch in a load, but isn't brittle like 2. Users would still need to add tags such as go_deps.patch_dependencies(repo_names = {"@brotli": "brotli"}).

[fmeum]

I found a fourth option that I currently like best:
4. Provide a repo_override tag on the module extension that accepts a repo_name and a list of targets and creates special repo named repo_name that contains aliases to all the targets under the same respective packages. Since this is an actual repo called repo_name, all other extension repos will see it under this name. Only downside is that the user has to manually list all the relevant targets.

[Wyverald]

So IIUC the capability is there (using label-typed attributes), just the usability is really bad because the user needs to somehow translate a BUILD file patch containing single-@ labels to a patch containing double-@ labels. Is that correct?

Assuming that it is, I'm sympathetic to the concern and agree that we should have a better story for this use case.

But first I wanted to make sure we're aware of a specific technical challenge, which is that any "repo mapping injection" we do is subject to conflicts. Suppose foo and bar are both using gazelle to bring in go-brotli, and would like to inject cc-brotli, we could have a conflict on our hands if foo and bar present different cc-brotlis to gazelle. We could of course say that only the root module can perform injections, or that conflicts are the user's business, but it's something we need to acknowledge.

---

Now, going over @fmeum's suggestions:

1. This seems extremely ugly to me, and then again could only be done by the root module, and it can only work if you use a non-registry override on gazelle (single_version_override wouldn't work because we don't apply the patch until we fetch the repo, and the MODULE.bazel file of gazelle is obtained from the registry, not from the [patched] fetch).

2. This is indeed too brittle IMO (and suffers from the injection conflict problem).

3. Makes writing a BUILD patch very unintuitive (and suffers from the injection conflict problem).

4. This is essentially a "repo alias" in its most ideal form, which ends up being a really obvious way to work around the deficiencies of the current repo mapping rules -- oh I can't map A to B? let me just create a C which is an alias of B and map A to C instead... at that point I'd rather fix it from Bazel's side. (And... you guessed it... this suffers from the injection conflict problem...)

The "reverse use_repo"/"make_visible_to" idea brought up in the other thread also suffers from the injection conflict problem, but has the advantage of being more ergonomic -- no funny label business.

---

To mitigate the injection conflict problem, I wonder if we can limit the scope of an injected repo mapping to select repos generated by the extension. Essentially this would be bringing back the repo_mapping faux-attribute on repo rules to all extension impl functions, but with semantics more akin to additional_repo_mappings. Then the user can pass labels into the extension, and the extension can selectively make those available to repos it generates (and decide what to do when two injections conflict).

But wait, isn't that #17493 (comment), where you said this was infeasible?? Well, not quite -- the repo_mapping attribute proposed in that issue changes the repo mapping of the Bazel modules using the extension, not the repos generated by the extension. By contrast, additional_repo_mappings only affects the repo mapping of extension-generated repos, which is already only computable after extensions run, so slapping some extra entries on there isn't a huge deal.

This approach still requires the user to "expose" a repo to an extension via a label (a capability), but then the extension can decide smartly what to do with that capability. Admittedly usability still isn't great; we could repurpose the "make_visible_to" idea to improve usability (module_ctx.modules[0].exports?), but I'd like to hear more thoughts first.

[meteorcloudy]

In the internal team discussion we had on how to pin a module extension generated repo, instead of using --override_repository, I had the following proposal of a repo_override directive in MODULE.bazel which can override the repospec and repo mapping of a given repo.

# Assuming I want to pin repo foo and bar in the following MODULE.bazel file

# MODULE.bazel
bazel_dep(name = "foo", version = "1.0")

ext = use_extension("//:extension.bzl", "my_extension")
ext.bar(name = "bar", ...)
use_repo(ext, "bar")

# After vendoring foo and bar, copy then out to <source dir>/third_party (or some arbitrary dir name)

# Introduce repo_override directive in MODULE.bazel and append the following content to MODULE.bazel

repo_override(
  repo_name = "foo", # The apparent repo name that's introduced by bazel_dep or use_repo
  repo_class = "local_repository",
  # Can it just be `path = "./third_party/foo",` ?
  attrs = {"path" = "./third_party/foo"},
)

repo_override(
  repo_name = "bar",
  repo_class = "local_repository",
  attrs = {"path": "./third_party/bar"},
  # If the overridden version requires additional repo mappings, users can fix them here.
  # The key is the apparent repo name seen by bar, and the value is the apparent repo name seen by the root module.
  additional_repo_mapping = {"baz": "my_barz"}
)

# repo_override will only be available in the root module like other overrides.
# Besides solving the pinning problem for vendor mode, it also solves the problem of overriding a module extension introduced repo.

If we only use the repo mapping functionality alone, for the go use case, it would be:

repo_override(
  repo_name = "go_sdk",
  # The first `go_sdk_linux_amd64` is the apparent repo name seen by `go_sdk`
  # The second `go_sdk_linux_amd64` is the **apparent repo name** seen by the root module
  additional_repo_mapping = {"go_sdk_linux_amd64": "go_sdk_linux_amd64"}
)

[iancha1992]

@bazel-io fork 7.1.0

[Wyverald]

This still needs some design work, so it's unlikely to happen for 7.1.0. Punting to 7.2.0 for now.

[bfsoyc]

I was blocked by the similar issue.
Here is my scenario: I try to build protobuf with cpp impl backend (with flag --define=use_fast_cpp_protos=true)

The protobuf require python toolchain for compiling. With legacy WORKSPACE:

# .bazelrc
build --define=use_fast_cpp_protos=true

# WORKSPACE
load("@rules_python//python:repositories.bzl", "python_register_toolchains")

# Register toolchain for hermetic build
python_register_toolchains(
    name = "python_3_9",
    python_version = "3.9",
)

# Use bind to override the //external: python_headers used in protobuf repo
bind(
    name = "python_headers",
    actual = "@python_3_9//:python_headers",
)

http_archive(
  name = "com_google_protobuf",
  # XXXX
)

Now the newest protobuf in BCR adapt for bazlmod by replacing use of //external: python_headers to //third_party: python_headers, is there anyway to override it?

# MODULE.bazel
python = use_extension("@rules_python//python/extensions:python.bzl", "python")

python.toolchain(
    configure_coverage_tool = True,
    is_default = True,
    python_version = "3.9",
)
use_repo(python, "python_3_9")

bazel_dep(name = "protobuf", version = "23.1", repo_name = "com_google_protobuf")
# !!!!!!!!!!! How to let the protobuf module adapt my registers python toolchain? !!!!!!!!!!!!

Any help will be appreciate, thanks.

[bfsoyc]

I was blocked by the similar issue. Here is my scenario: I try to build protobuf with cpp impl backend (with flag --define=use_fast_cpp_protos=true)

The protobuf require python toolchain for compiling. With legacy WORKSPACE:

# .bazelrc
build --define=use_fast_cpp_protos=true

# WORKSPACE
load("@rules_python//python:repositories.bzl", "python_register_toolchains")

# Register toolchain for hermetic build
python_register_toolchains(
   name = "python_3_9",
   python_version = "3.9",
)

# Use bind to override the //external: python_headers used in protobuf repo
bind(
   name = "python_headers",
   actual = "@python_3_9//:python_headers",
)

http_archive(
 name = "com_google_protobuf",
 # XXXX
)

Now the newest protobuf in BCR adapt for bazlmod by replacing use of //external: python_headers to //third_party: python_headers, is there anyway to override it?

# MODULE.bazel
python = use_extension("@rules_python//python/extensions:python.bzl", "python")

python.toolchain(
    configure_coverage_tool = True,
    is_default = True,
    python_version = "3.9",
)
use_repo(python, "python_3_9")

bazel_dep(name = "protobuf", version = "23.1", repo_name = "com_google_protobuf")
# !!!!!!!!!!! How to let the protobuf module adapt my registers python toolchain? !!!!!!!!!!!!

Any help will be appreciate, thanks.

I just found out my build ERROR is caused by pybind/pybind11_bazel#57 (comment)
"Bazel 7 broke pybind11_bazel on macOS"
Add --linkopt="-undefined dynamic_lookup" is a workaround.
Never mind my question. Protobuf repo defines itself well.
Please educate me if you have more to say. 😭

[alexeagle]

This seems like a fatal flaw for rules_go users, in the sense that bzlmod requires me to go visit the BUILD files of any Go package I depend on, and alter them to conform to the strict repo visibility semantics. @Wyverald is it still on-track for 7.2?

[Wyverald]

is it still on-track for 7.2?

Yes. Current plan is to do #19301 (comment). Does that address your concern?

[alexeagle]

It looks to me like that proposal still requires that users of a go dependency will have to include workarounds in their MODULE.bazel file? Or will rules_go be able to use that API internally such that every repository it generates works correctly? Probably a question for @fmeum

[Wyverald]

in the sense that bzlmod requires me to go visit the BUILD files of any Go package I depend on, and alter them to conform to the strict repo visibility semantics.

Sorry, I missed this part -- this doesn't sound like it would be addressed by the comment I linked. What is this problem, actually? Does it coincide with any of the other comments in this thread? I don't quite understand why you need to change other BUILD files.

[fmeum]

While writing a module extension that instantiates repos with user-supplied BUILD files, I used the following replacement for repo_override: I added an attr.label_list() called bazel_dep that accepts labels of the form @rules_java. Conveniently, when such a label goes through repo mapping, it will be of the form @@rules_java~1.2.3//:rules_java, i.e., you can get both the canonical and the apparent name from it. This information can then be used to rewrite labels in BUILD files (for which textually replacing "@rules_java// and "@rules_java" is probably okay).

repo_override or alternatives would still be much more reliable and powerful.

[sluongng]

FYI in the upcoming Gazelle release, there is a new workaround for this issue bazelbuild/bazel-gazelle@7d10bf7

Using the new mode, for a go_repository, gazelle will wipe all existing BUILD files and generate them new from scratch. This ensures that only MODULEs that are relevant to go builds are included in each BUILD file.

The fix is not ideal, but should address most simple cases in the Go ecosystem. I hope Bazel 7.3 release will provide a solution to this madness.

[fmeum]

If we decouple "overriding a repo definition" from "modifying a repo's repo mapping", we may be able to realize a cleaner API for the latter. This is a concept with different pros and cons:

use_repo(some_ext, my_repo_a = "repo_a")
use_repo(other_ext, my_repo_b = “repo_b”, my_repo_c = "repo_c")

repo_mapping_override(
    "my_repo_a", # required positional argument
    “my_repo_b”, # extra positionals
    io_bazel_repo_c = "my_repo_c", # extra keyword arguments
)

# vs. 
repo_override(
    repo_name = "my_repo_a",
    additional_repo_mapping = {
        "my_repo_b": "my_repo_b",
        “io_bazel_repo_c”: “my_repo_c”,
    },
)

Pros I see compared to the repo_override approach:

• Overall less verbose.
• Makes the "what is seen as what" part less symmetric in a way that corresponds to use_repo, thus making it easier to remember what goes where.
• In the common use case where the user needs to make a repo visible that they are referencing in a patch, the extra positionals form can be used and removes the need to duplicate the repo name. This is particularly useful as typos in the additional_repo_mapping keys are harder to spot (although we could add "did you mean" suggestions to the non-visible form of a RepositoryName to help with this).

Cons:

• The first positional argument has a different meaning than the other ones, but the same type. This could possibly be mitigated somewhat by better naming (e.g. make_visible_to), but that could be hard if we want to keep the _override suffix.

Generally speaking, I find the repo_override API easier to grok if you have very little idea what it's about due to it being more verbose/explicit and repo_mapping_override's API more pleasant if you work with it or wonder what it's doing exactly.

[Wyverald]

If we decouple "overriding a repo definition" from "modifying a repo's repo mapping", we may be able to realize a cleaner API for the latter.

Does this mean we'd need to introduce two new directives? One for repo definitions, one for repo mappings? (Since as we discussed elsewhere, I don't think we should reuse archive_override & co for overriding repo definitions.)

But now that I think about it, we could actually do this decoupling and make "modifying a repo's repo mapping" work on a per-extension level. The "overriding a repo definition" part simply becomes "define a repo using any mechanism, be it use_repo or use_repo_rule" (or maybe even bazel_dep, if we're feeling wild).

This even works around the "con" you named, since the first argument would just be the extension proxy object.

So, for Yun's earlier example:

# This earlier example:
ext = use_extension("//:extension.bzl", "my_extension")
ext.bar(name = "bar", ...)
use_repo(ext, "bar")
repo_override(
  repo_name = "bar",
  repo_class = "local_repository",
  attrs = {"path": "./third_party/bar"},
  # If the overridden version requires additional repo mappings, users can fix them here.
  # The key is the apparent repo name seen by bar, and the value is the apparent repo name seen by the root module.
  additional_repo_mapping = {"baz": "my_barz"}
)

########################
# ... would become:
local_repository = use_repo_rule("@bazel_tools//:blah.bzl", "local_repository")
local_repository(name="my_own_bar", path="./third_party/bar")

ext = use_extension("//:extension.bzl", "my_extension")
ext.bar(name = "bar", ...)
repo_override(ext, bar="my_own_bar", baz="my_barz")

One extra good thing: You don't need to use_repo(ext, "bar") just to override it, if you're not actually using bar at all.

One debatable thing: Is applying a repo mapping override across all repos in an extension actually desirable? In other words, can we claim "it's never a good idea to override repo mappings for some but not all repos generated by an extension"? I think the answer is yes, but I'm happy to be convinced otherwise.

[meteorcloudy]

@Wyverald I like this idea.

repo_override(ext, bar="my_own_bar", baz="my_barz")

IIUC, this basically means the old repo is completely replaced by the new one and won't be visible to anyone, right? If so, it's essentially the same the old repo_override, but with a much cleaner API. I do tend to agree it's a much more common case to override a repo for everyone than for a specific repo.

[fmeum]

Syntax-wise repo_override(ext, ...) is just great - it's concise and reuses the same concepts that users know from use_repo while distinguishing different argument roles by types.

To clarify your idea, is it that repo_override(ext, bar = "my_own_bar") means "the repo mapping of repos generated by the extension ext maps the apparent name bar to the canonical repo name associated with the apparent name my_own_bar by the root module"?

Assuming that is the case, I have some concerns:

1. If we allow this mechanism to not only add repo mapping entries, but replace them (which is needed if we want this to provide the functionality of replacing repos), users need to keep in mind that they are replacing some references to, not the definition of the repo. If ext lives in rules_foo, generates bar and then has the override applied, all other repos generated by ext would use my_own_bar instead, but if rules_foo itself contains use_repo(ext, "bar"), it would use the original bar. Wouldn't this make it impossible to e.g. replace non-module dependencies brought in via a "WORKSPACE macro turned module extension", which are usually only referred to via use_repos in the hosting module?
   I wonder if we could resolve this by also mapping all use_repo(ext, "bar") to my_own_bar, thus realizing what @meteorcloudy expressed with "this basically means the old repo is completely replaced by the new one and won't be visible to anyone".
2. This technique can't add to the repo mapping of a module. Note that the seemingly simple suggestion "just patch its MODULE.bazel file" isn't actually that simple since patches on the module file supplied via single_version_override are not effective (which reminds me that we should do something about this gotcha, e.g. have a warning shown). Users would have to reach for a non-registry override and thus replace the entire module at once.
   If we don't find another solution, we could always introduce an additional_repo_mapping attribute on single_version_override, just like in the original repo_override idea.

In other words, can we claim "it's never a good idea to override repo mappings for some but not all repos generated by an extension"?

I could see cases where someone would want to override the mapping for a single repo (e.g. have one particular Go dependency link against a particular OpenSSL version, without affecting any other Go deps), but I don't think that's common enough to make the API for it more complicated.

There is also always a workaround, assuming the module extension allows applying patches: You can first patch the references in the one extension repo to refer to a unique, new repo name and then inject a repo mapping entry for it. This seems acceptable for the rare cases in which one would need this and module extensions usually support patches. It's even more transparent since future readers of the BUILD files do not need to keep in mind that the same label may resolve to different labels in different extension repos.

[fmeum]

There is another type of reference which a repo_override based on modifying extension repo mappings wouldn't modify: If a repo rule wants to access a repo created by the same extension, it currently needs to go through a canonical name as there is no API for this, for example by trimming its own extension-provided name from the end and appending the name of the other repo. Such labels would still reference the "overridden" repo.

[Wyverald]

OK, so it looks like the biggest "hole" in this new idea is that the overridden repo is still accessible through its original canonical name.

This hole is rather hard to plug, since we'd need some sort of repo-level alias (#19927) for this to work. (The idea being that the overridden repo's canonical name aliases to the overriding repo's canonical name.) While this is easy to do on a surface level (just make a symlink), the proper fix for this is nigh impossible (involves making labels compare equal while cognizant of repo-level aliases).

We could work around this issue in the "use_repo from rules_foo" case by making use_repo "smart" (as in, have it be aware of override_repo directives). This will probably be good for 90% of cases. For the remaining 10% where people splice repo names, we could fix #19055 and call it a day.

---

This technique can't add to the repo mapping of a module.

While that's true, I'm still kind of sympathetic to the "patching MODULE.bazel with single_version_override" use case. Assuming we ever support that, it would be a "feature not a bug" that we have a single way to do this.

[fmeum]

Does that mean we have this plan?

1. Resolve Provide module extensions with a way to reference extension repos #19055 and document that canonical repo names should never be constructed by hand. If rulesets don't adopt it right away, it's not the end of the world as it only leads to issues when users repo_override repos that have canonical labels constructed for them by other repos in the extension.
2. Implement repo_override including smart use_repo.
3. Make single_version_override able to patch module files, either by fetching patched modules eagerly or by magically patching only the module file.

This looks good to me (and doable in time for 7.4.0/8.0.0)

[Wyverald]

SGTM.

One last bikeshed for the road: I'd prefer the name override_repo over repo_override. override_repo has a nice symmetry with use_repo, and repo_override is too similar to archive_override & co.