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

Specifying repo mapping in bzlmod module extension repos. #17493

Closed
matts1 opened this issue Feb 15, 2023 · 6 comments
Closed

Specifying repo mapping in bzlmod module extension repos. #17493

matts1 opened this issue Feb 15, 2023 · 6 comments
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@matts1
Copy link
Contributor

matts1 commented Feb 15, 2023

Description of the feature request:

One of the principles of module extensions is that each module has its own "repository namespace". This tends to result in the hub-and-spoke model, where we generate a hub for each module. However, this falls apart when you have multiple modules that define the same repo using the same module extension (which is relatively common). Consider:

workspace/Cargo.toml
same_dep = { version = "1.2.3" }
different_version = { version = "1.2.3"}
different_config = { version = "1.2.3", features = ["blah"]}
workspace/MODULE.bazel
bazel_dep(name = "my-dep")
...
rust_crates.crates_repository(repo_name = "crates", manifests=[":Cargo.toml"], lockfile=":Cargo.lock"))```
use_repo(rust_crates, "crates")
my_dep/Cargo.toml
same_dep = { version = "1.2.3" }
different_version = { version = "2.3.4"}
different_config = { version = "1.2.3", features = []}
implicit_dep = { version = "1.2.3", features = ["blah"]}
my_dep/MODULE.bazel
rust_crates.crates_repository(repo_name = "crates", manifests=[":Cargo.toml"], lockfile=":Cargo.lock"))
use_repo(rust_crates, "crates")

For an example such as above, the ideal semantics would be to generate repositories like the following:

  • same_dep_123_sources, different_version_123_sources, different_version_234_config, different_config_123_sources, implicit_dep_123_sources (private repos, cannot be use_repo'd)
  • same_dep_123_config1, different_version_123_config1, different_version_234_config1, different_config_123_config1, different_config_123_config2, implicit_dep_123_config1 (private repos, cannot be use_repo'd)
  • workspace_crates (visible as crates to workspace), invisible to my_dep
  • my_dep_crates (visible as crates to my_dep), invisible to workspace

In terms of implementation details, my idea of the top of my head was something along the lines of:

for mod in module_ctx.modules:
  repository_rule(name = "hub_repo_%s" % mod, repo_mapping = {mod: "crates"}, ...)
for spoke in spokes:
  other_repository_rule(name = spoke.name, repo_mapping = {}) # make repo invisible.

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

In the example above, there are a few different things where we could go wrong:

  • Implicit dependencies
    • If I remove same-dep from cargo.toml in the workspace, my deps = ["@crates//:same_dep"] will still compile
    • Conversely, I can add deps = ["@crates//:implicit_dep"], and if they remove implicit_dep in a future update to their module, my code will fail to compile.
  • Unintended access to implementation details
    • I can write use_repo(rust_crates, "crates_same_dep_sources") to access something that might be changed in future versions. This makes public vs private API unclear.
  • Impossible semantics
    • In the different config & different version examples, there is no way that I can provide @crates//:different_config, as they need to refer to different things, but can't.
  • If my_dep adds the use_repo line, when I update my module may break.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

release 6.0.0-pre.20221012.2

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

Have you found anything relevant by searching the web?

No

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

No

@fmeum
Copy link
Collaborator

fmeum commented Feb 15, 2023

I think that the idea of making the spoke repos invisible to anything other than themselves and giving each module its own dedicated hub repo is very good - it essentially transfers the bazel_dep semantics to the module extension layer.

Even without additions to Bzlmod, I believe that that concept can mostly be realized:

  1. Users can load their dedicated hub repo via use_repo(rust_crates, crates = "my_module_crates"). The explicit rename makes it relatively clear that what the user is getting is their own declared view on all crates, so that might be considered a feature rather than an inconvenience. This could break in multiple_version_override scenarios though, but I doubt anyone has any real experience with them yet.
  2. Instead of making the spokes fully invisible, just attach an auto-generated prefix such as internal__ to them.

There is unexplored design space that may allow for fully invisible spokes repos, but that feels pretty adventurous: You should be able to generate a module extension from a module extension and use it from the same module. If you forward the dependency information to the second module extension, it could generate repos that are truly separated from those in the first one, but still see each other.

We are planning to do something similar in bazelbuild/rules_go#3443, but applied to toolchains and thus without having to worry about strict deps.

@sgowroji sgowroji added type: feature request team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. untriaged area-Bzlmod Bzlmod-specific PRs, issues, and feature requests labels Feb 15, 2023
@matts1
Copy link
Contributor Author

matts1 commented Feb 16, 2023

For simplicity in future discussions, let's refer to the idea of giving each repo its own dedicated hub repo the "hubs and spokes" model.

I hadn't considered that I could do module renaming in the use_repo - that helps a lot, thanks. I think in the short term, that's a good approach, and it makes for a reasonable solution, but in the long term I'd like to see more power in module extensions to be able to do this. In the medium term, I think documentation on best practices for writing module extensions is the way to go - the only reason I became aware of any best practices was by filing a bug and having them pointed out to me there.

@Wyverald
Copy link
Member

This is indeed something we had considered in the beginning, but felt like would be too onerous on the extension author to include for Bzlmod v1. Now that Bzlmod is "launched", we could look at this again.

One major hurdle is that we picked a design where simply evaluating the MODULE.bazel file of a module gives us its full repo mapping without needing to run any extensions. If we allowed extensions themselves to do repo mappings, this rather important property is lost. It's not immediately clear to me how to approach this, but it's certainly worth keeping in mind.

@Wyverald Wyverald added P2 We'll consider working on this in future. (Assignee optional) not stale Issues or PRs that are inactive but not considered stale and removed untriaged labels Feb 21, 2023
@fmeum
Copy link
Collaborator

fmeum commented Apr 5, 2023

An update on some of these points now that #17908 is being implemented:

Implicit dependencies
If I remove same-dep from cargo.toml in the workspace, my deps = ["@crates//:same_dep"] will > still compile
Conversely, I can add deps = ["@crates//:implicit_dep"], and if they remove implicit_dep in a future update to their module, my code will fail to compile.

This is covered by the use_repo fixups: If your module extension knows that a repo is an implicit dep of your module, it will not include in the list of repos that your module should use_repo. If you still do, you will see a warning about depending on an implicit dependency.

Unintended access to implementation details
I can write use_repo(rust_crates, "crates_same_dep_sources") to access something that might be changed in future versions. This makes public vs private API unclear.

While the use_repo fixups would already show a warning if the root module depends on such an internal module, that may not go far enough. @Wyverald and I discussed having a private_repos field on module_ctx.extension_metadata that allows an extension to enforce that these repos are never imported by a different module. Would that fit your needs?

@matts1
Copy link
Contributor Author

matts1 commented Apr 6, 2023

It should mostly work with only some minor issues, but since I've already written a thing that generates one hub repo per tag, I think I'll stick with the existing one-hub-per-module mechanism.

We currently have over ~250 direct third party rust crate dependencies in our rust workspace, and I have concerns about potential repo name conflicts, since rust crates, unlike go modules, are usually just a single word name, and have no structure to them. For example, there is a rust crate called "platforms", which would conflict with @platforms (unless we turned it into something like "@crate-platforms//:crate").

@Wyverald
Copy link
Member

This is addressed by https://docs.google.com/document/d/1dj8SN5L6nwhNOufNqjBhYkk5f-BJI_FPYWKxlB3GAmA/edit. The API will be available in 6.2.0.

@sgowroji sgowroji removed the not stale Issues or PRs that are inactive but not considered stale label Jul 17, 2023
aherrmann added a commit to tweag/rules_nixpkgs that referenced this issue Sep 29, 2023
This uses the new isolated module extensions feature [1], instead of a
hub-repository approach [2] [3], to distinguish globally unified
repositories and locally specialized repositories.

[1]: bazelbuild/bazel#18529
[2]: bazelbuild/bazel#17048
[3]: bazelbuild/bazel#17493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Development

No branches or pull requests

4 participants