aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/test/java/com/google/devtools/build/lib
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 /src/test/java/com/google/devtools/build/lib
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 (limited to 'src/test/java/com/google/devtools/build/lib')
-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
10 files changed, 1046 insertions, 59 deletions
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-28 21:36:13 +0000