From 4c2889c4c9e1b2adb24ce904cbc32a318006f0b8 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Mon, 15 Jul 2024 18:35:33 -0600 Subject: [PATCH 01/10] Split _run and _test rules --- rules/proto_compile.bzl | 40 +++++++--- rules/proto_compile_gencopy.bzl | 127 ++++++++++++++++++++++++------- rules/proto_compiled_sources.bzl | 2 + rules/providers.bzl | 5 +- 4 files changed, 136 insertions(+), 38 deletions(-) diff --git a/rules/proto_compile.bzl b/rules/proto_compile.bzl index 4347b8fbb..630f2f0bb 100644 --- a/rules/proto_compile.bzl +++ b/rules/proto_compile.bzl @@ -93,18 +93,18 @@ def _strip_path_prefix(path, prefix): def is_windows(ctx): return ctx.configuration.host_path_separator == ";" +def _make_output_filename(ctx, name): + return name + ctx.attr.output_suffix + def _proto_compile_impl(ctx): # mut > outputs = [] + ctx.outputs.outputs - # mut If defined, we are using the srcs to predict the outputs - # srcgen_ext = None - if len(ctx.attr.srcs) > 0: - if len(ctx.outputs.outputs) > 0: - fail("rule must provide 'srcs' or 'outputs', but not both") + if len(ctx.attr.srcs) > 0 and len(ctx.outputs.outputs) > 0: + fail("rule must provide 'srcs' or 'outputs' (but not both)") - # srcgen_ext = ctx.attr.srcgen_ext - outputs = [ctx.actions.declare_file(name) for name in ctx.attr.srcs] + if len(ctx.attr.srcs) > 0: + outputs = [ctx.actions.declare_file(_make_output_filename(ctx, name)) for name in ctx.attr.srcs] ### ### Part 1: setup variables used in scope @@ -343,10 +343,23 @@ def _proto_compile_impl(ctx): env = {"BAZEL_BINDIR": ctx.bin_dir.path}, ) - return [ - ProtoCompileInfo(label = ctx.label, outputs = outputs), - DefaultInfo(files = depset(outputs)), + # if not ctx.attr.default_info: + # pass + + # comprehend a mapping of relpath -> File + output_file_map = {f.short_path[len(ctx.label.package):].lstrip("/"): f for f in outputs} + + providers = [ + ProtoCompileInfo( + label = ctx.label, + outputs = outputs, + output_file_map = output_file_map, + ), ] + if ctx.attr.default_info: + providers.append(DefaultInfo(files = depset(outputs))) + + return providers proto_compile = rule( implementation = _proto_compile_impl, @@ -360,6 +373,9 @@ proto_compile = rule( "srcs": attr.string_list( doc = "List of source files we expect to be regenerated (relative to package)", ), + "output_suffix": attr.string( + doc = "An optional suffix to be appended to output files", + ), "plugins": attr.label_list( doc = "List of ProtoPluginInfo providers", mandatory = True, @@ -388,6 +404,10 @@ proto_compile = rule( "verbose": attr.bool( doc = "The verbosity flag.", ), + "default_info": attr.bool( + doc = "If false, do not return the DefaultInfo provider", + default = True, + ), }, toolchains = ["@build_stack_rules_proto//toolchain:protoc"], ) diff --git a/rules/proto_compile_gencopy.bzl b/rules/proto_compile_gencopy.bzl index 975debd02..c9baf940c 100644 --- a/rules/proto_compile_gencopy.bzl +++ b/rules/proto_compile_gencopy.bzl @@ -22,7 +22,7 @@ def _copy_file(actions, src, dst): progress_message = "copying {} to {}".format(src.path, dst.path), ) -def _proto_compile_gencopy_impl(ctx): +def _proto_compile_gencopy_run_impl(ctx): config = gencopy_config(ctx) runfiles = [] @@ -31,13 +31,15 @@ def _proto_compile_gencopy_impl(ctx): srcfiles = {f.short_path[len(ctx.label.package):].lstrip("/"): f for f in ctx.files.srcs} for info in [dep[ProtoCompileInfo] for dep in ctx.attr.deps]: - runfiles += info.outputs + runfiles += info.output_file_map.values() + srcfiles = info.output_file_map srcs = [] # list of string for f in info.outputs: if config.mode == "check": - # if we are in 'check' mode, the src and dst cannot be the same file, so - # make a copy of it... but first, we need to find it in the srcs files! + # if we are in 'check' mode, the src and dst cannot be the same + # file, so make a copy of it... but first, we need to find it + # in the srcs files! found = False for srcfilename, srcfile in srcfiles.items(): if srcfilename == f.basename: @@ -76,27 +78,100 @@ def _proto_compile_gencopy_impl(ctx): executable = script, )] -def _proto_compile_gencopy_rule(is_test): - return rule( - implementation = _proto_compile_gencopy_impl, - attrs = dict( - gencopy_attrs, - deps = attr.label_list( - doc = "The ProtoCompileInfo providers", - providers = [ProtoCompileInfo], - ), - srcs = attr.label_list( - doc = "The source files", - allow_files = True, - ), - extension = attr.string( - doc = "optional file extension to add to the copied file", - mandatory = False, - ), +proto_compile_gencopy_run = rule( + implementation = _proto_compile_gencopy_run_impl, + attrs = dict( + gencopy_attrs, + deps = attr.label_list( + doc = "The ProtoCompileInfo providers", + providers = [ProtoCompileInfo], ), - executable = True, - test = is_test, - ) + srcs = attr.label_list( + doc = "The source files", + allow_files = True, + ), + extension = attr.string( + doc = "optional file extension to add to the copied file", + mandatory = False, + ), + ), + executable = True, + test = is_test, +) + +def _proto_compile_gencopy_test_impl(ctx): + config = gencopy_config(ctx) + + runfiles = [] + + # comprehend a mapping of relpath -> File + srcfiles = {f.short_path[len(ctx.label.package):].lstrip("/"): f for f in ctx.files.srcs} + + for info in [dep[ProtoCompileInfo] for dep in ctx.attr.deps]: + runfiles += info.output_file_map.values() + srcfiles = info.output_file_map -proto_compile_gencopy_test = _proto_compile_gencopy_rule(True) -proto_compile_gencopy_run = _proto_compile_gencopy_rule(False) + srcs = [] # list of string + for f in info.outputs: + if config.mode == "check": + # if we are in 'check' mode, the src and dst cannot be the same + # file, so make a copy of it... but first, we need to find it + # in the srcs files! + found = False + for srcfilename, srcfile in srcfiles.items(): + if srcfilename == f.basename: + replica = ctx.actions.declare_file(f.basename + ".actual", sibling = f) + _copy_file(ctx.actions, srcfile, replica) + runfiles.append(replica) + srcs.append(replica.short_path) + found = True + break + elif srcfilename == f.basename + ctx.attr.extension: + runfiles.append(srcfile) + srcs.append(srcfile.short_path) + found = True + break + if not found: + fail("could not find matching source file for generated file %s in %r" % (f.basename, srcfiles)) + + else: + srcs.append(f.short_path) + + config.packageConfigs.append( + struct( + targetLabel = str(info.label), + targetPackage = info.label.package, + targetWorkspaceRoot = info.label.workspace_root, + generatedFiles = [f.short_path for f in info.outputs], + sourceFiles = srcs, + ), + ) + + config_json, script, runfiles = gencopy_action(ctx, config, runfiles) + + return [DefaultInfo( + files = depset([config_json]), + runfiles = runfiles, + executable = script, + )] + +proto_compile_gencopy_rule_test = rule( + implementation = _proto_compile_gencopy_test_impl, + attrs = dict( + gencopy_attrs, + deps = attr.label_list( + doc = "The ProtoCompileInfo providers", + providers = [ProtoCompileInfo], + ), + srcs = attr.label_list( + doc = "The source files", + allow_files = True, + ), + extension = attr.string( + doc = "optional file extension to add to the copied file", + mandatory = False, + ), + ), + executable = True, + test = True, +) diff --git a/rules/proto_compiled_sources.bzl b/rules/proto_compiled_sources.bzl index f6f9c7c42..c13c57b46 100644 --- a/rules/proto_compiled_sources.bzl +++ b/rules/proto_compiled_sources.bzl @@ -32,6 +32,8 @@ def proto_compiled_sources(**kwargs): name = name, srcs = srcs, protoc = protoc, + # output_suffix = "2", + # default_info = False, **kwargs ) diff --git a/rules/providers.bzl b/rules/providers.bzl index 338c2dcb5..64703bef4 100644 --- a/rules/providers.bzl +++ b/rules/providers.bzl @@ -23,8 +23,9 @@ ProtoPluginInfo = provider( ProtoCompileInfo = provider( "ProtoCompileInfo provides downstream rules with the outputs of proto_compile", fields = { - "label": "The proto_compile rule label", - "outputs": "The output files from the rule", + "label": "The proto_compile rule label (type Label)", + "outputs": "The output files from the rule (type List[File])", + "output_file_map": "The output files from the rule (type Dict[String,List[File]]). The keys are the package-relative paths of the file and values are the file objects.", }, ) From 9ed8784d7e4af94fe615c957b6f8c893425ec277 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Mon, 15 Jul 2024 18:41:45 -0600 Subject: [PATCH 02/10] Initial run implementation (stripped down) --- rules/proto_compile_gencopy.bzl | 44 ++++++--------------------------- 1 file changed, 7 insertions(+), 37 deletions(-) diff --git a/rules/proto_compile_gencopy.bzl b/rules/proto_compile_gencopy.bzl index c9baf940c..da0a32578 100644 --- a/rules/proto_compile_gencopy.bzl +++ b/rules/proto_compile_gencopy.bzl @@ -26,46 +26,20 @@ def _proto_compile_gencopy_run_impl(ctx): config = gencopy_config(ctx) runfiles = [] - - # comprehend a mapping of relpath -> File - srcfiles = {f.short_path[len(ctx.label.package):].lstrip("/"): f for f in ctx.files.srcs} - for info in [dep[ProtoCompileInfo] for dep in ctx.attr.deps]: - runfiles += info.output_file_map.values() - srcfiles = info.output_file_map - - srcs = [] # list of string - for f in info.outputs: - if config.mode == "check": - # if we are in 'check' mode, the src and dst cannot be the same - # file, so make a copy of it... but first, we need to find it - # in the srcs files! - found = False - for srcfilename, srcfile in srcfiles.items(): - if srcfilename == f.basename: - replica = ctx.actions.declare_file(f.basename + ".actual", sibling = f) - _copy_file(ctx.actions, srcfile, replica) - runfiles.append(replica) - srcs.append(replica.short_path) - found = True - break - elif srcfilename == f.basename + ctx.attr.extension: - runfiles.append(srcfile) - srcs.append(srcfile.short_path) - found = True - break - if not found: - fail("could not find matching source file for generated file %s in %r" % (f.basename, srcfiles)) - - else: - srcs.append(f.short_path) + srcs = [] + dsts = [] + for [rel, f] in info.output_file_map.items(): + dsts.append(rel) + srcs.append(f.short_path) + runfiles.append(f) config.packageConfigs.append( struct( targetLabel = str(info.label), targetPackage = info.label.package, targetWorkspaceRoot = info.label.workspace_root, - generatedFiles = [f.short_path for f in info.outputs], + generatedFiles = dsts, sourceFiles = srcs, ), ) @@ -90,10 +64,6 @@ proto_compile_gencopy_run = rule( doc = "The source files", allow_files = True, ), - extension = attr.string( - doc = "optional file extension to add to the copied file", - mandatory = False, - ), ), executable = True, test = is_test, From da0ddad07eb77e34f311e09cf9fb1633e3ceda88 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Mon, 15 Jul 2024 19:05:25 -0600 Subject: [PATCH 03/10] Initial test implementation (stripped down) --- cmd/gencopy/gencopy.bzl | 4 ++ rules/proto_compile.bzl | 3 +- rules/proto_compile_gencopy.bzl | 82 +++++++++++++++------------------ 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/cmd/gencopy/gencopy.bzl b/cmd/gencopy/gencopy.bzl index 50f982cb5..06df5902c 100644 --- a/cmd/gencopy/gencopy.bzl +++ b/cmd/gencopy/gencopy.bzl @@ -19,6 +19,10 @@ gencopy_attrs = { doc = "The label.name used to regenerate targets", mandatory = True, ), + "extension": attr.string( + doc = "optional file extension to add to the copied file", + mandatory = False, + ), "_gencopy_script": attr.label( doc = "The gencopy script template", default = str(Label("//cmd/gencopy:gencopy.bash.in")), diff --git a/rules/proto_compile.bzl b/rules/proto_compile.bzl index 630f2f0bb..e5444389f 100644 --- a/rules/proto_compile.bzl +++ b/rules/proto_compile.bzl @@ -347,7 +347,8 @@ def _proto_compile_impl(ctx): # pass # comprehend a mapping of relpath -> File - output_file_map = {f.short_path[len(ctx.label.package):].lstrip("/"): f for f in outputs} + # output_file_map = {f.short_path[len(ctx.label.package):].lstrip("/"): f for f in outputs} + output_file_map = {f.short_path: f for f in outputs} providers = [ ProtoCompileInfo( diff --git a/rules/proto_compile_gencopy.bzl b/rules/proto_compile_gencopy.bzl index da0a32578..14e24db2a 100644 --- a/rules/proto_compile_gencopy.bzl +++ b/rules/proto_compile_gencopy.bzl @@ -29,10 +29,10 @@ def _proto_compile_gencopy_run_impl(ctx): for info in [dep[ProtoCompileInfo] for dep in ctx.attr.deps]: srcs = [] dsts = [] - for [rel, f] in info.output_file_map.items(): + for [rel, generated_file] in info.output_file_map.items(): + runfiles.append(generated_file) + srcs.append(generated_file.short_path) dsts.append(rel) - srcs.append(f.short_path) - runfiles.append(f) config.packageConfigs.append( struct( @@ -60,13 +60,9 @@ proto_compile_gencopy_run = rule( doc = "The ProtoCompileInfo providers", providers = [ProtoCompileInfo], ), - srcs = attr.label_list( - doc = "The source files", - allow_files = True, - ), ), executable = True, - test = is_test, + test = False, ) def _proto_compile_gencopy_test_impl(ctx): @@ -74,45 +70,30 @@ def _proto_compile_gencopy_test_impl(ctx): runfiles = [] - # comprehend a mapping of relpath -> File - srcfiles = {f.short_path[len(ctx.label.package):].lstrip("/"): f for f in ctx.files.srcs} + source_file_map = {f.short_path: f for f in ctx.files.srcs} for info in [dep[ProtoCompileInfo] for dep in ctx.attr.deps]: - runfiles += info.output_file_map.values() - srcfiles = info.output_file_map - - srcs = [] # list of string - for f in info.outputs: - if config.mode == "check": - # if we are in 'check' mode, the src and dst cannot be the same - # file, so make a copy of it... but first, we need to find it - # in the srcs files! - found = False - for srcfilename, srcfile in srcfiles.items(): - if srcfilename == f.basename: - replica = ctx.actions.declare_file(f.basename + ".actual", sibling = f) - _copy_file(ctx.actions, srcfile, replica) - runfiles.append(replica) - srcs.append(replica.short_path) - found = True - break - elif srcfilename == f.basename + ctx.attr.extension: - runfiles.append(srcfile) - srcs.append(srcfile.short_path) - found = True - break - if not found: - fail("could not find matching source file for generated file %s in %r" % (f.basename, srcfiles)) - - else: - srcs.append(f.short_path) + srcs = [] + dsts = [] + for [rel, generated_file] in info.output_file_map.items(): + source_file = source_file_map.get(rel) + if not source_file: + fail("could not find matching source file for generated file %s in %r" % (rel, source_file_map.keys())) + + if source_file.short_path == generated_file.short_path: + fail("source file path must be distinct from generated file path (src=%s, dst=%s)" % (source_file.short_path, generated_file.short_path)) + + runfiles.append(source_file) + runfiles.append(generated_file) + srcs.append(source_file.short_path) + dsts.append(generated_file.short_path) config.packageConfigs.append( struct( targetLabel = str(info.label), targetPackage = info.label.package, targetWorkspaceRoot = info.label.workspace_root, - generatedFiles = [f.short_path for f in info.outputs], + generatedFiles = dsts, sourceFiles = srcs, ), ) @@ -125,7 +106,7 @@ def _proto_compile_gencopy_test_impl(ctx): executable = script, )] -proto_compile_gencopy_rule_test = rule( +proto_compile_gencopy_test = rule( implementation = _proto_compile_gencopy_test_impl, attrs = dict( gencopy_attrs, @@ -137,11 +118,24 @@ proto_compile_gencopy_rule_test = rule( doc = "The source files", allow_files = True, ), - extension = attr.string( - doc = "optional file extension to add to the copied file", - mandatory = False, - ), ), executable = True, test = True, ) + +# found = False +# for srcfilename, srcfile in srcfiles.items(): +# if srcfilename == f.basename: +# replica = ctx.actions.declare_file(f.basename + ".actual", sibling = f) +# _copy_file(ctx.actions, srcfile, replica) +# runfiles.append(replica) +# srcs.append(replica.short_path) +# found = True +# break +# elif srcfilename == f.basename + ctx.attr.extension: +# runfiles.append(srcfile) +# srcs.append(srcfile.short_path) +# found = True +# break +# if not found: +# fail("could not find matching source file for generated file %s in %r" % (f.basename, srcfiles)) From dd8d47fc4387a6f959e255cee1bbfda3445b8d84 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Mon, 15 Jul 2024 19:31:11 -0600 Subject: [PATCH 04/10] Adapt rules with output_file_suffix --- rules/proto_compile.bzl | 22 +++++++++- rules/proto_compile_gencopy.bzl | 72 +++++++++++--------------------- rules/proto_compiled_sources.bzl | 4 +- 3 files changed, 47 insertions(+), 51 deletions(-) diff --git a/rules/proto_compile.bzl b/rules/proto_compile.bzl index e5444389f..346cb02f4 100644 --- a/rules/proto_compile.bzl +++ b/rules/proto_compile.bzl @@ -350,15 +350,30 @@ def _proto_compile_impl(ctx): # output_file_map = {f.short_path[len(ctx.label.package):].lstrip("/"): f for f in outputs} output_file_map = {f.short_path: f for f in outputs} + if ctx.attr.output_file_suffix: + for [rel, original_file] in output_file_map.items(): + dst_name = original_file.basename + ctx.attr.output_file_suffix + copy_file = ctx.actions.declare_file(dst_name, sibling = original_file) + ctx.actions.run_shell( + mnemonic = "ProtoCompileCopyFile", + inputs = [original_file], + outputs = [copy_file], + command = "cp '{}' '{}'".format(original_file.path, copy_file.path), + progress_message = "copying {} to {}".format(original_file.basename, copy_file.basename), + ) + output_file_map[rel] = copy_file + + output_files = output_file_map.values() + providers = [ ProtoCompileInfo( label = ctx.label, - outputs = outputs, + outputs = output_files, output_file_map = output_file_map, ), ] if ctx.attr.default_info: - providers.append(DefaultInfo(files = depset(outputs))) + providers.append(DefaultInfo(files = depset(output_files))) return providers @@ -409,6 +424,9 @@ proto_compile = rule( doc = "If false, do not return the DefaultInfo provider", default = True, ), + "output_file_suffix": attr.string( + doc = "If set, copy the output files to a new set having this suffix", + ), }, toolchains = ["@build_stack_rules_proto//toolchain:protoc"], ) diff --git a/rules/proto_compile_gencopy.bzl b/rules/proto_compile_gencopy.bzl index 14e24db2a..1578cfda8 100644 --- a/rules/proto_compile_gencopy.bzl +++ b/rules/proto_compile_gencopy.bzl @@ -4,43 +4,32 @@ load("//cmd/gencopy:gencopy.bzl", "gencopy_action", "gencopy_attrs", "gencopy_config") load(":providers.bzl", "ProtoCompileInfo") -def _copy_file(actions, src, dst): - """Copy a file to a new path destination - - Args: - actions: the object - src: the source file - dst: the destination path of the file - Returns: - for the copied file - """ - actions.run_shell( - mnemonic = "CopyFile", - inputs = [src], - outputs = [dst], - command = "cp '{}' '{}'".format(src.path, dst.path), - progress_message = "copying {} to {}".format(src.path, dst.path), - ) - def _proto_compile_gencopy_run_impl(ctx): config = gencopy_config(ctx) runfiles = [] for info in [dep[ProtoCompileInfo] for dep in ctx.attr.deps]: - srcs = [] - dsts = [] + # List[String]: names of files that represent the source files. In an + # update, these are the target filenames of a file copy operation. + source_files = [] + + # List[String]: names of files that represent the generated files. In + # an update, these are the source filenames of a file copy operation + # (although the file itself was generated by proto_compile). + generated_files = [] + for [rel, generated_file] in info.output_file_map.items(): runfiles.append(generated_file) - srcs.append(generated_file.short_path) - dsts.append(rel) + source_files.append(rel) + generated_files.append(generated_file.short_path) config.packageConfigs.append( struct( targetLabel = str(info.label), targetPackage = info.label.package, targetWorkspaceRoot = info.label.workspace_root, - generatedFiles = dsts, - sourceFiles = srcs, + generatedFiles = generated_files, + sourceFiles = source_files, ), ) @@ -73,8 +62,14 @@ def _proto_compile_gencopy_test_impl(ctx): source_file_map = {f.short_path: f for f in ctx.files.srcs} for info in [dep[ProtoCompileInfo] for dep in ctx.attr.deps]: - srcs = [] - dsts = [] + # List[String]: names of files that represent the source files. In a + # test, these are the file paths of actual source files that are in the + # workspace (and checked into source control). + source_files = [] + + # List[String]: names of files that represent the generated files. In a + # test, these are the outputs files from the proto_compile rule. + generated_files = [] for [rel, generated_file] in info.output_file_map.items(): source_file = source_file_map.get(rel) if not source_file: @@ -85,16 +80,16 @@ def _proto_compile_gencopy_test_impl(ctx): runfiles.append(source_file) runfiles.append(generated_file) - srcs.append(source_file.short_path) - dsts.append(generated_file.short_path) + source_files.append(source_file.short_path) + generated_files.append(generated_file.short_path) config.packageConfigs.append( struct( targetLabel = str(info.label), targetPackage = info.label.package, targetWorkspaceRoot = info.label.workspace_root, - generatedFiles = dsts, - sourceFiles = srcs, + generatedFiles = generated_files, + sourceFiles = source_files, ), ) @@ -122,20 +117,3 @@ proto_compile_gencopy_test = rule( executable = True, test = True, ) - -# found = False -# for srcfilename, srcfile in srcfiles.items(): -# if srcfilename == f.basename: -# replica = ctx.actions.declare_file(f.basename + ".actual", sibling = f) -# _copy_file(ctx.actions, srcfile, replica) -# runfiles.append(replica) -# srcs.append(replica.short_path) -# found = True -# break -# elif srcfilename == f.basename + ctx.attr.extension: -# runfiles.append(srcfile) -# srcs.append(srcfile.short_path) -# found = True -# break -# if not found: -# fail("could not find matching source file for generated file %s in %r" % (f.basename, srcfiles)) diff --git a/rules/proto_compiled_sources.bzl b/rules/proto_compiled_sources.bzl index c13c57b46..c2e3c315e 100644 --- a/rules/proto_compiled_sources.bzl +++ b/rules/proto_compiled_sources.bzl @@ -32,8 +32,8 @@ def proto_compiled_sources(**kwargs): name = name, srcs = srcs, protoc = protoc, - # output_suffix = "2", - # default_info = False, + output_file_suffix = ".gen", + default_info = False, **kwargs ) From 851a7e0ffa3d4b376b7a422660227117656b9a64 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Mon, 15 Jul 2024 19:43:47 -0600 Subject: [PATCH 05/10] Code review cleanup 1 --- rules/proto_compile.bzl | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/rules/proto_compile.bzl b/rules/proto_compile.bzl index 346cb02f4..d51511561 100644 --- a/rules/proto_compile.bzl +++ b/rules/proto_compile.bzl @@ -93,9 +93,6 @@ def _strip_path_prefix(path, prefix): def is_windows(ctx): return ctx.configuration.host_path_separator == ";" -def _make_output_filename(ctx, name): - return name + ctx.attr.output_suffix - def _proto_compile_impl(ctx): # mut > outputs = [] + ctx.outputs.outputs @@ -104,7 +101,7 @@ def _proto_compile_impl(ctx): fail("rule must provide 'srcs' or 'outputs' (but not both)") if len(ctx.attr.srcs) > 0: - outputs = [ctx.actions.declare_file(_make_output_filename(ctx, name)) for name in ctx.attr.srcs] + outputs = [ctx.actions.declare_file(name) for name in ctx.attr.srcs] ### ### Part 1: setup variables used in scope @@ -343,11 +340,6 @@ def _proto_compile_impl(ctx): env = {"BAZEL_BINDIR": ctx.bin_dir.path}, ) - # if not ctx.attr.default_info: - # pass - - # comprehend a mapping of relpath -> File - # output_file_map = {f.short_path[len(ctx.label.package):].lstrip("/"): f for f in outputs} output_file_map = {f.short_path: f for f in outputs} if ctx.attr.output_file_suffix: @@ -389,9 +381,6 @@ proto_compile = rule( "srcs": attr.string_list( doc = "List of source files we expect to be regenerated (relative to package)", ), - "output_suffix": attr.string( - doc = "An optional suffix to be appended to output files", - ), "plugins": attr.label_list( doc = "List of ProtoPluginInfo providers", mandatory = True, From cddaca0255573b05da060093c7499dcf2127fd2b Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Tue, 16 Jul 2024 11:27:57 -0600 Subject: [PATCH 06/10] proto_compile: prefer file.short_path --- pkg/BUILD.bazel | 1 + rules/proto_compile.bzl | 38 ++++++++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 1cc6e78c7..195a770ca 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -15,6 +15,7 @@ filegroup( "//pkg/plugin/grpc/grpcweb:all_files", "//pkg/plugin/grpcecosystem/grpcgateway:all_files", "//pkg/plugin/scalapb/scalapb:all_files", + "//pkg/plugin/scalapb/zio_grpc:all_files", "//pkg/plugin/stackb/grpc_js:all_files", "//pkg/plugin/stephenh/ts-proto:all_files", "//pkg/plugintest:all_files", diff --git a/rules/proto_compile.bzl b/rules/proto_compile.bzl index d51511561..ed1312b93 100644 --- a/rules/proto_compile.bzl +++ b/rules/proto_compile.bzl @@ -146,6 +146,12 @@ def _proto_compile_impl(ctx): # mut > post-processing modifications for the compile action mods = dict() + # out_dir is used in conjunction with file.short_path to determine output + # file paths + out_dir = ctx.bin_dir.path + if ctx.label.workspace_root: + out_dir = "/".join([out_dir, ctx.label.workspace_root]) + ### ### Part 2: per-plugin args ### @@ -276,16 +282,16 @@ def _proto_compile_impl(ctx): # into place if len(ctx.attr.output_mappings) > 0: copy_commands = [] - out_dir = ctx.bin_dir.path - if ctx.label.workspace_root: - out_dir = "/".join([out_dir, ctx.label.workspace_root]) for mapping in ctx.attr.output_mappings: basename, _, intermediate_filename = mapping.partition("=") - intermediate_filename = "/".join([out_dir, intermediate_filename]) output = outputs_by_basename.get(basename, None) if not output: fail("the mapped file '%s' was not listed in outputs" % basename) - copy_commands.append("cp '{}' '{}'".format(intermediate_filename, output.path)) + copy_commands.append("cp '{dir}/{src}' '{dir}/{dst}'".format( + dir = out_dir, + src = intermediate_filename, + dst = output.short_path, + )) copy_script = ctx.actions.declare_file(ctx.label.name + "_copy.sh") ctx.actions.write(copy_script, "\n".join(copy_commands), is_executable = True) inputs.append(copy_script) @@ -297,15 +303,22 @@ def _proto_compile_impl(ctx): for suffix, action in mods.items(): for f in outputs: if f.short_path.endswith(suffix): - mv_commands.append("awk '%s' %s > %s.tmp" % (action, f.path, f.path)) - mv_commands.append("mv %s.tmp %s" % (f.path, f.path)) + mv_commands.append("awk '{action}' {dir}/{short_path} > {dir}/{short_path}.tmp".format( + action = action, + dir = out_dir, + short_path = f.short_path, + )) + mv_commands.append("mv {dir}/{short_path}.tmp {dir}/{short_path}".format( + dir = out_dir, + short_path = f.short_path, + )) mv_script = ctx.actions.declare_file(ctx.label.name + "_mv.sh") ctx.actions.write(mv_script, "\n".join(mv_commands), is_executable = True) inputs.append(mv_script) commands.append(mv_script.path) if verbose: - before = ["env", "pwd", "ls -al .", "echo '\n##### SANDBOX BEFORE RUNNING PROTOC'", "find * -type l"] + before = ["env", "pwd", "ls -al .", "echo '\n##### SANDBOX BEFORE RUNNING PROTOC'", "find * -type l | grep -v node_modules"] after = ["echo '\n##### SANDBOX AFTER RUNNING PROTOC'", "find * -type f"] commands = before + commands + after @@ -350,8 +363,12 @@ def _proto_compile_impl(ctx): mnemonic = "ProtoCompileCopyFile", inputs = [original_file], outputs = [copy_file], - command = "cp '{}' '{}'".format(original_file.path, copy_file.path), - progress_message = "copying {} to {}".format(original_file.basename, copy_file.basename), + command = "cp '{dir}/{src}' '{dir}/{dst}'".format( + dir = ctx.bin_dir.path, + src = original_file.short_path, + dst = copy_file.short_path, + ), + progress_message = "copying output file {} to {}".format(original_file.short_path, copy_file.short_path), ) output_file_map[rel] = copy_file @@ -408,6 +425,7 @@ proto_compile = rule( ), "verbose": attr.bool( doc = "The verbosity flag.", + default = False, ), "default_info": attr.bool( doc = "If false, do not return the DefaultInfo provider", From 6af5158faac1b5fcdb2c69865e964991b5c37ef0 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Tue, 16 Jul 2024 11:30:12 -0600 Subject: [PATCH 07/10] Update proto_repository_tools_srcs.bzl --- rules/private/proto_repository_tools_srcs.bzl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rules/private/proto_repository_tools_srcs.bzl b/rules/private/proto_repository_tools_srcs.bzl index 94d34a147..eefbc29f9 100644 --- a/rules/private/proto_repository_tools_srcs.bzl +++ b/rules/private/proto_repository_tools_srcs.bzl @@ -100,6 +100,8 @@ PROTO_REPOSITORY_TOOLS_SRCS = [ "@build_stack_rules_proto//pkg/plugin/grpcecosystem/grpcgateway:protoc-gen-grpc-gateway.go", "@build_stack_rules_proto//pkg/plugin/scalapb/scalapb:BUILD.bazel", "@build_stack_rules_proto//pkg/plugin/scalapb/scalapb:protoc_gen_scala.go", + "@build_stack_rules_proto//pkg/plugin/scalapb/zio_grpc:BUILD.bazel", + "@build_stack_rules_proto//pkg/plugin/scalapb/zio_grpc:protoc_gen_zio_grpc.go", "@build_stack_rules_proto//pkg/plugin/stackb/grpc_js:BUILD.bazel", "@build_stack_rules_proto//pkg/plugin/stackb/grpc_js:protoc-gen-grpc-js.go", "@build_stack_rules_proto//pkg/plugin/stephenh/ts-proto:BUILD.bazel", From a678a5e73a9a1d82a70ebcbe748b9e8c3efc096e Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Tue, 16 Jul 2024 15:47:29 -0600 Subject: [PATCH 08/10] proto_compile: convert to dict for outputs --- rules/proto_compile.bzl | 55 ++++++++++++++++++++------------- rules/proto_compile_gencopy.bzl | 7 ++--- 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/rules/proto_compile.bzl b/rules/proto_compile.bzl index ed1312b93..cacd54bda 100644 --- a/rules/proto_compile.bzl +++ b/rules/proto_compile.bzl @@ -93,19 +93,37 @@ def _strip_path_prefix(path, prefix): def is_windows(ctx): return ctx.configuration.host_path_separator == ";" +def _get_package_relative_path(label, filename): + rel = filename + if rel.startswith(label.package): + rel = rel[len(label.package):] + return rel.lstrip("/") + def _proto_compile_impl(ctx): - # mut > - outputs = [] + ctx.outputs.outputs + ### + ### Part 1: setup variables used in scope + ### if len(ctx.attr.srcs) > 0 and len(ctx.outputs.outputs) > 0: fail("rule must provide 'srcs' or 'outputs' (but not both)") - if len(ctx.attr.srcs) > 0: - outputs = [ctx.actions.declare_file(name) for name in ctx.attr.srcs] + # : output files mapped by their package-relative path. + outputs = {} + + if len(ctx.attr.srcs): + # assume filenames in srcs are already package-relative + for name in ctx.attr.srcs: + rel = "/".join([ctx.label.package, name]) + f = ctx.actions.declare_file(name) + outputs[rel] = f + else: + for f in ctx.outputs.outputs: + # rel = _get_package_relative_path(ctx.label, f.short_path) + rel = f.short_path + outputs[rel] = f - ### - ### Part 1: setup variables used in scope - ### + # const . outputs indexed by basename. + outputs_by_basename = {f.basename: f for f in outputs.values()} # const verbosity flag verbose = ctx.attr.verbose @@ -122,9 +140,6 @@ def _proto_compile_impl(ctx): # const > outs = {_plugin_label_key(Label(k)): v for k, v in ctx.attr.outs.items()} - # const . outputs indexed by basename. - outputs_by_basename = {f.basename: f for f in outputs} - # mut > set of descriptors for the compile action descriptors = proto_info.transitive_descriptor_sets.to_list() @@ -301,7 +316,7 @@ def _proto_compile_impl(ctx): if len(mods): mv_commands = [] for suffix, action in mods.items(): - for f in outputs: + for f in outputs.values(): if f.short_path.endswith(suffix): mv_commands.append("awk '{action}' {dir}/{short_path} > {dir}/{short_path}.tmp".format( action = action, @@ -337,7 +352,7 @@ def _proto_compile_impl(ctx): for f in inputs: # buildifier: disable=print print("INPUT:", f.path) - for f in outputs: + for f in outputs.values(): # buildifier: disable=print print("EXPECTED OUTPUT:", f.path) @@ -346,17 +361,15 @@ def _proto_compile_impl(ctx): command = "\n".join(commands), inputs = inputs, mnemonic = "Protoc", - outputs = outputs, + outputs = outputs.values(), progress_message = "Compiling protoc outputs for %r" % [f.basename for f in protos], tools = tools, input_manifests = input_manifests, env = {"BAZEL_BINDIR": ctx.bin_dir.path}, ) - output_file_map = {f.short_path: f for f in outputs} - if ctx.attr.output_file_suffix: - for [rel, original_file] in output_file_map.items(): + for rel, original_file in outputs.items(): dst_name = original_file.basename + ctx.attr.output_file_suffix copy_file = ctx.actions.declare_file(dst_name, sibling = original_file) ctx.actions.run_shell( @@ -370,19 +383,17 @@ def _proto_compile_impl(ctx): ), progress_message = "copying output file {} to {}".format(original_file.short_path, copy_file.short_path), ) - output_file_map[rel] = copy_file - - output_files = output_file_map.values() + outputs[rel] = copy_file providers = [ ProtoCompileInfo( label = ctx.label, - outputs = output_files, - output_file_map = output_file_map, + outputs = outputs.values(), + output_file_map = outputs, ), ] if ctx.attr.default_info: - providers.append(DefaultInfo(files = depset(output_files))) + providers.append(DefaultInfo(files = depset(outputs.values()))) return providers diff --git a/rules/proto_compile_gencopy.bzl b/rules/proto_compile_gencopy.bzl index 1578cfda8..abfe94654 100644 --- a/rules/proto_compile_gencopy.bzl +++ b/rules/proto_compile_gencopy.bzl @@ -17,8 +17,7 @@ def _proto_compile_gencopy_run_impl(ctx): # an update, these are the source filenames of a file copy operation # (although the file itself was generated by proto_compile). generated_files = [] - - for [rel, generated_file] in info.output_file_map.items(): + for rel, generated_file in info.output_file_map.items(): runfiles.append(generated_file) source_files.append(rel) generated_files.append(generated_file.short_path) @@ -70,13 +69,13 @@ def _proto_compile_gencopy_test_impl(ctx): # List[String]: names of files that represent the generated files. In a # test, these are the outputs files from the proto_compile rule. generated_files = [] - for [rel, generated_file] in info.output_file_map.items(): + for rel, generated_file in info.output_file_map.items(): source_file = source_file_map.get(rel) if not source_file: fail("could not find matching source file for generated file %s in %r" % (rel, source_file_map.keys())) if source_file.short_path == generated_file.short_path: - fail("source file path must be distinct from generated file path (src=%s, dst=%s)" % (source_file.short_path, generated_file.short_path)) + fail("source file path must be distinct from generated file path (%s)" % source_file.short_path) runfiles.append(source_file) runfiles.append(generated_file) From f0507a6b725bfbfa2485637d04fc7f04cc51e2bf Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Tue, 16 Jul 2024 20:39:34 -0600 Subject: [PATCH 09/10] Use renames script to copy files to final location --- rules/proto_compile.bzl | 113 +++++++++++++++++++------------- rules/proto_compile_gencopy.bzl | 4 +- rules/providers.bzl | 2 +- 3 files changed, 69 insertions(+), 50 deletions(-) diff --git a/rules/proto_compile.bzl b/rules/proto_compile.bzl index cacd54bda..6bbb2ad46 100644 --- a/rules/proto_compile.bzl +++ b/rules/proto_compile.bzl @@ -93,37 +93,48 @@ def _strip_path_prefix(path, prefix): def is_windows(ctx): return ctx.configuration.host_path_separator == ";" -def _get_package_relative_path(label, filename): - rel = filename - if rel.startswith(label.package): - rel = rel[len(label.package):] - return rel.lstrip("/") - def _proto_compile_impl(ctx): ### ### Part 1: setup variables used in scope ### + # out_dir is used in conjunction with file.short_path to determine root + # output file paths + out_dir = ctx.bin_dir.path + if ctx.label.workspace_root: + out_dir = "/".join([out_dir, ctx.label.workspace_root]) + if len(ctx.attr.srcs) > 0 and len(ctx.outputs.outputs) > 0: fail("rule must provide 'srcs' or 'outputs' (but not both)") # : output files mapped by their package-relative path. - outputs = {} + # This struct is given to the provider. + output_files_by_rel_path = {} + + # const . The key is the file basename, value is the + # short_path of the output file. + output_short_paths_by_basename = {} + + # renames is a mapping from the output filename that was produced by the + # plugin to the actual name we want to output. + renames = {} if len(ctx.attr.srcs): # assume filenames in srcs are already package-relative for name in ctx.attr.srcs: rel = "/".join([ctx.label.package, name]) - f = ctx.actions.declare_file(name) - outputs[rel] = f + actual_name = name + ctx.attr.output_file_suffix + if actual_name != name: + renames[rel] = "/".join([ctx.label.package, actual_name]) + f = ctx.actions.declare_file(actual_name) + output_files_by_rel_path[rel] = f + output_short_paths_by_basename[name] = rel else: for f in ctx.outputs.outputs: # rel = _get_package_relative_path(ctx.label, f.short_path) rel = f.short_path - outputs[rel] = f - - # const . outputs indexed by basename. - outputs_by_basename = {f.basename: f for f in outputs.values()} + output_files_by_rel_path[rel] = f + output_short_paths_by_basename[f.basename] = rel # const verbosity flag verbose = ctx.attr.verbose @@ -161,12 +172,6 @@ def _proto_compile_impl(ctx): # mut > post-processing modifications for the compile action mods = dict() - # out_dir is used in conjunction with file.short_path to determine output - # file paths - out_dir = ctx.bin_dir.path - if ctx.label.workspace_root: - out_dir = "/".join([out_dir, ctx.label.workspace_root]) - ### ### Part 2: per-plugin args ### @@ -299,13 +304,13 @@ def _proto_compile_impl(ctx): copy_commands = [] for mapping in ctx.attr.output_mappings: basename, _, intermediate_filename = mapping.partition("=") - output = outputs_by_basename.get(basename, None) - if not output: + output_short_path = output_short_paths_by_basename.get(basename) + if not output_short_path: fail("the mapped file '%s' was not listed in outputs" % basename) copy_commands.append("cp '{dir}/{src}' '{dir}/{dst}'".format( dir = out_dir, src = intermediate_filename, - dst = output.short_path, + dst = output_short_path, )) copy_script = ctx.actions.declare_file(ctx.label.name + "_copy.sh") ctx.actions.write(copy_script, "\n".join(copy_commands), is_executable = True) @@ -316,22 +321,51 @@ def _proto_compile_impl(ctx): if len(mods): mv_commands = [] for suffix, action in mods.items(): - for f in outputs.values(): - if f.short_path.endswith(suffix): + for output_short_path in output_short_paths_by_basename.values(): + if output_short_path.endswith(suffix): mv_commands.append("awk '{action}' {dir}/{short_path} > {dir}/{short_path}.tmp".format( action = action, dir = out_dir, - short_path = f.short_path, + short_path = output_short_path, )) mv_commands.append("mv {dir}/{short_path}.tmp {dir}/{short_path}".format( dir = out_dir, - short_path = f.short_path, + short_path = output_short_path, )) mv_script = ctx.actions.declare_file(ctx.label.name + "_mv.sh") ctx.actions.write(mv_script, "\n".join(mv_commands), is_executable = True) inputs.append(mv_script) commands.append(mv_script.path) + # if the ctx.attr.output_file_suffix was set in conjunction with + # ctx.attr.srcs, we want to rename all the output files to a different + # suffix (e.g. foo.ts -> foo.ts.gen). The relocates the files that were + # generated by protoc plugins to a different name. This is used by the + # 'proto_compiled_sources' rule. The reason is that if we also have a + # `foo.ts` source file sitting in the workspace (checked into git), rules + # like `ts_project` will perform a 'copy_to_bin' action on the file. If we + # didn't do this rename, the ts_project rule and the proto_compile rule + # would attempt to create the same output file in bazel-bin (foo.ts), + # causing an error. + # + # In the case of proto_compiled_sources, executing `bazel run + # //proto:foo_ts.update` would generate the file + # `bazel-bin/proto/foo.ts.gen` and the gencopy operation will copy that file + # to `WORKSPACE/proto/foo.ts`, essentially making the `.gen` a + # temporary-like file. + if len(renames): + rename_commands = [] + for src, dst in renames.items(): + rename_commands.append("mv {dir}/{src} {dir}/{dst}".format( + dir = out_dir, + src = src, + dst = dst, + )) + rename_script = ctx.actions.declare_file(ctx.label.name + "_rename.sh") + ctx.actions.write(rename_script, "\n".join(rename_commands), is_executable = True) + inputs.append(rename_script) + commands.append(rename_script.path) + if verbose: before = ["env", "pwd", "ls -al .", "echo '\n##### SANDBOX BEFORE RUNNING PROTOC'", "find * -type l | grep -v node_modules"] after = ["echo '\n##### SANDBOX AFTER RUNNING PROTOC'", "find * -type f"] @@ -352,7 +386,7 @@ def _proto_compile_impl(ctx): for f in inputs: # buildifier: disable=print print("INPUT:", f.path) - for f in outputs.values(): + for f in output_files_by_rel_path.values(): # buildifier: disable=print print("EXPECTED OUTPUT:", f.path) @@ -361,39 +395,24 @@ def _proto_compile_impl(ctx): command = "\n".join(commands), inputs = inputs, mnemonic = "Protoc", - outputs = outputs.values(), + outputs = output_files_by_rel_path.values(), progress_message = "Compiling protoc outputs for %r" % [f.basename for f in protos], tools = tools, input_manifests = input_manifests, env = {"BAZEL_BINDIR": ctx.bin_dir.path}, ) - if ctx.attr.output_file_suffix: - for rel, original_file in outputs.items(): - dst_name = original_file.basename + ctx.attr.output_file_suffix - copy_file = ctx.actions.declare_file(dst_name, sibling = original_file) - ctx.actions.run_shell( - mnemonic = "ProtoCompileCopyFile", - inputs = [original_file], - outputs = [copy_file], - command = "cp '{dir}/{src}' '{dir}/{dst}'".format( - dir = ctx.bin_dir.path, - src = original_file.short_path, - dst = copy_file.short_path, - ), - progress_message = "copying output file {} to {}".format(original_file.short_path, copy_file.short_path), - ) - outputs[rel] = copy_file + outputs = output_files_by_rel_path.values() providers = [ ProtoCompileInfo( label = ctx.label, - outputs = outputs.values(), - output_file_map = outputs, + outputs = outputs, + output_files_by_rel_path = output_files_by_rel_path, ), ] if ctx.attr.default_info: - providers.append(DefaultInfo(files = depset(outputs.values()))) + providers.append(DefaultInfo(files = depset(outputs))) return providers diff --git a/rules/proto_compile_gencopy.bzl b/rules/proto_compile_gencopy.bzl index abfe94654..ae53f6ce9 100644 --- a/rules/proto_compile_gencopy.bzl +++ b/rules/proto_compile_gencopy.bzl @@ -17,7 +17,7 @@ def _proto_compile_gencopy_run_impl(ctx): # an update, these are the source filenames of a file copy operation # (although the file itself was generated by proto_compile). generated_files = [] - for rel, generated_file in info.output_file_map.items(): + for rel, generated_file in info.output_files_by_rel_path.items(): runfiles.append(generated_file) source_files.append(rel) generated_files.append(generated_file.short_path) @@ -69,7 +69,7 @@ def _proto_compile_gencopy_test_impl(ctx): # List[String]: names of files that represent the generated files. In a # test, these are the outputs files from the proto_compile rule. generated_files = [] - for rel, generated_file in info.output_file_map.items(): + for rel, generated_file in info.output_files_by_rel_path.items(): source_file = source_file_map.get(rel) if not source_file: fail("could not find matching source file for generated file %s in %r" % (rel, source_file_map.keys())) diff --git a/rules/providers.bzl b/rules/providers.bzl index 64703bef4..fe759228c 100644 --- a/rules/providers.bzl +++ b/rules/providers.bzl @@ -25,7 +25,7 @@ ProtoCompileInfo = provider( fields = { "label": "The proto_compile rule label (type Label)", "outputs": "The output files from the rule (type List[File])", - "output_file_map": "The output files from the rule (type Dict[String,List[File]]). The keys are the package-relative paths of the file and values are the file objects.", + "output_files_by_rel_path": "The output files from the rule (type Dict[String,List[File]]). The keys are the package-relative paths of the file and values are the file objects.", }, ) From b521f071508ae0aaa660f2290c5849d840d2d841 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Sun, 21 Jul 2024 15:01:05 -0600 Subject: [PATCH 10/10] Fix gencopy path for external protos --- cmd/gencopy/gencopy.bash.in | 17 +++++++++++------ cmd/gencopy/gencopy.bzl | 2 +- cmd/gencopy/gencopy.go | 2 -- cmd/gencopy/gencopy_test.go | 2 +- pkg/protoc/proto_compiled_sources.go | 3 +++ rules/private/proto_repository_tools_srcs.bzl | 1 + 6 files changed, 17 insertions(+), 10 deletions(-) diff --git a/cmd/gencopy/gencopy.bash.in b/cmd/gencopy/gencopy.bash.in index 7067102f3..504b741ae 100644 --- a/cmd/gencopy/gencopy.bash.in +++ b/cmd/gencopy/gencopy.bash.in @@ -11,7 +11,12 @@ CONFIG_SHORT_PATH=@@CONFIG_SHORT_PATH@@ # env # set -x +# echo "PWD: $PWD" # find . +# find .. + +# Get the directory of the current script (part of the runfiles tree) +script_dir=$(dirname "$0") # find_runfile prints the location of a runfile in the source workspace, # either by reading the symbolic link or reading the runfiles manifest. @@ -45,18 +50,18 @@ fi # Note that we don't change directories first; if we did, Generator wouldn't be # able to find runfiles, and some extensions rely on that. Generator can use # BUILD_WORKSPACE_DIRECTORY to interpret relative paths on the command line. -GENCOPY_short_path=$(find_runfile "$GENCOPY_SHORT_PATH") -if [ -z "$GENCOPY_short_path" ]; then +GENCOPY_path=$(find_runfile "$GENCOPY_SHORT_PATH") +if [ -z "$GENCOPY_path" ]; then echo "error: could not locate gencopy binary" >&2 exit 1 fi -CONFIG_short_path=$(find_runfile "$CONFIG_SHORT_PATH") -if [ -z "$CONFIG_short_path" ]; then +CONFIG_path=$(find_runfile "$CONFIG_SHORT_PATH") +if [ -z "$CONFIG_path" ]; then echo "error: could not locate gencopy configuration file" >&2 exit 1 fi -"$GENCOPY_short_path" \ - -config="$CONFIG_short_path" \ +"$GENCOPY_path" \ + -config="$CONFIG_path" \ -workspace_root_directory="${BUILD_WORKSPACE_DIRECTORY:-""}" \ No newline at end of file diff --git a/cmd/gencopy/gencopy.bzl b/cmd/gencopy/gencopy.bzl index 06df5902c..0e046d343 100644 --- a/cmd/gencopy/gencopy.bzl +++ b/cmd/gencopy/gencopy.bzl @@ -47,7 +47,7 @@ def gencopy_config(ctx): ) def gencopy_action(ctx, config, runfiles): - """gencopy_action declared a bazel action that runs the gencopy.bash script. + """gencopy_action declares a bazel action that runs the gencopy.bash script. Args: ctx: the context object. diff --git a/cmd/gencopy/gencopy.go b/cmd/gencopy/gencopy.go index b0980dd42..e65a66441 100644 --- a/cmd/gencopy/gencopy.go +++ b/cmd/gencopy/gencopy.go @@ -11,7 +11,6 @@ import ( "os" "path/filepath" "strconv" - "strings" "github.com/google/go-cmp/cmp" ) @@ -174,7 +173,6 @@ func makePkgSrcDstPairs(cfg *Config, pkg *PackageConfig) []*SrcDst { func makePkgSrcDstPair(cfg *Config, pkg *PackageConfig, src, dst string) *SrcDst { if pkg.TargetWorkspaceRoot != "" { - src = filepath.Join("external", strings.TrimPrefix(src, "..")) dst = filepath.Join(pkg.TargetWorkspaceRoot, dst) } dst = filepath.Join(cfg.WorkspaceRootDirectory, dst) diff --git a/cmd/gencopy/gencopy_test.go b/cmd/gencopy/gencopy_test.go index 9db097a5b..d16d28a10 100644 --- a/cmd/gencopy/gencopy_test.go +++ b/cmd/gencopy/gencopy_test.go @@ -68,7 +68,7 @@ func TestMakePkgSrcDstPair(t *testing.T) { pkg: PackageConfig{TargetWorkspaceRoot: "external/foo"}, src: "../foo/file.txt", dst: "file.txt", - want: SrcDst{Src: "external/foo/file.txt", Dst: "/home/external/foo/file.txt"}, + want: SrcDst{Src: "../foo/file.txt", Dst: "/home/external/foo/file.txt"}, }, } { t.Run(name, func(t *testing.T) { diff --git a/pkg/protoc/proto_compiled_sources.go b/pkg/protoc/proto_compiled_sources.go index 28cab0d26..a9e61bd9e 100644 --- a/pkg/protoc/proto_compiled_sources.go +++ b/pkg/protoc/proto_compiled_sources.go @@ -44,6 +44,9 @@ func (s *protoCompiledSources) LoadInfo() rule.LoadInfo { // ProvideRule implements part of the LanguageRule interface. func (s *protoCompiledSources) ProvideRule(cfg *LanguageRuleConfig, config *ProtocConfiguration) RuleProvider { + if len(config.Outputs) == 0 { + return nil + } return &protoCompileRule{ kind: "proto_compiled_sources", nameSuffix: "compiled_sources", diff --git a/rules/private/proto_repository_tools_srcs.bzl b/rules/private/proto_repository_tools_srcs.bzl index eefbc29f9..462b8167e 100644 --- a/rules/private/proto_repository_tools_srcs.bzl +++ b/rules/private/proto_repository_tools_srcs.bzl @@ -181,6 +181,7 @@ PROTO_REPOSITORY_TOOLS_SRCS = [ "@build_stack_rules_proto//plugin/grpc/grpc-web:BUILD.bazel", "@build_stack_rules_proto//plugin/grpc-ecosystem/grpc-gateway:BUILD.bazel", "@build_stack_rules_proto//plugin/scalapb/scalapb:BUILD.bazel", + "@build_stack_rules_proto//plugin/scalapb/zio-grpc:BUILD.bazel", "@build_stack_rules_proto//plugin/stackb/grpc_js:BUILD.bazel", "@build_stack_rules_proto//plugin/stephenh/ts-proto:BUILD.bazel", "@build_stack_rules_proto//rules:BUILD.bazel",