Make constraints and select() work well with each other.

This implements most of a design proposal that splits constraint checking
into two pieces: *static* checks, which apply the standard constraint checking
done today, and *refined* checks, which selectively prune environments based
on select paths and check that not every environment gets pruned out.

As a result of this change, dependencies like:

   java_library(
       name = "lib",
       restricted_to = [":A", ":B"],
       deps = select({
           ":config_a": [":depA"],
           ":config_b": [":depB"],
       }))
   java_library(
       name = "depA",
       restricted_to = [":A"])
   java_library(
       name = "depB",
       restricted_to = [":B"])

are allowed.

Specifically, even though neither "depA" nor "depB" supports [":A", ":B"], the combination of the two does. So the select as a whole supports all environments declared in lib, even though only one of those environments actually gets chosen for a given build.

Refinement makes lib "match" the chosen path. So for "config_a" builds, lib's environment set is "refined" down to [":A"], meaning [":B"]-restricted rules cannot depend on it. Likewise, for "config_b" builds, lib's environment set is "refined" down to [":B"], meaning [":A"]-restricted rules cannot depend on it. This guarantees that the  restrictions imposed by the chosen select path propagate faithfully up the dependency chain. 

See new documentation in ConstraintSemantics.java for more details.

--
MOS_MIGRATED_REVID=120464241

2 files changed, 257 insertions, 2 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java
index 1a7ee31680..721a796cc2 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java
@@ -53,6 +53,9 @@ public class ConstraintsTest extends AbstractConstraintsTest {
         "sh_library(name = 'implicit', srcs = ['implicit.sh'])",
         "sh_library(name = 'latebound', srcs = ['latebound.sh'])",
         "sh_library(name = 'default', srcs = ['default.sh'])");
+    scratch.file("config/BUILD",
+        "config_setting(name = 'a', values = {'define': 'mode=a'})",
+        "config_setting(name = 'b', values = {'define': 'mode=b'})");
   }
 
   /**
@@ -773,13 +776,13 @@ public class ConstraintsTest extends AbstractConstraintsTest {
   public void testConfigSettingRulesAreNotChecked() throws Exception {
     new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults().make();
     scratch.file("hello/BUILD",
-        "config_setting(name = 'setting', values = {'compilation_mode': 'fastbuild'})",
         "sh_library(",
         "    name = 'shlib',",
         "    srcs = select({",
-        "        ':setting': ['shlib.sh'],",
+        "        '//config:a': ['shlib.sh'],",
         "    }),",
         "    compatible_with = ['//buildenv/foo:a'])");
+    useConfiguration("--define", "mode=a");
     assertNotNull(getConfiguredTarget("//hello:shlib"));
     assertNoEvents();
   }
@@ -897,4 +900,234 @@ public class ConstraintsTest extends AbstractConstraintsTest {
         .make();
     assertNotNull(getConfiguredTarget("//buildenv/foo:foo"));
   }
+
+  private void writeDepsForSelectTests() throws Exception {
+    scratch.file("deps/BUILD",
+        "cc_library(",
+        "    name = 'dep_a',",
+        "    srcs = [],",
+        "    restricted_to = ['//buildenv/foo:a'])",
+        "cc_library(",
+        "    name = 'dep_b',",
+        "    srcs = [],",
+        "    restricted_to = ['//buildenv/foo:b'])");
+  }
+
+  @Test
+  public void testSelectableDepsCanMissEnvironments() throws Exception {
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults().make();
+    writeDepsForSelectTests();
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//config:a': ['//deps:dep_a'],",
+        "        '//config:b': ['//deps:dep_b'],",
+        "    }),",
+        "    compatible_with = ['//buildenv/foo:a', '//buildenv/foo:b'])");
+    useConfiguration("--define", "mode=a");
+    assertNotNull(getConfiguredTarget("//hello:lib"));
+  }
+
+  @Test
+  public void testStaticCheckingOnSelectsTemporarilyDisabled() throws Exception {
+    // TODO(bazel-team): update this test once static checking on selects is implemented. When
+    // that happens, the union of all deps in the select must support the environments in the
+    // depending rule. So the logic here is constraint-invalid because //buildenv/foo:c isn't
+    // fulfilled by any of the deps.
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b", "c").setDefaults().make();
+    writeDepsForSelectTests();
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//config:a': ['//deps:dep_a'],",
+        "        '//config:b': ['//deps:dep_b'],",
+        "    }),",
+        "    compatible_with = ['//buildenv/foo:a', '//buildenv/foo:b', '//buildenv/foo:c'])");
+    useConfiguration("--define", "mode=a");
+    assertNotNull(getConfiguredTarget("//hello:lib"));
+  }
+
+  @Test
+  public void testDepInBothSelectAndUnconditionalListIsAlwaysChecked() throws Exception {
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults().make();
+    writeDepsForSelectTests();
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//config:a': ['//deps:dep_a'],",
+        "        '//config:b': ['//deps:dep_b'],",
+        "    }),",
+        "    data = ['//deps:dep_a'],",
+        "    compatible_with = ['//buildenv/foo:a', '//buildenv/foo:b'])");
+    useConfiguration("--define", "mode=a");
+    reporter.removeHandler(failFastHandler);
+    assertNull(getConfiguredTarget("//hello:lib"));
+    assertContainsEvent(
+        "dependency //deps:dep_a doesn't support expected environment: //buildenv/foo:b");
+  }
+
+  @Test
+  public void testUnconditionalSelectsAlwaysChecked() throws Exception {
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults().make();
+    writeDepsForSelectTests();
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//conditions:default': ['//deps:dep_a'],",
+        "    }),",
+        "    compatible_with = ['//buildenv/foo:a', '//buildenv/foo:b'])");
+    reporter.removeHandler(failFastHandler);
+    assertNull(getConfiguredTarget("//hello:lib"));
+    assertContainsEvent(
+        "dependency //deps:dep_a doesn't support expected environment: //buildenv/foo:b");
+  }
+
+  @Test
+  public void testRefinedEnvironmentCheckingValidCaseDirect() throws Exception {
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults().make();
+    writeDepsForSelectTests();
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//config:a': ['//deps:dep_a'],",
+        "        '//config:b': ['//deps:dep_b'],",
+        "    }),",
+        "    compatible_with = ['//buildenv/foo:a'])");
+    useConfiguration("--define", "mode=a");
+    // Valid because "--define mode=a" refines :lib to "compatible_with = ['//buildenv/foo:a']".
+    assertNotNull(getConfiguredTarget("//hello:lib"));
+  }
+
+  @Test
+  public void testRefinedEnvironmentCheckingBadCaseDirect() throws Exception {
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults().make();
+    writeDepsForSelectTests();
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//config:a': ['//deps:dep_a'],",
+        "        '//config:b': ['//deps:dep_b'],",
+        "    }),",
+        "    compatible_with = ['//buildenv/foo:b'])");
+    useConfiguration("--define", "mode=a");
+    reporter.removeHandler(failFastHandler);
+    // Invalid because "--define mode=a" refines :lib to "compatible_with = []" (empty).
+    assertNull(getConfiguredTarget("//hello:lib"));
+    assertContainsEvent("//hello:lib: all environments have been refined out of the following"
+        + " groups: //buildenv/foo:foo");
+  }
+
+  @Test
+  public void testRefinedEnvironmentCheckingValidCaseTransitive() throws Exception {
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults().make();
+    writeDepsForSelectTests();
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//config:a': ['//deps:dep_a'],",
+        "        '//config:b': ['//deps:dep_b'],",
+        "    }),",
+        "    compatible_with = ['//buildenv/foo:a', '//buildenv/foo:b'])",
+        "cc_library(",
+        "    name = 'depender',",
+        "    srcs = [],",
+        "    deps = [':lib'],",
+        "    compatible_with = ['//buildenv/foo:a'])");
+    useConfiguration("--define", "mode=a");
+    // Valid because "--define mode=a" refines :lib to "compatible_with = ['//buildenv/foo:a']".
+    assertNotNull(getConfiguredTarget("//hello:depender"));
+  }
+
+  @Test
+  public void testRefinedEnvironmentCheckingBadCaseTransitive() throws Exception {
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults().make();
+    writeDepsForSelectTests();
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//config:a': ['//deps:dep_a'],",
+        "        '//config:b': ['//deps:dep_b'],",
+        "    }),",
+        "    compatible_with = ['//buildenv/foo:a', '//buildenv/foo:b'])",
+        "cc_library(",
+        "    name = 'depender',",
+        "    srcs = [],",
+        "    deps = [':lib'],",
+        "    compatible_with = ['//buildenv/foo:b'])");
+    useConfiguration("--define", "mode=a");
+    reporter.removeHandler(failFastHandler);
+    // Invalid because "--define mode=a" refines :lib to "compatible_with = ['//buildenv/foo:a']".
+    assertNull(getConfiguredTarget("//hello:depender"));
+    assertContainsEvent("//hello:depender: all environments have been refined out of the following"
+        + " groups: //buildenv/foo:foo");
+  }
+
+  @Test
+  public void testEnvironmentRefiningAccountsForImplicitDefaults() throws Exception {
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults("b").make();
+    writeDepsForSelectTests();
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//config:a': ['//deps:dep_a'],",
+        "        '//config:b': ['//deps:dep_b'],",
+        "    }))");
+    useConfiguration("--define", "mode=a");
+    reporter.removeHandler(failFastHandler);
+    // Invalid because :lib has an implicit default of ['//buildenv/foo:b'] and "--define mode=a"
+    // refines it to "compatible_with = []" (empty).
+    assertNull(getConfiguredTarget("//hello:lib"));
+    assertContainsEvent("//hello:lib: all environments have been refined out of the following"
+        + " groups: //buildenv/foo:foo");
+  }
+
+  @Test
+  public void testEnvironmentRefiningChecksAllEnvironmentGroups() throws Exception {
+    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults().make();
+    new EnvironmentGroupMaker("buildenv/bar").setEnvironments("c", "d").setDefaults().make();
+    scratch.file("deps/BUILD",
+        "cc_library(",
+        "    name = 'dep_a',",
+        "    srcs = [],",
+        "    restricted_to = ['//buildenv/foo:a', '//buildenv/bar:d'])",
+        "cc_library(",
+        "    name = 'dep_b',",
+        "    srcs = [],",
+        "    restricted_to = ['//buildenv/foo:b', '//buildenv/bar:c'])");
+    scratch.file("hello/BUILD",
+        "cc_library(",
+        "    name = 'lib',",
+        "    srcs = [],",
+        "    deps = select({",
+        "        '//config:a': ['//deps:dep_a'],",
+        "        '//config:b': ['//deps:dep_b'],",
+        "    }),",
+        "    compatible_with = ['//buildenv/foo:a', '//buildenv/bar:c'])");
+        useConfiguration("--define", "mode=a");
+        reporter.removeHandler(failFastHandler);
+        // Invalid because while the //buildenv/foo refinement successfully refines :lib to
+        // ['//buildenv/foo:a'], the bar refinement refines it to [].
+        assertNull(getConfiguredTarget("//hello:lib"));
+        assertContainsEvent("//hello:lib: all environments have been refined out of the following"
+            + " groups: //buildenv/bar:bar");
+  }
 }
+
diff --git a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
index a5754a4860..6be376a141 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java
@@ -266,6 +266,28 @@ public class BuildTypeTest {
         Label.parseAbsolute(BuildType.Selector.DEFAULT_CONDITION_KEY)));
   }
 
+  @Test
+  public void testUnconditionalSelects() throws Exception {
+    assertFalse(
+        new Selector<Label>(
+            ImmutableMap.of("//conditions:a", "//a:a"),
+            null, currentRule, BuildType.LABEL
+        ).isUnconditional());
+    assertFalse(
+        new Selector<Label>(
+            ImmutableMap.of(
+                "//conditions:a", "//a:a",
+                BuildType.Selector.DEFAULT_CONDITION_KEY, "//b:b"),
+            null, currentRule, BuildType.LABEL
+        ).isUnconditional());
+    assertTrue(
+        new Selector<Label>(
+            ImmutableMap.of(
+                BuildType.Selector.DEFAULT_CONDITION_KEY, "//b:b"),
+            null, currentRule, BuildType.LABEL
+        ).isUnconditional());
+  }
+
   private static FilesetEntry makeFilesetEntry() {
     try {
       return new FilesetEntry(