Use prebuilt protoc #443
Use prebuilt protoc #443
Conversation
agluszak
force-pushed
the
prebuilt-proto
branch
from
ed080c7 to
991ef27
Compare
|
Looks like there're some issues on CI but I think this is a nice change we should get in |
|
WORKSPACE seems uncooperative unfortunately, I'll first figure out how to handle these problems in rules_kotlin |
| bazel_dep(name = "rules_java", version = "9.0.3") | ||
| bazel_dep(name = "bazel_skylib", version = "1.8.1") | ||
| bazel_dep(name = "rules_jvm_external", version = "6.7") | ||
| bazel_dep(name = "protobuf", version = "33.4", repo_name = "com_google_protobuf") |
There was a problem hiding this comment.
Is this necessary? Apps shouldn't need to declare dependencies that are used by the rules.
|
|
||
| # Use prebuilt protoc | ||
| common --incompatible_enable_proto_toolchain_resolution | ||
| common --@com_google_protobuf//bazel/toolchains:prefer_prebuilt_protoc=true |
There was a problem hiding this comment.
Is there another way to pass this flag to the underlying proto rules/actions? This line assumes that the user's app depends on protobuf directly, which might not be the case.
| "@rules_android_maven//:com_google_errorprone_error_prone_annotations", | ||
| "@rules_android_maven//:com_google_guava_guava", | ||
| "@rules_android_maven//:com_google_protobuf_protobuf_java", | ||
| "@com_google_protobuf//java/core", |
There was a problem hiding this comment.
Do we still need protobuf in the Maven deps if you're replacing their references with @com_google_protobuf?
|
@agluszak Thanks for this PR! It definitely moves us in the right direction towards reducing our CI times. If you wouldn't mind, I'd really prefer this PR to be split into two pieces:
|
3 participants
No description provided.