`rules_ruby` is required to run conformance tests in external repos

[ouillie]

rules_ruby is a dev_dependency, but it's actually required if you want to run conformance tests in an external repo with a Bzlmod dependency on protobuf (even when that repo has nothing to do with Ruby). That's because conformance/BUILD loads from @rules_ruby//ruby:defs.bzl and //ruby:defs.bzl.

I see 2 ways to deal with this:

• ideal: Move conformance_ruby and related stuff to a different directory, so I can depend on conformance_proto in my external repo without needing the rules_ruby dev dependency. We could move it to a new directory /conformance/ruby, or /ruby/conformance, or somewhere else. Happy to open this PR.
• Make the rules_ruby dependency non-dev-only, so it's included in the dependency tree in the external repo.

[esrauchg]

Thanks for the report!

The root cause problem mostly stems the move from workspace to bzlmod, and the generalized topic is one that we don't have a clear path on yet.

We're following up with the bazel team internally for advice/long term plan here, and we should be able to report back next week with more about what we expect to do here.

[meteorcloudy]

The root cause problem mostly stems the move from workspace to bzlmod

I think the root cause is not the move from workspace to bzlmod, with WORKSPACE, rules_ruby would still be fetched in order to use any target in conformance/BUILD.

The root cause is that you are defining targets of different languages in the same package, to just correctly parsing all those targets, Bazel needs to download all rulesets. This has nothing to do with whether you introduce those dependencies with WORKSPACE or Bzlmod.

Ideally, those targets should be defined in their own bazel modules, so that end users would only need to depend on the ones they need and don't have to introduce unnecessary dependencies.

[esrauchg]

workspace to bzlmod, with WORKSPACE, rules_ruby would still be fetched in order to use any target in conformance/BUILD

The relevance of WORKSPACE to bzlmod shift is that this setup is "not perfect but ok" on workspace where users who want to use this directory will have to fetch rules_ruby, and thats a choice they are able to make themselves. Basically the flow ends up being:

• rules_ruby may or may not be available, but some BUILD files are only usable if it is available
• User tries to do this, hits the error
• User adds rules_ruby to their WORKSPACE
• Everything works.

Post-bzlmod is the situation is instead:

• rules_ruby is a dev_dependency only
• User tries to do this, hits the error
• User has no way to move forward this package

Ideally, those targets should be defined in their own bazel modules

Unfortunately I think taken to the conclusion this would require very drastic reshape of Protobuf's codebase: the implication is that we would need to ban having BUILD files that contain e.g. both a java_proto_library() and a ruby_proto_library(). We currently have an starlark-enforced rule that $lang_proto_library() is colocated with its corresponding proto_library() as a hygiene problem (it avoids ODR by precluding multiple $lang_proto_library being defined for the same proto_library). So at the minimum breaking up this one BUILD file into ~5-10 separate modules would also require a real policy change in addition to the fundamental reshape of our codebase (we would suddenly do like a @wkt_python @wkt_rust @wkt_java @wkt_cc etc instead of being able to have one directory that has the wkt.proto files and all of the language targets next to each-other).

[meteorcloudy]

Thanks for the clarification! Sounds like "making an additional dependency available to a non-root module" is what's missing in Bzlmod.

Maybe we can implement something like inject_repo for bazel modules, then the end users could do

inject_repo_for_module(module_name="protobuf", rules_ruby = "my_rules_ruby")

@fmeum @Wyverald WDYT?

[fmeum]

I want to make sure that we don't miss alternative solutions:

it avoids ODR by precluding multiple $lang_proto_library being defined for the same proto_library

Is this still a concern now that most $lang_proto_library rules are based on aspects? Ideally each user-defined $lang_proto_library referencing the same proto_library target would share the same aspect-generated action. That would simplify the situation immensely as most packages would only have to contain proto_library targets. Even if that requires effort to change the layout, the result would potentially be much cleaner in addition to being more Bzlmod-friendly.

Maybe we can implement something like inject_repo for bazel modules, then the end users could do...

This looks a bit like optional deps, but entirely managed by the end user. I would consider that an "eternal workaround". If it's all it takes to make protobuf highly usable with Bzlmod for most users then it might be worth it, otherwise I would be skeptical. Users can already patch the MODULE.bazel file of the protobuf module via single_version_override.

[meteorcloudy]

Is it possible to use alias to work around the issue? We keep those language specific targets in subdir and make them private, then create public alias targets in the top level package.

[esrauchg]

We keep those language specific targets in subdir and make them private, then create public alias targets in the top level package.

For a local workaround to the narrow problem identified here, it does seem like we likely can move ~all of the language-specific things to a subdir (and leave behind an alias at top level where relevant), it seems like that should be a relative local no-op to get the dev-dependency-loads() off of the public BUILD file. Its maybe a minor question if we should just do one conformance/internal/BUILD and dump all of the junk as a generic mitigation-shield of the top level, or if we should try to make it a dir per dev-only language already.

The relevant meta-topic is that we will need to follow up on a more holistic plan for (slower and separately from this one GH issue) is just the topic of Protobuf slicing our BUILD files by language and releasing many small bzlmods as the way to avoid dependency bloat from bzlmod monolith.

I understand that's the Bazel team's expectation of where Protobuf would necessarily go post-bzlmod, but Protobuf is (today) not subscribed that slicing-by-language + publishing many small mods as being a healthy direction for our project to move. And for things that are trying to actually be public (instead of just conflicting with an internal thing like this report was) I think we don't have any mitigations available today.

For example this concern is the the only real blocker from us having Rust support in bazel, where today we are caught in between not wanting to do small mods and the problem of bloat caused by taking rules_rust as a dep from our monolith mod as the main blocker from us shipping it.

Is this still a concern now that most $lang_proto_library rules are based on aspects?

The rules-of-concern in this specific report aren't on aspects; for example internal_ruby_proto_library() is defined here. You can see in the attached PR that moving the $lang_proto_library() actually changed the import paths in the .rb code.

The rules which just use .proto file on srcs instead of dep-on-proto_library normally have collocation-enforced 'implicitly', but with the proposed PR effectively loses that by having to export the .proto file which is what locally unlocks the possibility of multiple directories having dupe rules with the same .proto src.

I could be wrong but I also do believe there's still some concern in some cases where aspects is in use; Go and Rust ones use packages with names in the source tied to the $lang_proto_library name at least, then I believe there should be some risks at least on Rust without collocation.

Broadly though the collocation enforcement at best blocks only one of many different ways that ODR violations could be introduced (you could just copy paste the .proto file or hand-run protoc and check in the gencode, etc). Especially for the internal_* rules there's not much risk. I don't think we need to view collocation as an immovable concept that we necessarily need to stay tied to even if doing it wrong could cause ODR violations. It's probably more relevant as just one example of an broader ethos of "we do want BUILD files to be able to be multi-language, hopefully without bloating our users" rather than a killer problem in isolation.

[ouillie]

I know I already attached a more-complicated PR, but just a reminder that a pretty simple short-term solution could be to just make rules_ruby non-dev_only. That change could always be reversed later after figuring out the proper directory structure, and by my estimate, would not move the needle much in terms of overhead: it only adds 28 kB to the repository cache, and a mere few hundred bytes to the build cache for the dependent repository (as long as it does not actually use any of the Ruby-specific targets, but just loads some BUILD files that require rules_ruby during the analysis phase).

However, I suspect I may be misunderstanding something because of what @esrauchg said earlier about rules_rust:

For example this concern is the the only real blocker from us having Rust support in bazel, where today we are caught in between not wanting to do small mods and the problem of bloat caused by taking rules_rust as a dep from our monolith mod as the main blocker from us shipping it.

I would have thought Bazel's lazy approach to downloading dependencies would make this pretty negligible. It's not like dependent repositories would actually have to download a Rust toolchain just to be able to load a BUILD file that loads something from rules_rust. Would appreciate any elucidation of my certainly incomplete understanding here.

[Wyverald]

re inject_repo_for_module: I'd only consider this as a last resort. Imagine that you have a dependency offering some rb_proto_library or some such and that's the only reason you need to add a dependency on rules_ruby and specify this injection in your root module. (Also note what Fabian said about patching MODULE.bazel.)

re alias: this could indeed help. But do note that it requires splitting the package first -- at that point, IMO the majority of the work is done.

[esrauchg]

I would have thought Bazel's lazy approach to downloading dependencies would make this pretty negligible.

The problem/topic is that toolchains unfortunately can't be lazy in the same way, so adding rules_ruby / rules_rust, etc actually does eagerly affect people even if they don't reach any BUILD file that needs it in practice.

So there's bloat concerns with Protobuf's module taking deps on any more rules_* in the status quo, so your PR is much appreciated if it does unblock deps into the conformance directory without needing to do that today.

Thanks!