aspect-build / toolchains_protoc

Pre-built protoc binary toolchain for Bazel, fetched from official protobuf releases
Apache License 2.0
22 stars 3 forks source link

fix: ensure module toolchains are dev dependencies #9

Closed jrandolf closed 5 months ago

jrandolf commented 5 months ago

These cause errors when a user tries to declare their own toolchain using the default repository. For example:

protoc = use_extension("@toolchains_protoc//protoc:extensions.bzl", "protoc")
protoc.toolchain(
    version = "v27.0-rc3",
)
use_repo(protoc, "toolchains_protoc_hub")
CLAassistant commented 5 months ago

CLA assistant check
All committers have signed the CLA.

jrandolf commented 5 months ago

So after investigating things further, I was able to fix this using

protoc.toolchain(
    name = "protoc_toolchains",
    version = "v27.0-rc3",
)
use_repo(protoc, "protoc_toolchains")

which makes sense because protoc_toolchains is a different repo that doesn't clash with the default one.

However, I find this counter intuitive to other stable toolchains such as rust and nodejs. I suggest we either remove the default one and enforce users to pick their own (which is almost always the way to do things to ensure there are no surprises with versioning) or we somehow remove/decouple the com_google_protobuf attribute from the toolchain and require users to choose their protobuf type implementation (this is what's causing the clash in the repro).

alexeagle commented 5 months ago

Here is where the logic is intended to allow the root module to pick the version https://github.com/aspect-build/toolchains_protoc/blob/main/protoc/extensions.bzl#L18-L21

alexeagle commented 5 months ago

Let's start with an issue rather than a PR since we don't agree on what the problem is nor the expected behavior.