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.

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

use the released binary of protoc instead of compiling it from source #88

Open
dmivankov opened this issue Oct 26, 2020 · 10 comments
Open
Labels
blocked-external Waiting on external issue/release enhancement New feature or request

Comments

@dmivankov
Copy link
Contributor

dmivankov commented Oct 26, 2020

Similar to bazelbuild/rules_proto#35
it would be nice for some users to save time on compiling protoc

https://github.com/grpc/grpc-java/blob/master/compiler/BUILD.bazel#L14 protoc plugin depends on "@com_google_protobuf//:protoc_lib" which is a cc_library so may be not that trivial to fix

@aaliddell aaliddell added the enhancement New feature or request label Oct 26, 2020
@aaliddell
Copy link
Member

Interesting! This’d save my sanity when doing clean rebuilds too.

@aaliddell aaliddell mentioned this issue Feb 9, 2021
@caseyduquettesc
Copy link

Is this being actively worked on or is help required?

@aaliddell
Copy link
Member

I've tried to do this a couple of times, but it's not quite such the obvious win that it was in rules_proto. For sure it should be doable, but I need to figure a maintainable way of doing it, since I don't want to manually update every arch's hash each time we bump protobuf versions.

@aaliddell aaliddell added this to the 4.0.0 milestone Sep 14, 2021
@aaliddell aaliddell mentioned this issue Sep 16, 2021
10 tasks
@aaliddell
Copy link
Member

I've tested this in the dev branch and it doesn't actually make the build any faster since protoc still gets built. This is due to protoc still being built anyway for proto_library targets, perhaps due to an interaction of rules_proto with rules_proto_grpc surrounding the definition of com_google_protobuf and how they shim in the pre-built protoc. For the time being, I am going to revert this again, since there's no benefit to doing so at present and makes maintenance more difficult.

@aaliddell aaliddell added the blocked-external Waiting on external issue/release label Sep 18, 2021
@aaliddell aaliddell removed this from the 4.0.0 milestone Sep 18, 2021
aaliddell added a commit that referenced this issue Sep 19, 2021
@codersasha
Copy link
Contributor

codersasha commented Jun 15, 2022

This PR helps our team a lot. It would be great to find a way to re-land it in the next release.

Because all our developers use linux x64, we've found a way to skip protoc builds for the following target types on that platform:

  • proto_library
  • php_proto_compile
  • go_proto_library (from io_bazel_rules_go)

But can't yet figure out:

  • php_grpc_compile

To do this, we use your patch @aaliddell in rules_proto_grpc_use_prebuilt_protoc_binary.patch:

diff --git a/protobuf/BUILD.bazel b/protobuf/BUILD.bazel
index c45d5193..2b416c81 100644
--- a/protobuf/BUILD.bazel
+++ b/protobuf/BUILD.bazel
@@ -1,5 +1,14 @@
 load(":toolchains.bzl", "protoc_toolchain")
 
+alias(
+    name = "protoc_bin",
+    actual = select({
+        "@bazel_tools//src/conditions:linux_x86_64": "@com_google_protobuf_protoc_linux_x86_64//:bin/protoc",
+        "//conditions:default": "@com_google_protobuf//:protoc",
+    }),
+    visibility = ["//visibility:public"],
+)
+
 # Define toolchain type for protoc compiler
 toolchain_type(
     name = "toolchain_type",
diff --git a/protobuf/toolchains.bzl b/protobuf/toolchains.bzl
index ca90158f..7c08c9f8 100644
--- a/protobuf/toolchains.bzl
+++ b/protobuf/toolchains.bzl
@@ -12,8 +12,9 @@ protoc_toolchain = rule(
     attrs = {
         "protoc": attr.label(
             doc = "The protocol compiler tool",
-            default = "@com_google_protobuf//:protoc",
+            default = "//protobuf:protoc_bin",
             executable = True,
+            allow_files = True,
             cfg = "exec",
         ),
         "fixer": attr.label(
diff --git a/repositories.bzl b/repositories.bzl
index baa1caa8..e4bf9b4e 100644
--- a/repositories.bzl
+++ b/repositories.bzl
@@ -63,6 +63,14 @@ VERSIONS = {
         "sha256": "07b4117379dde7ab382345c3b0f5edfc6b7cff6c93756eac63da121e0bbcc5de",
     },
 
+    # Protobuf binary.
+    "com_google_protobuf_protoc_linux_x86_64": {
+        "type": "http",
+        "urls": ["https://github.com/protocolbuffers/protobuf/releases/download/v3.20.0/protoc-3.20.0-linux-x86_64.zip"],
+        "sha256": "75d8a9d7a2c42566e46411750d589c51276242d8b6247a5724bac0f9283e05a8",
+        "build_file_content": 'exports_files(["bin/protoc"])',
+    },
+
     # Android
     "build_bazel_rules_android": {
         "type": "github",
@@ -463,6 +471,7 @@ def rules_proto_grpc_repos(**kwargs):
 
     six(**kwargs)
     com_google_protobuf(**kwargs)
+    com_google_protobuf_protoc_linux_x86_64(**kwargs)
     com_github_grpc_grpc(**kwargs)
     external_zlib(**kwargs)
 
@@ -481,6 +490,9 @@ def com_google_protobuf(**kwargs):
 def com_github_grpc_grpc(**kwargs):
     _generic_dependency("com_github_grpc_grpc", **kwargs)
 
+def com_google_protobuf_protoc_linux_x86_64(**kwargs):
+    _generic_dependency("com_google_protobuf_protoc_linux_x86_64", **kwargs)
+
 def external_zlib(**kwargs):
     _generic_dependency("zlib", **kwargs)
 

io_bazel_rules_go_use_prebuilt_protoc_binary.patch:

diff --git a/go/private/repositories.bzl b/go/private/repositories.bzl
index 79f2a4c6..2571a64e 100644
--- a/go/private/repositories.bzl
+++ b/go/private/repositories.bzl
@@ -130,6 +130,14 @@ def go_rules_dependencies():
     # * Most proto repos are updated more frequently than rules_go, and
     #   we can't keep up.
 
+    _maybe(
+        http_archive,
+        name = "com_google_protobuf_protoc_linux_x86_64",
+        build_file_content = 'exports_files(["bin/protoc"])',
+        sha256 = "75d8a9d7a2c42566e46411750d589c51276242d8b6247a5724bac0f9283e05a8",
+        urls = ["https://github.com/protocolbuffers/protobuf/releases/download/v3.20.0/protoc-3.20.0-linux-x86_64.zip"],
+    )
+
     # Go protobuf runtime library and utilities.
     # releaser:upgrade-dep protocolbuffers protobuf-go
     _maybe(
diff --git a/proto/BUILD.bazel b/proto/BUILD.bazel
index 2125a938..c5f7d92f 100644
--- a/proto/BUILD.bazel
+++ b/proto/BUILD.bazel
@@ -15,6 +15,15 @@ load(
     "WELL_KNOWN_TYPE_RULES",
 )
 
+alias(
+    name = "protoc_bin",
+    actual = select({
+        "@bazel_tools//src/conditions:linux_x86_64": "@com_google_protobuf_protoc_linux_x86_64//:bin/protoc",
+        "//conditions:default": ":protoc",
+    }),
+    visibility = ["//visibility:public"],
+)
+
 go_proto_compiler(
     name = "go_proto_bootstrap",
     visibility = ["//visibility:public"],
diff --git a/proto/compiler.bzl b/proto/compiler.bzl
index 71f5ab22..bd5f7516 100644
--- a/proto/compiler.bzl
+++ b/proto/compiler.bzl
@@ -200,7 +200,8 @@ _go_proto_compiler = rule(
         "_protoc": attr.label(
             executable = True,
             cfg = "exec",
-            default = "//proto:protoc",
+            allow_files = True,
+            default = "//proto:protoc_bin",
         ),
         "_go_context_data": attr.label(
             default = "//:go_context_data",

WORKSPACE file sections:

http_archive(
    name = "rules_proto_grpc",
    patch_args = ["-p1"],
    patches = [
        # Use the prebuilt protoc binary from protocolbuffers/protobuf, instead of building it from source.
        "//:tools/bazel/patches/rules_proto_grpc_use_prebuilt_protoc_binary.patch",
    ],
    sha256 = "507e38c8d95c7efa4f3b1c0595a8e8f139c885cb41a76cab7e20e4e67ae87731",
    strip_prefix = "rules_proto_grpc-4.1.1",
    urls = [
        "https://github.com/rules-proto-grpc/rules_proto_grpc/archive/4.1.1.tar.gz",
    ],
)
http_archive(
    name = "io_bazel_rules_go",
    patch_args = ["-p1"],
    patches = [
        # Use the prebuilt protoc binary from protocolbuffers/protobuf, instead of building it from source.
        "//:tools/bazel/patches/io_bazel_rules_go_use_prebuilt_protoc_binary.patch",
    ],
    sha256 = "ab21448cef298740765f33a7f5acee0607203e4ea321219f2a4c85a6e0fb0a27",
    urls = [
        "https://github.com/bazelbuild/rules_go/releases/download/v0.32.0/rules_go-v0.32.0.zip",
    ],
)

To prevent the issue of proto_library targets building protoc as well, we also add the following to our .bazelrc:

build --proto_compiler=@com_google_protobuf_protoc_linux_x86_64//:bin/protoc

Not building protoc from scratch for these targets significantly speeds up all builds, especially those done on a clean checkout (such as on CI). It can remove as many as 1500 targets on a single proto target build. If we can get that working for our php_grpc_compile targets, that would make a huge difference to our workflow.

@Gentatsu
Copy link

Any more news on this? Would be super useful for our CI, too.

@aaliddell
Copy link
Member

As far as I am aware, there is no change to the constraints raised before

@Gentatsu
Copy link

Is there any reason proto_library needs to compile protoc? It's defined in rules_proto but can it not be monkey patched (or something) to use the precompiled binary as well?

@dmivankov
Copy link
Contributor Author

rules-proto-grpc need not only protoc but also language plugins for it like

// @rules_proto_grpc//java:grpc_java_plugin
proto_plugin(
  name = "grpc_java_plugin",
  visibility = ["//visibility:public"],
  tool = "@io_grpc_grpc_java//compiler:grpc_java_plugin",
  out = "{name}_grpc.jar",
)
// @io_grpc_grpc_java//compiler:grpc_java_plugin
cc_binary(
  name = "grpc_java_plugin",
  visibility = ["//visibility:public"],
  tags = ["__CC_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__"],
  generator_name = "grpc_java_plugin",
  generator_function = "cc_binary",
  generator_location = "compiler/BUILD.bazel:5:10",
  srcs = ["@io_grpc_grpc_java//compiler:src/java_plugin/cpp/java_generator.cpp", "@io_grpc_grpc_java//compiler:src/java_plugin/cpp/java_generator.h", "@io_grpc_grpc_java//compiler:src/java_plugin/cpp/java_plugin.cpp"],
  deps = ["@com_google_protobuf//:protoc_lib"],
)
// @com_google_protobuf//:protoc_lib
cc_library(
  name = "protoc_lib",
  visibility = ["//visibility:public"],
  srcs = ["@com_google_protobuf//:src/google/protobuf/compiler/code_generator.cc",...],
  linkopts = select({"@com_google_protobuf//build_defs:config_android": [], "@com_google_protobuf//build_defs:config_android-stlport": [], "@com_google_protobuf//build_defs:config_android-libcpp": [], "@com_google_protobuf//build_defs:config_android-gnu-libstdcpp": [], "@com_google_protobuf//build_defs:config_android-default": [], "@com_google_protobuf//build_defs:config_msvc": ["-ignore:4221"], "//conditions:default": ["-lpthread", "-lm"]}),
  includes = ["src/"],
  copts = select({"@com_google_protobuf//build_defs:config_msvc": ["/wd4065", "/wd4244", "/wd4251", "/wd4267", "/wd4305", "/wd4307", "/wd4309", "/wd4334", "/wd4355", "/wd4506", "/wd4800", "/wd4996"], "//conditions:default": ["-DHAVE_ZLIB", "-Woverloaded-virtual", "-Wno-sign-compare"]}),
  deps = ["@com_google_protobuf//:protobuf"],
)

If they get packaged and distributed as binaries it should be possible to switch to precompiled versions.

Looks like plugin is a hidden parameter, but if you have a precompiled plugin you can probably follow https://github.com/rules-proto-grpc/rules_proto_grpc/blob/master/docs/custom_plugins.rst to use it instead of default one

@rdesgroppes
Copy link

If they get packaged and distributed as binaries it should be possible to switch to precompiled versions.

@dmivankov protoc as well as a number of protoc-gen-* plugins are distributed as binaries. Leveraging bazel-contrib/rules_jvm_external#908, I've been fetching them through maven_install, for instance:

maven_install(
    artifacts = [
        # [...]
        "com.google.protobuf:protoc:exe:linux-aarch_64:3.21.12",
        "com.google.protobuf:protoc:exe:linux-x86_64:3.21.12",
        "com.google.protobuf:protoc:exe:osx-aarch_64:3.21.12",
        "com.google.protobuf:protoc:exe:osx-x86_64:3.21.12",
        # [...]
        "io.grpc:protoc-gen-grpc-java:linux-aarch_64:exe:1.54.0",
        "io.grpc:protoc-gen-grpc-java:linux-x86_64:exe:1.54.0",
        "io.grpc:protoc-gen-grpc-java:osx-aarch_64:exe:1.54.0",
        "io.grpc:protoc-gen-grpc-java:osx-x86_64:exe:1.54.0",
        # [...]
    ],
    # [...]
)

... which can be then be referred to as executable targets:

  • "@maven//:com_google_protobuf_protoc_linux_x86_64"
  • "@maven//:io_grpc_protoc_gen_grpc_java_osx_aarch_64"
  • etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked-external Waiting on external issue/release enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants