Add basic incompatible target skipping
This patch aims to implement a basic version of incompatible target
skipping outlined here:
https://docs.google.com/document/d/12n5QNHmFSkuh5yAbdEex64ot4hRgR-moL1zRimU7wHQ/edit?usp=sharing
The implementation in this patch supports target skipping based
on the target platform. In a `BUILD` file you can now add constraints
that the target platform must satisfy in order for the target to be
built and/or tested. For example, use the following snippet to declare
a target to be compatible with Windows platforms only:
cc_binary(
name = "bin",
srcs = ["bin.cc"],
target_compatible_with = [
"@platforms//os:windows",
],
)
Builds triggered with `:all` or `...` on a non-Windows platform will
simply skip the incompatible target. An appropriate note is shown on
the command line if the `--show_result` threshold is high enough.
Targets that transitively depend on incompatible targets are
themselves considered incompatible and will also be skipped.
Explicitly requesting an incompatible target on the command line is an
error and will cause the build to fail. Bazel will print out an
appropriate error message and inform the user what constraint could
not be satisfied.
See the new documentation in this patch for more information. In
particular, https://docs.bazel.build/versions/master/platforms.html
should be a good bit more informative.
This implementation does not make any effort to support expressing
compatibility with toolchains. It is possible that using `select()`
(commented on below) already makes this possible, but it's not
validated or explicitly supported in this patch.
During implementation we noticed that `select()` can be quite powerful
in combination with `target_compatible_with`. A basic summary of this
is also documented on the Platforms page.
It may be useful to create helper functions in, say, skylib to help
make complex `select()` statements more readable. For example, we
could replace the following:
target_compatible_with = select({
"@platforms//os:linux": [],
"@platforms//os:macos": [],
"//conditions:default": [":not_compatible"],
})
with something like:
target_compatible_with = constraints.any_of([
"@platforms//os:linux",
"@platforms//os:macos",
])
That, however, is work for follow-up patches.
Many thanks to Austin Schuh (@AustinSchuh) and Greg Estren
(@gregestren) for working on the proposal and helping a ton on this
patch itself. Also thanks to many others who provided feedback on the
implementation.
RELNOTES: Bazel skips incompatible targets based on target platform
and `target_compatible_with` contents. See
https://docs.bazel.build/versions/master/platforms.html for more
details.
Closes #10945.
PiperOrigin-RevId: 339243111
diff --git a/src/main/java/com/google/devtools/build/docgen/PredefinedAttributes.java b/src/main/java/com/google/devtools/build/docgen/PredefinedAttributes.java
index 2c6629f..48dbbdd 100644
--- a/src/main/java/com/google/devtools/build/docgen/PredefinedAttributes.java
+++ b/src/main/java/com/google/devtools/build/docgen/PredefinedAttributes.java
@@ -54,6 +54,7 @@
"templates/attributes/common/licenses.html",
"templates/attributes/common/restricted_to.html",
"templates/attributes/common/tags.html",
+ "templates/attributes/common/target_compatible_with.html",
"templates/attributes/common/testonly.html",
"templates/attributes/common/toolchains.html",
"templates/attributes/common/visibility.html");
diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/target_compatible_with.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/target_compatible_with.html
new file mode 100644
index 0000000..c65186a
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/target_compatible_with.html
@@ -0,0 +1,32 @@
+<p>
+<code>List of <a href="../build-ref.html#labels">labels</a>; optional; default
+is the empty list</code>
+</p>
+
+<p>
+A list of
+<code><a href="platform.html#constraint_value">constraint_value</a></code>s
+that must be present in the target platform for this target to be considered
+"compatible". This is in addition to any constraints already set by the rule
+type. If the target platform does not satisfy all listed constraints then the
+target is considered "incompatible". Incompatible targets are skipped for
+building and testing when the target pattern is expanded
+(e.g. `//...`, `:all`). When explicitly specified on the command line,
+incompatible targets cause Bazel to print an error and cause a build or test
+failure.
+</p>
+
+<p>
+Targets that transitively depend on incompatible targets are themselves
+considered incompatible. They are also skipped for building and testing.
+</p>
+
+<p>
+An empty list (which is the default) signifies that the target is compatible
+with all platforms.
+<p>
+
+<p>
+See the <a href="../platforms.html#skipping-incompatible-targets">Platforms</a>
+page for more information about incompatible target skipping.
+</p>
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 6399623..daf24df 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -329,6 +329,7 @@
":extra/extra_action_info_file_write_action",
":extra_action_artifacts_provider",
":file_provider",
+ ":incompatible_platform_provider",
":inconsistent_aspect_order_exception",
":label_and_location",
":label_expander",
@@ -378,6 +379,7 @@
"//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink",
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
+ "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
"//src/main/java/com/google/devtools/build/lib/analysis/stringtemplate",
"//src/main/java/com/google/devtools/build/lib/bugreport",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
@@ -601,6 +603,7 @@
":config/build_options",
":config/invalid_configuration_exception",
":configured_target",
+ ":constraints/platform_restrictions_result",
":constraints/top_level_constraint_semantics",
":extra_action_artifacts_provider",
":make_environment_event",
@@ -777,6 +780,19 @@
)
java_library(
+ name = "incompatible_platform_provider",
+ srcs = ["IncompatiblePlatformProvider.java"],
+ deps = [
+ ":configured_target",
+ ":transitive_info_provider",
+ "//src/main/java/com/google/devtools/build/lib/analysis/platform",
+ "//third_party:auto_value",
+ "//third_party:guava",
+ "//third_party:jsr305",
+ ],
+)
+
+java_library(
name = "inconsistent_aspect_order_exception",
srcs = ["InconsistentAspectOrderException.java"],
deps = [
@@ -1983,8 +1999,21 @@
)
java_library(
+ name = "constraints/platform_restrictions_result",
+ srcs = ["constraints/PlatformRestrictionsResult.java"],
+ deps = [
+ ":configured_target",
+ "//third_party:auto_value",
+ "//third_party:guava",
+ ],
+)
+
+java_library(
name = "constraints/top_level_constraint_semantics",
- srcs = ["constraints/TopLevelConstraintSemantics.java"],
+ srcs = [
+ "constraints/PlatformRestrictionsResult.java",
+ "constraints/TopLevelConstraintSemantics.java",
+ ],
deps = [
":analysis_cluster",
":config/build_configuration",
@@ -1992,14 +2021,17 @@
":constraints/constraint_semantics",
":constraints/environment_collection",
":constraints/supported_environments_provider",
+ ":incompatible_platform_provider",
":transitive_info_collection",
":view_creation_failed_exception",
+ "//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration_value",
"//src/main/protobuf:failure_details_java_proto",
+ "//third_party:auto_value",
"//third_party:guava",
"//third_party:jsr305",
],
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index 3eace72..12a54a0 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -34,6 +34,7 @@
import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.analysis.constraints.ConstraintConstants;
+import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute;
@@ -335,6 +336,10 @@
.nonconfigurable(
"special logic for constraints and select: see ConstraintSemantics"))
.add(
+ attr("target_compatible_with", LABEL_LIST)
+ .mandatoryProviders(ConstraintValueInfo.PROVIDER.id())
+ .allowedFileTypes(FileTypeSet.NO_FILE))
+ .add(
attr(RuleClass.CONFIG_SETTING_DEPS_ATTRIBUTE, LABEL_LIST)
.nonconfigurable("stores configurability keys"))
.add(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 7dd75d0..cac212e4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigurationResolver.TopLevelTargetsAndConfigsResult;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
+import com.google.devtools.build.lib.analysis.constraints.PlatformRestrictionsResult;
import com.google.devtools.build.lib.analysis.constraints.TopLevelConstraintSemantics;
import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory;
import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory.CoverageReportActionsWrapper;
@@ -212,6 +213,7 @@
TargetPatternPhaseValue loadingResult,
BuildOptions targetOptions,
Set<String> multiCpu,
+ ImmutableSet<String> explicitTargetPatterns,
List<String> aspects,
AnalysisOptions viewOptions,
boolean keepGoing,
@@ -430,6 +432,24 @@
skyframeBuildView.clearInvalidatedConfiguredTargets();
}
+ TopLevelConstraintSemantics topLevelConstraintSemantics =
+ new TopLevelConstraintSemantics(
+ skyframeExecutor.getPackageManager(),
+ input -> skyframeExecutor.getConfiguration(eventHandler, input),
+ eventHandler);
+
+ PlatformRestrictionsResult platformRestrictions =
+ topLevelConstraintSemantics.checkPlatformRestrictions(
+ skyframeAnalysisResult.getConfiguredTargets(), explicitTargetPatterns, keepGoing);
+
+ if (!platformRestrictions.targetsWithErrors().isEmpty()) {
+ // If there are any errored targets (e.g. incompatible targets that are explicitly specified
+ // on the command line), remove them from the list of targets to be built.
+ skyframeAnalysisResult =
+ skyframeAnalysisResult.withAdditionalErroredTargets(
+ ImmutableSet.copyOf(platformRestrictions.targetsWithErrors()));
+ }
+
int numTargetsToAnalyze = topLevelTargetsWithConfigs.size();
int numSuccessful = skyframeAnalysisResult.getConfiguredTargets().size();
if (0 < numSuccessful && numSuccessful < numTargetsToAnalyze) {
@@ -440,11 +460,11 @@
}
Set<ConfiguredTarget> targetsToSkip =
- new TopLevelConstraintSemantics(
- skyframeExecutor.getPackageManager(),
- input -> skyframeExecutor.getConfiguration(eventHandler, input),
- eventHandler)
- .checkTargetEnvironmentRestrictions(skyframeAnalysisResult.getConfiguredTargets());
+ Sets.union(
+ topLevelConstraintSemantics.checkTargetEnvironmentRestrictions(
+ skyframeAnalysisResult.getConfiguredTargets()),
+ platformRestrictions.targetsToSkip())
+ .immutableCopy();
AnalysisResult result =
createResult(
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 52c24aa..ee92dce 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.analysis.configuredtargets.OutputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.PackageGroupConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
+import com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics;
import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleConfiguredTargetUtil;
import com.google.devtools.build.lib.analysis.test.AnalysisFailure;
import com.google.devtools.build.lib.analysis.test.AnalysisFailureInfo;
@@ -322,6 +323,12 @@
prerequisiteMap.values()))
.build();
+ ConfiguredTarget incompatibleTarget =
+ RuleContextConstraintSemantics.incompatibleConfiguredTarget(ruleContext, prerequisiteMap);
+ if (incompatibleTarget != null) {
+ return incompatibleTarget;
+ }
+
List<NestedSet<AnalysisFailure>> analysisFailures = depAnalysisFailures(ruleContext);
if (!analysisFailures.isEmpty()) {
return erroredConfiguredTargetWithFailures(ruleContext, analysisFailures);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/IncompatiblePlatformProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/IncompatiblePlatformProvider.java
new file mode 100644
index 0000000..da72929
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/IncompatiblePlatformProvider.java
@@ -0,0 +1,70 @@
+// Copyright 2020 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.auto.value.AutoValue;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
+import javax.annotation.Nullable;
+
+/**
+ * Provider instance for the {@code target_compatible_with} attribute.
+ *
+ * <p>The presence of this provider is used to indicate that a target is incompatible with the
+ * current platform. Any target that provides this will automatically be excluded from {@link
+ * SkyframeAnalysisResult}'s list of configured targets.
+ *
+ * <p>This provider is able to keep track of _why_ the corresponding target is considered
+ * incompatible. If the target is incompatible because the target platform didn't satisfy one of the
+ * constraints in target_compatible_with, then all the relevant constraints are accessible via
+ * {@code getConstraintsResponsibleForIncompatibility()}. On the other hand, if the corresponding
+ * target is incompatible because one of its dependencies is incompatible, then all the incompatible
+ * dependencies are available via {@code getTargetResponsibleForIncompatibility()}.
+ */
+@AutoValue
+public abstract class IncompatiblePlatformProvider implements TransitiveInfoProvider {
+ public static IncompatiblePlatformProvider incompatibleDueToTargets(
+ ImmutableList<ConfiguredTarget> targetsResponsibleForIncompatibility) {
+ Preconditions.checkNotNull(targetsResponsibleForIncompatibility);
+ Preconditions.checkArgument(!targetsResponsibleForIncompatibility.isEmpty());
+ return new AutoValue_IncompatiblePlatformProvider(targetsResponsibleForIncompatibility, null);
+ }
+
+ public static IncompatiblePlatformProvider incompatibleDueToConstraints(
+ ImmutableList<ConstraintValueInfo> constraints) {
+ Preconditions.checkNotNull(constraints);
+ Preconditions.checkArgument(!constraints.isEmpty());
+ return new AutoValue_IncompatiblePlatformProvider(null, constraints);
+ }
+
+ /**
+ * Returns the incompatible dependencies that caused this provider to be present.
+ *
+ * <p>This may be null. If it is null, then {@code getConstraintsResponsibleForIncompatibility()}
+ * is guaranteed to be non-null. It will have at least one element in it if it is not null.
+ */
+ @Nullable
+ public abstract ImmutableList<ConfiguredTarget> targetsResponsibleForIncompatibility();
+
+ /**
+ * Returns the constraints that the target platform didn't satisfy.
+ *
+ * <p>This may be null. If it is null, then {@code getTargetsResponsibleForIncompatibility()} is
+ * guaranteed to be non-null. It will have at least one element in it if it is not null.
+ */
+ @Nullable
+ public abstract ImmutableList<ConstraintValueInfo> constraintsResponsibleForIncompatibility();
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/PlatformRestrictionsResult.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/PlatformRestrictionsResult.java
new file mode 100644
index 0000000..420cbd9
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/PlatformRestrictionsResult.java
@@ -0,0 +1,44 @@
+// Copyright 2020 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.constraints;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+
+/** Targets that have additional restrictions based on the current platform. */
+@AutoValue
+public abstract class PlatformRestrictionsResult {
+ /** Targets that need be skipped. */
+ public abstract ImmutableSet<ConfiguredTarget> targetsToSkip();
+ /** Targets that should be skipped, but were explicitly requested on the command line. */
+ public abstract ImmutableSet<ConfiguredTarget> targetsWithErrors();
+
+ public static Builder builder() {
+ return new AutoValue_PlatformRestrictionsResult.Builder()
+ .targetsToSkip(ImmutableSet.of())
+ .targetsWithErrors(ImmutableSet.of());
+ }
+
+ /** {@link PlatformRestrictionsResult}Builder. */
+ @AutoValue.Builder
+ public interface Builder {
+ Builder targetsToSkip(ImmutableSet<ConfiguredTarget> targetsToSkip);
+
+ Builder targetsWithErrors(ImmutableSet<ConfiguredTarget> targetsWithErrors);
+
+ PlatformRestrictionsResult build();
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java
index a6de036..321f7cd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java
@@ -14,21 +14,38 @@
package com.google.devtools.build.lib.analysis.constraints;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.Streams.stream;
+
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.base.Verify;
import com.google.common.collect.ImmutableCollection;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.FailAction;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.DependencyKind;
+import com.google.devtools.build.lib.analysis.IncompatiblePlatformProvider;
import com.google.devtools.build.lib.analysis.LabelAndLocation;
+import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.Runfiles;
+import com.google.devtools.build.lib.analysis.RunfilesProvider;
+import com.google.devtools.build.lib.analysis.RunfilesSupport;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.constraints.EnvironmentCollection.EnvironmentWithGroup;
import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironmentsProvider.RemovedEnvironmentCulprit;
+import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
+import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
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.packages.Attribute;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.BuildType;
@@ -40,6 +57,9 @@
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.Type.LabelClass;
import com.google.devtools.build.lib.packages.Type.LabelVisitor;
+import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
+import com.google.devtools.build.lib.util.OrderedSetMultimap;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedHashSet;
@@ -808,4 +828,160 @@
type.visitLabels(visitor, value, /*context=*/ null);
}
}
+
+ /**
+ * Creates an incompatible {@link ConfiguredTarget} if the corresponding rule is incompatible.
+ *
+ * <p>Returns null if the target is not incompatible.
+ *
+ * <p>"Incompatible" here means either:
+ *
+ * <ol>
+ * <li>the corresponding <code>target_compatible_with</code> list contains a constraint that the
+ * current platform doesn't satisfy, or
+ * <li>a transitive dependency is incompatible.
+ * </ol>
+ *
+ * @param ruleContext analysis context for the rule
+ * @param prerequisiteMap the dependencies of the rule
+ * @throws ActionConflictException if the underlying {@link RuleConfiguredTargetBuilder}
+ * encounters a problem when assembling a dummy action for the incompatible {@link
+ * ConfiguredTarget}.
+ */
+ @Nullable
+ public static ConfiguredTarget incompatibleConfiguredTarget(
+ RuleContext ruleContext,
+ OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> prerequisiteMap)
+ throws ActionConflictException {
+ if (!ruleContext.getRule().getRuleClassObject().useToolchainResolution()) {
+ return null;
+ }
+
+ // This is incompatible if explicitly specified to be.
+ if (ruleContext.attributes().has("target_compatible_with")) {
+ ImmutableList<ConstraintValueInfo> invalidConstraintValues =
+ stream(
+ PlatformProviderUtils.constraintValues(
+ ruleContext.getPrerequisites("target_compatible_with")))
+ .filter(cv -> !ruleContext.targetPlatformHasConstraint(cv))
+ .collect(toImmutableList());
+
+ if (!invalidConstraintValues.isEmpty()) {
+ return createIncompatibleConfiguredTarget(ruleContext, null, invalidConstraintValues);
+ }
+ }
+
+ // This is incompatible if one of the dependencies is as well.
+ ImmutableList.Builder<ConfiguredTarget> incompatibleDependenciesBuilder =
+ ImmutableList.builder();
+ for (ConfiguredTargetAndData infoCollection : prerequisiteMap.values()) {
+ ConfiguredTarget dependency = infoCollection.getConfiguredTarget();
+ if (dependency instanceof OutputFileConfiguredTarget) {
+ // For generated files, we want to query the generating rule for providers. genrule() for
+ // example doesn't attach providers like IncompatiblePlatformProvider to its outputs.
+ dependency = ((OutputFileConfiguredTarget) dependency).getGeneratingRule();
+ }
+ if (dependency.getProvider(IncompatiblePlatformProvider.class) != null) {
+ incompatibleDependenciesBuilder.add(dependency);
+ }
+ }
+
+ ImmutableList<ConfiguredTarget> incompatibleDependencies =
+ incompatibleDependenciesBuilder.build();
+ if (!incompatibleDependencies.isEmpty()) {
+ return createIncompatibleConfiguredTarget(ruleContext, incompatibleDependencies, null);
+ }
+
+ return null;
+ }
+
+ /**
+ * A helper function for incompatibleConfiguredTarget() to actually create the incompatible {@link
+ * ConfiguredTarget}.
+ *
+ * @param ruleContext analysis context for the rule
+ * @param targetsResponsibleForIncompatibility the targets that are responsible this target's
+ * incompatibility. If null, that means that target is responsible for its own
+ * incompatibility. I.e. it has constraints in target_compatible_with that were not satisfied
+ * on the target platform. This must be null if violatedConstraints is set. This must be set
+ * if violatedConstraints is null.
+ * @param violatedConstraints the constraints that the target platform doesn't satisfy. This must
+ * be null if targetRsesponsibleForIncompatibility is set.
+ * @throws ActionConflictException if the underlying {@link RuleConfiguredTargetBuilder}
+ * encounters a problem when assembling a dummy action for the incompatible {@link
+ * ConfiguredTarget}.
+ */
+ private static ConfiguredTarget createIncompatibleConfiguredTarget(
+ RuleContext ruleContext,
+ @Nullable ImmutableList<ConfiguredTarget> targetsResponsibleForIncompatibility,
+ @Nullable ImmutableList<ConstraintValueInfo> violatedConstraints)
+ throws ActionConflictException {
+ // Create a dummy ConfiguredTarget that has the IncompatiblePlatformProvider set.
+ ImmutableList<Artifact> outputArtifacts = ruleContext.getOutputArtifacts();
+
+ if (ruleContext.isTestTarget() && outputArtifacts.isEmpty()) {
+ // Test targets require RunfilesSupport to be added to the RuleConfiguredTargetBuilder
+ // which needs an "executable". Create one here if none exist already.
+ //
+ // It would be ideal if we could avoid this. Currently the problem is that some rules like
+ // sh_test only declare an output artifact in the corresponding ConfiguredTarget factory
+ // function (see ShBinary.java). Since this code path here replaces the factory function rules
+ // like sh_test never get a chance to declare an output artifact.
+ //
+ // On top of that, the rest of the code base makes the assumption that test targets provide an
+ // instance RunfilesSupport. This can be seen in the TestProvider, the TestActionBuilder, and
+ // the RuleConfiguredTargetBuilder classes. There might be a way to break this assumption, but
+ // it's currently too heavily baked in to work around it more nicely than this.
+ //
+ // Theoretically, this hack shouldn't be an issue because the corresponding actions will never
+ // get executed. They cannot be queried either.
+ outputArtifacts = ImmutableList.of(ruleContext.createOutputArtifact());
+ }
+
+ NestedSet<Artifact> filesToBuild =
+ NestedSetBuilder.<Artifact>stableOrder().addAll(outputArtifacts).build();
+
+ Runfiles.Builder runfilesBuilder =
+ new Runfiles.Builder(
+ ruleContext.getWorkspaceName(),
+ ruleContext.getConfiguration().legacyExternalRunfiles());
+ Runfiles runfiles =
+ runfilesBuilder
+ .addTransitiveArtifacts(filesToBuild)
+ .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES)
+ .build();
+
+ RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext);
+ builder.setFilesToBuild(filesToBuild);
+
+ if (targetsResponsibleForIncompatibility != null) {
+ builder.add(
+ IncompatiblePlatformProvider.class,
+ IncompatiblePlatformProvider.incompatibleDueToTargets(
+ targetsResponsibleForIncompatibility));
+ } else if (violatedConstraints != null) {
+ builder.add(
+ IncompatiblePlatformProvider.class,
+ IncompatiblePlatformProvider.incompatibleDueToConstraints(violatedConstraints));
+ } else {
+ throw new IllegalArgumentException(
+ "Both violatedConstraints and targetsResponsibleForIncompatibility are null");
+ }
+
+ builder.add(RunfilesProvider.class, RunfilesProvider.simple(runfiles));
+ if (!outputArtifacts.isEmpty()) {
+ Artifact executable = outputArtifacts.get(0);
+ RunfilesSupport runfilesSupport =
+ RunfilesSupport.withExecutable(ruleContext, runfiles, executable);
+ builder.setRunfilesSupport(runfilesSupport, executable);
+
+ ruleContext.registerAction(
+ new FailAction(
+ ruleContext.getActionOwner(),
+ outputArtifacts,
+ "Can't build this. This target is incompatible.",
+ Code.CANT_BUILD_INCOMPATIBLE_TARGET));
+ }
+ return builder.build();
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
index f8ea49f..29e9122 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.analysis.constraints;
+import com.google.common.base.Preconditions;
import com.google.common.base.Predicates;
import com.google.common.base.Verify;
import com.google.common.collect.ArrayListMultimap;
@@ -21,11 +22,13 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Multimap;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.IncompatiblePlatformProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironmentsProvider.RemovedEnvironmentCulprit;
+import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
@@ -59,6 +62,8 @@
private final PackageManager packageManager;
private final Function<Key, BuildConfiguration> configurationProvider;
private final ExtendedEventHandler eventHandler;
+ private static final String TARGET_INCOMPATIBLE_ERROR_TEMPLATE =
+ "Target %s is incompatible and cannot be built, but was explicitly requested.%s";
/**
* Constructor with helper classes for loading targets.
@@ -89,6 +94,132 @@
}
/**
+ * Checks that the all top-level targets are compatible with the target platform.
+ *
+ * <p>If any target doesn't support the target platform it will be either marked as "to be
+ * skipped" or marked as "errored".
+ *
+ * <p>Targets that are incompatible with the target platform and are not explicitly requested on
+ * the command line should be skipped.
+ *
+ * <p>Targets that are incompatible with the target platform and *are* explicitly requested on the
+ * command line are errored. Having one or more errored targets will cause the entire build to
+ * fail with an error message.
+ *
+ * @param topLevelTargets the build's top-level targets
+ * @param explicitTargetPatterns the set of explicit target patterns specified by the user on the
+ * command line. Every target must be in the unambiguous canonical form (i.e., with the "@"
+ * prefix for all targets including in the main repository).
+ * @return the set of to-be-skipped and errored top-level targets.
+ * @throws ViewCreationFailedException if any top-level target was explicitly requested on the
+ * command line.
+ */
+ public PlatformRestrictionsResult checkPlatformRestrictions(
+ ImmutableList<ConfiguredTarget> topLevelTargets,
+ ImmutableSet<String> explicitTargetPatterns,
+ boolean keepGoing)
+ throws ViewCreationFailedException {
+ ImmutableSet.Builder<ConfiguredTarget> incompatibleTargets = ImmutableSet.builder();
+ ImmutableSet.Builder<ConfiguredTarget> incompatibleButRequestedTargets = ImmutableSet.builder();
+
+ for (ConfiguredTarget target : topLevelTargets) {
+ IncompatiblePlatformProvider incompatibleProvider =
+ target.getProvider(IncompatiblePlatformProvider.class);
+
+ // Move on to the next target if this one is compatible.
+ if (incompatibleProvider == null) {
+ continue;
+ }
+
+ // We need the label in unambiguous form here. I.e. with the "@" prefix for targets in the
+ // main repository. explicitTargetPatterns is also already in the unambiguous form to make
+ // comparison succeed regardless of the provided form.
+ String labelString = target.getLabel().getUnambiguousCanonicalForm();
+ if (explicitTargetPatterns.contains(labelString)) {
+ if (!keepGoing) {
+ // Use the slightly simpler form for printing error messages. I.e. no "@" prefix for
+ // targets in the main repository.
+ String targetIncompatibleMessage =
+ String.format(
+ TARGET_INCOMPATIBLE_ERROR_TEMPLATE,
+ target.getLabel().toString(),
+ reportOnIncompatibility(target));
+ throw new ViewCreationFailedException(
+ targetIncompatibleMessage,
+ FailureDetail.newBuilder()
+ .setMessage(targetIncompatibleMessage)
+ .setAnalysis(Analysis.newBuilder().setCode(Code.INCOMPATIBLE_TARGET_REQUESTED))
+ .build());
+ }
+ this.eventHandler.handle(
+ Event.warn(String.format(TARGET_INCOMPATIBLE_ERROR_TEMPLATE, labelString, "")));
+ incompatibleButRequestedTargets.add(target);
+ } else {
+ // If this is not an explicitly requested target we can safely skip it.
+ incompatibleTargets.add(target);
+ }
+ }
+
+ return PlatformRestrictionsResult.builder()
+ .targetsToSkip(ImmutableSet.copyOf(incompatibleTargets.build()))
+ .targetsWithErrors(ImmutableSet.copyOf(incompatibleButRequestedTargets.build()))
+ .build();
+ }
+
+ /**
+ * Assembles the explanation for a platform incompatibility.
+ *
+ * <p>This is useful when trying to explain to the user why an explicitly requested target on the
+ * command line is considered incompatible. The goal is to print out the dependency chain and the
+ * constraint that wasn't satisfied so that the user can immediately figure out what happened.
+ *
+ * @param target the incompatible target that was explicitly requested on the command line.
+ * @return the verbose error message to show to the user.
+ */
+ private static String reportOnIncompatibility(ConfiguredTarget target) {
+ Preconditions.checkNotNull(target);
+
+ String message = "\nDependency chain:";
+ IncompatiblePlatformProvider provider = null;
+
+ // TODO(austinschuh): While the first eror is helpful, reporting all the errors at once would
+ // save the user bazel round trips.
+ while (target != null) {
+ message += "\n " + target.getLabel();
+ provider = target.getProvider(IncompatiblePlatformProvider.class);
+ ImmutableList<ConfiguredTarget> targetList = provider.targetsResponsibleForIncompatibility();
+ if (targetList == null) {
+ target = null;
+ } else {
+ target = targetList.get(0);
+ }
+ }
+
+ message += " <-- target platform didn't satisfy constraint";
+ if (provider.constraintsResponsibleForIncompatibility().size() == 1) {
+ message += " " + provider.constraintsResponsibleForIncompatibility().get(0).label();
+ return message;
+ }
+
+ message += "s [";
+
+ boolean first = true;
+ for (ConstraintValueInfo constraintValueInfo :
+ provider.constraintsResponsibleForIncompatibility()) {
+ if (first) {
+ first = false;
+ } else {
+ message += ", ";
+ }
+ message += constraintValueInfo.label();
+ }
+
+ message += "]";
+
+ return message;
+ }
+
+ /**
* Checks that if this is an environment-restricted build, all top-level targets support expected
* top-level environments. Expected top-level environments can be declared explicitly through
* {@code --target_environment} or implicitly through {@code --auto_cpu_environment_group}. For
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
index b2b0c43..6270462 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -34,6 +34,7 @@
import com.google.devtools.build.lib.buildtool.buildevent.TestFilteringCompleteEvent;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.pkgcache.LoadingFailedException;
@@ -54,6 +55,7 @@
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.Collection;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
@@ -203,6 +205,9 @@
Stopwatch timer = Stopwatch.createStarted();
env.getReporter().handle(Event.progress("Loading complete. Analyzing..."));
+ ImmutableSet<String> explicitTargetPatterns =
+ getExplicitTargetPatterns(env, request.getTargets());
+
BuildView view =
new BuildView(
env.getDirectories(),
@@ -214,6 +219,7 @@
loadingResult,
targetOptions,
multiCpu,
+ explicitTargetPatterns,
request.getAspects(),
request.getViewOptions(),
request.getKeepGoing(),
@@ -259,6 +265,7 @@
new TestFilteringCompleteEvent(
analysisResult.getTargetsToBuild(),
analysisResult.getTargetsToTest(),
+ analysisResult.getTargetsToSkip(),
configurationMap));
return analysisResult;
}
@@ -294,4 +301,49 @@
"Found " + targetCount + (targetCount == 1 ? " target..." : " targets...")));
}
}
+
+ /**
+ * Turns target patterns from the command line into parsed equivalents for single targets.
+ *
+ * <p>Globbing targets like ":all" and "..." are ignored here and will not be in the returned set.
+ *
+ * @param env the action's environment.
+ * @param requestedTargetPatterns the list of target patterns specified on the command line.
+ * @return the set of stringified labels of target patterns that represent single targets. The
+ * stringified labels are in the "unambiguous canonical form".
+ * @throws ViewCreationFailedException if a pattern fails to parse for some reason.
+ */
+ private static ImmutableSet<String> getExplicitTargetPatterns(
+ CommandEnvironment env, List<String> requestedTargetPatterns)
+ throws ViewCreationFailedException {
+ ImmutableSet.Builder<String> explicitTargetPatterns = ImmutableSet.builder();
+ TargetPattern.Parser parser = new TargetPattern.Parser(env.getRelativeWorkingDirectory());
+
+ for (String requestedTargetPattern : requestedTargetPatterns) {
+ if (requestedTargetPattern.startsWith("-")) {
+ // Excluded patterns are by definition not explicitly requested so we can move on to the
+ // next target pattern.
+ continue;
+ }
+
+ // Parse the pattern. This should always work because this is at least the second time we're
+ // doing it. The previous time is in runAnalysisPhase(). Still, if parsing does fail we
+ // propagate the exception up.
+ TargetPattern parsedPattern;
+ try {
+ parsedPattern = parser.parse(requestedTargetPattern);
+ } catch (TargetParsingException e) {
+ throw new ViewCreationFailedException(
+ "Failed to parse target pattern even though it was previously parsed successfully",
+ e.getDetailedExitCode().getFailureDetail(),
+ e);
+ }
+
+ if (parsedPattern.getType() == TargetPattern.Type.SINGLE_TARGET) {
+ explicitTargetPatterns.add(parsedPattern.getSingleTargetPath());
+ }
+ }
+
+ return ImmutableSet.copyOf(explicitTargetPatterns.build());
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
index d4a0e89..f3bcdbd 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
@@ -86,16 +86,22 @@
if (targetsToPrint.size() > request.getBuildOptions().maxResultTargets) {
return;
}
- // Filter the targets we care about into two buckets:
+ // Filter the targets we care about into three buckets:
Collection<ConfiguredTarget> succeeded = new ArrayList<>();
Collection<ConfiguredTarget> failed = new ArrayList<>();
+ Collection<ConfiguredTarget> skipped = new ArrayList<>();
Collection<ConfiguredTarget> successfulTargets = result.getSuccessfulTargets();
for (ConfiguredTarget target : targetsToPrint) {
- (successfulTargets.contains(target) ? succeeded : failed).add(target);
+ if (configuredTargetsToSkip.contains(target)) {
+ skipped.add(target);
+ } else {
+ (successfulTargets.contains(target) ? succeeded : failed).add(target);
+ }
}
- // TODO(bazel-team): convert these to a new "SKIPPED" status when ready: b/62191890.
- failed.addAll(configuredTargetsToSkip);
+ for (ConfiguredTarget target : skipped) {
+ outErr.printErr("Target " + target.getLabel() + " was skipped\n");
+ }
TopLevelArtifactContext context = request.getTopLevelArtifactContext();
for (ConfiguredTarget target : succeeded) {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestFilteringCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestFilteringCompleteEvent.java
index d8e8876..febb37c 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestFilteringCompleteEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestFilteringCompleteEvent.java
@@ -27,13 +27,14 @@
/**
* This event is fired after test filtering.
*
- * The test filtering phase always expands test_suite rules, so
- * the set of active targets should never contain test_suites.
+ * <p>The test filtering phase always expands test_suite rules, so the set of active targets should
+ * never contain test_suites.
*/
@Immutable
public class TestFilteringCompleteEvent {
private final Collection<ConfiguredTarget> targets;
private final Collection<ConfiguredTarget> testTargets;
+ private final Collection<ConfiguredTarget> skippedTests;
private final Map<BuildConfigurationValue.Key, BuildConfiguration> configurationMap;
/**
@@ -41,14 +42,17 @@
*
* @param targets The set of active targets that remain.
* @param testTargets The collection of tests to be run. May be null.
+ * @param targetsToSkip The collection of tests that are to be skipped.
* @param configurationMap A map from configuration keys of all targets to the configurations.
*/
public TestFilteringCompleteEvent(
Collection<? extends ConfiguredTarget> targets,
Collection<? extends ConfiguredTarget> testTargets,
+ Collection<? extends ConfiguredTarget> targetsToSkip,
Map<BuildConfigurationValue.Key, BuildConfiguration> configurationMap) {
this.targets = ImmutableList.copyOf(targets);
this.testTargets = testTargets == null ? null : ImmutableList.copyOf(testTargets);
+ this.skippedTests = ImmutableList.copyOf(targetsToSkip);
this.configurationMap = configurationMap;
if (testTargets == null) {
return;
@@ -74,6 +78,11 @@
return testTargets;
}
+ /** Returns the set of tests that should be skipped. */
+ public Collection<ConfiguredTarget> getSkippedTests() {
+ return skippedTests;
+ }
+
public BuildConfiguration getConfigurationForTarget(ConfiguredTarget target) {
return Preconditions.checkNotNull(configurationMap.get(target.getConfigurationKey()));
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 715d6ba..5890fc2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -265,6 +265,12 @@
public static final String COMPATIBLE_ENVIRONMENT_ATTR = "compatible_with";
/**
+ * For Bazel's constraint system: the attribute that declares the list of constraints that the
+ * target must satisfy to be considered compatible.
+ */
+ public static final String TARGET_RESTRICTED_TO_ATTR = "target_compatible_with";
+
+ /**
* For Bazel's constraint system: the implicit attribute used to store rule class restriction
* defaults as specified by {@link Builder#restrictedTo}.
*/
@@ -1410,6 +1416,7 @@
this.supportsConstraintChecking = false;
attributes.remove(RuleClass.COMPATIBLE_ENVIRONMENT_ATTR);
attributes.remove(RuleClass.RESTRICTED_ENVIRONMENT_ATTR);
+ attributes.remove(RuleClass.TARGET_RESTRICTED_TO_ATTR);
return this;
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
index 62be54b..63d8c5a 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
@@ -110,7 +110,11 @@
continue;
}
TestResultAggregator aggregator =
- new TestResultAggregator(target, event.getConfigurationForTarget(target), policy);
+ new TestResultAggregator(
+ target,
+ event.getConfigurationForTarget(target),
+ policy,
+ event.getSkippedTests().contains(target));
TestResultAggregator oldAggregator = aggregators.put(asKey(target), aggregator);
Preconditions.checkState(
oldAggregator == null, "target: %s, values: %s %s", target, oldAggregator, aggregator);
@@ -140,21 +144,40 @@
}
}
+ private void targetSkipped(ConfiguredTargetKey configuredTargetKey) {
+ TestResultAggregator aggregator = aggregators.get(configuredTargetKey);
+ if (aggregator != null) {
+ aggregator.targetSkipped();
+ }
+ }
+
@VisibleForTesting
void buildComplete(
- Collection<ConfiguredTarget> actualTargets, Collection<ConfiguredTarget> successfulTargets) {
+ Collection<ConfiguredTarget> actualTargets,
+ Collection<ConfiguredTarget> skippedTargets,
+ Collection<ConfiguredTarget> successfulTargets) {
if (actualTargets == null || successfulTargets == null) {
return;
}
+ ImmutableSet<ConfiguredTarget> nonSuccessfulTargets =
+ Sets.difference(ImmutableSet.copyOf(actualTargets), ImmutableSet.copyOf(successfulTargets))
+ .immutableCopy();
for (ConfiguredTarget target :
Sets.difference(
- ImmutableSet.copyOf(actualTargets), ImmutableSet.copyOf(successfulTargets))) {
+ ImmutableSet.copyOf(nonSuccessfulTargets), ImmutableSet.copyOf(skippedTargets))) {
if (isAlias(target)) {
continue;
}
targetFailure(asKey(target));
}
+
+ for (ConfiguredTarget target : skippedTargets) {
+ if (isAlias(target)) {
+ continue;
+ }
+ targetSkipped(asKey(target));
+ }
}
@Subscribe
@@ -164,7 +187,8 @@
blazeHalted = true;
}
skipTargetsOnFailure = result.getStopOnFirstFailure();
- buildComplete(result.getActualTargets(), result.getSuccessfulTargets());
+ buildComplete(
+ result.getActualTargets(), result.getSkippedTargets(), result.getSuccessfulTargets());
}
@Subscribe
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
index c518194..b72ca55 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
@@ -317,7 +317,9 @@
addFailureToErrorList(results, "locally", stats.failedLocallyCount);
addFailureToErrorList(results, "remotely", stats.failedRemotelyCount);
addToErrorList(results, "was", "were", "skipped", stats.noStatusCount);
- printer.print(String.format("\nExecuted %d out of %d %s: %s.\n",
+ printer.print(
+ String.format(
+ "\nExecuted %d out of %d %s: %s.\n",
stats.numberOfExecutedTargets,
stats.numberOfTargets,
stats.numberOfTargets == 1 ? "test" : "tests",
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAggregator.java b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAggregator.java
index 6650bcb..6c64522 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAggregator.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAggregator.java
@@ -70,7 +70,10 @@
private final Map<Artifact, TestResult> statusMap = new HashMap<>();
public TestResultAggregator(
- ConfiguredTarget target, BuildConfiguration configuration, AggregationPolicy policy) {
+ ConfiguredTarget target,
+ BuildConfiguration configuration,
+ AggregationPolicy policy,
+ boolean skippedThisTest) {
this.testTarget = target;
this.policy = policy;
@@ -83,6 +86,7 @@
this.summary.setConfiguration(configuration);
}
this.summary.setStatus(BlazeTestStatus.NO_STATUS);
+ this.summary.setSkipped(skippedThisTest);
this.remainingRuns = new HashSet<>(TestProvider.getTestStatusArtifacts(target));
}
@@ -150,6 +154,21 @@
policy.eventBus.post(summary.build());
}
+ synchronized void targetSkipped() {
+ if (remainingRuns.isEmpty()) {
+ // Blaze does not guarantee that BuildResult.getSuccessfulTargets() and posted TestResult
+ // events are in sync. Thus, it is possible that a test event was posted, but the target is
+ // not present in the set of successful targets.
+ return;
+ }
+
+ summary.setStatus(BlazeTestStatus.NO_STATUS);
+
+ // These are never going to run; removing them marks the target complete.
+ remainingRuns.clear();
+ policy.eventBus.post(summary.build());
+ }
+
/** Returns the known aggregate results for the given target at the current moment. */
synchronized TestSummary.Builder getCurrentSummaryForTesting() {
return summary;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java b/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java
index 6b2845a..fe0a79c 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java
@@ -133,6 +133,12 @@
return this;
}
+ public Builder setSkipped(boolean skipped) {
+ checkMutation(skipped);
+ summary.skipped = skipped;
+ return this;
+ }
+
public Builder addCoverageFiles(List<Path> coverageFiles) {
checkMutation(coverageFiles);
summary.coverageFiles.addAll(coverageFiles);
@@ -342,6 +348,7 @@
private ConfiguredTarget target;
private BuildConfiguration configuration;
private BlazeTestStatus status;
+ private boolean skipped;
// Currently only populated if --runs_per_test_detects_flakes is enabled.
private Multimap<Integer, BlazeTestStatus> shardRunStatuses = ArrayListMultimap.create();
private int numCached;
@@ -389,6 +396,10 @@
return status;
}
+ public boolean isSkipped() {
+ return skipped;
+ }
+
/**
* Whether or not any results associated with this test were cached locally or remotely.
*
@@ -525,7 +536,10 @@
return lastStopTimeMillis;
}
- static Mode getStatusMode(BlazeTestStatus status) {
+ Mode getStatusMode() {
+ if (skipped) {
+ return Mode.WARNING;
+ }
return status == BlazeTestStatus.PASSED
? Mode.INFO
: (status == BlazeTestStatus.FLAKY ? Mode.WARNING : Mode.ERROR);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java b/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java
index ce7052a..adb74ce 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java
@@ -60,8 +60,16 @@
for (Path path : summary.getPassedLogs()) {
allLogs.add(testLogPathFormatter.getPathStringToPrint(path));
}
- printer.printLn("" + TestSummary.getStatusMode(summary.getStatus()) + summary.getStatus() + ": "
- + Mode.DEFAULT + testName + " (see " + Joiner.on(' ').join(allLogs) + ")");
+ printer.printLn(
+ ""
+ + summary.getStatusMode()
+ + summary.getStatus()
+ + ": "
+ + Mode.DEFAULT
+ + testName
+ + " (see "
+ + Joiner.on(' ').join(allLogs)
+ + ")");
printer.printLn(Mode.INFO + "INFO: " + Mode.DEFAULT + "From Testing " + testName);
// Whether to output the target at all was checked by the caller.
@@ -90,12 +98,18 @@
}
}
- private static String statusString(BlazeTestStatus status) {
- return status.toString().replace('_', ' ');
+ private static String statusString(TestSummary summary) {
+ if (summary.isSkipped()) {
+ // If the test was skipped then its status will be something like NO_STATUS. That's not
+ // informative enough to a user. Instead, return "SKIPPED" for skipped tests.
+ return "SKIPPED";
+ }
+ return summary.getStatus().toString().replace('_', ' ');
}
/**
* Prints summary status for a single test.
+ *
* @param terminalPrinter The printer to print to
*/
public static void print(
@@ -130,15 +144,19 @@
|| status == BlazeTestStatus.BLAZE_HALTED_BEFORE_TESTING) {
return;
}
- String message = getCacheMessage(summary) + statusString(summary.getStatus());
+ String message = getCacheMessage(summary) + statusString(summary);
String targetName = summary.getLabel().toString();
if (withConfigurationName) {
targetName += " (" + summary.getConfiguration().getMnemonic() + ")";
}
terminalPrinter.print(
Strings.padEnd(targetName, 78 - message.length(), ' ')
- + " " + TestSummary.getStatusMode(summary.getStatus()) + message + Mode.DEFAULT
- + (verboseSummary ? getAttemptSummary(summary) + getTimeSummary(summary) : "") + "\n");
+ + " "
+ + summary.getStatusMode()
+ + message
+ + Mode.DEFAULT
+ + (verboseSummary ? getAttemptSummary(summary) + getTimeSummary(summary) : "")
+ + "\n");
if (printFailedTestCases && summary.getStatus() == BlazeTestStatus.FAILED) {
if (summary.getFailedTestCasesStatus() == FailedTestCasesStatus.NOT_AVAILABLE) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
index 47e8806..933a7da 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
@@ -15,6 +15,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.PackageRoots;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -82,4 +84,22 @@
public PackageRoots getPackageRoots() {
return packageRoots;
}
+
+ /**
+ * Returns an equivalent {@link SkyframeAnalysisResult}, except with errored targets removed from
+ * the configured target list.
+ */
+ public SkyframeAnalysisResult withAdditionalErroredTargets(
+ ImmutableSet<ConfiguredTarget> erroredTargets) {
+ return new SkyframeAnalysisResult(
+ hasLoadingError,
+ /*hasAnalysisError=*/ true,
+ hasActionConflicts,
+ Sets.difference(ImmutableSet.copyOf(configuredTargets), erroredTargets)
+ .immutableCopy()
+ .asList(),
+ walkableGraph,
+ aspects,
+ packageRoots);
+ }
}
