Skip to content

Commit

Permalink
Revert "Remove --incompatible_blacklisted_protos_requires_proto_info"
Browse files Browse the repository at this point in the history
This reverts commit fe4a680.
  • Loading branch information
philwo committed Dec 10, 2020
1 parent 96a2551 commit e43825d
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,19 @@ public static class Options extends FragmentOptions {
help = "If true, add --allowed_public_imports to the java compile actions.")
public boolean experimentalJavaProtoAddAllowedPublicImports;

@Option(
name = "incompatible_blacklisted_protos_requires_proto_info",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If enabled, 'proto_lang_toolchain.blacklisted_protos' requires provider 'ProtoInfo'")
public boolean blacklistedProtosRequiresProtoInfo;

@Override
public FragmentOptions getHost() {
Options host = (Options) super.getHost();
Expand All @@ -187,6 +200,7 @@ public FragmentOptions getHost() {
host.experimentalJavaProtoAddAllowedPublicImports =
experimentalJavaProtoAddAllowedPublicImports;
host.generatedProtosInVirtualImports = generatedProtosInVirtualImports;
host.blacklistedProtosRequiresProtoInfo = blacklistedProtosRequiresProtoInfo;
return host;
}
}
Expand Down Expand Up @@ -260,4 +274,8 @@ public boolean strictPublicImports() {
public boolean generatedProtosInVirtualImports() {
return options.generatedProtosInVirtualImports;
}

public boolean blacklistedProtosRequiresProtoInfo() {
return options.blacklistedProtosRequiresProtoInfo;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.packages.Type;

Expand All @@ -34,9 +36,22 @@ public class ProtoLangToolchain implements RuleConfiguredTargetFactory {
public ConfiguredTarget create(RuleContext ruleContext)
throws InterruptedException, RuleErrorException, ActionConflictException {
NestedSetBuilder<Artifact> blacklistedProtos = NestedSetBuilder.stableOrder();
for (ProtoInfo protoInfo :
ruleContext.getPrerequisites("blacklisted_protos", ProtoInfo.PROVIDER)) {
blacklistedProtos.addTransitive(protoInfo.getOriginalTransitiveProtoSources());
for (TransitiveInfoCollection protos : ruleContext.getPrerequisites("blacklisted_protos")) {
ProtoInfo protoInfo = protos.get(ProtoInfo.PROVIDER);
if (protoInfo == null
&& ruleContext
.getFragment(ProtoConfiguration.class)
.blacklistedProtosRequiresProtoInfo()) {
ruleContext.ruleError(
"'" + ruleContext.getLabel() + "' does not have mandatory provider 'ProtoInfo'.");
}
if (protoInfo != null) {
blacklistedProtos.addTransitive(protoInfo.getOriginalTransitiveProtoSources());
} else {
// Only add files from FileProvider if |protos| is not a proto_library to avoid adding
// the descriptor_set of proto_library to the list of blacklisted files.
blacklistedProtos.addTransitive(protos.getProvider(FileProvider.class).getFilesToBuild());
}
}

return new RuleConfiguredTargetBuilder(ruleContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
import static com.google.devtools.build.lib.packages.BuildType.LABEL;
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier;
import com.google.devtools.build.lib.packages.Type;

/** Implements {code proto_lang_toolchain}. */
Expand Down Expand Up @@ -72,7 +74,8 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
.add(
attr("blacklisted_protos", LABEL_LIST)
.allowedFileTypes()
.mandatoryProviders(StarlarkProviderIdentifier.forKey(ProtoInfo.PROVIDER.getKey())))
.mandatoryBuiltinProviders(
ImmutableList.<Class<? extends TransitiveInfoProvider>>of(FileProvider.class)))
.requiresConfigurationFragments(ProtoConfiguration.class)
.advertiseProvider(ProtoLangToolchainProvider.class)
.removeAttribute("data")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,44 @@ public void protoToolchainBlacklistTransitiveProtos() throws Exception {
getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class));
}

@Test
public void protoToolchainMixedBlacklist() throws Exception {
// Tests legacy behaviour.
useConfiguration("--incompatible_blacklisted_protos_requires_proto_info=false");

scratch.file(
"third_party/x/BUILD",
TestConstants.LOAD_PROTO_LIBRARY,
"licenses(['unencumbered'])",
"cc_binary(name = 'plugin', srcs = ['plugin.cc'])",
"cc_library(name = 'runtime', srcs = ['runtime.cc'])",
"proto_library(name = 'metadata', srcs = ['metadata.proto'])",
"proto_library(",
" name = 'descriptor',",
" srcs = ['descriptor.proto'],",
" strip_import_prefix = '/third_party')",
"filegroup(name = 'any', srcs = ['any.proto'])");

scratch.file(
"foo/BUILD",
TestConstants.LOAD_PROTO_LANG_TOOLCHAIN,
"proto_lang_toolchain(",
" name = 'toolchain',",
" command_line = 'cmd-line',",
" plugin = '//third_party/x:plugin',",
" runtime = '//third_party/x:runtime',",
" blacklisted_protos = [",
" '//third_party/x:metadata',",
" '//third_party/x:descriptor',",
" '//third_party/x:any']",
")");

update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus());

validateProtoLangToolchain(
getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class));
}

@Test
public void optionalFieldsAreEmpty() throws Exception {
scratch.file(
Expand Down

0 comments on commit e43825d

Please sign in to comment.