From 7cd602160baec001f353e8933771764fc6353f7d Mon Sep 17 00:00:00 2001 From: kczulko Date: Mon, 17 Mar 2025 10:32:36 +0100 Subject: [PATCH 01/12] feat: bit more configurable proto toolchain --- scala/toolchains.bzl | 4 +- scala/toolchains_repo.bzl | 8 ++-- scala_proto/default/default_deps.bzl | 3 +- scala_proto/private/scala_proto_aspect.bzl | 3 +- scala_proto/scala_proto_toolchain.bzl | 43 +++++----------------- scala_proto/toolchains.bzl | 26 +++++-------- test/proto/BUILD | 14 +++++-- 7 files changed, 39 insertions(+), 62 deletions(-) diff --git a/scala/toolchains.bzl b/scala/toolchains.bzl index a0d2e017e..dac91be51 100644 --- a/scala/toolchains.bzl +++ b/scala/toolchains.bzl @@ -42,7 +42,7 @@ def scala_toolchains( scalafmt = False, scalafmt_default_config_path = ".scalafmt.conf", scala_proto = False, - scala_proto_enable_all_options = False, + scala_proto_options = [], jmh = False, twitter_scrooge = False, twitter_scrooge_deps = {}): @@ -180,7 +180,7 @@ def scala_toolchains( specs2 = specs2, scalafmt = scalafmt, scala_proto = scala_proto, - scala_proto_enable_all_options = scala_proto_enable_all_options, + scala_proto_options = scala_proto_options, jmh = jmh, twitter_scrooge = twitter_scrooge, twitter_scrooge_deps = twitter_scrooge_deps, diff --git a/scala/toolchains_repo.bzl b/scala/toolchains_repo.bzl index 1e4c01b12..141c11ee3 100644 --- a/scala/toolchains_repo.bzl +++ b/scala/toolchains_repo.bzl @@ -46,7 +46,7 @@ def _scala_toolchains_repo_impl(repository_ctx): repo_attr = repository_ctx.attr format_args = { "rules_scala_repo": Label("//:all").repo_name, - "proto_enable_all_options": repo_attr.scala_proto_enable_all_options, + "proto_options": repo_attr.scala_proto_options, } toolchains = {} @@ -96,9 +96,9 @@ _scala_toolchains_repo = repository_rule( "scala_proto": attr.bool( doc = "Instantiate the scala_proto toolchain", ), - "scala_proto_enable_all_options": attr.bool( + "scala_proto_options": attr.string_list( doc = ( - "Enable all scala_proto_options; " + + "Protobuf generator options; " + "scala_proto must also be True for this to take effect" ), ), @@ -216,7 +216,7 @@ load( setup_scala_proto_toolchains( name = "scala_proto", - enable_all_options = {proto_enable_all_options}, + default_gen_opts = {proto_options}, ) declare_deps_provider( diff --git a/scala_proto/default/default_deps.bzl b/scala_proto/default/default_deps.bzl index a6a8945ab..fb9d2e925 100644 --- a/scala_proto/default/default_deps.bzl +++ b/scala_proto/default/default_deps.bzl @@ -17,9 +17,8 @@ _DEFAULT_DEP_PROVIDER_FORMAT = ( def scala_proto_deps_providers( compile = _DEFAULT_DEP_PROVIDER_FORMAT % "compile", - grpc = _DEFAULT_DEP_PROVIDER_FORMAT % "grpc", worker = _DEFAULT_DEP_PROVIDER_FORMAT % "worker"): - return [compile, grpc, worker] + return [compile, worker] DEFAULT_SCALAPB_COMPILE_DEPS = [ Label("//scala/private/toolchain_deps:scala_library_classpath"), diff --git a/scala_proto/private/scala_proto_aspect.bzl b/scala_proto/private/scala_proto_aspect.bzl index 7abb89b13..c05da751a 100644 --- a/scala_proto/private/scala_proto_aspect.bzl +++ b/scala_proto/private/scala_proto_aspect.bzl @@ -70,7 +70,8 @@ def _generate_sources(ctx, toolchain, proto): args.use_param_file(param_file_arg = "@%s", use_always = True) for gen, out in outputs.items(): args.add("--" + gen + "_out", out) - args.add("--" + gen + "_opt", toolchain.generators_opts) + if gen in toolchain.generators_opts: + args.add_all(toolchain.generators_opts[gen], format_each = "--{}_opt=%s".format(gen)) args.add_joined("--descriptor_set_in", descriptors, join_with = ctx.configuration.host_path_separator) args.add_all(sources) diff --git a/scala_proto/scala_proto_toolchain.bzl b/scala_proto/scala_proto_toolchain.bzl index b579c69d2..f13d9439f 100644 --- a/scala_proto/scala_proto_toolchain.bzl +++ b/scala_proto/scala_proto_toolchain.bzl @@ -4,12 +4,6 @@ load( _scala_proto_deps_providers = "scala_proto_deps_providers", ) -def _generators(ctx): - return dict( - ctx.attr.named_generators, - scala = ctx.attr.main_generator, - ) - def _generators_jars(ctx): generator_deps = ctx.attr.extra_generator_dependencies + [ ctx.attr._main_generator_dep, @@ -19,22 +13,6 @@ def _generators_jars(ctx): for dep in generator_deps ]) -def _generators_opts(ctx): - opts = [] - if ctx.attr.with_grpc: - opts.append("grpc") - if ctx.attr.with_flat_package: - opts.append("flat_package") - if ctx.attr.with_single_line_to_string: - opts.append("single_line_to_proto_string") - return ",".join(opts) - -def _compile_dep_ids(ctx): - deps = ["scalapb_compile_deps"] - if ctx.attr.with_grpc: - deps.append("scalapb_grpc_deps") - return deps - def _ignored_proto_targets_by_label(ctx): return {p.label: p for p in ctx.attr.blacklisted_protos} @@ -49,13 +27,14 @@ def _worker_flags(ctx, generators, jars): return "--jvm_flags=" + " ".join(["-D%s=%s" % i for i in env.items()]) def _scala_proto_toolchain_impl(ctx): - generators = _generators(ctx) + generators = ctx.attr.named_generators generators_jars = _generators_jars(ctx) + compile_dep_ids = ["scalapb_compile_deps"] toolchain = platform_common.ToolchainInfo( generators = generators, generators_jars = generators_jars, - generators_opts = _generators_opts(ctx), - compile_dep_ids = _compile_dep_ids(ctx), + generators_opts = ctx.attr.generators_opts, + compile_dep_ids = compile_dep_ids, blacklisted_protos = _ignored_proto_targets_by_label(ctx), protoc = ctx.executable.protoc, scalac = ctx.attr.scalac.files_to_run, @@ -66,17 +45,11 @@ def _scala_proto_toolchain_impl(ctx): return [toolchain] # Args: -# with_grpc: Enables generation of grpc service bindings for services -# with_flat_package: When true, ScalaPB will not append the protofile base name to the package name -# with_single_line_to_string: Enables generation of toString() methods that use the single line format # blacklisted_protos: list of protobuf targets to exclude from recursive building # code_generator: what code generator to use, usually you'll want the default scala_proto_toolchain = rule( implementation = _scala_proto_toolchain_impl, attrs = { - "with_grpc": attr.bool(), - "with_flat_package": attr.bool(), - "with_single_line_to_string": attr.bool(), "blacklisted_protos": attr.label_list(default = []), "code_generator": attr.label( executable = True, @@ -91,10 +64,12 @@ scala_proto_toolchain = rule( # If we drop 2.11 support, restore `scalapb.ScalaPbCodeGenerator` here, # remove `_main_generator_dep`, and delete # `//src/scala/scripts:scalapb_codegenerator_wrapper` and its files. - "main_generator": attr.string( - default = "scripts.ScalaPbCodeGenerator", + "named_generators": attr.string_dict( + default = { + "scala": "scripts.ScalaPbCodeGenerator", + }, ), - "named_generators": attr.string_dict(), + "generators_opts": attr.string_list_dict(), "extra_generator_dependencies": attr.label_list( providers = [JavaInfo], ), diff --git a/scala_proto/toolchains.bzl b/scala_proto/toolchains.bzl index 866ee19f6..e3c14a310 100644 --- a/scala_proto/toolchains.bzl +++ b/scala_proto/toolchains.bzl @@ -4,7 +4,11 @@ load( "scala_proto_toolchain", ) -def setup_scala_proto_toolchains(name, enable_all_options = False): +_default_gen_opts = [ + "grpc", +] + +def setup_scala_proto_toolchains(name, default_gen_opts = _default_gen_opts): """Used by @rules_scala_toolchains//scala_proto/BUILD. See //scala/private:macros/toolchains_repo.bzl for details, especially the @@ -12,14 +16,13 @@ def setup_scala_proto_toolchains(name, enable_all_options = False): Args: name: prefix for all generate toolchains - enable_all_options: set to True to enable the `with_flat_package` and - `with_single_line_to_string` attributes of `scala_proto_toolchain` + default_gen_opts: parameters passed to the default generator """ scala_proto_deps_toolchain( name = "%s_default_deps_toolchain_impl" % name, dep_providers = [ ":scalapb_%s_deps_provider" % p - for p in ["compile", "grpc", "worker"] + for p in ["compile", "worker"] ], visibility = ["//visibility:public"], ) @@ -32,22 +35,13 @@ def setup_scala_proto_toolchains(name, enable_all_options = False): toolchain_name = "%s_default_toolchain" % name toolchain_impl_name = "%s_default_toolchain_impl" % name - toolchain_options = { - "with_flat_package": False, - "with_grpc": True, - "with_single_line_to_string": False, - } - - if enable_all_options: - toolchain_name = "%s_enable_all_options_toolchain" % name - toolchain_impl_name = "%s_enable_all_options_toolchain_impl" % name - toolchain_options["with_flat_package"] = True - toolchain_options["with_single_line_to_string"] = True scala_proto_toolchain( name = toolchain_impl_name, visibility = ["//visibility:public"], - **toolchain_options + generators_opts = { + "scala": default_gen_opts, + }, ) native.toolchain( diff --git a/test/proto/BUILD b/test/proto/BUILD index 751edd6f8..222494c11 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -24,13 +24,21 @@ scala_proto_toolchain( extra_generator_dependencies = [ "//test/src/main/scala/scalarules/test/extra_protobuf_generator", ], + generators_opts = { + "scala": [ + "grpc", + "single_line_to_proto_string", + ], + "jvm_extra_protobuf_generator": [ + "grpc", + "single_line_to_proto_string", + ], + }, named_generators = { + "scala": "scripts.ScalaPbCodeGenerator", "jvm_extra_protobuf_generator": "scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator", }, visibility = ["//visibility:public"], - with_flat_package = False, - with_grpc = True, - with_single_line_to_string = True, ) toolchain( From 7001dd39d354f1a132006432f98d5cd798e3eb56 Mon Sep 17 00:00:00 2001 From: kczulko Date: Mon, 17 Mar 2025 11:10:14 +0100 Subject: [PATCH 02/12] fix: tests --- test/proto/custom_generator/BUILD.bazel | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/proto/custom_generator/BUILD.bazel b/test/proto/custom_generator/BUILD.bazel index a4585d0e5..eae296d14 100644 --- a/test/proto/custom_generator/BUILD.bazel +++ b/test/proto/custom_generator/BUILD.bazel @@ -62,7 +62,9 @@ toolchain( scala_proto_toolchain( name = "scala_proto_toolchain_def", - main_generator = "test.proto.custom_generator.DummyGenerator", + named_generators = { + "scala": "test.proto.custom_generator.DummyGenerator", + }, ) toolchain( @@ -100,7 +102,9 @@ toolchain( scala_proto_toolchain( name = "failing_scala_proto_toolchain_def", - main_generator = "test.proto.custom_generator.FailingGenerator", + named_generators = { + "scala": "test.proto.custom_generator.FailingGenerator" + }, ) toolchain( From 9a43a736acc3ec9df17f23379e467148d564d92f Mon Sep 17 00:00:00 2001 From: kczulko Date: Mon, 17 Mar 2025 13:57:30 +0100 Subject: [PATCH 03/12] fix: grpc deps --- scala/toolchains_repo.bzl | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/scala/toolchains_repo.bzl b/scala/toolchains_repo.bzl index 141c11ee3..3fd256a56 100644 --- a/scala/toolchains_repo.bzl +++ b/scala/toolchains_repo.bzl @@ -223,14 +223,7 @@ declare_deps_provider( name = "scalapb_compile_deps_provider", deps_id = "scalapb_compile_deps", visibility = ["//visibility:public"], - deps = DEFAULT_SCALAPB_COMPILE_DEPS, -) - -declare_deps_provider( - name = "scalapb_grpc_deps_provider", - deps_id = "scalapb_grpc_deps", - visibility = ["//visibility:public"], - deps = DEFAULT_SCALAPB_GRPC_DEPS, + deps = DEFAULT_SCALAPB_COMPILE_DEPS + DEFAULT_SCALAPB_GRPC_DEPS, ) declare_deps_provider( From 8557ee57214a62cda5c942eb3459156e88be962d Mon Sep 17 00:00:00 2001 From: kczulko Date: Mon, 17 Mar 2025 15:46:38 +0100 Subject: [PATCH 04/12] fix: test_version suite --- scala/toolchains.bzl | 5 ++--- test_version/WORKSPACE.template | 1 + 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scala/toolchains.bzl b/scala/toolchains.bzl index dac91be51..8d041a871 100644 --- a/scala/toolchains.bzl +++ b/scala/toolchains.bzl @@ -84,9 +84,8 @@ def scala_toolchains( scalafmt_default_config_path: the relative path to the default Scalafmt config file within the repository scala_proto: whether to instantiate the scala_proto toolchain - scala_proto_enable_all_options: whether to instantiate the scala_proto - toolchain with all options enabled; `scala_proto` must also be - `True` for this to take effect + scala_proto_options: protobuf options, like 'scala3_sources' or 'grpc'; + `scala_proto` must also be `True` for this to take effect jmh: whether to instantiate the Java Microbenchmarks Harness toolchain twitter_scrooge: whether to instantiate the twitter_scrooge toolchain twitter_scrooge_deps: dictionary of string to Label containing overrides diff --git a/test_version/WORKSPACE.template b/test_version/WORKSPACE.template index 77101c65c..e367fc3aa 100644 --- a/test_version/WORKSPACE.template +++ b/test_version/WORKSPACE.template @@ -65,6 +65,7 @@ load( scala_toolchains( fetch_sources = True, scala_proto = True, + scala_proto_options = ["grpc"], scalatest = True, specs2 = True, ) From 6073b6524fefb92c17e2214a2d0f6b565fb5a3e0 Mon Sep 17 00:00:00 2001 From: kczulko Date: Mon, 17 Mar 2025 16:25:07 +0100 Subject: [PATCH 05/12] fix: linting --- test/proto/custom_generator/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/proto/custom_generator/BUILD.bazel b/test/proto/custom_generator/BUILD.bazel index eae296d14..e3f7f998c 100644 --- a/test/proto/custom_generator/BUILD.bazel +++ b/test/proto/custom_generator/BUILD.bazel @@ -103,7 +103,7 @@ toolchain( scala_proto_toolchain( name = "failing_scala_proto_toolchain_def", named_generators = { - "scala": "test.proto.custom_generator.FailingGenerator" + "scala": "test.proto.custom_generator.FailingGenerator", }, ) From 58763d0ce80969cc59143aa18c7974613c6a657b Mon Sep 17 00:00:00 2001 From: kczulko Date: Tue, 18 Mar 2025 16:36:57 +0100 Subject: [PATCH 06/12] fix: docs --- README.md | 2 +- docs/scala_proto_library.md | 29 ++++++++++++++--------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index e03f99ff2..b47695a28 100644 --- a/README.md +++ b/README.md @@ -494,7 +494,7 @@ with the following `scala_toolchains()` parameters: ```py scala_toolchains( scala_proto = True, - scala_proto_enable_all_options = True, + scala_proto_options = [], ) ``` diff --git a/docs/scala_proto_library.md b/docs/scala_proto_library.md index 4ebe76bcb..33f2d3614 100644 --- a/docs/scala_proto_library.md +++ b/docs/scala_proto_library.md @@ -7,6 +7,11 @@ which adds a few dependencies needed for ScalaPB: scala_toolchains( # Other toolchains settings... scala_proto = True, + scala_proto_options = [ + "grpc", + "flat_package", + "scala3_sources", + ], ) scala_register_toolchains() @@ -47,9 +52,13 @@ load( scala_proto_toolchain( name = "scala_proto_toolchain", - with_grpc = False, - with_flat_package = False, - with_single_line_to_string = False, + generators_opts = { + "scala": [ + "grpc", + "flat_package", + "scala3_sources", + ] + }, visibility = ["//visibility:public"], ) @@ -66,9 +75,7 @@ toolchain( | Attribute name | Description | | ----------------------------- | ----------------------------------------------------- | | name | `Name, required`
A unique name for this toolchain. | -| with_grpc | `boolean, optional (default False)`
Enables generation of grpc service bindings for services. | -| with_flat_package | `boolean, optional (default False)`
When true, ScalaPB will not append the protofile base name to the package name. | -| with_single_line_to_string | `boolean, optional (default False)`
Enables generation of toString() methods that use a single line format. | +| generators_opts | `List of strings, optional`
Additional protobuf options like 'grpc', 'flat_package' or 'scala3_sources'. | | blacklisted_protos | `List of labels, optional`
List of protobuf targets to exclude from recursive building. | | code_generator | `Label, optional (has default)`
Which code generator to use. A sensible default is provided. | | named_generators | `String dict, optional` | @@ -89,7 +96,6 @@ scala_proto_deps_toolchain( visibility = ["//visibility:public"], dep_providers = [ ":my_compile_deps", - ":my_grpc_deps", ], ) @@ -105,17 +111,10 @@ declare_deps_provider( deps = ["@dep1", "@dep2"], visibility = ["//visibility:public"], ) - -declare_deps_provider( - name = "my_grpc_deps", - deps_id = "scalapb_grpc_deps", - deps = ["@dep3", "@dep4"], - visibility = ["//visibility:public"], -) ``` ### `scala_proto_deps_toolchain` Toolchain Attributes | Attribute name | Description | | ----------------------------- | ----------------------------------------------------- | -| dep_providers | `List of labels, optional (has default)`
allows injection of gRPC (deps_id - `scalapb_grpc_deps`) and ScalaPB (deps_id `scalapb_compile_deps`) dependencies | +| dep_providers | `List of labels, optional (has default)`
allows injection of gRPC (deps_id - `scalapb_worker_deps`) and ScalaPB (deps_id `scalapb_compile_deps`) dependencies | From 5d4c0d7e9dd6d396664574c32b485518e91000cd Mon Sep 17 00:00:00 2001 From: kczulko Date: Wed, 26 Mar 2025 13:25:54 +0100 Subject: [PATCH 07/12] fix: code review --- docs/scala_proto_library.md | 2 +- scala_proto/scala_proto_toolchain.bzl | 41 +++++++++++++++++-------- scala_proto/toolchains.bzl | 8 ++--- test/proto/BUILD | 2 +- test/proto/custom_generator/BUILD.bazel | 4 +-- 5 files changed, 35 insertions(+), 22 deletions(-) diff --git a/docs/scala_proto_library.md b/docs/scala_proto_library.md index 33f2d3614..ab77b6077 100644 --- a/docs/scala_proto_library.md +++ b/docs/scala_proto_library.md @@ -78,7 +78,7 @@ toolchain( | generators_opts | `List of strings, optional`
Additional protobuf options like 'grpc', 'flat_package' or 'scala3_sources'. | | blacklisted_protos | `List of labels, optional`
List of protobuf targets to exclude from recursive building. | | code_generator | `Label, optional (has default)`
Which code generator to use. A sensible default is provided. | -| named_generators | `String dict, optional` | +| generators | `String dict, optional` | | extra_generator_dependencies | `List of labels, optional` | | scalac | `Label, optional (has default)`
Target for scalac. A sensible default is provided. | diff --git a/scala_proto/scala_proto_toolchain.bzl b/scala_proto/scala_proto_toolchain.bzl index f13d9439f..60325e234 100644 --- a/scala_proto/scala_proto_toolchain.bzl +++ b/scala_proto/scala_proto_toolchain.bzl @@ -27,7 +27,7 @@ def _worker_flags(ctx, generators, jars): return "--jvm_flags=" + " ".join(["-D%s=%s" % i for i in env.items()]) def _scala_proto_toolchain_impl(ctx): - generators = ctx.attr.named_generators + generators = ctx.attr.generators generators_jars = _generators_jars(ctx) compile_dep_ids = ["scalapb_compile_deps"] toolchain = platform_common.ToolchainInfo( @@ -57,18 +57,7 @@ scala_proto_toolchain = rule( default = Label("//src/scala/scripts:scalapb_worker"), allow_files = True, ), - # `scripts.ScalaPbCodeGenerator` and `_main_generator_dep` are currently - # necessary to support protoc-bridge < 0.9.8, specifically 0.7.14 - # required by Scala 2.11. See #1647 and scalapb/ScalaPB#1771. - # - # If we drop 2.11 support, restore `scalapb.ScalaPbCodeGenerator` here, - # remove `_main_generator_dep`, and delete - # `//src/scala/scripts:scalapb_codegenerator_wrapper` and its files. - "named_generators": attr.string_dict( - default = { - "scala": "scripts.ScalaPbCodeGenerator", - }, - ), + "generators": attr.string_dict(), "generators_opts": attr.string_list_dict(), "extra_generator_dependencies": attr.label_list( providers = [JavaInfo], @@ -96,6 +85,13 @@ scala_proto_toolchain = rule( [proto rules documentation](https://docs.bazel.build/versions/master/be/protocol-buffer.html#proto_library) """, ), + # `scripts.ScalaPbCodeGenerator` and `_main_generator_dep` are currently + # necessary to support protoc-bridge < 0.9.8, specifically 0.7.14 + # required by Scala 2.11. See #1647 and scalapb/ScalaPB#1771. + # + # If we drop 2.11 support, restore `scalapb.ScalaPbCodeGenerator` here, + # remove `_main_generator_dep`, and delete + # `//src/scala/scripts:scalapb_codegenerator_wrapper` and its files. "_main_generator_dep": attr.label( default = Label( "//src/scala/scripts:scalapb_codegenerator_wrapper", @@ -107,6 +103,24 @@ scala_proto_toolchain = rule( }, ) +def scalapb_toolchain(name, opts = [], **kwargs): + """Setups default scala protobuf (scalapb) toolchain + + Args: + name: A unique name for this target + opts: scalapb generator options like 'grpc' or 'flat_package' + """ + scala_proto_toolchain( + name = name, + generators = { + "scala": "scripts.ScalaPbCodeGenerator", + }, + generators_opts = { + "scala": opts + }, + **kwargs, + ) + def _scala_proto_deps_toolchain(ctx): toolchain = platform_common.ToolchainInfo( dep_providers = ctx.attr.dep_providers, @@ -125,3 +139,4 @@ scala_proto_deps_toolchain = rule( ) scala_proto_deps_providers = _scala_proto_deps_providers + diff --git a/scala_proto/toolchains.bzl b/scala_proto/toolchains.bzl index e3c14a310..a315732a4 100644 --- a/scala_proto/toolchains.bzl +++ b/scala_proto/toolchains.bzl @@ -1,7 +1,7 @@ load( "//scala_proto:scala_proto_toolchain.bzl", "scala_proto_deps_toolchain", - "scala_proto_toolchain", + "scalapb_toolchain", ) _default_gen_opts = [ @@ -36,12 +36,10 @@ def setup_scala_proto_toolchains(name, default_gen_opts = _default_gen_opts): toolchain_name = "%s_default_toolchain" % name toolchain_impl_name = "%s_default_toolchain_impl" % name - scala_proto_toolchain( + scalapb_toolchain( name = toolchain_impl_name, + opts = default_gen_opts, visibility = ["//visibility:public"], - generators_opts = { - "scala": default_gen_opts, - }, ) native.toolchain( diff --git a/test/proto/BUILD b/test/proto/BUILD index 222494c11..78091c902 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -34,7 +34,7 @@ scala_proto_toolchain( "single_line_to_proto_string", ], }, - named_generators = { + generators = { "scala": "scripts.ScalaPbCodeGenerator", "jvm_extra_protobuf_generator": "scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator", }, diff --git a/test/proto/custom_generator/BUILD.bazel b/test/proto/custom_generator/BUILD.bazel index e3f7f998c..261301ac6 100644 --- a/test/proto/custom_generator/BUILD.bazel +++ b/test/proto/custom_generator/BUILD.bazel @@ -62,7 +62,7 @@ toolchain( scala_proto_toolchain( name = "scala_proto_toolchain_def", - named_generators = { + generators = { "scala": "test.proto.custom_generator.DummyGenerator", }, ) @@ -102,7 +102,7 @@ toolchain( scala_proto_toolchain( name = "failing_scala_proto_toolchain_def", - named_generators = { + generators = { "scala": "test.proto.custom_generator.FailingGenerator", }, ) From b6c36956961906672bfc6ce38fd08f619adc20c2 Mon Sep 17 00:00:00 2001 From: kczulko Date: Wed, 26 Mar 2025 13:39:15 +0100 Subject: [PATCH 08/12] chore: README.md section for scala_proto_toolchain changes --- README.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/README.md b/README.md index b47695a28..ae2185701 100644 --- a/README.md +++ b/README.md @@ -819,6 +819,35 @@ under WORKSPACE](#6.5.0), with the maximum dependency versions specified in that section. While this may continue to work for some time, it is not officially supported. +### `scala_proto_toolchain` changes + +Since #1718 `scala_proto_toolchain`'s `main_generator` was removed in flavor of more +flexible protobuf plugins configuration. Now each generator (plugin) will get a corresponding name +that can be used for further plugin options setup: + +``` +scala_proto_toolchain( + name = "example", + generators_opts = { + "scala": [ + "grpc", + "single_line_to_proto_string", + ], + "jvm_extra_protobuf_generator": [ + "grpc", + "single_line_to_proto_string", + ], + }, + generators = { + "scala": "scripts.ScalaPbCodeGenerator", + "jvm_extra_protobuf_generator": "scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator", + }, +) +``` + +Also, `scalapb_grpc_deps` was removed since this changes moves the responsibility +on the user side to configure dependencies based on the provided generators and their options. + ### Removal of `bind()` aliases for `twitter_scrooge` dependencies `rules_scala` 7.x removes all of the obsolete [`bind()`][] aliases under From f08dd28275ff5ede4a87572dec779af334d544ae Mon Sep 17 00:00:00 2001 From: kczulko Date: Wed, 26 Mar 2025 13:46:28 +0100 Subject: [PATCH 09/12] fix: better attrs ordering for the README example --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index ae2185701..376c5b6fb 100644 --- a/README.md +++ b/README.md @@ -828,6 +828,10 @@ that can be used for further plugin options setup: ``` scala_proto_toolchain( name = "example", + generators = { + "scala": "scripts.ScalaPbCodeGenerator", + "jvm_extra_protobuf_generator": "scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator", + }, generators_opts = { "scala": [ "grpc", @@ -838,10 +842,6 @@ scala_proto_toolchain( "single_line_to_proto_string", ], }, - generators = { - "scala": "scripts.ScalaPbCodeGenerator", - "jvm_extra_protobuf_generator": "scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator", - }, ) ``` From f82d0ae5b2f9b8ab95779b5bc5183d579c43aec7 Mon Sep 17 00:00:00 2001 From: kczulko Date: Wed, 26 Mar 2025 14:11:05 +0100 Subject: [PATCH 10/12] fix: failing test + linter --- scala_proto/scala_proto_toolchain.bzl | 5 ++--- test/proto/BUILD | 8 ++++---- .../missing_direct_deps/scala_proto_deps/BUILD | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/scala_proto/scala_proto_toolchain.bzl b/scala_proto/scala_proto_toolchain.bzl index 60325e234..6bea53688 100644 --- a/scala_proto/scala_proto_toolchain.bzl +++ b/scala_proto/scala_proto_toolchain.bzl @@ -116,9 +116,9 @@ def scalapb_toolchain(name, opts = [], **kwargs): "scala": "scripts.ScalaPbCodeGenerator", }, generators_opts = { - "scala": opts + "scala": opts, }, - **kwargs, + **kwargs ) def _scala_proto_deps_toolchain(ctx): @@ -139,4 +139,3 @@ scala_proto_deps_toolchain = rule( ) scala_proto_deps_providers = _scala_proto_deps_providers - diff --git a/test/proto/BUILD b/test/proto/BUILD index 78091c902..7e01ec55f 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -24,6 +24,10 @@ scala_proto_toolchain( extra_generator_dependencies = [ "//test/src/main/scala/scalarules/test/extra_protobuf_generator", ], + generators = { + "scala": "scripts.ScalaPbCodeGenerator", + "jvm_extra_protobuf_generator": "scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator", + }, generators_opts = { "scala": [ "grpc", @@ -34,10 +38,6 @@ scala_proto_toolchain( "single_line_to_proto_string", ], }, - generators = { - "scala": "scripts.ScalaPbCodeGenerator", - "jvm_extra_protobuf_generator": "scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator", - }, visibility = ["//visibility:public"], ) diff --git a/test_expect_failure/missing_direct_deps/scala_proto_deps/BUILD b/test_expect_failure/missing_direct_deps/scala_proto_deps/BUILD index e3173d0dd..44b0596b1 100644 --- a/test_expect_failure/missing_direct_deps/scala_proto_deps/BUILD +++ b/test_expect_failure/missing_direct_deps/scala_proto_deps/BUILD @@ -2,7 +2,7 @@ load("@rules_proto//proto:defs.bzl", "proto_library") load("@rules_java//java:defs.bzl", "java_library") load("//scala:scala.bzl", "scala_library") load("//scala_proto:scala_proto.bzl", "scala_proto_library") -load("//scala_proto:scala_proto_toolchain.bzl", "scala_proto_toolchain") +load("//scala_proto:scala_proto_toolchain.bzl", "scalapb_toolchain") load(":customized_scala_proto.bzl", "custom_stamping_convention", "custom_stamping_scala_proto_library") proto_library( @@ -28,7 +28,7 @@ java_library( deps = [":some_scala_proto"], ) -scala_proto_toolchain( +scalapb_toolchain( name = "stamp_by_convention_toolchain_impl", stamp_by_convention = True, ) From 78acc0062230231b711940ee5f6e49bd96b9abab Mon Sep 17 00:00:00 2001 From: kczulko Date: Wed, 26 Mar 2025 15:19:19 +0100 Subject: [PATCH 11/12] fix: README.md and docstring --- README.md | 38 ++++++++++++++++++++++----- scala_proto/scala_proto_toolchain.bzl | 7 ++--- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 376c5b6fb..c9e1cab3a 100644 --- a/README.md +++ b/README.md @@ -819,13 +819,22 @@ under WORKSPACE](#6.5.0), with the maximum dependency versions specified in that section. While this may continue to work for some time, it is not officially supported. -### `scala_proto_toolchain` changes +### `scala_proto_toolchain` changes and new `scalapb_toolchain` macro -Since #1718 `scala_proto_toolchain`'s `main_generator` was removed in flavor of more -flexible protobuf plugins configuration. Now each generator (plugin) will get a corresponding name +`scala_proto_toolchain` has a more flexible plugin configuration schema. The +new `generators` and `generators_opts` attributes replace the following +attributes: + +- `with_grpc` +- `with_flat_package` +- `with_single_line_to_string` +- `main_generator` +- `named_generators` + +Now each generator (plugin) will get a corresponding name that can be used for further plugin options setup: -``` +```py scala_proto_toolchain( name = "example", generators = { @@ -845,8 +854,25 @@ scala_proto_toolchain( ) ``` -Also, `scalapb_grpc_deps` was removed since this changes moves the responsibility -on the user side to configure dependencies based on the provided generators and their options. +`scalapb_grpc_deps` no longer exists since it's now the user's responsibility +to configure dependencies based on the provided generators and their options. + +The new `scalapb_toolchain` convenience macro wraps `scala_proto_toolchain` +to provide the default [ScalaPB](https://scalapb.github.io/) implementation: + +.``py +load("//scala_proto:scala_proto_toolchain.bzl", "scalapb_toolchain") + +scalapb_toolchain( + name = "my_toolchain", + opts = [ + "grpc", + "single_line_to_proto_string", + ], + visibility = ["//visibility:public"], +) +.`` + ### Removal of `bind()` aliases for `twitter_scrooge` dependencies diff --git a/scala_proto/scala_proto_toolchain.bzl b/scala_proto/scala_proto_toolchain.bzl index 6bea53688..51bcb6e55 100644 --- a/scala_proto/scala_proto_toolchain.bzl +++ b/scala_proto/scala_proto_toolchain.bzl @@ -104,11 +104,12 @@ scala_proto_toolchain = rule( ) def scalapb_toolchain(name, opts = [], **kwargs): - """Setups default scala protobuf (scalapb) toolchain + """Sets up a scala_proto_toolchain using ScalaPB. Args: - name: A unique name for this target - opts: scalapb generator options like 'grpc' or 'flat_package' + name: A unique name for this target + opts: scalapb generator options like 'grpc' or 'flat_package' + kwargs: remaining arguments to `scala_proto_toolchain` """ scala_proto_toolchain( name = name, From 3c25cdfaa3007fd2dc0926a017ffb495c08e9ee5 Mon Sep 17 00:00:00 2001 From: kczulko Date: Wed, 26 Mar 2025 18:44:57 +0100 Subject: [PATCH 12/12] fix: code fence --- README.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c9e1cab3a..cca4f0073 100644 --- a/README.md +++ b/README.md @@ -860,7 +860,7 @@ to configure dependencies based on the provided generators and their options. The new `scalapb_toolchain` convenience macro wraps `scala_proto_toolchain` to provide the default [ScalaPB](https://scalapb.github.io/) implementation: -.``py +```py load("//scala_proto:scala_proto_toolchain.bzl", "scalapb_toolchain") scalapb_toolchain( @@ -871,8 +871,7 @@ scalapb_toolchain( ], visibility = ["//visibility:public"], ) -.`` - +``` ### Removal of `bind()` aliases for `twitter_scrooge` dependencies