From 50c55a9810a974dc5a9e7dd1a5c0d295d525f283 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Mon, 9 Dec 2019 20:50:12 +0100 Subject: [PATCH] [bazel] Correctly handle proto files under `_virtual_imports` This can happen if: 1. `proto_library` contains at least one of `import_prefix/strip_import_prefix`, or 2. (for Bazel >= 1.0) at least one of `proto_library`'s `srcs` is generated. Fixes #1094 --- examples/proto/examplepb/BUILD.bazel | 7 +- protoc-gen-swagger/defs.bzl | 218 +++++++++++++++------------ 2 files changed, 129 insertions(+), 96 deletions(-) diff --git a/examples/proto/examplepb/BUILD.bazel b/examples/proto/examplepb/BUILD.bazel index 9928fa47e8e..b483934a865 100644 --- a/examples/proto/examplepb/BUILD.bazel +++ b/examples/proto/examplepb/BUILD.bazel @@ -4,20 +4,23 @@ load("@grpc_ecosystem_grpc_gateway//protoc-gen-swagger:defs.bzl", "protoc_gen_sw package(default_visibility = ["//visibility:public"]) +# TODO(yannic): Add examples/tests that use import_prefix/strip_import_prefix. + # gazelle:exclude a_bit_of_everything.pb.gw.go # gazelle:exclude echo_service.pb.gw.go # gazelle:exclude flow_combination.pb.gw.go +# gazelle:exclude generated_input.proto # gazelle:exclude non_standard_names.pb.gw.go +# gazelle:exclude response_body_service.pb.gw.go # gazelle:exclude stream.pb.gw.go # gazelle:exclude use_go_template.pb.gw.go # gazelle:exclude wrappers.pb.gw.go -# gazelle:exclude response_body_service.pb.gw.go genrule( name = "generated_proto", srcs = ["generated_input.proto"], outs = ["generated_output.proto"], - cmd = "cp $< $@", # A simple copy simulates a generated proto file. + cmd = "cp $< $@", # A simple copy simulates a generated proto file. ) proto_library( diff --git a/protoc-gen-swagger/defs.bzl b/protoc-gen-swagger/defs.bzl index 652e4a20b1a..fac06f3b9b7 100644 --- a/protoc-gen-swagger/defs.bzl +++ b/protoc-gen-swagger/defs.bzl @@ -4,133 +4,163 @@ Reads the the api spec in protobuf format and generate an open-api spec. Optionally applies settings from the grpc-service configuration. """ -def _collect_includes(gen_dir, srcs): - """Build an include path mapping. - - It is important to not just collect unique dirnames, to also support - proto files of the same name from different packages. - - The implementation below is similar to what bazel does in its - ProtoCompileActionBuilder.java +# TODO(yannic): Replace with |proto_common.direct_source_infos| when +# https://github.com/bazelbuild/rules_proto/pull/22 lands. +def _direct_source_infos(proto_info, provided_sources = []): + """Returns sequence of `ProtoFileInfo` for `proto_info`'s direct sources. + + Files that are both in `proto_info`'s direct sources and in + `provided_sources` are skipped. This is useful, e.g., for well-known + protos that are already provided by the Protobuf runtime. + + Args: + proto_info: An instance of `ProtoInfo`. + provided_sources: Optional. A sequence of files to ignore. + Usually, these files are already provided by the + Protocol Buffer runtime (e.g. Well-Known protos). + + Returns: A sequence of `ProtoFileInfo` containing information about + `proto_info`'s direct sources. """ - includes = [] - for src in srcs: - ref_path = src.path - if ref_path.startswith(gen_dir): - ref_path = ref_path[len(gen_dir):].lstrip("/") + source_root = proto_info.proto_source_root + if "." == source_root: + return [struct(file = src, import_path = src.path) for src in proto_info.direct_sources] + + offset = len(source_root) + 1 # + '/'. + + infos = [] + for src in proto_info.direct_sources: + # TODO(yannic): Remove this hack when we drop support for Bazel < 1.0. + local_offset = offset + if src.root.path and not source_root.startswith(src.root.path): + # Before Bazel 1.0, `proto_source_root` wasn't guaranteed to be a + # prefix of `src.path`. This could happend, e.g., if `file` was + # generated (https://github.com/bazelbuild/bazel/issues/9215). + local_offset += len(src.root.path) + 1 # + '/'. + infos.append(struct(file = src, import_path = src.path[local_offset:])) + + return infos + +def _run_proto_gen_swagger( + actions, + proto_info, + target_name, + transitive_proto_srcs, + protoc, + protoc_gen_swagger, + grpc_api_configuration, + single_output, + json_names_for_fields): + args = actions.args() + + args.add("--plugin", "protoc-gen-swagger=%s" % protoc_gen_swagger.path) + + args.add("--swagger_opt", "logtostderr=true") + args.add("--swagger_opt", "allow_repeated_fields_in_body=true") + + extra_inputs = [] + if grpc_api_configuration: + extra_inputs.append(grpc_api_configuration) + args.add("--swagger_opt", "grpc_api_configuration=%s" % grpc_api_configuration.path) - if src.owner.workspace_root: - workspace_root = src.owner.workspace_root - ref_path = ref_path[len(workspace_root):].lstrip("/") + if json_names_for_fields: + args.add("--swagger_opt", "json_names_for_fields=true") - include = ref_path + "=" + src.path - if include not in includes: - includes.append(include) + proto_file_infos = _direct_source_infos(proto_info) - return includes + # TODO(yannic): Use |proto_info.transitive_descriptor_sets| when + # https://github.com/bazelbuild/bazel/issues/9337 is fixed. + args.add_all(proto_info.transitive_proto_path, format_each = "--proto_path=%s") -def _run_proto_gen_swagger(ctx, direct_proto_srcs, transitive_proto_srcs, actions, protoc, protoc_gen_swagger, grpc_api_configuration, single_output, json_names_for_fields): - swagger_files = [] + if single_output: + args.add("--swagger_opt", "allow_merge=true") + args.add("--swagger_opt", "merge_file_name=%s" % target_name) - inputs = direct_proto_srcs + transitive_proto_srcs - tools = [protoc_gen_swagger] + swagger_file = actions.declare_file("%s.swagger.json" % target_name) + args.add("--swagger_out", swagger_file.dirname) - options = ["logtostderr=true", "allow_repeated_fields_in_body=true"] - if grpc_api_configuration: - options.append("grpc_api_configuration=%s" % grpc_api_configuration.path) - inputs.append(grpc_api_configuration) + args.add_all([f.import_path for f in proto_file_infos]) - if json_names_for_fields: - options.append("json_names_for_fields=true") + actions.run( + executable = protoc, + tools = [protoc_gen_swagger], + inputs = depset( + direct = extra_inputs, + transitive = [transitive_proto_srcs], + ), + outputs = [swagger_file], + arguments = [args], + ) - includes = _collect_includes(ctx.genfiles_dir.path, direct_proto_srcs + transitive_proto_srcs) + return [swagger_file] - if single_output: + # TODO(yannic): We may be able to generate all files in a single action, + # but that will change at least the semantics of `use_go_template.proto`. + swagger_files = [] + for proto_file_info in proto_file_infos: + # TODO(yannic): This probably doesn't work as expected: we only add this + # option after we have seen it, so `.proto` sources that happen to be + # in the list of `.proto` files before `use_go_template.proto` will be + # compiled without this option, and all sources that get compiled after + # `use_go_template.proto` will have this option on. + if proto_file_info.file.basename == "use_go_template.proto": + args.add("--swagger_opt", "use_go_templates=true") + + file_name = "%s.swagger.json" % proto_file_info.import_path[:-len(".proto")] swagger_file = actions.declare_file( - "%s.swagger.json" % ctx.attr.name, - sibling = direct_proto_srcs[0], + "_virtual_imports/%s/%s" % (target_name, file_name), ) - output_dir = ctx.bin_dir.path - if direct_proto_srcs[0].owner.workspace_root: - output_dir = "/".join([output_dir, direct_proto_srcs[0].owner.workspace_root]) - output_dir = "/".join([output_dir, direct_proto_srcs[0].dirname]) - options.append("allow_merge=true") - options.append("merge_file_name=%s" % ctx.attr.name) + file_args = actions.args() + + offset = len(file_name) + 1 # + '/'. + file_args.add("--swagger_out", swagger_file.path[:-offset]) - args = actions.args() - args.add("--plugin=protoc-gen-swagger=%s" % protoc_gen_swagger.path) - args.add("--swagger_out=%s:%s" % (",".join(options), output_dir)) - args.add_all(["-I%s" % include for include in includes]) - args.add_all([src.path for src in direct_proto_srcs]) + file_args.add(proto_file_info.import_path) actions.run( executable = protoc, - inputs = inputs, - tools = tools, + tools = [protoc_gen_swagger], + inputs = depset( + direct = extra_inputs, + transitive = [transitive_proto_srcs], + ), outputs = [swagger_file], - arguments = [args], + arguments = [args, file_args], ) - swagger_files.append(swagger_file) - else: - for proto in direct_proto_srcs: - if proto.basename == "use_go_template.proto": - options.append("use_go_templates=true") - - swagger_file = actions.declare_file( - "%s.swagger.json" % proto.basename[:-len(".proto")], - sibling = proto, - ) - - output_dir = ctx.bin_dir.path - if proto.owner.workspace_root: - output_dir = "/".join([output_dir, proto.owner.workspace_root]) - - args = actions.args() - args.add("--plugin=protoc-gen-swagger=%s" % protoc_gen_swagger.path) - args.add("--swagger_out=%s:%s" % (",".join(options), output_dir)) - args.add_all(["-I%s" % include for include in includes]) - args.add(proto.path) - - actions.run( - executable = protoc, - inputs = inputs, - tools = tools, - outputs = [swagger_file], - arguments = [args], - ) - swagger_files.append(swagger_file) return swagger_files def _proto_gen_swagger_impl(ctx): proto = ctx.attr.proto[ProtoInfo] - grpc_api_configuration = ctx.file.grpc_api_configuration - - return [DefaultInfo( - files = depset( - _run_proto_gen_swagger( - ctx, - direct_proto_srcs = proto.direct_sources, - transitive_proto_srcs = ctx.files._well_known_protos + proto.transitive_sources.to_list(), - actions = ctx.actions, - protoc = ctx.executable._protoc, - protoc_gen_swagger = ctx.executable._protoc_gen_swagger, - grpc_api_configuration = grpc_api_configuration, - single_output = ctx.attr.single_output, - json_names_for_fields = ctx.attr.json_names_for_fields, + return [ + DefaultInfo( + files = depset( + _run_proto_gen_swagger( + actions = ctx.actions, + proto_info = proto, + target_name = ctx.attr.name, + transitive_proto_srcs = depset( + direct = ctx.files._well_known_protos, + transitive = [proto.transitive_sources], + ), + protoc = ctx.executable._protoc, + protoc_gen_swagger = ctx.executable._protoc_gen_swagger, + grpc_api_configuration = ctx.file.grpc_api_configuration, + single_output = ctx.attr.single_output, + json_names_for_fields = ctx.attr.json_names_for_fields, + ), ), ), - )] + ] protoc_gen_swagger = rule( attrs = { "proto": attr.label( - allow_rules = ["proto_library"], mandatory = True, - providers = ["proto"], + providers = [ProtoInfo], ), "grpc_api_configuration": attr.label( allow_single_file = True,