Skip to content

Commit

Permalink
Honor the feature flag to enable generation from raw proto sources.
Browse files Browse the repository at this point in the history
The DescriptorSets ProtoInfo exposes don't have source info, so the generated
source don't have the comment. bazelbuild/bazel#9337
is open to attempt to get that, but this provides a way to opt into forcing it.

This does come with a minor risk for cross repository and/or generated proto
files where the protoc command line might not be crafted correctly, so it
remains opt in.

This is followup to the previous change that did this for swift_proto_library.

NOTE: The current version of swift grpc depended on doesn't include comments,
but the NIO version appears to support it for methods, so landing the work
in advance of things updating to that version.

RELNOTES: None
PiperOrigin-RevId: 330729710
  • Loading branch information
thomasvl authored and swiple-rules-gardener committed Sep 9, 2020
1 parent d39ddf0 commit c4e2328
Showing 1 changed file with 64 additions and 23 deletions.
87 changes: 64 additions & 23 deletions swift/internal/swift_grpc_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ load(
load(
":feature_names.bzl",
"SWIFT_FEATURE_ENABLE_TESTING",
"SWIFT_FEATURE_GENERATE_FROM_RAW_PROTO_FILES",
"SWIFT_FEATURE_NO_GENERATED_HEADER",
)
load(":linking.bzl", "register_libraries_to_link")
Expand All @@ -49,6 +50,7 @@ def _register_grpcswift_generate_action(
proto_source_root,
transitive_descriptor_sets,
module_mapping_file,
generate_from_proto_sources,
mkdir_and_run,
protoc_executable,
protoc_plugin_executable,
Expand All @@ -69,6 +71,10 @@ def _register_grpcswift_generate_action(
target being analyzed. May be `None`, in which case no module
mapping will be passed (the case for leaf nodes in the dependency
graph).
generate_from_proto_sources: True/False for is generation should happen
from proto source file vs just via the DescriptorSets. The Sets
don't have source info, so the generated sources won't have
comments (https://github.com/bazelbuild/bazel/issues/9337).
mkdir_and_run: The `File` representing the `mkdir_and_run` executable.
protoc_executable: The `File` representing the `protoc` executable.
protoc_plugin_executable: The `File` representing the `protoc` plugin
Expand Down Expand Up @@ -135,14 +141,34 @@ def _register_grpcswift_generate_action(
module_mapping_file,
format = "--swiftgrpc_opt=ProtoPathModuleMappings=%s",
)
protoc_args.add("--descriptor_set_in")
protoc_args.add_joined(transitive_descriptor_sets, join_with = ":")

protoc_args.add_joined(
transitive_descriptor_sets,
join_with = ":",
format_joined = "--descriptor_set_in=%s",
omit_if_empty = True,
)

if generate_from_proto_sources:
# ProtoCompileActionBuilder.java's XPAND_TRANSITIVE_PROTO_PATH_FLAGS
# leaves this off also.
if proto_source_root != ".":
protoc_args.add(proto_source_root, format = "--proto_path=%s")

# Follow ProtoCompileActionBuilder.java's
# ExpandImportArgsFn::expandToCommandLine() logic and provide a mapping
# for each file to the proto path.
for f in direct_srcs:
protoc_args.add("-I%s=%s" % (proto_import_path(f, proto_source_root), f.path))

protoc_args.add_all([
proto_import_path(f, proto_source_root)
for f in direct_srcs
])

additional_command_inputs = []
if generate_from_proto_sources:
additional_command_inputs.extend(direct_srcs)
if module_mapping_file:
additional_command_inputs.append(module_mapping_file)

Expand Down Expand Up @@ -181,16 +207,41 @@ def _swift_grpc_library_impl(ctx):

swift_toolchain = ctx.attr._toolchain[SwiftToolchainInfo]

# Direct sources are passed as arguments to protoc to generate *only* the
# files in this target, but we need to pass the transitive sources as inputs
# to the generating action so that all the dependent files are available for
# protoc to parse.
# Instead of providing all those files and opening/reading them, we use
# protoc's support for reading descriptor sets to resolve things.
direct_srcs = ctx.attr.srcs[0][ProtoInfo].direct_sources
transitive_descriptor_sets = (
ctx.attr.srcs[0][ProtoInfo].transitive_descriptor_sets
unsupported_features = ctx.disabled_features
if ctx.attr.flavor != "client_stubs":
unsupported_features.append(SWIFT_FEATURE_ENABLE_TESTING)

feature_configuration = swift_common.configure_features(
ctx = ctx,
requested_features = ctx.features + [SWIFT_FEATURE_NO_GENERATED_HEADER],
swift_toolchain = swift_toolchain,
unsupported_features = unsupported_features,
)

generate_from_proto_sources = swift_common.is_enabled(
feature_configuration = feature_configuration,
feature_name = SWIFT_FEATURE_GENERATE_FROM_RAW_PROTO_FILES,
)

srcs_proto_info = ctx.attr.srcs[0][ProtoInfo]

# Only the files for direct sources should be generated, but the
# transitive descriptor sets are still need to be able to parse/load
# those descriptors.
direct_srcs = srcs_proto_info.direct_sources
if generate_from_proto_sources:
# Take the transitive descriptor sets from the proto_library deps,
# so the direct sources won't be in any descriptor sets to reduce
# the input to the action (and what protoc has to parse).
direct_descriptor_set = srcs_proto_info.direct_descriptor_set
transitive_descriptor_sets = depset(direct = [
x
for x in srcs_proto_info.transitive_descriptor_sets.to_list()
if x != direct_descriptor_set
])
else:
transitive_descriptor_sets = srcs_proto_info.transitive_descriptor_sets

deps = ctx.attr.deps

minimal_module_mappings = deps[0][SwiftProtoInfo].module_mappings
Expand All @@ -209,9 +260,10 @@ def _swift_grpc_library_impl(ctx):
ctx.label,
ctx.actions,
direct_srcs,
ctx.attr.srcs[0][ProtoInfo].proto_source_root,
srcs_proto_info.proto_source_root,
transitive_descriptor_sets,
transitive_module_mapping_file,
generate_from_proto_sources,
ctx.executable._mkdir_and_run,
ctx.executable._protoc,
ctx.executable._protoc_gen_swiftgrpc,
Expand All @@ -225,17 +277,6 @@ def _swift_grpc_library_impl(ctx):
# action.
compile_deps = deps + ctx.attr._proto_support

unsupported_features = ctx.disabled_features
if ctx.attr.flavor != "client_stubs":
unsupported_features.append(SWIFT_FEATURE_ENABLE_TESTING)

feature_configuration = swift_common.configure_features(
ctx = ctx,
requested_features = ctx.features + [SWIFT_FEATURE_NO_GENERATED_HEADER],
swift_toolchain = swift_toolchain,
unsupported_features = unsupported_features,
)

module_name = swift_common.derive_module_name(ctx.label)

compilation_outputs = swift_common.compile(
Expand Down

1 comment on commit c4e2328

@keith
Copy link
Member

@keith keith commented on c4e2328 Sep 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.