aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar mstaib <mstaib@google.com>2018-05-16 10:13:51 -0700
committerGravatar Copybara-Service <copybara-piper@google.com>2018-05-16 10:15:49 -0700
commit188b578c0e9995c864d284f22de4e48c220b0596 (patch)
tree104640b011b04e2d787098eacdd52df5bf22981d
parent238079669ec280a972adf68ee987c5d407e15b26 (diff)
Enable manual trimming of config_feature_flags.
This enables users of config_feature_flags to specify the flags used by the transitive closure of a particular target in the transitive_configs attribute of all targets. It also adds a flag - --enforce_transitive_configs_for_config_feature_flag - which enforces this specification and uses it to trim the set of flags available to that target. RELNOTES: None. PiperOrigin-RevId: 196846092
Diffstat
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java10
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfiguration.java72
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagOptions.java34
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java114
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java7
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java4
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/config/ConfigRules.java6
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/AbstractAndroidLocalTestTestBase.java58
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java70
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfigurationTest.java48
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java13
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java122
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java770
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java10
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java10
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java3
-rw-r--r--src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java1
17 files changed, 1283 insertions, 69 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index 712be737a2..a1454b23d2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -181,6 +181,12 @@ public class BaseRuleClasses {
}
/**
+ * The attribute used to list the configuration properties used by a target and its transitive
+ * dependencies. Currently only supports config_feature_flag.
+ */
+ public static final String TAGGED_TRIMMING_ATTR = "transitive_configs";
+
+ /**
* Share common attributes across both base and Skylark base rules.
*/
public static RuleClass.Builder commonCoreAndSkylarkAttributes(RuleClass.Builder builder) {
@@ -196,6 +202,10 @@ public class BaseRuleClasses {
.nonconfigurable(
"special attribute integrated more deeply into Bazel's core logic"))
.add(
+ attr(TAGGED_TRIMMING_ATTR, NODEP_LABEL_LIST)
+ .orderIndependent()
+ .nonconfigurable("Used in determining configuration"))
+ .add(
attr("deprecation", STRING)
.value(deprecationDefault)
.nonconfigurable("Used in core loading phase logic with no access to configs"))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfiguration.java
index 6a1e7c770e..708a8258d3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfiguration.java
@@ -14,8 +14,11 @@
package com.google.devtools.build.lib.rules.config;
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
+import com.google.common.collect.ImmutableSortedSet;
import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
import com.google.devtools.build.lib.actions.ArtifactOwner;
@@ -24,6 +27,7 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
+import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import java.util.Map;
@@ -44,8 +48,25 @@ public final class ConfigFeatureFlagConfiguration extends BuildConfiguration.Fra
public static final class Loader implements ConfigurationFragmentFactory {
@Override
public BuildConfiguration.Fragment create(
- ConfigurationEnvironment env, BuildOptions buildOptions) {
- return new ConfigFeatureFlagConfiguration(buildOptions.get(ConfigFeatureFlagOptions.class));
+ ConfigurationEnvironment env, BuildOptions buildOptions)
+ throws InvalidConfigurationException {
+ ConfigFeatureFlagOptions options = buildOptions.get(ConfigFeatureFlagOptions.class);
+ if (!options.unknownFlags.isEmpty()) {
+ ImmutableList.Builder<String> errorMessage = new ImmutableList.Builder<>();
+ for (Label missingLabel : options.unknownFlags) {
+ errorMessage.add(
+ String.format(
+ "Feature flag %1$s was accessed in a configuration it is not present in. All "
+ + "targets which depend on %1$s directly or indirectly must name it in their "
+ + "transitive_configs attribute.",
+ missingLabel));
+ }
+ throw new InvalidConfigurationException(
+ "Some feature flags were incorrectly specified:\n"
+ + Joiner.on("\n").join(errorMessage.build()));
+ }
+
+ return new ConfigFeatureFlagConfiguration(options);
}
@Override
@@ -59,18 +80,46 @@ public final class ConfigFeatureFlagConfiguration extends BuildConfiguration.Fra
}
}
+ /** Exception thrown when a flag is accessed in a configuration it is not present in. */
+ public static final class MissingFlagException extends RuntimeException {
+ public MissingFlagException(Label label) {
+ super(
+ String.format(
+ "Feature flag %1$s was accessed in a configuration it was trimmed from.", label));
+ }
+ }
+
private final ImmutableSortedMap<Label, String> flagValues;
+ private final ImmutableSortedSet<Label> knownDefaultFlags;
+ private final boolean isTrimmed;
@Nullable private final String flagHash;
/** Creates a new configuration fragment from the given {@link ConfigFeatureFlagOptions}. */
public ConfigFeatureFlagConfiguration(ConfigFeatureFlagOptions options) {
- this(options.getFlagValues());
+ // TODO(mstaib): we'd love to only construct these when we're trimmed, but this is still what
+ // the top level looks like - make constructing an untrimmed configuration an error when no
+ // configurations are constructed untrimmed.
+ this(
+ options.getFlagValues(),
+ options.getKnownDefaultFlags().orElse(ImmutableSortedSet.of()),
+ options.enforceTransitiveConfigsForConfigFeatureFlag && options.isTrimmed());
}
@AutoCodec.Instantiator
- ConfigFeatureFlagConfiguration(ImmutableSortedMap<Label, String> flagValues) {
+ ConfigFeatureFlagConfiguration(
+ ImmutableSortedMap<Label, String> flagValues,
+ ImmutableSortedSet<Label> knownDefaultFlags,
+ boolean isTrimmed) {
this.flagValues = flagValues;
- this.flagHash = this.flagValues.isEmpty() ? null : hashFlags(this.flagValues);
+ this.knownDefaultFlags = knownDefaultFlags;
+ this.isTrimmed = isTrimmed;
+ // We don't hash flags set to their default values; all valid configurations of a target have
+ // the same set of known flags, so the set of flags set to something other than their default
+ // values is enough to disambiguate configurations. Similarly, isTrimmed need not be hashed;
+ // enforceTransitiveConfigsForConfigFeatureFlag should not change within a build, and when it's
+ // enabled, the only configuration which is untrimmed (the top-level configuration) shouldn't
+ // be used for any actual targets.
+ this.flagHash = flagValues.isEmpty() ? null : hashFlags(flagValues);
}
/** Converts the given flag values into a string hash for use as an output directory fragment. */
@@ -92,13 +141,24 @@ public final class ConfigFeatureFlagConfiguration extends BuildConfiguration.Fra
*
* <p>If the flag is not set in the current configuration, then the returned value will be absent.
*
+ * <p>If the flag has been trimmed from the current configuration, a RuntimeException
+ * (MissingFlagException) will be thrown. Because the configuration should fail to construct if a
+ * required flag is missing, and because config_feature_flag (the only intended user of this
+ * method) automatically requires itself, this should not come to pass.
+ *
* <p>This method should only be used by the rule whose label is passed here. Other rules should
* depend on that rule and read a provider exported by it. To encourage callers of this method to
* do the right thing, this class takes {@link ArtifactOwner} instead of {@link Label}; to get the
* ArtifactOwner for a rule, call {@code ruleContext.getOwner()}.
*/
public Optional<String> getFeatureFlagValue(ArtifactOwner owner) {
- return Optional.ofNullable(flagValues.get(owner.getLabel()));
+ if (flagValues.containsKey(owner.getLabel())) {
+ return Optional.of(flagValues.get(owner.getLabel()));
+ } else if (!isTrimmed || knownDefaultFlags.contains(owner.getLabel())) {
+ return Optional.empty();
+ } else {
+ throw new MissingFlagException(owner.getLabel());
+ }
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagOptions.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagOptions.java
index dda852b30d..dcbeb01c15 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagOptions.java
@@ -62,8 +62,6 @@ public final class ConfigFeatureFlagOptions extends FragmentOptions {
/**
* Whether to perform user-guided trimming of feature flags based on the tagging in the
* transitive_configs attribute.
- *
- * <p>Currently a no-op.
*/
@Option(
name = "enforce_transitive_configs_for_config_feature_flag",
@@ -277,4 +275,36 @@ public final class ConfigFeatureFlagOptions extends FragmentOptions {
this.knownDefaultFlags = knownDefaultFlagsBuilder.build();
this.unknownFlags = unknownFlagsBuilder.build();
}
+
+ /** Returns whether work needs to be done to trim this configuration to the given set of flags. */
+ public boolean requiresTrimming(Set<Label> requiredFlags) {
+ if (!this.isTrimmed()) {
+ // this configuration is untrimmed, so there is definitely work to do
+ return true;
+ }
+ if (requiredFlags.size()
+ != this.flagValues.size() + this.knownDefaultFlags.size() + this.unknownFlags.size()) {
+ // the set of flags we know about isn't the same size as the set of flags we need to know
+ // about, so there must be work to do
+ return true;
+ }
+ for (Label flag : requiredFlags) {
+ if (!(flagValues.containsKey(flag)
+ || knownDefaultFlags.contains(flag)
+ || unknownFlags.contains(flag))) {
+ // we need a flag we don't know anything about, so there must be work to do
+ return true;
+ }
+ }
+ // the sets are the same size and contain the same elements, so trimming would just get us back
+ // to the state we're in right now
+ return false;
+ }
+
+ @Override
+ public ConfigFeatureFlagOptions getHost() {
+ ConfigFeatureFlagOptions options = (ConfigFeatureFlagOptions) super.getHost();
+ options.enforceTransitiveConfigsForConfigFeatureFlag = false;
+ return options;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java
new file mode 100644
index 0000000000..04806e5fcc
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java
@@ -0,0 +1,114 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License
+
+package com.google.devtools.build.lib.rules.config;
+
+import static com.google.devtools.build.lib.packages.BuildType.LABEL_KEYED_STRING_DICT;
+import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL_LIST;
+
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Ordering;
+import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
+import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.packages.RuleClass;
+import com.google.devtools.build.lib.packages.RuleTransitionFactory;
+
+/**
+ * A transition factory for trimming feature flags manually via an attribute which specifies the
+ * feature flags used by transitive dependencies.
+ */
+public class ConfigFeatureFlagTaggedTrimmingTransitionFactory implements RuleTransitionFactory {
+
+ private static final class ConfigFeatureFlagTaggedTrimmingTransition implements PatchTransition {
+ public static final ConfigFeatureFlagTaggedTrimmingTransition EMPTY =
+ new ConfigFeatureFlagTaggedTrimmingTransition(ImmutableSortedSet.of());
+
+ private final ImmutableSortedSet<Label> flags;
+ private final int cachedHashCode;
+
+ ConfigFeatureFlagTaggedTrimmingTransition(ImmutableSortedSet<Label> flags) {
+ this.flags = flags;
+ this.cachedHashCode = this.flags.hashCode();
+ }
+
+ @Override
+ public BuildOptions apply(BuildOptions options) {
+ if (!(options.contains(ConfigFeatureFlagOptions.class)
+ && options.get(ConfigFeatureFlagOptions.class)
+ .enforceTransitiveConfigsForConfigFeatureFlag
+ && options.get(ConfigFeatureFlagOptions.class).requiresTrimming(flags))) {
+ return options;
+ }
+ BuildOptions result = options.clone();
+ result.get(ConfigFeatureFlagOptions.class).trimFlagValues(flags);
+ return result;
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ return other instanceof ConfigFeatureFlagTaggedTrimmingTransition
+ && this.flags.equals(((ConfigFeatureFlagTaggedTrimmingTransition) other).flags);
+ }
+
+ @Override
+ public int hashCode() {
+ return cachedHashCode;
+ }
+
+ @Override
+ public String toString() {
+ return String.format("ConfigFeatureFlagTaggedTrimmingTransition{flags=%s}", flags);
+ }
+ }
+
+ private final String attributeName;
+
+ public ConfigFeatureFlagTaggedTrimmingTransitionFactory(String attributeName) {
+ this.attributeName = attributeName;
+ }
+
+ @Override
+ public PatchTransition buildTransitionFor(Rule rule) {
+ NonconfigurableAttributeMapper attrs = NonconfigurableAttributeMapper.of(rule);
+ RuleClass ruleClass = rule.getRuleClassObject();
+ if (ruleClass.getName().equals(ConfigRuleClasses.ConfigFeatureFlagRule.RULE_NAME)) {
+ return new ConfigFeatureFlagTaggedTrimmingTransition(ImmutableSortedSet.of(rule.getLabel()));
+ }
+
+ ImmutableSortedSet.Builder<Label> requiredLabelsBuilder =
+ new ImmutableSortedSet.Builder<>(Ordering.natural());
+ if (attrs.isAttributeValueExplicitlySpecified(attributeName)
+ && !attrs.get(attributeName, NODEP_LABEL_LIST).isEmpty()) {
+ requiredLabelsBuilder.addAll(attrs.get(attributeName, NODEP_LABEL_LIST));
+ }
+ if (ruleClass.getTransitionFactory() instanceof ConfigFeatureFlagTransitionFactory) {
+ String settingAttribute =
+ ((ConfigFeatureFlagTransitionFactory) ruleClass.getTransitionFactory())
+ .getAttributeName();
+ // Because the process of setting a flag also creates a dependency on that flag, we need to
+ // include all the set flags, even if they aren't actually declared as used by this rule.
+ requiredLabelsBuilder.addAll(attrs.get(settingAttribute, LABEL_KEYED_STRING_DICT).keySet());
+ }
+
+ ImmutableSortedSet<Label> requiredLabels = requiredLabelsBuilder.build();
+ if (requiredLabels.isEmpty()) {
+ return ConfigFeatureFlagTaggedTrimmingTransition.EMPTY;
+ }
+
+ return new ConfigFeatureFlagTaggedTrimmingTransition(requiredLabels);
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java
index 72b91b58a9..28fbf5aecb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java
@@ -105,6 +105,13 @@ public class ConfigFeatureFlagTransitionFactory implements RuleTransitionFactory
}
}
+ /**
+ * Returns the attribute examined by this transition factory.
+ */
+ public String getAttributeName() {
+ return this.attributeName;
+ }
+
@Override
public boolean equals(Object other) {
return other instanceof ConfigFeatureFlagTransitionFactory
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java
index 45a7163f78..bf0256ab92 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java
@@ -396,6 +396,7 @@ platform(
/** Rule definition for Android's config_feature_flag rule. */
public static final class ConfigFeatureFlagRule implements RuleDefinition {
+ public static final String RULE_NAME = "config_feature_flag";
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
@@ -412,13 +413,14 @@ platform(
attr("default_value", STRING)
.nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON))
.add(ConfigFeatureFlag.getWhitelistAttribute(env))
+ .removeAttribute(BaseRuleClasses.TAGGED_TRIMMING_ATTR)
.build();
}
@Override
public RuleDefinition.Metadata getMetadata() {
return RuleDefinition.Metadata.builder()
- .name("config_feature_flag")
+ .name(RULE_NAME)
.ancestors(ConfigBaseRule.class)
.factoryClass(ConfigFeatureFlag.class)
.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRules.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRules.java
index de5e22f779..4d49eafa58 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRules.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRules.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules.config;
import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.RuleSet;
import com.google.devtools.build.lib.rules.core.CoreRules;
@@ -30,9 +31,12 @@ public final class ConfigRules implements RuleSet {
@Override
public void init(ConfiguredRuleClassProvider.Builder builder) {
+ builder.setTrimmingTransitionFactory(
+ new ConfigFeatureFlagTaggedTrimmingTransitionFactory(BaseRuleClasses.TAGGED_TRIMMING_ATTR));
builder.addRuleDefinition(new ConfigRuleClasses.ConfigBaseRule());
builder.addRuleDefinition(new ConfigRuleClasses.ConfigSettingRule());
- builder.addConfig(ConfigFeatureFlagOptions.class, new ConfigFeatureFlagConfiguration.Loader());
+ builder.addConfig(ConfigFeatureFlagOptions.class,
+ new ConfigFeatureFlagConfiguration.Loader());
builder.addRuleDefinition(new ConfigRuleClasses.ConfigFeatureFlagRule());
builder.addSkylarkAccessibleTopLevels("config_common", new ConfigSkylarkCommon());
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AbstractAndroidLocalTestTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/android/AbstractAndroidLocalTestTestBase.java
index 301d2acc12..bf62f4f71c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AbstractAndroidLocalTestTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AbstractAndroidLocalTestTestBase.java
@@ -93,7 +93,9 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
@Test
public void testFeatureFlagsAttributeSetsSelectInDependency() throws Exception {
- useConfiguration("--experimental_dynamic_configs=on");
+ useConfiguration(
+ "--experimental_dynamic_configs=on",
+ "--enforce_transitive_configs_for_config_feature_flag");
writeFile(
"java/com/foo/BUILD",
"load('//java/bar:foo.bzl', 'extra_deps')",
@@ -105,6 +107,7 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
"config_setting(",
" name = 'flag1@on',",
" flag_values = {':flag1': 'on'},",
+ " transitive_configs = [':flag1'],",
")",
"config_feature_flag(",
" name = 'flag2',",
@@ -114,6 +117,7 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
"config_setting(",
" name = 'flag2@on',",
" flag_values = {':flag2': 'on'},",
+ " transitive_configs = [':flag2'],",
")",
"android_library(",
" name = 'lib',",
@@ -124,6 +128,7 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
" ':flag2@on': ['Flag2On.java'],",
" '//conditions:default': ['Flag2Off.java'],",
" }),",
+ " transitive_configs = [':flag1', ':flag2'],",
")",
"android_local_test(",
" name = 'foo',",
@@ -131,7 +136,8 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
" deps = [':lib'] + extra_deps,",
" feature_flags = {",
" 'flag1': 'on',",
- " }",
+ " },",
+ " transitive_configs = [':flag1', ':flag2'],",
")");
ConfiguredTarget binary = getConfiguredTarget("//java/com/foo");
List<String> inputs =
@@ -144,7 +150,9 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
@Test
public void testFeatureFlagsAttributeSetsSelectInTest() throws Exception {
- useConfiguration("--experimental_dynamic_configs=on");
+ useConfiguration(
+ "--experimental_dynamic_configs=on",
+ "--enforce_transitive_configs_for_config_feature_flag");
writeFile(
"java/com/foo/BUILD",
"load('//java/bar:foo.bzl', 'extra_deps')",
@@ -156,6 +164,7 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
"config_setting(",
" name = 'flag1@on',",
" flag_values = {':flag1': 'on'},",
+ " transitive_configs = [':flag1'],",
")",
"config_feature_flag(",
" name = 'flag2',",
@@ -165,6 +174,7 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
"config_setting(",
" name = 'flag2@on',",
" flag_values = {':flag2': 'on'},",
+ " transitive_configs = [':flag2'],",
")",
"android_local_test(",
" name = 'foo',",
@@ -178,7 +188,8 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
" }),",
" feature_flags = {",
" 'flag1': 'on',",
- " }",
+ " },",
+ " transitive_configs = [':flag1', ':flag2'],",
")");
ConfiguredTarget binary = getConfiguredTarget("//java/com/foo");
List<String> inputs =
@@ -192,7 +203,9 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
@Test
public void testFeatureFlagsAttributeFailsAnalysisIfFlagValueIsInvalid() throws Exception {
reporter.removeHandler(failFastHandler);
- useConfiguration("--experimental_dynamic_configs=on");
+ useConfiguration(
+ "--experimental_dynamic_configs=on",
+ "--enforce_transitive_configs_for_config_feature_flag");
writeFile(
"java/com/foo/BUILD",
"load('//java/bar:foo.bzl', 'extra_deps')",
@@ -204,13 +217,15 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
"config_setting(",
" name = 'flag1@on',",
" flag_values = {':flag1': 'on'},",
+ " transitive_configs = [':flag1'],",
")",
"android_library(",
" name = 'lib',",
" srcs = select({",
" ':flag1@on': ['Flag1On.java'],",
" '//conditions:default': ['Flag1Off.java'],",
- " })",
+ " }),",
+ " transitive_configs = [':flag1'],",
")",
"android_local_test(",
" name = 'foo',",
@@ -218,7 +233,8 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
" deps = [':lib',] + extra_deps,",
" feature_flags = {",
" 'flag1': 'invalid',",
- " }",
+ " },",
+ " transitive_configs = [':flag1'],",
")");
assertThat(getConfiguredTarget("//java/com/foo")).isNull();
assertContainsEvent(
@@ -230,7 +246,9 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
public void testFeatureFlagsAttributeFailsAnalysisIfFlagValueIsInvalidEvenIfNotUsed()
throws Exception {
reporter.removeHandler(failFastHandler);
- useConfiguration("--experimental_dynamic_configs=on");
+ useConfiguration(
+ "--experimental_dynamic_configs=on",
+ "--enforce_transitive_configs_for_config_feature_flag");
writeFile(
"java/com/foo/BUILD",
"load('//java/bar:foo.bzl', 'extra_deps')",
@@ -259,7 +277,9 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
@Test
public void testFeatureFlagsAttributeSetsFeatureFlagProviderValues() throws Exception {
- useConfiguration("--experimental_dynamic_configs=on");
+ useConfiguration(
+ "--experimental_dynamic_configs=on",
+ "--enforce_transitive_configs_for_config_feature_flag");
writeFile(
"java/com/foo/reader.bzl",
"def _impl(ctx):",
@@ -291,6 +311,7 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
"flag_reader(",
" name = 'FooFlags',",
" flags = [':flag1', ':flag2'],",
+ " transitive_configs = [':flag1', ':flag2'],",
")",
"android_local_test(",
" name = 'foo',",
@@ -298,7 +319,8 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
" deps = extra_deps,",
" feature_flags = {",
" 'flag1': 'on',",
- " }",
+ " },",
+ " transitive_configs = [':flag1', ':flag2'],",
")");
Artifact flagList =
actionsTestUtil().getFirstArtifactEndingWith(
@@ -314,7 +336,9 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
public void testFeatureFlagsAttributeFailsAnalysisIfFlagIsAliased()
throws Exception {
reporter.removeHandler(failFastHandler);
- useConfiguration("--experimental_dynamic_configs=on");
+ useConfiguration(
+ "--experimental_dynamic_configs=on",
+ "--enforce_transitive_configs_for_config_feature_flag");
writeFile(
"java/com/foo/BUILD",
"load('//java/bar:foo.bzl', 'extra_deps')",
@@ -326,6 +350,7 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
"alias(",
" name = 'alias',",
" actual = 'flag1',",
+ " transitive_configs = [':flag1'],",
")",
"android_local_test(",
" name = 'foo',",
@@ -333,7 +358,8 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
" deps = extra_deps,",
" feature_flags = {",
" 'alias': 'on',",
- " }",
+ " },",
+ " transitive_configs = [':flag1'],",
")");
assertThat(getConfiguredTarget("//java/com/foo")).isNull();
assertContainsEvent(String.format(
@@ -345,6 +371,7 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
@Test
public void testFeatureFlagPolicyMustBeVisibleToRuleToUseFeatureFlags() throws Exception {
reporter.removeHandler(failFastHandler); // expecting an error
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
overwriteFile(
"tools/whitelists/config_feature_flag/BUILD",
"package_group(",
@@ -367,7 +394,8 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
" deps = extra_deps,",
" feature_flags = {",
" '//flag:flag': 'right',",
- " }",
+ " },",
+ " transitive_configs = ['//flag:flag'],",
")");
assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNull();
assertContainsEvent(
@@ -378,6 +406,7 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
@Test
public void testFeatureFlagPolicyDoesNotBlockRuleIfInPolicy() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
overwriteFile(
"tools/whitelists/config_feature_flag/BUILD",
"package_group(",
@@ -400,7 +429,8 @@ public abstract class AbstractAndroidLocalTestTestBase extends BuildViewTestCase
" deps = extra_deps,",
" feature_flags = {",
" '//flag:flag': 'right',",
- " }",
+ " },",
+ " transitive_configs = ['//flag:flag'],",
")");
assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNotNull();
assertNoEvents();
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 3a9bb90190..7d41301892 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
@@ -3006,7 +3006,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
@Test
public void testFeatureFlagsAttributeSetsSelectInDependency() throws Exception {
- useConfiguration("--experimental_dynamic_configs=notrim");
+ useConfiguration(
+ "--experimental_dynamic_configs=notrim",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"java/com/foo/BUILD",
"config_feature_flag(",
@@ -3017,6 +3019,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"config_setting(",
" name = 'flag1@on',",
" flag_values = {':flag1': 'on'},",
+ " transitive_configs = [':flag1'],",
")",
"config_feature_flag(",
" name = 'flag2',",
@@ -3026,6 +3029,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"config_setting(",
" name = 'flag2@on',",
" flag_values = {':flag2': 'on'},",
+ " transitive_configs = [':flag2'],",
")",
"android_library(",
" name = 'lib',",
@@ -3036,6 +3040,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
" ':flag2@on': ['Flag2On.java'],",
" '//conditions:default': ['Flag2Off.java'],",
" }),",
+ " transitive_configs = [':flag1', ':flag2'],",
")",
"android_binary(",
" name = 'foo',",
@@ -3043,7 +3048,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
" deps = [':lib'],",
" feature_flags = {",
" 'flag1': 'on',",
- " }",
+ " },",
+ " transitive_configs = [':flag1', ':flag2'],",
")");
ConfiguredTarget binary = getConfiguredTarget("//java/com/foo");
List<String> inputs =
@@ -3055,7 +3061,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
@Test
public void testFeatureFlagsAttributeSetsSelectInBinary() throws Exception {
- useConfiguration("--experimental_dynamic_configs=notrim");
+ useConfiguration(
+ "--experimental_dynamic_configs=notrim",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"java/com/foo/BUILD",
"config_feature_flag(",
@@ -3066,6 +3074,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"config_setting(",
" name = 'flag1@on',",
" flag_values = {':flag1': 'on'},",
+ " transitive_configs = [':flag1'],",
")",
"config_feature_flag(",
" name = 'flag2',",
@@ -3075,6 +3084,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"config_setting(",
" name = 'flag2@on',",
" flag_values = {':flag2': 'on'},",
+ " transitive_configs = [':flag2'],",
")",
"android_binary(",
" name = 'foo',",
@@ -3088,7 +3098,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
" }),",
" feature_flags = {",
" 'flag1': 'on',",
- " }",
+ " },",
+ " transitive_configs = [':flag1', ':flag2'],",
")");
ConfiguredTarget binary = getConfiguredTarget("//java/com/foo");
List<String> inputs =
@@ -3100,7 +3111,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
@Test
public void testFeatureFlagsAttributeSetsSelectInBinaryAlias() throws Exception {
- useConfiguration("--experimental_dynamic_configs=notrim");
+ useConfiguration(
+ "--experimental_dynamic_configs=notrim",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"java/com/foo/BUILD",
"config_feature_flag(",
@@ -3111,6 +3124,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"config_setting(",
" name = 'flag1@on',",
" flag_values = {':flag1': 'on'},",
+ " transitive_configs = [':flag1'],",
")",
"android_binary(",
" name = 'foo',",
@@ -3121,7 +3135,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
" }),",
" feature_flags = {",
" 'flag1': 'on',",
- " }",
+ " },",
+ " transitive_configs = [':flag1'],",
")",
"alias(",
" name = 'alias',",
@@ -3138,7 +3153,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
@Test
public void testFeatureFlagsAttributeFailsAnalysisIfFlagValueIsInvalid() throws Exception {
reporter.removeHandler(failFastHandler);
- useConfiguration("--experimental_dynamic_configs=on");
+ useConfiguration(
+ "--experimental_dynamic_configs=on",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"java/com/foo/BUILD",
"config_feature_flag(",
@@ -3149,13 +3166,15 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"config_setting(",
" name = 'flag1@on',",
" flag_values = {':flag1': 'on'},",
+ " transitive_configs = [':flag1'],",
")",
"android_library(",
" name = 'lib',",
" srcs = select({",
" ':flag1@on': ['Flag1On.java'],",
" '//conditions:default': ['Flag1Off.java'],",
- " })",
+ " }),",
+ " transitive_configs = [':flag1'],",
")",
"android_binary(",
" name = 'foo',",
@@ -3163,7 +3182,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
" deps = [':lib'],",
" feature_flags = {",
" 'flag1': 'invalid',",
- " }",
+ " },",
+ " transitive_configs = [':flag1'],",
")");
assertThat(getConfiguredTarget("//java/com/foo")).isNull();
assertContainsEvent(
@@ -3175,7 +3195,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
public void testFeatureFlagsAttributeFailsAnalysisIfFlagValueIsInvalidEvenIfNotUsed()
throws Exception {
reporter.removeHandler(failFastHandler);
- useConfiguration("--experimental_dynamic_configs=on");
+ useConfiguration(
+ "--experimental_dynamic_configs=on",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"java/com/foo/BUILD",
"config_feature_flag(",
@@ -3186,13 +3208,15 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"config_setting(",
" name = 'flag1@on',",
" flag_values = {':flag1': 'on'},",
+ " transitive_configs = [':flag1'],",
")",
"android_binary(",
" name = 'foo',",
" manifest = 'AndroidManifest.xml',",
" feature_flags = {",
" 'flag1': 'invalid',",
- " }",
+ " },",
+ " transitive_configs = [':flag1'],",
")");
assertThat(getConfiguredTarget("//java/com/foo")).isNull();
assertContainsEvent(
@@ -3204,7 +3228,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
public void testFeatureFlagsAttributeFailsAnalysisIfFlagIsAliased()
throws Exception {
reporter.removeHandler(failFastHandler);
- useConfiguration("--experimental_dynamic_configs=notrim");
+ useConfiguration(
+ "--experimental_dynamic_configs=notrim",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"java/com/foo/BUILD",
"config_feature_flag(",
@@ -3215,13 +3241,15 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"alias(",
" name = 'alias',",
" actual = 'flag1',",
+ " transitive_configs = [':flag1'],",
")",
"android_binary(",
" name = 'foo',",
" manifest = 'AndroidManifest.xml',",
" feature_flags = {",
" 'alias': 'on',",
- " }",
+ " },",
+ " transitive_configs = [':flag1'],",
")");
assertThat(getConfiguredTarget("//java/com/foo")).isNull();
assertContainsEvent(
@@ -3232,7 +3260,9 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
@Test
public void testFeatureFlagsAttributeSetsFeatureFlagProviderValues() throws Exception {
- useConfiguration("--experimental_dynamic_configs=notrim");
+ useConfiguration(
+ "--experimental_dynamic_configs=notrim",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"java/com/foo/reader.bzl",
"def _impl(ctx):",
@@ -3263,6 +3293,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
"flag_reader(",
" name = 'FooFlags',",
" flags = [':flag1', ':flag2'],",
+ " transitive_configs = [':flag1', ':flag2'],",
")",
"android_binary(",
" name = 'foo',",
@@ -3270,7 +3301,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
" srcs = [':FooFlags.java'],",
" feature_flags = {",
" 'flag1': 'on',",
- " }",
+ " },",
+ " transitive_configs = [':flag1', ':flag2'],",
")");
Artifact flagList =
getFirstArtifactEndingWith(
@@ -3316,6 +3348,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
@Test
public void testFeatureFlagPolicyMustContainRuleToUseFeatureFlags() throws Exception {
reporter.removeHandler(failFastHandler); // expecting an error
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratch.overwriteFile(
"tools/whitelists/config_feature_flag/BUILD",
"package_group(",
@@ -3337,7 +3370,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
" srcs = [':FooFlags.java'],",
" feature_flags = {",
" '//flag:flag': 'right',",
- " }",
+ " },",
+ " transitive_configs = ['//flag:flag'],",
")");
assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNull();
assertContainsEvent(
@@ -3348,6 +3382,7 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
@Test
public void testFeatureFlagPolicyDoesNotBlockRuleIfInPolicy() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratch.overwriteFile(
"tools/whitelists/config_feature_flag/BUILD",
"package_group(",
@@ -3369,7 +3404,8 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
" srcs = [':FooFlags.java'],",
" feature_flags = {",
" '//flag:flag': 'right',",
- " }",
+ " },",
+ " transitive_configs = ['//flag:flag'],",
")");
assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNotNull();
assertNoEvents();
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfigurationTest.java
index 07dcae5dc2..7811335294 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfigurationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagConfigurationTest.java
@@ -16,12 +16,15 @@ package com.google.devtools.build.lib.rules.config;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
+import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.util.LabelArtifactOwner;
import com.google.devtools.build.lib.cmdline.Label;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -40,14 +43,40 @@ public final class ConfigFeatureFlagConfigurationTest {
}
@Test
- public void getFeatureFlagValue_returnsEmptyOptionalWhenRequestingUnsetFlag() throws Exception {
+ public void
+ getFeatureFlagValue_returnsEmptyOptionalWhenRequestingUnknownFlagFromUntrimmedOptions()
+ throws Exception {
Optional<String> flagValue =
- getConfigurationWithFlags(ImmutableMap.of(Label.parseAbsoluteUnchecked("//a:a"), "valued"))
+ getUntrimmedConfigurationWithFlags(
+ ImmutableMap.of(Label.parseAbsoluteUnchecked("//a:a"), "valued"))
.getFeatureFlagValue(new LabelArtifactOwner(Label.parseAbsoluteUnchecked("//b:b")));
assertThat(flagValue).isEmpty();
}
@Test
+ public void getFeatureFlagValue_returnsEmptyOptionalWhenRequestingKnownDefaultFlag()
+ throws Exception {
+ Optional<String> flagValue =
+ getConfigurationWithFlags(
+ ImmutableMap.of(Label.parseAbsoluteUnchecked("//a:a"), "valued"),
+ ImmutableSet.of(
+ Label.parseAbsoluteUnchecked("//a:a"), Label.parseAbsoluteUnchecked("//b:b")))
+ .getFeatureFlagValue(new LabelArtifactOwner(Label.parseAbsoluteUnchecked("//b:b")));
+ assertThat(flagValue).isEmpty();
+ }
+
+ @Test
+ public void getFeatureFlagValue_throwsExceptionWhenRequestingUnknownFlag() throws Exception {
+ try {
+ getConfigurationWithFlags(ImmutableMap.of(Label.parseAbsoluteUnchecked("//a:a"), "valued"))
+ .getFeatureFlagValue(new LabelArtifactOwner(Label.parseAbsoluteUnchecked("//b:b")));
+ fail("no exception was thrown");
+ } catch (ConfigFeatureFlagConfiguration.MissingFlagException expected) {
+ assertThat(expected).hasMessageThat().contains("//b:b");
+ }
+ }
+
+ @Test
public void getOutputDirectoryName_returnsNullWhenFlagMapIsEmpty() throws Exception {
assertThat(getConfigurationWithFlags(ImmutableMap.<Label, String>of()).getOutputDirectoryName())
.isNull();
@@ -133,7 +162,22 @@ public final class ConfigFeatureFlagConfigurationTest {
/** Generates a configuration fragment with the given set of flag-value pairs. */
private static ConfigFeatureFlagConfiguration getConfigurationWithFlags(
Map<Label, String> flags) {
+ return getConfigurationWithFlags(flags, flags.keySet());
+ }
+
+ private static ConfigFeatureFlagConfiguration getConfigurationWithFlags(
+ Map<Label, String> flags, Set<Label> availableFlags) {
+ ConfigFeatureFlagOptions options = new ConfigFeatureFlagOptions();
+ options.enforceTransitiveConfigsForConfigFeatureFlag = true;
+ options.replaceFlagValues(flags);
+ options.trimFlagValues(availableFlags);
+ return new ConfigFeatureFlagConfiguration(options);
+ }
+
+ private static ConfigFeatureFlagConfiguration getUntrimmedConfigurationWithFlags(
+ Map<Label, String> flags) {
ConfigFeatureFlagOptions options = new ConfigFeatureFlagOptions();
+ options.enforceTransitiveConfigsForConfigFeatureFlag = false;
options.replaceFlagValues(flags);
return new ConfigFeatureFlagConfiguration(options);
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
index 411294abbd..e9861dfc66 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java
@@ -38,7 +38,9 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
@Before
public void useTrimmedConfigurations() throws Exception {
- useConfiguration("--experimental_dynamic_configs=on");
+ useConfiguration(
+ "--experimental_dynamic_configs=on",
+ "--enforce_transitive_configs_for_config_feature_flag");
}
@Override
@@ -72,6 +74,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
" flag_values = {",
" ':flag': 'configured',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -91,6 +94,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
" flag_values = {",
" ':flag': 'configured',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -157,10 +161,12 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
" flag_values = {",
" ':flag': 'configured',",
" },",
+ " transitive_configs = [':flag'],",
")",
"flag_reading_wrapper(",
" name = 'wrapper',",
" flag = ':flag',",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -215,6 +221,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
"flag_reading_wrapper(",
" name = 'wrapper',",
" flag = ':flag',",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -253,6 +260,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
" flag_values = {",
" ':other': 'configured',",
" },",
+ " transitive_configs = [':flag', ':other'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -280,6 +288,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
" flag_values = {",
" ':other': 'configured',",
" },",
+ " transitive_configs = [':flag', ':other'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -339,6 +348,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
" flag_values = {",
" ':flag': 'legal',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -362,6 +372,7 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase {
" flag_values = {",
" ':flag': 'invalid',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
index 8faf64d8e4..46042fdce3 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
@@ -399,7 +399,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void matchesIfFlagValuesAndValuesBothMatch() throws Exception {
- useConfiguration("--copt=-Dright");
+ useConfiguration("--copt=-Dright", "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -410,6 +410,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dright',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -421,6 +422,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void matchesIfFlagValuesMatchAndValuesAreEmpty() throws Exception {
+ useConfiguration("--copt=-Dright", "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -429,6 +431,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" ':flag': 'right',",
" },",
" values = {},",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -455,7 +458,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotMatchIfNeitherFlagValuesNorValuesMatches() throws Exception {
- useConfiguration("--copt=-Dright");
+ useConfiguration("--copt=-Dright", "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -466,6 +469,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dwrong',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -477,6 +481,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotMatchIfFlagValuesDoNotMatchAndValuesAreEmpty() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -485,6 +490,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" ':flag': 'wrong',",
" },",
" values = {},",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -496,7 +502,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotMatchIfFlagValuesDoNotMatchButValuesDo() throws Exception {
- useConfiguration("--copt=-Dright");
+ useConfiguration("--copt=-Dright", "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -507,6 +513,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dright',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -533,7 +540,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotMatchIfValuesDoNotMatchButFlagValuesDo() throws Exception {
- useConfiguration("--copt=-Dright");
+ useConfiguration("--copt=-Dright", "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -544,6 +551,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dwrong',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -555,6 +563,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotMatchIfEvenOneFlagValueDoesNotMatch() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -564,6 +573,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" ':flag2': 'bad',",
" },",
" values = {},",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -580,12 +590,14 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void matchesIfNonDefaultIsSpecifiedAndFlagValueIsThatValue() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"feature_flag_setter(",
" name = 'setter',",
" exports_setting = ':match',",
" flag_values = {':flag': 'actual'},",
+ " transitive_configs = [':flag'],",
")",
"config_setting(",
" name = 'match',",
@@ -593,6 +605,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" ':flag': 'actual',",
" },",
" values = {},",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -604,12 +617,14 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotMatchIfDefaultIsSpecifiedAndFlagValueIsNotDefault() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"feature_flag_setter(",
" name = 'setter',",
" exports_setting = ':match',",
" flag_values = {':flag': 'actual'},",
+ " transitive_configs = [':flag'],",
")",
"config_setting(",
" name = 'match',",
@@ -617,6 +632,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" ':flag': 'default',",
" },",
" values = {},",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -628,7 +644,10 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotRefineSettingWithSameValuesAndSameFlagValues() throws Exception {
- useConfiguration("--copt=-Dright", "--javacopt=-Dgood");
+ useConfiguration(
+ "--copt=-Dright",
+ "--javacopt=-Dgood",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -641,6 +660,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" 'copt': '-Dright',",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_setting(",
" name = 'other',",
@@ -652,6 +672,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" 'copt': '-Dright',",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -671,7 +692,10 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotRefineSettingWithDifferentValuesAndSameFlagValues() throws Exception {
- useConfiguration("--copt=-Dright", "--javacopt=-Dgood");
+ useConfiguration(
+ "--copt=-Dright",
+ "--javacopt=-Dgood",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -683,6 +707,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_setting(",
" name = 'other',",
@@ -693,6 +718,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dright',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -712,7 +738,10 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotRefineSettingWithSameValuesAndDifferentFlagValues() throws Exception {
- useConfiguration("--copt=-Dright", "--javacopt=-Dgood");
+ useConfiguration(
+ "--copt=-Dright",
+ "--javacopt=-Dgood",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -724,6 +753,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" 'copt': '-Dright',",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_setting(",
" name = 'other',",
@@ -734,6 +764,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" 'copt': '-Dright',",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag2'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -753,7 +784,10 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotRefineSettingWithDifferentValuesAndDifferentFlagValues() throws Exception {
- useConfiguration("--copt=-Dright", "--javacopt=-Dgood");
+ useConfiguration(
+ "--copt=-Dright",
+ "--javacopt=-Dgood",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -764,6 +798,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dright',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_setting(",
" name = 'other',",
@@ -773,6 +808,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag2'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -792,7 +828,10 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotRefineSettingWithDifferentValuesAndSubsetFlagValues() throws Exception {
- useConfiguration("--copt=-Dright", "--javacopt=-Dgood");
+ useConfiguration(
+ "--copt=-Dright",
+ "--javacopt=-Dgood",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -804,6 +843,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dright',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_setting(",
" name = 'other',",
@@ -813,6 +853,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -832,7 +873,10 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void doesNotRefineSettingWithSubsetValuesAndDifferentFlagValues() throws Exception {
- useConfiguration("--copt=-Dright", "--javacopt=-Dgood");
+ useConfiguration(
+ "--copt=-Dright",
+ "--javacopt=-Dgood",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -844,6 +888,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" 'copt': '-Dright',",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_setting(",
" name = 'other',",
@@ -853,6 +898,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dright',",
" },",
+ " transitive_configs = [':flag2'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -872,7 +918,10 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void refinesSettingWithSubsetValuesAndSameFlagValues() throws Exception {
- useConfiguration("--copt=-Dright", "--javacopt=-Dgood");
+ useConfiguration(
+ "--copt=-Dright",
+ "--javacopt=-Dgood",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -885,6 +934,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" 'copt': '-Dright',",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_setting(",
" name = 'other',",
@@ -895,6 +945,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dright',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -914,7 +965,10 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void refinesSettingWithSameValuesAndSubsetFlagValues() throws Exception {
- useConfiguration("--copt=-Dright", "--javacopt=-Dgood");
+ useConfiguration(
+ "--copt=-Dright",
+ "--javacopt=-Dgood",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -927,6 +981,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" 'copt': '-Dright',",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_setting(",
" name = 'other',",
@@ -937,6 +992,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" 'copt': '-Dright',",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -956,7 +1012,10 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void refinesSettingWithSubsetValuesAndSubsetFlagValues() throws Exception {
- useConfiguration("--copt=-Dright", "--javacopt=-Dgood");
+ useConfiguration(
+ "--copt=-Dright",
+ "--javacopt=-Dgood",
+ "--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -969,6 +1028,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" 'copt': '-Dright',",
" 'javacopt': '-Dgood',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_setting(",
" name = 'other',",
@@ -978,6 +1038,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
" values = {",
" 'copt': '-Dright',",
" },",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -997,6 +1058,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void matchesAliasedFlagsInFlagValues() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -1004,10 +1066,12 @@ public class ConfigSettingTest extends BuildViewTestCase {
" flag_values = {",
" ':alias': 'right',",
" },",
+ " transitive_configs = [':flag'],",
")",
"alias(",
" name = 'alias',",
" actual = 'flag',",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -1019,6 +1083,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void aliasedFlagsAreCountedInRefining() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratch.file(
"test/BUILD",
"config_setting(",
@@ -1027,16 +1092,19 @@ public class ConfigSettingTest extends BuildViewTestCase {
" ':alias': 'right',",
" ':flag2': 'good',",
" },",
+ " transitive_configs = [':flag', ':flag2'],",
")",
"config_setting(",
" name = 'other',",
" flag_values = {",
" ':flag': 'right',",
" },",
+ " transitive_configs = [':flag'],",
")",
"alias(",
" name = 'alias',",
" actual = 'flag',",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
@@ -1056,6 +1124,7 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void referencingSameFlagViaMultipleAliasesFails() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
checkError(
"test",
"multialias",
@@ -1067,10 +1136,12 @@ public class ConfigSettingTest extends BuildViewTestCase {
" ':alias': 'right',",
" ':direct': 'right',",
" },",
+ " transitive_configs = [':direct'],",
")",
"alias(",
" name = 'alias',",
" actual = 'direct',",
+ " transitive_configs = [':direct'],",
")",
"config_feature_flag(",
" name = 'direct',",
@@ -1098,15 +1169,19 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void requiresValidValueForFlagValues() throws Exception {
- checkError("test", "invalid_flag",
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
+ checkError(
+ "test",
+ "invalid_flag",
"in flag_values attribute of config_setting rule //test:invalid_flag: "
- + "error while parsing user-defined configuration values: "
- + "'invalid' is not a valid value for '//test:flag'",
+ + "error while parsing user-defined configuration values: "
+ + "'invalid' is not a valid value for '//test:flag'",
"config_setting(",
" name = 'invalid_flag',",
" flag_values = {",
" ':flag': 'invalid',",
- " })",
+ " },",
+ " transitive_configs = [':flag'])",
"config_feature_flag(",
" name = 'flag',",
" allowed_values = ['right', 'valid'],",
@@ -1116,18 +1191,23 @@ public class ConfigSettingTest extends BuildViewTestCase {
@Test
public void usesAliasLabelWhenReportingErrorInFlagValues() throws Exception {
- checkError("test", "invalid_flag",
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
+ checkError(
+ "test",
+ "invalid_flag",
"in flag_values attribute of config_setting rule //test:invalid_flag: "
- + "error while parsing user-defined configuration values: "
- + "'invalid' is not a valid value for '//test:alias'",
+ + "error while parsing user-defined configuration values: "
+ + "'invalid' is not a valid value for '//test:alias'",
"config_setting(",
" name = 'invalid_flag',",
" flag_values = {",
" ':alias': 'invalid',",
- " })",
+ " },",
+ " transitive_configs = [':flag'])",
"alias(",
" name = 'alias',",
" actual = ':flag',",
+ " transitive_configs = [':flag'],",
")",
"config_feature_flag(",
" name = 'flag',",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java
new file mode 100644
index 0000000000..e433fb98bf
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java
@@ -0,0 +1,770 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License
+
+package com.google.devtools.build.lib.rules.config;
+
+import static com.google.common.collect.ImmutableSortedMap.toImmutableSortedMap;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableSortedMap;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Ordering;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.skylark.util.SkylarkTestCase;
+import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for manual trimming of feature flags with the transitive_configs attribute. */
+@RunWith(JUnit4.class)
+public final class FeatureFlagManualTrimmingTest extends SkylarkTestCase {
+
+ @Before
+ public void enableManualTrimming() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
+ }
+
+ @Override
+ protected ConfiguredRuleClassProvider getRuleClassProvider() {
+ ConfiguredRuleClassProvider.Builder builder =
+ new ConfiguredRuleClassProvider.Builder().addRuleDefinition(new FeatureFlagSetterRule());
+ TestRuleClassProvider.addStandardRules(builder);
+ return builder.build();
+ }
+
+ @Before
+ public void setUpFlagReadingRule() throws Exception {
+ scratch.file(
+ "test/read_flags.bzl",
+ "def _read_flags_impl(ctx):",
+ " ctx.actions.write(",
+ " ctx.outputs.flagdict,",
+ " '\\n'.join(['%s:::%s' % (dep.label, dep[config_common.FeatureFlagInfo].value)",
+ " for dep in ctx.attr.flags]))",
+ " return [DefaultInfo(files = depset([ctx.outputs.flagdict]))]",
+ "read_flags = rule(",
+ " implementation = _read_flags_impl,",
+ " attrs = {'flags': attr.label_list()},",
+ " outputs = {'flagdict': '%{name}.flags'},",
+ ")");
+ }
+
+ @Before
+ public void setUpHostTransitionRule() throws Exception {
+ scratch.file(
+ "test/host_transition.bzl",
+ "def _host_transition_impl(ctx):",
+ " files = depset(transitive = [src[DefaultInfo].files for src in ctx.attr.srcs])",
+ " return [DefaultInfo(files = files)]",
+ "host_transition = rule(",
+ " implementation = _host_transition_impl,",
+ " attrs = {'srcs': attr.label_list(cfg='host')},",
+ ")");
+ }
+
+ private ImmutableSortedMap<Label, String> getFlagMapFromConfiguration(BuildConfiguration config) {
+ return config.getOptions().get(ConfigFeatureFlagOptions.class).getFlagValues();
+ }
+
+ private ImmutableSortedMap<Label, String> getFlagValuesFromOutputFile(Artifact flagDict) {
+ String fileContents =
+ ((FileWriteAction) getActionGraph().getGeneratingAction(flagDict)).getFileContents();
+ return Splitter.on('\n')
+ .withKeyValueSeparator(":::")
+ .split(fileContents)
+ .entrySet()
+ .stream()
+ .collect(
+ toImmutableSortedMap(
+ Ordering.natural(),
+ (entry) -> Label.parseAbsoluteUnchecked(entry.getKey()),
+ Map.Entry::getValue));
+ }
+
+ @Test
+ public void duplicateTargetsCreatedWithTrimmingDisabled() throws Exception {
+ useConfiguration("--noenforce_transitive_configs_for_config_feature_flag");
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'left',",
+ " deps = [':common'],",
+ " flag_values = {",
+ " ':different_flag': 'left',",
+ " ':common_flag': 'configured',",
+ " },",
+ " transitive_configs = [':common_flag'],",
+ ")",
+ "feature_flag_setter(",
+ " name = 'right',",
+ " deps = [':common'],",
+ " flag_values = {",
+ " ':different_flag': 'right',",
+ " ':common_flag': 'configured',",
+ " },",
+ " transitive_configs = [':common_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'common',",
+ " flags = [':common_flag'],",
+ " transitive_configs = [':common_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'different_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")",
+ "config_feature_flag(",
+ " name = 'common_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ Artifact leftFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:left")).toList());
+ Artifact rightFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:right")).toList());
+
+ assertThat(leftFlags).isNotEqualTo(rightFlags);
+ }
+
+ @Test
+ public void featureFlagSetAndInTransitiveConfigs_GetsSetValue() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':trimmed_flag': 'left',",
+ " ':used_flag': 'configured',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [':used_flag'],",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'trimmed_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ Artifact targetFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:target")).toList());
+
+ Label usedFlag = Label.parseAbsolute("//test:used_flag");
+ assertThat(getFlagValuesFromOutputFile(targetFlags)).containsEntry(usedFlag, "configured");
+ }
+
+ @Test
+ public void featureFlagSetButNotInTransitiveConfigs_IsTrimmedOutAndCollapsesDuplicates()
+ throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'left',",
+ " deps = [':common'],",
+ " flag_values = {",
+ " ':different_flag': 'left',",
+ " ':common_flag': 'configured',",
+ " },",
+ " transitive_configs = [':common_flag'],",
+ ")",
+ "feature_flag_setter(",
+ " name = 'right',",
+ " deps = [':common'],",
+ " flag_values = {" ,
+ " ':different_flag': 'right',",
+ " ':common_flag': 'configured',",
+ " },",
+ " transitive_configs = [':common_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'common',",
+ " flags = [':common_flag'],",
+ " transitive_configs = [':common_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'different_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")",
+ "config_feature_flag(",
+ " name = 'common_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ Artifact leftFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:left")).toList());
+ Artifact rightFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:right")).toList());
+
+ assertThat(leftFlags).isEqualTo(rightFlags);
+ assertThat(leftFlags.getArtifactOwner()).isEqualTo(rightFlags.getArtifactOwner());
+ }
+
+ @Test
+ public void featureFlagInTransitiveConfigsButNotSet_GetsDefaultValue() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':trimmed_flag': 'left',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [':used_flag'],",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'trimmed_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ Artifact targetFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:target")).toList());
+
+ Label usedFlag = Label.parseAbsolute("//test:used_flag");
+ assertThat(getFlagValuesFromOutputFile(targetFlags)).containsEntry(usedFlag, "default");
+ }
+
+ @Test
+ public void featureFlagInTransitiveConfigsButNotInTransitiveClosure_IsWastefulButDoesNotError()
+ throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'left',",
+ " deps = [':common'],",
+ " flag_values = {",
+ " ':different_flag': 'left',",
+ " ':common_flag': 'configured',",
+ " },",
+ " transitive_configs = [':different_flag', ':common_flag'],",
+ ")",
+ "feature_flag_setter(",
+ " name = 'right',",
+ " deps = [':common'],",
+ " flag_values = {",
+ " ':different_flag': 'right',",
+ " ':common_flag': 'configured',",
+ " },",
+ " transitive_configs = [':different_flag', ':common_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'common',",
+ " flags = [':common_flag'],",
+ " transitive_configs = [':different_flag', ':common_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'different_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")",
+ "config_feature_flag(",
+ " name = 'common_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ Artifact leftFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:left")).toList());
+ Artifact rightFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:right")).toList());
+
+ assertThat(leftFlags).isNotEqualTo(rightFlags);
+ assertThat(leftFlags.getArtifactOwner()).isNotEqualTo(rightFlags.getArtifactOwner());
+ }
+
+ @Test
+ public void emptyTransitiveConfigs_EquivalentRegardlessOfFeatureFlags() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'left',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':used_flag': 'left',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "feature_flag_setter(",
+ " name = 'right',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':used_flag': 'right',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " transitive_configs = [],",
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")");
+
+ Artifact leftFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:left")).toList());
+ Artifact rightFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:right")).toList());
+ Artifact directFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:reader")).toList());
+
+ assertThat(leftFlags).isEqualTo(rightFlags);
+ assertThat(leftFlags).isEqualTo(directFlags);
+ }
+
+ @Test
+ public void absentTransitiveConfigs_EquivalentRegardlessOfFeatureFlags() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'left',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':used_flag': 'left',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "feature_flag_setter(",
+ " name = 'right',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':used_flag': 'right',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ // no transitive_configs = equivalent to []
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")");
+
+ Artifact leftFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:left")).toList());
+ Artifact rightFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:right")).toList());
+ Artifact directFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:reader")).toList());
+
+ assertThat(leftFlags).isEqualTo(rightFlags);
+ assertThat(leftFlags).isEqualTo(directFlags);
+ }
+
+ @Test
+ public void nonexistentLabelInTransitiveConfigs_DoesNotError() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':trimmed_flag': 'left',",
+ " },",
+ " transitive_configs = [':false_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " transitive_configs = [':false_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'trimmed_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")");
+
+ getConfiguredTarget("//test:target");
+ assertNoEvents();
+ }
+
+ @Test
+ public void flagSetBySetterButNotInTransitiveConfigs_CanBeUsedByDeps() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':not_actually_trimmed_flag': 'left',",
+ " },",
+ " transitive_configs = [],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [':not_actually_trimmed_flag'],",
+ " transitive_configs = [':not_actually_trimmed_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'not_actually_trimmed_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")");
+
+ getConfiguredTarget("//test:target");
+ assertNoEvents();
+ }
+
+ @Test
+ public void featureFlagInUnusedSelectBranchButNotInTransitiveConfigs_DoesNotError()
+ throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':trimmed_flag': 'left',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = select({':used_flag@other': [':trimmed_flag'], '//conditions:default': []}),",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "config_setting(",
+ " name = 'used_flag@other',",
+ " flag_values = {':used_flag': 'other'},",
+ ")",
+ "config_feature_flag(",
+ " name = 'trimmed_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ getConfiguredTarget("//test:target");
+ assertNoEvents();
+ }
+
+ @Test
+ public void featureFlagTarget_IsTrimmedToOnlyItself() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " exports_flag = ':read_flag',",
+ " flag_values = {",
+ " ':trimmed_flag': 'left',",
+ " ':read_flag': 'configured',",
+ " },",
+ " transitive_configs = [':trimmed_flag', ':read_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'trimmed_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")",
+ "config_feature_flag(",
+ " name = 'read_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ ConfiguredTarget target = getConfiguredTarget("//test:target");
+ RuleContext ruleContext = getRuleContext(target);
+ BuildConfiguration childConfiguration =
+ Iterables.getOnlyElement(ruleContext.getPrerequisiteConfiguredTargetAndTargets("exports_flag", Mode.TARGET)).getConfiguration();
+
+ Label childLabel = Label.parseAbsoluteUnchecked("//test:read_flag");
+ assertThat(getFlagMapFromConfiguration(childConfiguration).keySet())
+ .containsExactly(childLabel);
+ }
+
+ @Test
+ public void featureFlagAccessedByPathWithMissingLabel_ProducesError() throws Exception {
+ reporter.removeHandler(failFastHandler); // expecting an error
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':broken'],",
+ " flag_values = {",
+ " ':used_flag': 'configured',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "filegroup(",
+ " name = 'broken',",
+ " srcs = [':reader'],",
+ " transitive_configs = [],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [':used_flag'],",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ assertThat(getConfiguredTarget("//test:target")).isNull();
+ assertContainsEvent(
+ "Feature flag //test:used_flag was accessed in a configuration it is not present in. All "
+ + "targets which depend on //test:used_flag directly or indirectly must name it in "
+ + "their transitive_configs attribute.");
+ }
+
+ @Test
+ public void featureFlagAccessedByPathWithMissingTransitiveConfigs_ProducesError()
+ throws Exception {
+ reporter.removeHandler(failFastHandler); // expecting an error
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':broken'],",
+ " flag_values = {",
+ " ':used_flag': 'configured',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "filegroup(",
+ " name = 'broken',",
+ " srcs = [':reader'],",
+ // no transitive_configs = equivalent to []
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [':used_flag'],",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ assertThat(getConfiguredTarget("//test:target")).isNull();
+ assertContainsEvent(
+ "Feature flag //test:used_flag was accessed in a configuration it is not present in. All "
+ + "targets which depend on //test:used_flag directly or indirectly must name it in "
+ + "their transitive_configs attribute.");
+ }
+
+ @Test
+ public void featureFlagInHostConfiguration_HasDefaultValue() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':host_transition.bzl', 'host_transition')",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':host'],",
+ " flag_values = {",
+ " ':used_flag': 'configured',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "host_transition(",
+ " name = 'host',",
+ " srcs = [':reader'],",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [':used_flag'],",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ Artifact targetFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:target")).toList());
+
+ Label usedFlag = Label.parseAbsolute("//test:used_flag");
+ assertThat(getFlagValuesFromOutputFile(targetFlags)).containsEntry(usedFlag, "default");
+ }
+
+ @Test
+ public void featureFlagInHostConfiguration_HasNoTransitiveConfigEnforcement() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':host_transition.bzl', 'host_transition')",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':host'],",
+ " flag_values = {",
+ " ':used_flag': 'configured',",
+ " },",
+ // no transitive_configs
+ ")",
+ "host_transition(",
+ " name = 'host',",
+ " srcs = [':reader'],",
+ // no transitive_configs
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [':used_flag'],",
+ // no transitive_configs
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ getConfiguredTarget("//test:target");
+ assertNoEvents();
+ }
+
+ @Test
+ public void featureFlagAccessedDirectly_ReturnsDefaultValue() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ assertThat(
+ ConfigFeatureFlagProvider.fromTarget(getConfiguredTarget("//test:used_flag"))
+ .getValue())
+ .isEqualTo("default");
+ }
+
+ @Test
+ public void featureFlagAccessedViaTopLevelLibraryTarget_ReturnsDefaultValue() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [':used_flag'],",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+ Artifact targetFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:reader")).toList());
+
+ Label usedFlag = Label.parseAbsolute("//test:used_flag");
+ assertThat(getFlagValuesFromOutputFile(targetFlags)).containsEntry(usedFlag, "default");
+ }
+
+ @Test
+ public void featureFlagSettingRules_OverrideFlagsFromReverseTransitiveClosure()
+ throws Exception {
+ // In other words: if you have a dependency which sets feature flags itself, you don't need to
+ // name any of the feature flags used by that target or its transitive closure, as it sets
+ // feature flags itself.
+ // This is because the feature flag setting transition (which calls replaceFlagValues) runs
+ // before the trimming transition and completely replaces the feature flag set. Thus, when
+ // the trimming transition (which calls trimFlagValues) runs, its requests are always satisfied.
+
+ scratch.file(
+ "test/BUILD",
+ "load(':read_flags.bzl', 'read_flags')",
+ "filegroup(",
+ " name = 'toplevel',",
+ " srcs = [':target'],",
+ // no transitive_configs
+ ")",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':reader'],",
+ " flag_values = {",
+ " ':trimmed_flag': 'left',",
+ " ':used_flag': 'configured',",
+ " },",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [':used_flag'],",
+ " transitive_configs = [':used_flag'],",
+ ")",
+ "config_feature_flag(",
+ " name = 'trimmed_flag',",
+ " allowed_values = ['default', 'left', 'right'],",
+ " default_value = 'default',",
+ ")",
+ "config_feature_flag(",
+ " name = 'used_flag',",
+ " allowed_values = ['default', 'configured', 'other'],",
+ " default_value = 'default',",
+ ")");
+
+ Artifact targetFlags =
+ Iterables.getOnlyElement(getFilesToBuild(getConfiguredTarget("//test:toplevel")).toList());
+
+ Label usedFlag = Label.parseAbsolute("//test:used_flag");
+ assertThat(getFlagValuesFromOutputFile(targetFlags)).containsEntry(usedFlag, "configured");
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
index 51c145a84f..cc8fb258eb 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
@@ -1494,6 +1494,7 @@ public class AppleBinaryTest extends ObjcRuleTestCase {
@Test
public void testFeatureFlags_offByDefault() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratchFeatureFlagTestLib();
scratch.file(
"test/BUILD",
@@ -1501,6 +1502,7 @@ public class AppleBinaryTest extends ObjcRuleTestCase {
" name = 'bin',",
" deps = ['//lib:objcLib'],",
" platform_type = 'ios',",
+ " transitive_configs = ['//lib:flag1', '//lib:flag2'],",
")");
CommandAction linkAction = linkAction("//test:bin");
@@ -1522,6 +1524,7 @@ public class AppleBinaryTest extends ObjcRuleTestCase {
@Test
public void testFeatureFlags_oneFlagOn() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratchFeatureFlagTestLib();
scratch.file(
"test/BUILD",
@@ -1531,7 +1534,8 @@ public class AppleBinaryTest extends ObjcRuleTestCase {
" platform_type = 'ios',",
" feature_flags = {",
" '//lib:flag2': 'on',",
- " }",
+ " },",
+ " transitive_configs = ['//lib:flag1', '//lib:flag2'],",
")");
CommandAction linkAction = linkAction("//test:bin");
@@ -1553,6 +1557,7 @@ public class AppleBinaryTest extends ObjcRuleTestCase {
@Test
public void testFeatureFlags_allFlagsOn() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratchFeatureFlagTestLib();
scratch.file(
"test/BUILD",
@@ -1563,7 +1568,8 @@ public class AppleBinaryTest extends ObjcRuleTestCase {
" feature_flags = {",
" '//lib:flag1': 'on',",
" '//lib:flag2': 'on',",
- " }",
+ " },",
+ " transitive_configs = ['//lib:flag1', '//lib:flag2'],",
")");
CommandAction linkAction = linkAction("//test:bin");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
index f8d056ad9e..2649ebca97 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java
@@ -567,6 +567,7 @@ public class AppleStaticLibraryTest extends ObjcRuleTestCase {
@Test
public void testFeatureFlags_offByDefault() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratchFeatureFlagTestLib();
scratch.file(
"test/BUILD",
@@ -574,6 +575,7 @@ public class AppleStaticLibraryTest extends ObjcRuleTestCase {
" name = 'static_lib',",
" deps = ['//lib:objcLib'],",
" platform_type = 'ios',",
+ " transitive_configs = ['//lib:flag1', '//lib:flag2'],",
")");
CommandAction linkAction = linkLibAction("//test:static_lib");
@@ -595,6 +597,7 @@ public class AppleStaticLibraryTest extends ObjcRuleTestCase {
@Test
public void testFeatureFlags_oneFlagOn() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratchFeatureFlagTestLib();
scratch.file(
"test/BUILD",
@@ -604,7 +607,8 @@ public class AppleStaticLibraryTest extends ObjcRuleTestCase {
" platform_type = 'ios',",
" feature_flags = {",
" '//lib:flag2': 'on',",
- " }",
+ " },",
+ " transitive_configs = ['//lib:flag1', '//lib:flag2'],",
")");
CommandAction linkAction = linkLibAction("//test:static_lib");
@@ -626,6 +630,7 @@ public class AppleStaticLibraryTest extends ObjcRuleTestCase {
@Test
public void testFeatureFlags_allFlagsOn() throws Exception {
+ useConfiguration("--enforce_transitive_configs_for_config_feature_flag");
scratchFeatureFlagTestLib();
scratch.file(
"test/BUILD",
@@ -636,7 +641,8 @@ public class AppleStaticLibraryTest extends ObjcRuleTestCase {
" feature_flags = {",
" '//lib:flag1': 'on',",
" '//lib:flag2': 'on',",
- " }",
+ " },",
+ " transitive_configs = ['//lib:flag1', '//lib:flag2'],",
")");
CommandAction linkAction = linkLibAction("//test:static_lib");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
index 598eda0fc0..4b72583b21 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
@@ -2267,6 +2267,7 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase {
"config_setting(",
" name = 'flag1@on',",
" flag_values = {':flag1': 'on'},",
+ " transitive_configs = [':flag1'],",
")",
"config_feature_flag(",
" name = 'flag2',",
@@ -2276,6 +2277,7 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase {
"config_setting(",
" name = 'flag2@on',",
" flag_values = {':flag2': 'on'},",
+ " transitive_configs = [':flag2'],",
")",
"objc_library(",
" name = 'objcLib',",
@@ -2293,6 +2295,7 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase {
" ':flag2@on': ['-FLAG_2_ON'],",
" '//conditions:default': ['-FLAG_2_OFF'],",
" }),",
+ " transitive_configs = [':flag1', ':flag2'],",
")");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 8c4b7df066..e449ed1ceb 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -613,6 +613,7 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
assertThat(result).containsExactly(
"name",
"visibility",
+ "transitive_configs",
"tags",
"generator_name",
"generator_function",
generated by cgit v1.2.3 (git 2.39.1) at 2024-06-22 15:55:27 +0000