bazelbuild / rules_proto

Protocol buffer rules for Bazel
Apache License 2.0
167 stars 69 forks source link

Add current_proto_toolchain #214

Open psalaberria002 opened 6 months ago

psalaberria002 commented 6 months ago

After asking in https://bazelbuild.slack.com/archives/C04281DTLH0/p1716279966809639 I gave it a tried, and it seems to be working.

The implementation is based on https://github.com/bazelbuild/rules_python/blob/730a2e39bd2702910f28629d4583b3ec49f4ee5e/python/current_py_toolchain.bzl

Tested with the following snippet:

load("@rules_proto//proto:defs.bzl", "current_proto_toolchain")

current_proto_toolchain(
    name = "current_proto_toolchain",
    visibility = ["//visibility:public"],
)

genrule(
    name = "x",
    srcs = [":current_proto_toolchain"],
    outs = ["x"],
    cmd = "$(PROTOC) > $(OUTS)",
    toolchains = [":current_proto_toolchain"],
)

How should we test this feature?

Is there any other preferred alternative for exposing the current protoc binary after toolchain resolution?

psalaberria002 commented 6 months ago

LGTM, maybe a genrule test for it so we know it works?

I get While resolving toolchains for target //tests/current_toolchain:current_proto_toolchain (312a038): No matching toolchains found for types //proto:toolchain_type. because there aren't any registered toolchains in this module.

Is it possible to register toolchains only for running tests, but not when publishing the module? Does that require a totally separate folder with its own MODULE.bazel?

thesayyn commented 6 months ago

i think so yes, https://bazel.build/rules/lib/globals/module#register_toolchains has an attribute dev_dependency.

thesayyn commented 6 months ago

we already depend on protobuf repo which has a protoc binary which can be registered as a toolchain only as a dev dep. https://github.com/bazelbuild/rules_proto/blob/d205d37866925569d99b4d6cdcba172326ecf812/MODULE.bazel#L18

comius commented 6 months ago

current toolchain pattern is something we're deprecating in other toolchains. What is the reason to introduce it for protos?

psalaberria002 commented 6 months ago

current toolchain pattern is something we're deprecating in other toolchains. What is the reason to introduce it for protos?

It was recommended in the Slack workspace.

Whats the alternative for accessing the protoc binary when using the toolchain? I have some rust code that expects protoc to be available as the PROTOC environment variable.

thesayyn commented 6 months ago

toolchain pattern is something we're deprecating in other toolchains.

Hmm, what is it being replaced with? Currently, in order to use a toolchain in a genrule, you need a target to materialize a toolchain type and return some providers according to this issue.

This target is also important to workaround issues such as https://github.com/bazelbuild/bazel/issues/19645