From 8e7c692524dff4afc26b38e67cdc5b75205b75d6 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sat, 7 Nov 2020 18:02:25 +0100 Subject: [PATCH] Remove --incompatible_blacklisted_protos_requires_proto_info Cleanup after flipping the flag in Bazel 4.0 (53705f6a014067a8f17cf0af4da0ad99fe1169be) --- .../lib/rules/proto/ProtoConfiguration.java | 18 --------- .../lib/rules/proto/ProtoLangToolchain.java | 18 +-------- .../rules/proto/ProtoLangToolchainRule.java | 7 +--- .../rules/proto/ProtoLangToolchainTest.java | 38 ------------------- 4 files changed, 4 insertions(+), 77 deletions(-) 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 f7a536bbaeafe7..8200dab3cf89a3 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 @@ -169,19 +169,6 @@ 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(); @@ -201,7 +188,6 @@ public FragmentOptions getHost() { host.experimentalJavaProtoAddAllowedPublicImports = experimentalJavaProtoAddAllowedPublicImports; host.generatedProtosInVirtualImports = generatedProtosInVirtualImports; - host.blacklistedProtosRequiresProtoInfo = blacklistedProtosRequiresProtoInfo; return host; } } @@ -295,8 +281,4 @@ public boolean strictPublicImports() { public boolean generatedProtosInVirtualImports() { return options.generatedProtosInVirtualImports; } - - public boolean blacklistedProtosRequiresProtoInfo() { - return options.blacklistedProtosRequiresProtoInfo; - } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchain.java index db9c3ebdfed7d6..6e340d86233740 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchain.java @@ -36,22 +36,8 @@ public class ProtoLangToolchain implements RuleConfiguredTargetFactory { public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException, RuleErrorException, ActionConflictException { NestedSetBuilder blacklistedProtos = NestedSetBuilder.stableOrder(); - 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()); - } + for (ProtoInfo protoInfo : ruleContext.getPrerequisites("blacklisted_protos", ProtoInfo.PROVIDER)) { + blacklistedProtos.addTransitive(protoInfo.getOriginalTransitiveProtoSources()); } return new RuleConfiguredTargetBuilder(ruleContext) diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainRule.java index 65e27e963c4793..13425f5a2b1b5e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainRule.java @@ -18,14 +18,12 @@ 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.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.config.HostTransition; 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}. */ @@ -70,8 +68,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi .add( attr("blacklisted_protos", LABEL_LIST) .allowedFileTypes() - .mandatoryBuiltinProviders( - ImmutableList.>of(FileProvider.class))) + .mandatoryProviders(StarlarkProviderIdentifier.forKey(ProtoInfo.PROVIDER.getKey()))) .requiresConfigurationFragments(ProtoConfiguration.class) .advertiseProvider(ProtoLangToolchainProvider.class) .removeAttribute("data") diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java index c9e32316d6dc88..c75aace52a95fb 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java @@ -140,44 +140,6 @@ 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(