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.

Sign up for GitHub

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

Jump to bottom

Support --incompatible_enable_proto_toolchain_resolution #3895

Closed
alexeagle opened this issue Mar 19, 2024 · 9 comments · Fixed by #3919
Closed

Support --incompatible_enable_proto_toolchain_resolution #3895

alexeagle opened this issue Mar 19, 2024 · 9 comments · Fixed by #3919

Comments

@alexeagle
Copy link
Contributor

What version of rules_go are you using?

Current

What version of Bazel are you using?

7.0.0 or later

What did you do?

Try to build a go_library target with the flag enabled and a different toolchain registered for @rules_proto//proto:toolchain_type

What did you expect to see?

When --incompatible_enable_proto_toolchain_resolution is enabled, rules should consult the relevant toolchain_type to locate protoc.
Example PR from rules_python:
bazelbuild/rules_python#1577

That means I ought to be able to add a Go example to https://github.com/alexeagle/toolchains_protoc/tree/main/examples
which is a project that ensures we never have to build the cc_binary target for protoc from sources.

What did you see instead?

rules_go/proto/BUILD.bazel

Line 129 in cac338e

dep = "@com_google_protobuf//:protoc",
depends on @com_google_protobuf//:protoc directly.

In this case I registered a pre-built protoc toolchain (using https://github.com/alexeagle/toolchains_protoc) but get an error:

$ bazel query --output=build //cli/core/pkg/aspect/config:config
# /home/alex/Projects/silo/cli/core/pkg/aspect/config/BUILD.bazel:3:11
go_library(
  name = "config",
  visibility = ["//visibility:public"],
  generator_name = "config",
  generator_function = "go_library_macro",
  generator_location = "cli/core/pkg/aspect/config/BUILD.bazel:3:11",
  srcs = ["//cli/core/pkg/aspect/config:config.go"],
  deps = ["//cli/core/pkg/bazel:bazel", "//cli/core/pkg/ioutils:ioutils", "@com_github_spf13_cobra//:cobra"],
  importpath = "github.com/aspect-build/silo/cli/core/pkg/aspect/config",
)
# Rule config instantiated at (most recent call last):
#   /home/alex/Projects/silo/cli/core/pkg/aspect/config/BUILD.bazel:3:11                                                                in <toplevel>
#   /home/alex/.cache/bazel/_bazel_alex/cf34354d28c19c286eb12fd35d42d565/external/io_bazel_rules_go/go/private/rules/wrappers.bzl:45:15 in go_library_macro
# Rule go_library defined at (most recent call last):
#   /home/alex/.cache/bazel/_bazel_alex/cf34354d28c19c286eb12fd35d42d565/external/io_bazel_rules_go/go/private/rules/library.bzl:61:18 in <toplevel>

alex@a:~/Projects/silo$ code $(bazel info output_base)/external
alex@a:~/Projects/silo$ bazel build //cli/core/pkg/aspect/config:config
ERROR: /home/alex/.cache/bazel/_bazel_alex/cf34354d28c19c286eb12fd35d42d565/external/com_google_protobuf/BUILD: no such target '@@com_google_protobuf//:protoc': target 'protoc' not declared in package '' defined by /home/alex/.cache/bazel/_bazel_alex/cf34354d28c19c286eb12fd35d42d565/external/com_google_protobuf/BUILD (Tip: use `query "@com_google_protobuf//:*"` to see all the targets in that package)
ERROR: /home/alex/.cache/bazel/_bazel_alex/cf34354d28c19c286eb12fd35d42d565/external/io_bazel_rules_go/proto/BUILD.bazel:127:20: no such target '@@com_google_protobuf//:protoc': target 'protoc' not declared in package '' defined by /home/alex/.cache/bazel/_bazel_alex/cf34354d28c19c286eb12fd35d42d565/external/com_google_protobuf/BUILD (Tip: use `query "@com_google_protobuf//:*"` to see all the targets in that package) and referenced by '@@io_bazel_rules_go//proto:protoc'
@fmeum
Copy link
Collaborator

fmeum commented Mar 19, 2024 •
edited

I am a huge fan of this and will try to work on it in early April unless someone gets to it earlier.

Cc @linzhp

@alexeagle alexeagle mentioned this issue Apr 11, 2024
Merged
github-merge-queue bot pushed a commit to bazelbuild/rules_python that referenced this issue Apr 11, 2024
@thesayyn @alexeagle
refactor: toolchainize py_proto_library (#1577)
4be00a6 
Enables use of `--incompatible_enable_proto_toolchain_resolution` flag
that launched in Bazel 7. This allows users to choose a pre-built
`protoc` or use the runtime from https://pypi.org/project/protobuf/
rather than be forced to use hard-coded values in Bazel core.

This change is also happening in other language rulesets that provide
first-class protobuf support, e.g.
bazelbuild/rules_go#3895

No update to CHANGELOG.md in this PR as the feature is not yet
documented for end-users, this just makes it possible to enable the
flag. A follow-up PR will provide user instructions.

---------

Co-authored-by: Alex Eagle <alex@aspect.dev>
@alexeagle
Copy link
Contributor Author

@fmeum note the rules_python change just landed, and @thesayyn tells me rules_java is already switched. Would be nice to include rules_go in this when rules_proto 6.0.0 final goes out (it's currently RC2)

@fmeum fmeum mentioned this issue Apr 12, 2024
Merged
@fmeum
Copy link
Collaborator

fmeum commented Apr 12, 2024

@thesayyn I tried using rules_java with --incompatible_enable_proto_toolchain_resolution, but it fails to find a proto_lang_toolchain for Java even with Bazel@HEAD. Do you know what's needed to get this to work?

@thesayyn
Copy link

Toolchain for java comes from protobuf repo, that's only one we i know of unless @alexeagle added one in his prebuilt toolchains ruleset.

@alexeagle
Copy link
Contributor Author

Right, the flag decouples from the hardcoded toolchain but doesn't automatically supply a replacement. It should be easy to fetch the runtime from Maven exactly like the protobuf docs instruct all non-Bazel users/tools should do. I'll add to the toolchains_protoc

@alexeagle
Copy link
Contributor Author

correction, https://github.com/bazelbuild/examples/blob/never_compile_protoc_again/proto/java/Main.java already shows how this works for Java.

This was referenced Apr 23, 2024
Closed
Open
@johanbrandhorst
Copy link
Contributor

Is there an example of using this with rules_go?

@fmeum
Copy link
Collaborator

fmeum commented May 23, 2024 •
edited

Is there an example of using this with rules_go?

You don't need to do anything extra for rules_go, just flip the flag and have a protoc toolchain registered (e.g. from toolchains_protoc). rules_go doesn't use proto_lang_toolchain, so you also don't have to register one.

@alexeagle
Copy link
Contributor Author

Adding the example here: aspect-build/toolchains_protoc#10
(but it's trivial like @fmeum says, no additional toolchain to register)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for --incompatible_enable_proto_toolchain_resolution bazelbuild/rules_go
4 participants
@alexeagle @fmeum @johanbrandhorst @thesayyn