Provide clearer messaging when a build fails because a rule's

supported environments get refined away by incompatible
select paths.

This is a fix of TODO:   
https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java#L597

Old message:
  ERROR: /workspace/foo/BUILD:3:1: in cc_binary rule //foo:main: all environments have been refined out of the following groups: //buildenv:environment_group

New message:
  ERROR: /workspace/foo/BUILD:3:1: in cc_binary rule //foo:main: the current command-line flags disqualify all supported environments because of incompatible select() paths:
 environment: //buildenv:gce removed by: //util/lib:some_dep_with_select (/workspace/util/lib/BUILD:12:1)

--
MOS_MIGRATED_REVID=125788804

5 files changed, 98 insertions, 14 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 17617e7d07..eb9822f36e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -24,10 +24,12 @@ import com.google.devtools.build.lib.analysis.constraints.ConstraintSemantics;
 import com.google.devtools.build.lib.analysis.constraints.EnvironmentCollection;
 import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironments;
 import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironmentsProvider;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.test.ExecutionInfoProvider;
 import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
@@ -180,9 +182,12 @@ public final class RuleConfiguredTargetBuilder {
         ConstraintSemantics.getSupportedEnvironments(ruleContext);
     if (supportedEnvironments != null) {
       EnvironmentCollection.Builder refinedEnvironments = new EnvironmentCollection.Builder();
-      ConstraintSemantics.checkConstraints(ruleContext, supportedEnvironments, refinedEnvironments);
+      Map<Label, Target> removedEnvironmentCulprits = new LinkedHashMap<>();
+      ConstraintSemantics.checkConstraints(ruleContext, supportedEnvironments, refinedEnvironments,
+          removedEnvironmentCulprits);
       add(SupportedEnvironmentsProvider.class,
-          new SupportedEnvironments(supportedEnvironments, refinedEnvironments.build()));
+          new SupportedEnvironments(supportedEnvironments, refinedEnvironments.build(),
+              removedEnvironmentCulprits));
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java
index 398056c2e1..cab7c42832 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java
@@ -511,9 +511,13 @@ public class ConstraintSemantics {
    *     value of {@link #getSupportedEnvironments}. In particular, for any environment group that's
    *     not in this collection, the rule is assumed to support the defaults for that group.
    * @param refinedEnvironments a builder for populating this rule's refined environments
+   * @param removedEnvironmentCulprits a builder for populating the core dependencies that trigger
+   *     pruning away environments through refinement. If multiple dependencies qualify (e.g.
+   *     two direct deps under the current rule), one is arbitrarily chosen.
    */
   public static void checkConstraints(RuleContext ruleContext,
-      EnvironmentCollection staticEnvironments, EnvironmentCollection.Builder refinedEnvironments) {
+      EnvironmentCollection staticEnvironments, EnvironmentCollection.Builder refinedEnvironments,
+      Map<Label, Target> removedEnvironmentCulprits) {
     Set<EnvironmentWithGroup> refinedEnvironmentsSoFar = new LinkedHashSet<>();
     // Start with the full set of static environments:
     refinedEnvironmentsSoFar.addAll(staticEnvironments.getGroupedEnvironments());
@@ -565,11 +569,13 @@ public class ConstraintSemantics {
         }
         refinedEnvironmentsSoFar.remove(envToPrune);
         groupsWithEnvironmentsRemoved.add(envToPrune.group());
+        removedEnvironmentCulprits.put(envToPrune.environment(),
+            findOriginalRefiner(ruleContext, depEnvironments, envToPrune));
       }
     }
 
     checkRefinedEnvironmentConstraints(ruleContext, groupsWithEnvironmentsRemoved,
-        refinedEnvironmentsSoFar, refinedEnvironments);
+        refinedEnvironmentsSoFar, refinedEnvironments, removedEnvironmentCulprits);
   }
 
   /**
@@ -582,9 +588,11 @@ public class ConstraintSemantics {
    * <p>Violations of this expectation trigger rule analysis errors.
    */
   private static void checkRefinedEnvironmentConstraints(
-      RuleContext ruleContext, Set<EnvironmentGroup> groupsWithEnvironmentsRemoved,
+      RuleContext ruleContext,
+      Set<EnvironmentGroup> groupsWithEnvironmentsRemoved,
       Set<EnvironmentWithGroup> refinedEnvironmentsSoFar,
-      EnvironmentCollection.Builder refinedEnvironments) {
+      EnvironmentCollection.Builder refinedEnvironments,
+      Map<Label, Target> removedEnvironmentCulprits) {
     Set<EnvironmentGroup> refinedGroups = new LinkedHashSet<>();
     for (EnvironmentWithGroup envWithGroup : refinedEnvironmentsSoFar) {
       refinedEnvironments.put(envWithGroup.group(), envWithGroup.environment());
@@ -594,14 +602,51 @@ public class ConstraintSemantics {
         ? ImmutableSet.<EnvironmentGroup>of()
         : Sets.difference(groupsWithEnvironmentsRemoved, refinedGroups);
     if (!newlyEmptyGroups.isEmpty()) {
-      // TODO(bazel-team): specify exactly which deps violated expectations.
-      Set<Label> groupsAsLabels = new LinkedHashSet<>();
-      for (EnvironmentGroup group : newlyEmptyGroups) {
-        groupsAsLabels.add(group.getLabel());
+      ruleContext.ruleError(getOverRefinementError(newlyEmptyGroups, removedEnvironmentCulprits));
+    }
+  }
+
+  /**
+   * Constructs an error message for when all environments have been pruned out of one
+   * or more environment groups due to refining.
+   */
+  private static String getOverRefinementError(Set<EnvironmentGroup> newlyEmptyGroups,
+      Map<Label, Target> removedEnvironmentCulprits) {
+    StringBuilder message = new StringBuilder("the current command-line flags disqualify "
+        + "all supported environments because of incompatible select() paths:");
+    for (EnvironmentGroup group : newlyEmptyGroups) {
+      if (newlyEmptyGroups.size() > 1) {
+        message.append("\n\nenvironment group: " + group.getLabel() + ":");
+      }
+      for (Label prunedEnvironment : group.getEnvironments()) {
+        Target culprit = removedEnvironmentCulprits.get(prunedEnvironment);
+        if (culprit != null) { // Only environments this rule declared support for have culprits.
+          message.append("\n environment: " + prunedEnvironment
+              + " removed by: " + culprit.getLabel() + " (" + culprit.getLocation() + ")");
+        }
       }
-      ruleContext.ruleError("all environments have been refined out of the following groups: "
-          + Joiner.on(", ").join(groupsAsLabels));
     }
+    return message.toString();
+  }
+
+  /**
+   * Given an environment that should be refined out of the current rule because of the given dep,
+   * returns the original dep that caused the removal.
+   *
+   * <p>For example, say we have R -> D1 -> D2 and all rules support environment E. If the
+   * refinement happens because D2 has
+   * <pre>
+   *   deps = select({":foo": ["restricted_to_E"], ":bar": ["restricted_to_F"]}}  # Choose F.
+   * </pre>
+   *
+   * <p>then D2 is the original refiner (even though D1 and R inherit the same pruning).
+   */
+  private static Target findOriginalRefiner(RuleContext ruleContext,
+      SupportedEnvironmentsProvider dep, EnvironmentWithGroup envToPrune) {
+    Target depCulprit = dep.getRemovedEnvironmentCulprit(envToPrune.environment());
+    // If the dep has no record of this environment being refined, that means the current rule
+    // is the culprit.
+    return depCulprit == null ? ruleContext.getTarget() : depCulprit;
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java
index 52e10c8e48..dfdc7e4347 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.analysis.constraints;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
@@ -23,6 +24,7 @@ import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.EnvironmentGroup;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
+import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 
 /**
@@ -47,7 +49,8 @@ public class Environment implements RuleConfiguredTargetFactory {
 
     EnvironmentCollection env = new EnvironmentCollection.Builder().put(group, label).build();
     return new RuleConfiguredTargetBuilder(ruleContext)
-        .addProvider(SupportedEnvironmentsProvider.class, new SupportedEnvironments(env, env))
+        .addProvider(SupportedEnvironmentsProvider.class,
+            new SupportedEnvironments(env, env, ImmutableMap.<Label, Target>of()))
         .addProvider(RunfilesProvider.class, RunfilesProvider.EMPTY)
         .add(FileProvider.class, FileProvider.EMPTY)
         .add(FilesToRunProvider.class, FilesToRunProvider.EMPTY)
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/SupportedEnvironments.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/SupportedEnvironments.java
index 681bea01d4..f1fcae13af 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/SupportedEnvironments.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/SupportedEnvironments.java
@@ -14,17 +14,25 @@
 
 package com.google.devtools.build.lib.analysis.constraints;
 
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.Target;
+
+import java.util.Map;
+
 /**
  * Standard {@link SupportedEnvironmentsProvider} implementation.
  */
 public class SupportedEnvironments implements SupportedEnvironmentsProvider {
   private final EnvironmentCollection staticEnvironments;
   private final EnvironmentCollection refinedEnvironments;
+  private final ImmutableMap<Label, Target> removedEnvironmentCulprits;
 
   public SupportedEnvironments(EnvironmentCollection staticEnvironments,
-      EnvironmentCollection refinedEnvironments) {
+      EnvironmentCollection refinedEnvironments, Map<Label, Target> removedEnvironmentCulprits) {
     this.staticEnvironments = staticEnvironments;
     this.refinedEnvironments = refinedEnvironments;
+    this.removedEnvironmentCulprits = ImmutableMap.copyOf(removedEnvironmentCulprits);
   }
 
   @Override
@@ -36,4 +44,9 @@ public class SupportedEnvironments implements SupportedEnvironmentsProvider {
   public EnvironmentCollection getRefinedEnvironments() {
     return refinedEnvironments;
   }
+
+  @Override
+  public Target getRemovedEnvironmentCulprit(Label environment) {
+    return removedEnvironmentCulprits.get(environment);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/SupportedEnvironmentsProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/SupportedEnvironmentsProvider.java
index 3337a440f5..81909fc9c4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/SupportedEnvironmentsProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/SupportedEnvironmentsProvider.java
@@ -15,6 +15,8 @@
 package com.google.devtools.build.lib.analysis.constraints;
 
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.Target;
 
 /**
  * A provider that advertises which environments the associated target is compatible with
@@ -39,4 +41,20 @@ public interface SupportedEnvironmentsProvider extends TransitiveInfoProvider {
    * {@link ConstraintSemantics} for details.
    */
   EnvironmentCollection getRefinedEnvironments();
+
+  /**
+   * If the given environment was refined away from this target's set of supported environments,
+   * returns the dependency that originally removed the environment.
+   *
+   * <p>For example, if the current rule is restricted_to [E] and depends on D1, D1 is
+   * restricted_to [E] and depends on D2, and D2 is restricted_to [E, F] and has a select()
+   * with one path following an E-restricted dep and the other path following an F-restricted dep,
+   * then when the build chooses the F path the current rule has [E] refined to [] and D2 is the
+   * culprit.
+   *
+   * <p>If the given environment was not refined away for this rule, returns null.
+   *
+   * <p>See {@link ConstraintSemantics} class documentation for more details on refinement.
+   */
+  Target getRemovedEnvironmentCulprit(Label environment);
 }