Skip to content

Commit 3dac61c

Browse files
committed
fix: code review
1 parent ab9a57f commit 3dac61c

File tree

5 files changed

+35
-22
lines changed

5 files changed

+35
-22
lines changed

docs/scala_proto_library.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ toolchain(
7878
| generators_opts | `List of strings, optional`<br/>Additional protobuf options like 'grpc', 'flat_package' or 'scala3_sources'. |
7979
| blacklisted_protos | `List of labels, optional`<br/>List of protobuf targets to exclude from recursive building. |
8080
| code_generator | `Label, optional (has default)`<br/>Which code generator to use. A sensible default is provided. |
81-
| named_generators | `String dict, optional` |
81+
| generators | `String dict, optional` |
8282
| extra_generator_dependencies | `List of labels, optional` |
8383
| scalac | `Label, optional (has default)`<br/>Target for scalac. A sensible default is provided. |
8484

scala_proto/scala_proto_toolchain.bzl

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def _worker_flags(ctx, generators, jars):
2727
return "--jvm_flags=" + " ".join(["-D%s=%s" % i for i in env.items()])
2828

2929
def _scala_proto_toolchain_impl(ctx):
30-
generators = ctx.attr.named_generators
30+
generators = ctx.attr.generators
3131
generators_jars = _generators_jars(ctx)
3232
compile_dep_ids = ["scalapb_compile_deps"]
3333
toolchain = platform_common.ToolchainInfo(
@@ -57,18 +57,7 @@ scala_proto_toolchain = rule(
5757
default = Label("//src/scala/scripts:scalapb_worker"),
5858
allow_files = True,
5959
),
60-
# `scripts.ScalaPbCodeGenerator` and `_main_generator_dep` are currently
61-
# necessary to support protoc-bridge < 0.9.8, specifically 0.7.14
62-
# required by Scala 2.11. See #1647 and scalapb/ScalaPB#1771.
63-
#
64-
# If we drop 2.11 support, restore `scalapb.ScalaPbCodeGenerator` here,
65-
# remove `_main_generator_dep`, and delete
66-
# `//src/scala/scripts:scalapb_codegenerator_wrapper` and its files.
67-
"named_generators": attr.string_dict(
68-
default = {
69-
"scala": "scripts.ScalaPbCodeGenerator",
70-
},
71-
),
60+
"generators": attr.string_dict(),
7261
"generators_opts": attr.string_list_dict(),
7362
"extra_generator_dependencies": attr.label_list(
7463
providers = [JavaInfo],
@@ -96,6 +85,13 @@ scala_proto_toolchain = rule(
9685
[proto rules documentation](https://docs.bazel.build/versions/master/be/protocol-buffer.html#proto_library)
9786
""",
9887
),
88+
# `scripts.ScalaPbCodeGenerator` and `_main_generator_dep` are currently
89+
# necessary to support protoc-bridge < 0.9.8, specifically 0.7.14
90+
# required by Scala 2.11. See #1647 and scalapb/ScalaPB#1771.
91+
#
92+
# If we drop 2.11 support, restore `scalapb.ScalaPbCodeGenerator` here,
93+
# remove `_main_generator_dep`, and delete
94+
# `//src/scala/scripts:scalapb_codegenerator_wrapper` and its files.
9995
"_main_generator_dep": attr.label(
10096
default = Label(
10197
"//src/scala/scripts:scalapb_codegenerator_wrapper",
@@ -107,6 +103,24 @@ scala_proto_toolchain = rule(
107103
},
108104
)
109105

106+
def scalapb_toolchain(name, opts = [], **kwargs):
107+
"""Setups default scala protobuf (scalapb) toolchain
108+
109+
Args:
110+
name: A unique name for this target
111+
opts: scalapb generator options like 'grpc' or 'flat_package'
112+
"""
113+
scala_proto_toolchain(
114+
name = name,
115+
generators = {
116+
"scala": "scripts.ScalaPbCodeGenerator",
117+
},
118+
generators_opts = {
119+
"scala": opts
120+
},
121+
**kwargs,
122+
)
123+
110124
def _scala_proto_deps_toolchain(ctx):
111125
toolchain = platform_common.ToolchainInfo(
112126
dep_providers = ctx.attr.dep_providers,
@@ -125,3 +139,4 @@ scala_proto_deps_toolchain = rule(
125139
)
126140

127141
scala_proto_deps_providers = _scala_proto_deps_providers
142+

scala_proto/toolchains.bzl

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
load(
22
"//scala_proto:scala_proto_toolchain.bzl",
33
"scala_proto_deps_toolchain",
4-
"scala_proto_toolchain",
4+
"scalapb_toolchain",
55
)
66

77
_default_gen_opts = [
@@ -36,12 +36,10 @@ def setup_scala_proto_toolchains(name, default_gen_opts = _default_gen_opts):
3636
toolchain_name = "%s_default_toolchain" % name
3737
toolchain_impl_name = "%s_default_toolchain_impl" % name
3838

39-
scala_proto_toolchain(
39+
scalapb_toolchain(
4040
name = toolchain_impl_name,
41+
opts = default_gen_opts,
4142
visibility = ["//visibility:public"],
42-
generators_opts = {
43-
"scala": default_gen_opts,
44-
},
4543
)
4644

4745
native.toolchain(

test/proto/BUILD

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ scala_proto_toolchain(
3434
"single_line_to_proto_string",
3535
],
3636
},
37-
named_generators = {
37+
generators = {
3838
"scala": "scripts.ScalaPbCodeGenerator",
3939
"jvm_extra_protobuf_generator": "scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator",
4040
},

test/proto/custom_generator/BUILD.bazel

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ toolchain(
6262

6363
scala_proto_toolchain(
6464
name = "scala_proto_toolchain_def",
65-
named_generators = {
65+
generators = {
6666
"scala": "test.proto.custom_generator.DummyGenerator",
6767
},
6868
)
@@ -102,7 +102,7 @@ toolchain(
102102

103103
scala_proto_toolchain(
104104
name = "failing_scala_proto_toolchain_def",
105-
named_generators = {
105+
generators = {
106106
"scala": "test.proto.custom_generator.FailingGenerator",
107107
},
108108
)

0 commit comments

Comments
 (0)