Don't crash when building environment groups directly
(following PackageGroup's precedent).
Also cleanup: generalize the pattern by which we determine
non-configurable target types.
--
MOS_MIGRATED_REVID=104125803
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index e1c0def..1a2488f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -35,6 +35,7 @@
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.ConstantRuleVisibility;
+import com.google.devtools.build.lib.packages.EnvironmentGroup;
import com.google.devtools.build.lib.packages.InputFile;
import com.google.devtools.build.lib.packages.OutputFile;
import com.google.devtools.build.lib.packages.PackageGroup;
@@ -201,6 +202,8 @@
} else if (target instanceof PackageGroup) {
PackageGroup packageGroup = (PackageGroup) target;
return new PackageGroupConfiguredTarget(targetContext, packageGroup);
+ } else if (target instanceof EnvironmentGroup) {
+ return new EnvironmentGroupConfiguredTarget(targetContext, (EnvironmentGroup) target);
} else {
throw new AssertionError("Unexpected target class: " + target.getClass().getName());
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java
new file mode 100644
index 0000000..41741c7
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java
@@ -0,0 +1,46 @@
+// Copyright 2015 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.analysis;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.UnmodifiableIterator;
+import com.google.devtools.build.lib.packages.EnvironmentGroup;
+
+/**
+ * Dummy ConfiguredTarget for environment groups. Contains no functionality, since
+ * environment groups are not really first-class Targets.
+ */
+public final class EnvironmentGroupConfiguredTarget extends AbstractConfiguredTarget {
+ EnvironmentGroupConfiguredTarget(TargetContext targetContext, EnvironmentGroup envGroup) {
+ super(targetContext);
+ Preconditions.checkArgument(targetContext.getConfiguration() == null);
+ }
+
+ @Override
+ public EnvironmentGroup getTarget() {
+ return (EnvironmentGroup) super.getTarget();
+ }
+
+ @Override
+ public Object get(String providerKey) {
+ // No providers.
+ return null;
+ }
+
+ @Override
+ public UnmodifiableIterator<TransitiveInfoProvider> iterator() {
+ throw new UnsupportedOperationException();
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java
index c238b74..d0382dc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java
@@ -73,6 +73,6 @@
@Override
public UnmodifiableIterator<TransitiveInfoProvider> iterator() {
- throw new IllegalStateException();
+ throw new UnsupportedOperationException();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
index 6fada4c..82d8e49 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
@@ -23,8 +23,6 @@
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
import com.google.devtools.build.lib.packages.Attribute.Transition;
-import com.google.devtools.build.lib.packages.InputFile;
-import com.google.devtools.build.lib.packages.PackageGroup;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
@@ -77,7 +75,7 @@
public static BuildConfiguration configureTopLevelTarget(BuildConfiguration topLevelConfiguration,
Target toTarget) {
- if (toTarget instanceof InputFile || toTarget instanceof PackageGroup) {
+ if (!toTarget.isConfigurable()) {
return null;
}
return topLevelConfiguration.getTransitions().toplevelConfigurationHook(toTarget);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
index a5b977c..0536dc3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java
@@ -310,6 +310,11 @@
return ConstantRuleVisibility.PRIVATE; // No rule should be referencing an environment_group.
}
+ @Override
+ public boolean isConfigurable() {
+ return false;
+ }
+
public static String targetKind() {
return "environment group";
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/InputFile.java b/src/main/java/com/google/devtools/build/lib/packages/InputFile.java
index 1cb2cc0..f136aef 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/InputFile.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/InputFile.java
@@ -71,6 +71,11 @@
}
}
+ @Override
+ public boolean isConfigurable() {
+ return false;
+ }
+
public boolean isLicenseSpecified() {
return license != null && license != License.NO_LICENSE;
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java b/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java
index b7259da..eed4c63 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/OutputFile.java
@@ -38,6 +38,11 @@
return generatingRule.getVisibility();
}
+ @Override
+ public boolean isConfigurable() {
+ return true;
+ }
+
/**
* Returns the rule which generates this output file.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java b/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
index d7fe3df..3695e36 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageGroup.java
@@ -146,6 +146,11 @@
return ConstantRuleVisibility.PUBLIC;
}
+ @Override
+ public boolean isConfigurable() {
+ return false;
+ }
+
public static String targetKind() {
return "package group";
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index 6f8e60c..24ed1f7 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -604,6 +604,11 @@
}
@Override
+ public boolean isConfigurable() {
+ return true;
+ }
+
+ @Override
@SuppressWarnings("unchecked")
public Set<DistributionType> getDistributions() {
if (isAttrDefined("distribs", BuildType.DISTRIBUTIONS)
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Target.java b/src/main/java/com/google/devtools/build/lib/packages/Target.java
index f515f4f..7fb42b6 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Target.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Target.java
@@ -78,4 +78,9 @@
* Returns the visibility of this target.
*/
RuleVisibility getVisibility();
+
+ /**
+ * Returns whether this target type can be configured (e.g. accepts non-null configurations).
+ */
+ boolean isConfigurable();
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/FakeSubincludeTarget.java b/src/main/java/com/google/devtools/build/lib/query2/FakeSubincludeTarget.java
index c42f7cb..7ce3a88 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/FakeSubincludeTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/FakeSubincludeTarget.java
@@ -83,4 +83,9 @@
public RuleVisibility getVisibility() {
return ConstantRuleVisibility.PUBLIC;
}
+
+ @Override
+ public boolean isConfigurable() {
+ return true;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index acb45eb..c98da4e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -170,11 +170,7 @@
// that doesn't match; we can even have the same value multiple times. However, I think it's
// only triggered in tests (i.e., in normal operation, the configuration passed in is already
// null).
- if (target instanceof InputFile) {
- // InputFileConfiguredTarget expects its configuration to be null since it's not used.
- configuration = null;
- } else if (target instanceof PackageGroup) {
- // Same for PackageGroupConfiguredTarget.
+ if (!target.isConfigurable()) {
configuration = null;
}
TargetAndConfiguration ctgValue =
