Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create experimental flag to include source info in proto descriptors #10925

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -337,7 +337,10 @@ public static void writeDescriptorSet(
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),
Expand All @@ -353,7 +356,13 @@ public static void writeDescriptorSet(
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(
Expand All @@ -365,7 +374,8 @@ private static ToolchainInvocation createDescriptorSetToolchain(CharSequence out
/* pluginExecutable= */ null,
/* runtime= */ null,
/* blacklistedProtos= */ NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER)),
outReplacement);
outReplacement,
protocOpts.build());
}

/** Whether to use exports in the proto compile action. */
Expand Down Expand Up @@ -552,6 +562,8 @@ static CustomCommandLine createCommandLineFromToolchains(
"--plugin=protoc-gen-PLUGIN_%s=%s",
invocation.name, toolchain.pluginExecutable().getExecutable().getExecPath());
}

cmdLine.addAll(invocation.protocOpts);
}

cmdLine.addAll(protocOpts);
Expand Down Expand Up @@ -749,13 +761,23 @@ public static class ToolchainInvocation {
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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,16 @@ public static class Options extends FragmentOptions {
)
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",
Expand Down Expand Up @@ -208,6 +218,8 @@ public FragmentOptions getHost() {
loadProtoToolchainForJavaliteFromComGoogleProtobuf;
host.protoCompiler = protoCompiler;
host.protocOpts = protocOpts;
host.experimentalProtoDescriptorSetsIncludeSourceInfo =
experimentalProtoDescriptorSetsIncludeSourceInfo;
host.experimentalProtoExtraActions = experimentalProtoExtraActions;
host.protoCompiler = protoCompiler;
host.protoToolchainForJava = protoToolchainForJava;
Expand Down Expand Up @@ -261,6 +273,10 @@ public ImmutableList<String> protocOpts() {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1003,4 +1003,33 @@ public void testMigrationLabelNotRequiredWhenDisabled() throws Exception {

getConfiguredTarget("//a");
}

@Test
public void testNoExperimentalProtoDescriptorSetsIncludeSourceInfo() throws Exception {
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 {
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");
}
}