Create experimental flag to include source info in proto descriptors
`protoc` has an option to include source info (e.g. comments) in the
generated descriptor sets. Well done proto generators also relay the
comments into the generated source so IDEs that do source indexing
and pick them up to provide contextual tooltips/documentation/etc.
This change adds an experimental flag to include this info in the
descriptors generated by `proto_library` so we can collect some
data whether including source info by default is feasible.
See #9337 for more context.
//cc @thomasvl
Closes #10925.
PiperOrigin-RevId: 299838533
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
index f26499a..1694a3f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
@@ -14,12 +14,12 @@
package com.google.devtools.build.lib.rules.proto;
-import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.Iterables.isEmpty;
import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER;
import static com.google.devtools.build.lib.rules.proto.ProtoCommon.areDepsStrict;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -337,7 +337,9 @@
SpawnAction.Builder actions =
createActions(
ruleContext,
- ImmutableList.of(createDescriptorSetToolchain(output.getExecPathString())),
+ ImmutableList.of(
+ createDescriptorSetToolchain(
+ ruleContext.getFragment(ProtoConfiguration.class), output.getExecPathString())),
protoInfo,
ruleContext.getLabel(),
ImmutableList.of(output),
@@ -353,7 +355,13 @@
ruleContext.registerAction(actions.build(ruleContext));
}
- private static ToolchainInvocation createDescriptorSetToolchain(CharSequence outReplacement) {
+ private static ToolchainInvocation createDescriptorSetToolchain(
+ ProtoConfiguration configuration, CharSequence outReplacement) {
+ ImmutableList.Builder<String> protocOpts = ImmutableList.builder();
+ if (configuration.experimentalProtoDescriptorSetsIncludeSourceInfo()) {
+ protocOpts.add("--include_source_info");
+ }
+
return new ToolchainInvocation(
"dontcare",
ProtoLangToolchainProvider.create(
@@ -365,7 +373,8 @@
/* pluginExecutable= */ null,
/* runtime= */ null,
/* blacklistedProtos= */ NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER)),
- outReplacement);
+ outReplacement,
+ protocOpts.build());
}
/** Whether to use exports in the proto compile action. */
@@ -552,6 +561,8 @@
"--plugin=protoc-gen-PLUGIN_%s=%s",
invocation.name, toolchain.pluginExecutable().getExecutable().getExecPath());
}
+
+ cmdLine.addAll(invocation.protocOpts);
}
cmdLine.addAll(protocOpts);
@@ -749,13 +760,23 @@
final String name;
public final ProtoLangToolchainProvider toolchain;
final CharSequence outReplacement;
+ final ImmutableList<String> protocOpts;
public ToolchainInvocation(
String name, ProtoLangToolchainProvider toolchain, CharSequence outReplacement) {
- checkState(!name.contains(" "), "Name %s should not contain spaces", name);
+ this(name, toolchain, outReplacement, ImmutableList.of());
+ }
+
+ public ToolchainInvocation(
+ String name,
+ ProtoLangToolchainProvider toolchain,
+ CharSequence outReplacement,
+ ImmutableList<String> protocOpts) {
+ Preconditions.checkState(!name.contains(" "), "Name %s should not contain spaces", name);
this.name = name;
this.toolchain = toolchain;
this.outReplacement = outReplacement;
+ this.protocOpts = Preconditions.checkNotNull(protocOpts);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
index 9ada5aa..dfe5e9e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
@@ -75,6 +75,15 @@
public boolean experimentalProtoExtraActions;
@Option(
+ name = "experimental_proto_descriptor_sets_include_source_info",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.OUTPUT_SELECTION,
+ effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOADING_AND_ANALYSIS},
+ metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+ help = "Run extra actions for alternative Java api versions in a proto_library.")
+ public boolean experimentalProtoDescriptorSetsIncludeSourceInfo;
+
+ @Option(
name = "proto_compiler",
defaultValue = "@com_google_protobuf//:protoc",
converter = CoreOptionConverters.LabelConverter.class,
@@ -181,6 +190,8 @@
host.loadProtoRulesFromBzl = loadProtoRulesFromBzl;
host.protoCompiler = protoCompiler;
host.protocOpts = protocOpts;
+ host.experimentalProtoDescriptorSetsIncludeSourceInfo =
+ experimentalProtoDescriptorSetsIncludeSourceInfo;
host.experimentalProtoExtraActions = experimentalProtoExtraActions;
host.protoCompiler = protoCompiler;
host.protoToolchainForJava = protoToolchainForJava;
@@ -234,6 +245,10 @@
return protocOpts;
}
+ public boolean experimentalProtoDescriptorSetsIncludeSourceInfo() {
+ return options.experimentalProtoDescriptorSetsIncludeSourceInfo;
+ }
+
/**
* Returns true if we will run extra actions for actions that are not run by default. If this
* is enabled, e.g. all extra_actions for alternative api-versions or language-flavours of a
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
index a905862..d3d2608 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
@@ -1003,4 +1003,41 @@
getConfiguredTarget("//a");
}
+
+ @Test
+ public void testNoExperimentalProtoDescriptorSetsIncludeSourceInfo() throws Exception {
+ if (!isThisBazel()) {
+ return;
+ }
+
+ scratch.file(
+ "x/BUILD",
+ TestConstants.LOAD_PROTO_LIBRARY,
+ "proto_library(",
+ " name = 'a_proto',",
+ " srcs = ['a.proto'],",
+ ")");
+
+ Iterable<String> commandLine = paramFileArgsForAction(getDescriptorWriteAction("//x:a_proto"));
+ assertThat(commandLine).doesNotContain("--include_source_info");
+ }
+
+ @Test
+ public void testExperimentalProtoDescriptorSetsIncludeSourceInfo() throws Exception {
+ if (!isThisBazel()) {
+ return;
+ }
+
+ useConfiguration("--experimental_proto_descriptor_sets_include_source_info");
+ scratch.file(
+ "x/BUILD",
+ TestConstants.LOAD_PROTO_LIBRARY,
+ "proto_library(",
+ " name = 'a_proto',",
+ " srcs = ['a.proto'],",
+ ")");
+
+ Iterable<String> commandLine = paramFileArgsForAction(getDescriptorWriteAction("//x:a_proto"));
+ assertThat(commandLine).contains("--include_source_info");
+ }
}
