bazelbuild / rules_proto

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

feat: a convenient toolchain to register prebuilt protoc #206

Closed alexeagle closed 6 months ago

comius commented 8 months ago

cc @haberman @bellspice

comius commented 6 months ago

Closing because there was no activity for 3 months.

alexeagle commented 6 months ago

That's true, there was no reply from the reviewers in all that time. But it's disingenuous to use that as a justification when you're the maintainer who didn't reply.

comius commented 6 months ago

On March 14th I posted review feedback and there was no reply from you about that. Also this PR is set against mirror branch, not main.

I’d like to understand better your reaction. Do you want this PR reopened?

davido commented 5 months ago

That's really pity, that this was closed.

@alexeagle Thank you so much for releasing a prebuilt protoc toolchain on: https://github.com/aspect-build/toolchains_protoc.

@comius Thank you so much for adding --incompatible_enable_proto_toolchain_resolution to Bazel and allowing to use prebuilt protoc toolchains and stop building it again and again from the source.

Needless to say, that prebuilt protoc toolchain should be exposed in rules_proto itself. That's the case for all other rules_* projects.

It would be great if this PR could be re-opened and finalized.

alexeagle commented 5 months ago

I'm meeting with Ivo next week and can discuss, but Bazel team is currently planning to archive this repo, and the protobuf repo maintainers rejected this design proposal, so I'm guessing the separate toolchain repo will remain.

davido commented 5 months ago

@alexeagle Can you share the reference to a discussion as for why the current rules_protoc maintainer would reject the proposal to host the prebuilt protoc toolchain in this repository?

I import rules_java and expect to consume prebuilt JDK without building it from source. I import rules_go, rules_nodejs, rule_kotlin, and what not and I always get the prebuilt toolchain as expected.

rules_protoc was the only exception: import it and either wait for 15 minutes to build it from source, or (even worse) it wouldn't build at all due to outdated C++ Toolchain on the host machine and/or outdated protobuf versions in @com_google_protobuf.

The right course of action is obviously, to flip --incompatible_enable_proto_toolchain_resolution to true per default, and include the prebuilt protoc toolchain in rules_protoc, so that the folks could import it and move forward with the life without messing around with the external prebuilt protoc providers...

haberman commented 5 months ago

Hi @davido, I can share some thoughts from the perspective of someone on the proto team.

We agree with the premise that most users have no need to build protoc from source, and should not need to wait for this build to complete, especially given that we already release prebuilt binaries.

We would like the proto rules to use prebuilt binaries by default so that users can get the benefit of faster builds. We don't have a concrete timeline for delivering this, but I would consider it part of our roadmap. We need to make sure that the design aligns with our versioning and compatibility policies. For example, we do not support mixing and matching arbitrary versions of protoc and the language runtimes.

In the future, rules_protoc will not be a separate repo, but instead the proto rules will live in the protobuf repo. But even though the rules will be colocated with the protoc sources, we can still recognize if a prebuilt binary is available for the current repo version, and use that instead of building from source.