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

Added proto_compile_deps and grpc_compile_deps to @io_bazel_rules_rust//proto:toolchain #519

Closed
wants to merge 3 commits into from

Conversation

UebelAndre
Copy link
Collaborator

Closes #483

This PR adds proto_compile_deps and grpc_compile_deps to the @io_bazel_rules_rust//proto:toolchain toolchain to close a comment

# TODO(damienmg): Once bazelbuild/bazel#6889 is fixed, reintroduce
# proto_compile_deps and grpc_compile_deps and remove them from the
# rust_proto_library and grpc_proto_library.

This is doable now that bazelbuild/bazel#6889 is closed.

Though, I'm not entirely clear on what the earliest version of Bazel that supports this fix is. It looks like 3.4 as per bazelbuild/bazel@bc4ef8c from bazelbuild/bazel#10523

Unsure of what to do with these changes in that case.

@google-cla google-cla bot added the cla: yes label Dec 2, 2020
@UebelAndre
Copy link
Collaborator Author

Oh, the CI failure is caused by missing changes from #518 @damienmg would you be able to take a look there and here?

@UebelAndre
Copy link
Collaborator Author

Yeah, I don't even think this will pass CI without updating the version of Bazel 😢

@damienmg
Copy link
Collaborator

damienmg commented Dec 2, 2020

@UebelAndre I don't understand, what missing change? CI seems to be green @Head.

@UebelAndre
Copy link
Collaborator Author

Oh, the CI failure is caused by missing changes from #518 @damienmg would you be able to take a look there and here?

@damienmg That's been resolved

There's a new failure, at least for Build #1722

@UebelAndre
Copy link
Collaborator Author

@UebelAndre I don't understand, what missing change? CI seems to be green @Head.

Oh, and I was referring to the fact that CI in this PR had failed and I didn't expect it to. The recent builds are passing except for the Minimum Supported Version test. This makes sense due to the addition of incompatible_use_toolchain_transition (as per bazelbuild/bazel#11584). This is what I was referring to when I was saying I don't think CI will pass since this functionality seems relatively new.

@UebelAndre
Copy link
Collaborator Author

UebelAndre commented Dec 3, 2020

I think the minimum supported version would have to go up to 3.4 for this based on this commit: bazelbuild/bazel@099cf2f

@UebelAndre
Copy link
Collaborator Author

This is blocked by #529

@UebelAndre
Copy link
Collaborator Author

Gonna close this for now. Doesn't seem there's any interest in this change and I don't have the bandwidth to drive it forward.

I'm dumping a diff below since the changes are relatively small and I'm not sure what happens to changes on a PR once it's closed and the original branch is deleted.

diff --git a/proto/proto.bzl b/proto/proto.bzl
index 00769f8..5ff2082 100644
--- a/proto/proto.bzl
+++ b/proto/proto.bzl
@@ -34,8 +34,6 @@ rust_proto_repositories()
 
 load(
     "@io_bazel_rules_rust//proto:toolchain.bzl",
-    "GRPC_COMPILE_DEPS",
-    "PROTO_COMPILE_DEPS",
     _generate_proto = "rust_generate_proto",
     _generated_file_stem = "generated_file_stem",
 )
@@ -209,6 +207,12 @@ def _rust_proto_compile(protos, descriptor_sets, imports, crate_name, ctx, is_gr
         output_hash,
     ))
 
+    # Gather all dependencies for compilation
+    compile_action_deps = depset(
+        compile_deps +
+        proto_toolchain.grpc_compile_deps if is_grpc else proto_toolchain.proto_compile_deps,
+    ).to_list()
+
     return rustc_compile_action(
         ctx = ctx,
         toolchain = find_toolchain(ctx),
@@ -217,7 +221,7 @@ def _rust_proto_compile(protos, descriptor_sets, imports, crate_name, ctx, is_gr
             type = "rlib",
             root = lib_rs,
             srcs = srcs,
-            deps = compile_deps,
+            deps = compile_action_deps,
             proc_macro_deps = [],
             aliases = {},
             output = rust_lib,
@@ -280,7 +284,6 @@ rust_proto_library = rule(
         ),
         "rust_deps": attr.label_list(
             doc = "The crates the generated library depends on.",
-            default = PROTO_COMPILE_DEPS,
         ),
         "_cc_toolchain": attr.label(
             default = "@bazel_tools//tools/cpp:current_cc_toolchain",
@@ -306,6 +309,9 @@ rust_proto_library = rule(
         "@io_bazel_rules_rust//rust:toolchain",
         "@bazel_tools//tools/cpp:toolchain_type",
     ],
+    # TODO: Remove once (bazelbuild/bazel#11584) is closed and the rules use
+    # the version of Bazel that issue was closed on as the min supported version
+    incompatible_use_toolchain_transition = True,
     doc = """\
 Builds a Rust library crate from a set of `proto_library`s.
 
@@ -359,7 +365,6 @@ rust_grpc_library = rule(
         ),
         "rust_deps": attr.label_list(
             doc = "The crates the generated library depends on.",
-            default = GRPC_COMPILE_DEPS,
         ),
         "_cc_toolchain": attr.label(
             default = "@bazel_tools//tools/cpp:current_cc_toolchain",
@@ -385,6 +390,9 @@ rust_grpc_library = rule(
         "@io_bazel_rules_rust//rust:toolchain",
         "@bazel_tools//tools/cpp:toolchain_type",
     ],
+    # TODO: Remove once (bazelbuild/bazel#11584) is closed and the rules use
+    # the version of Bazel that issue was closed on as the min supported version
+    incompatible_use_toolchain_transition = True,
     doc = """\
 Builds a Rust library crate from a set of `proto_library`s suitable for gRPC.
 
diff --git a/proto/toolchain.bzl b/proto/toolchain.bzl
index 67ccbe5..0f573ea 100644
--- a/proto/toolchain.bzl
+++ b/proto/toolchain.bzl
@@ -16,8 +16,21 @@
 
 load("@io_bazel_rules_rust//rust:private/rust.bzl", "name_to_crate_name")
 
-def generated_file_stem(f):
-    basename = f.rsplit("/", 2)[-1]
+def generated_file_stem(file_path):
+    """Returns the basename of a file without any extensions.
+
+    Example:
+    ```python
+    content.append("pub mod %s;" % _generated_file_stem(f))
+    ```
+
+    Args:
+        file_path (string): A path to a file
+
+    Returns:
+        string: The file stem of the filename
+    """
+    basename = file_path.rsplit("/", 2)[-1]
     basename = name_to_crate_name(basename)
     return basename.rsplit(".", 2)[0]
 
@@ -107,7 +120,9 @@ def _rust_proto_toolchain_impl(ctx):
     return platform_common.ToolchainInfo(
         protoc = ctx.executable.protoc,
         proto_plugin = ctx.file.proto_plugin,
+        proto_compile_deps = ctx.attr.proto_compile_deps,
         grpc_plugin = ctx.file.grpc_plugin,
+        grpc_compile_deps = ctx.attr.grpc_compile_deps,
         edition = ctx.attr.edition,
     )
 
@@ -123,9 +138,6 @@ GRPC_COMPILE_DEPS = PROTO_COMPILE_DEPS + [
     "@io_bazel_rules_rust//proto/raze:tls_api_stub",
 ]
 
-# TODO(damienmg): Once bazelbuild/bazel#6889 is fixed, reintroduce
-# proto_compile_deps and grpc_compile_deps and remove them from the
-# rust_proto_library and grpc_proto_library.
 rust_proto_toolchain = rule(
     implementation = _rust_proto_toolchain_impl,
     attrs = {
@@ -143,6 +155,11 @@ rust_proto_toolchain = rule(
                 "@io_bazel_rules_rust//proto:protoc_gen_rust",
             ),
         ),
+        "proto_compile_deps": attr.label_list(
+            doc = "The crates the generated protobuf libraries depends on.",
+            cfg = "target",
+            default = [Label(dep) for dep in PROTO_COMPILE_DEPS],
+        ),
         "grpc_plugin": attr.label(
             doc = "The location of the Rust protobuf compiler plugin to generate rust gRPC stubs.",
             allow_single_file = True,
@@ -151,6 +168,11 @@ rust_proto_toolchain = rule(
                 "@io_bazel_rules_rust//proto:protoc_gen_rust_grpc",
             ),
         ),
+        "grpc_compile_deps": attr.label_list(
+            doc = "The crates the generated grpc libraries depends on.",
+            cfg = "target",
+            default = [Label(dep) for dep in GRPC_COMPILE_DEPS],
+        ),
         "edition": attr.string(
             doc = "The edition used by the generated rust source.",
             default = "2015",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reintroduce proto_compile_deps and grpc_compile_deps to rust_proto_toolchain
2 participants