diff options
Diffstat (limited to 'src')
6 files changed, 49 insertions, 15 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/whitelisting/Whitelist.java b/src/main/java/com/google/devtools/build/lib/analysis/whitelisting/Whitelist.java index 8850f1a9ec..10e776a3be 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/whitelisting/Whitelist.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/whitelisting/Whitelist.java @@ -44,13 +44,10 @@ public final class Whitelist { * * @param whitelistName The name of the whitelist. This has to comply with attribute naming * standards and will be used as a suffix for the attribute name. - * @param packageGroupWhitelist Label for the package group with the whitelist. */ - public static Attribute.Builder<Label> getAttributeFromWhitelistName( - String whitelistName, Label packageGroupWhitelist) { + public static Attribute.Builder<Label> getAttributeFromWhitelistName(String whitelistName) { String attributeName = getAttributeNameFromWhitelistName(whitelistName); return attr(attributeName, LABEL) - .value(packageGroupWhitelist) .cfg(HOST) .mandatoryNativeProviders(ImmutableList.of(PackageSpecificationProvider.class)); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java index 549bbfeda0..ecfdcb65b2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java @@ -19,11 +19,13 @@ import com.google.common.base.Optional; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.AliasProvider; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; @@ -53,6 +55,14 @@ public abstract class AndroidFeatureFlagSetProvider implements TransitiveInfoPro } /** + * Constructs a definition for the attribute used to restrict access to feature flags. The + * whitelist will only be reached if the feature_flags attribute is explicitly set. + */ + public static Attribute.Builder<Label> getWhitelistAttribute(RuleDefinitionEnvironment env) { + return ConfigFeatureFlag.getWhitelistAttribute(env, FEATURE_FLAG_ATTR); + } + + /** * Builds a map which can be used with create, confirming that the desired flag values were * actually received, and producing an error if they were not (because aliases were used). * diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index b2f064848e..763a134cba 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java @@ -54,7 +54,6 @@ import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidManifestMerger; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ConfigurationDistinguisher; -import com.google.devtools.build.lib.rules.config.ConfigFeatureFlag; import com.google.devtools.build.lib.rules.config.ConfigFeatureFlagProvider; import com.google.devtools.build.lib.rules.cpp.CppOptions; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; @@ -917,9 +916,8 @@ public final class AndroidRuleClasses { .allowedRuleClasses("config_feature_flag") .allowedFileTypes() .nonconfigurable("defines an aspect of configuration") - .mandatoryProviders( - ImmutableList.of(ConfigFeatureFlagProvider.id()))) - .add(ConfigFeatureFlag.getWhitelistAttribute(env)) + .mandatoryProviders(ImmutableList.of(ConfigFeatureFlagProvider.id()))) + .add(AndroidFeatureFlagSetProvider.getWhitelistAttribute(env)) // The resource extractor is used at the binary level to extract java resources from the // deploy jar so that they can be added to the APK. .add( diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java index 70fa67d8d1..68bf177ce5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java @@ -35,6 +35,8 @@ import com.google.devtools.build.lib.analysis.whitelisting.Whitelist; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.Attribute; +import com.google.devtools.build.lib.packages.Attribute.ComputedDefault; +import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.syntax.Printer; import java.util.List; @@ -46,11 +48,32 @@ public class ConfigFeatureFlag implements RuleConfiguredTargetFactory { /** The name of the policy that is used to restrict access to the config_feature_flag rule. */ private static final String WHITELIST_NAME = "config_feature_flag"; + /** The label of the policy that is used to restrict access to the config_feature_flag rule. */ + private static final String WHITELIST_LABEL = + "//tools/whitelists/config_feature_flag:config_feature_flag"; + /** Constructs a definition for the attribute used to restrict access to config_feature_flag. */ public static Attribute.Builder<Label> getWhitelistAttribute(RuleDefinitionEnvironment env) { - return Whitelist.getAttributeFromWhitelistName( - WHITELIST_NAME, - env.getToolsLabel("//tools/whitelists/config_feature_flag:config_feature_flag")); + return Whitelist.getAttributeFromWhitelistName(WHITELIST_NAME) + .value(env.getToolsLabel(WHITELIST_LABEL)); + } + + /** + * Constructs a definition for the attribute used to restrict access to config_feature_flag. The + * whitelist will only be reached if the given {@code attributeToInspect} has a value explicitly + * specified. It must be non-configurable. + */ + public static Attribute.Builder<Label> getWhitelistAttribute( + RuleDefinitionEnvironment env, String attributeToInspect) { + final Label label = env.getToolsLabel(WHITELIST_LABEL); + return Whitelist.getAttributeFromWhitelistName(WHITELIST_NAME) + .value( + new ComputedDefault() { + @Override + public Label getDefault(AttributeMap rule) { + return rule.isAttributeValueExplicitlySpecified(attributeToInspect) ? label : null; + } + }); } /** diff --git a/src/test/java/com/google/devtools/build/lib/analysis/whitelisting/WhitelistDummyRule.java b/src/test/java/com/google/devtools/build/lib/analysis/whitelisting/WhitelistDummyRule.java index 28a5b19c09..05d7bda898 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/whitelisting/WhitelistDummyRule.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/whitelisting/WhitelistDummyRule.java @@ -34,7 +34,8 @@ public final class WhitelistDummyRule implements RuleDefinition, RuleConfiguredT public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { return builder .add( - Whitelist.getAttributeFromWhitelistName("dummy", env.getLabel("//whitelist:whitelist"))) + Whitelist.getAttributeFromWhitelistName("dummy") + .value(env.getLabel("//whitelist:whitelist"))) .build(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java index 212f2700a1..686ddc1a66 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java @@ -3040,13 +3040,12 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { } @Test - public void testFeatureFlagPolicyDoesNotBlockRuleIfFlagValuesNotUsed() throws Exception { + public void testFeatureFlagPolicyIsNotUsedIfFlagValuesNotUsed() throws Exception { scratch.overwriteFile( "tools/whitelists/config_feature_flag/BUILD", "package_group(", " name = 'config_feature_flag',", - " packages = ['//flag'])"); - scratch.file("flag/BUILD"); + " packages = ['*super* busted package group'])"); scratch.file( "java/com/google/android/foo/BUILD", "android_binary(", @@ -3055,7 +3054,13 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " srcs = [':FooFlags.java'],", ")"); assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNotNull(); + // the package_group is busted, so we would have failed to get this far if we depended on it assertNoEvents(); + // sanity check time: does this test actually test what we're testing for? + reporter.removeHandler(failFastHandler); + assertThat(getConfiguredTarget("//tools/whitelists/config_feature_flag:config_feature_flag")) + .isNull(); + assertContainsEvent("*super* busted package group"); } @Test |