Support --include_source_info in native proto_library

[htuch]

It's useful to be able to specify an optional inclusion of source_code_info in the proto descriptors generated by proto_library, for the purpose of documentation generation tools etc. I was working on an aspect that used the proto_library output descriptors, and this is stripped away. It's useful to strip by default, but it would be also good to have a param to native.proto_library that allows us to specify targets that provide the source code info.

[cgrushko]

We can add a global flag to control the command line created by proto_library (or switch proto_library's descriptor creating code to use a real proto_lang_toolchain instead of hardcoding).

I emphasize it being global - allowing some proto_library to have it and some not has two problems:

1. it complicates the rule, which makes it harder to maintain and harder to use. I see better alternatives (see below)
2. once people start relying on this for their compilation, they're in a world of pain - in order to depend add a dependency from your proto to another team's proto, you need to add this hypothetical attribute to all of the other proto's transitive dependencies. We had bad experience with this internally.

Alternative - modify your aspect to create a command line to compile srcs with the flag you need.

Anyway - I'm no longer on the proto project, though I'm happy to give my 2 cents.

[htuch]

As a workaround, we switched the aspect to compile from srcs, but that loses some of the advantages that proto_library output descriptor sets have, in particular around taming the import path complexity (which gets tricky with externals).

What if we made the proto_library by default produce descriptor sets and then provide a stripped_proto_library (or something to that effect) that can reduce then remove the unnecessary source code info for those parts of a project that need a reduced size descriptor set?

[abscondment]

I'm running into the need for this, too. We have a plugin that does some code generation directed by comment-based annotations; without the source info, all of the annotations are missing.

[cgrushko]

@abscondment the recommended approach is to use proto options - they show up in the descriptor set and are verified by the compiler at build time.

[abscondment]

@cgrushko Neat - thanks for the pointer!

[cgrushko]

heh, I was getting ready for push back :) I realize the migration to options will take time, but I think the outcome will be better :)

[ashi009]

I think global option is not ideal, as when generating doc/swagger from proto files, SourceCodeInfo is critical, and it's likely that a single build requires both stripped and unstripped descriptor set.

I'd prefer to have proto_library generate two sets of descriptor set, one with all SourceCodeInfo stripped out, and one with them. Say, adding a proto.unstripped_transitive_descriptor_sets next to the proto.transitive_descriptor_sets.

Whoever needs SourceCodeInfo can use proto.unstripped_transitive_descriptor_sets to create their rule, and I think bazel is smart enough not to build those unstripped descriptors if no one depends on them.

[ashi009]

Hey @cgrushko and @lberki, is there any update on this FR?

I just stumbled into this issue again, as I found that I can't use pb.go files in bazel-bin to host godoc server as all the comments are stripped out. For this use case the proto extension option won't work for all the existing protoc-gen plugins.

[ashi009]

ATM gapic now provides proto_library_with_info rule for doing this:

load("@com_google_api_codegen//rules_gapic:gapic.bzl", "proto_library_with_info")

proto_library(
    name = "iam_admin_proto",
    srcs = ["iam.proto"],
    deps = [
        "//google/api:annotations_proto",
        "//google/iam/v1:iam_policy_proto",
        "//google/iam/v1:policy_proto",
        "@com_google_protobuf//:empty_proto",
        "@com_google_protobuf//:field_mask_proto",
        "@com_google_protobuf//:timestamp_proto",
    ],
)

proto_library_with_info(
    name = "iam_admin_proto_with_info",
    deps = [":iam_admin_proto"],
)

Still, it would be nice to have this shipped with bazel natively.

[Yannic]

Duplicating into bazelbuild/bazel#9337

TL;DR: Bazel introduced --experimental_proto_descriptor_sets_include_source_info which passes --include_source_info when compiling the descriptor set of proto_library targets. It may become the default in the future.