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 a1cb8b660ebc93..7781ae6fa82bae 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -328,6 +328,7 @@ java_library( ":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 @@ java_library( "//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", @@ -778,6 +780,15 @@ java_library( ], ) +java_library( + name = "incompatible_platform_provider", + srcs = ["IncompatiblePlatformProvider.java"], + deps = [ + ":transitive_info_provider", + "//src/main/java/com/google/devtools/build/lib/concurrent", + ], +) + java_library( name = "inconsistent_aspect_order_exception", srcs = ["InconsistentAspectOrderException.java"], @@ -1982,6 +1993,7 @@ java_library( srcs = ["constraints/TopLevelConstraintSemantics.java"], deps = [ ":analysis_cluster", + ":incompatible_platform_provider", ":config/build_configuration", ":configured_target", ":constraints/constraint_semantics", 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 586d9cc716b404..f0f35719e91c78 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; @@ -327,6 +328,14 @@ public static RuleClass.Builder commonCoreAndStarlarkAttributes(RuleClass.Builde .dontCheckConstraints() .nonconfigurable( "special logic for constraints and select: see ConstraintSemantics")) + /* + A list of constraint_values that must be satisfied by the target platform in + order for this toolchain to be selected for a target building for that platform. + */ + .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")) 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 74f0b84b62a85f..046adba4aa8627 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 @@ -212,6 +212,7 @@ public AnalysisResult update( TargetPatternPhaseValue loadingResult, BuildOptions targetOptions, Set multiCpu, + List requestedTargetPatterns, List aspects, AnalysisOptions viewOptions, boolean keepGoing, @@ -420,6 +421,22 @@ public AnalysisResult update( skyframeBuildView.clearInvalidatedConfiguredTargets(); } + TopLevelConstraintSemantics topLevelConstraintSemantics = + new TopLevelConstraintSemantics( + skyframeExecutor.getPackageManager(), + input -> skyframeExecutor.getConfiguration(eventHandler, input), + eventHandler); + + TopLevelConstraintSemantics.PlatformRestrictionsResult platformRestrictions = + topLevelConstraintSemantics.checkPlatformRestrictions( + skyframeAnalysisResult.getConfiguredTargets(), requestedTargetPatterns, keepGoing); + + if (platformRestrictions.targetsWithErrors.size() > 0) { + skyframeAnalysisResult = + skyframeAnalysisResult.withAdditionalErroredTargets( + ImmutableSet.copyOf(platformRestrictions.targetsWithErrors)); + } + int numTargetsToAnalyze = topLevelTargetsWithConfigs.size(); int numSuccessful = skyframeAnalysisResult.getConfiguredTargets().size(); if (0 < numSuccessful && numSuccessful < numTargetsToAnalyze) { @@ -430,11 +447,11 @@ public AnalysisResult update( } Set targetsToSkip = - new TopLevelConstraintSemantics( - skyframeExecutor.getPackageManager(), - input -> skyframeExecutor.getConfiguration(eventHandler, input), - eventHandler) - .checkTargetEnvironmentRestrictions(skyframeAnalysisResult.getConfiguredTargets()); + ImmutableSet.copyOf( + Sets.union( + topLevelConstraintSemantics.checkTargetEnvironmentRestrictions( + skyframeAnalysisResult.getConfiguredTargets()), + platformRestrictions.targetsToSkip)); 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 5d3d61fc5e48aa..ad43b4c7cd7a94 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,8 @@ 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.platform.ConstraintValueInfo; +import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils; 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; @@ -318,6 +320,12 @@ private ConfiguredTarget createRule( prerequisiteMap.values())) .build(); + ConfiguredTarget incompatibleTarget = + incompatibleConfiguredTarget(ruleContext, prerequisiteMap); + if (incompatibleTarget != null) { + return incompatibleTarget; + } + List> analysisFailures = depAnalysisFailures(ruleContext); if (!analysisFailures.isEmpty()) { return erroredConfiguredTargetWithFailures(ruleContext, analysisFailures); @@ -372,6 +380,89 @@ private ConfiguredTarget createRule( } } + // TODO(phil): Document this. + // TODO(phil): Move this into ConstraintSemantics somehow. + @Nullable + private ConfiguredTarget incompatibleConfiguredTarget( + RuleContext ruleContext, + OrderedSetMultimap prerequisiteMap) + throws ActionConflictException { + if (!"toolchain".equals(ruleContext.getRule().getRuleClass())) { + boolean isIncompatible = false; + + // This is incompatible if one of the dependencies is as well. + 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) { + isIncompatible = true; + } + } + + // This is incompatible if explicitly specified to be. + if (!isIncompatible && ruleContext.attributes().has("target_compatible_with")) { + Iterable constraintValues = + PlatformProviderUtils.constraintValues( + ruleContext.getPrerequisites("target_compatible_with", TransitionMode.DONT_CHECK)); + for (ConstraintValueInfo constraintValue : constraintValues) { + if (!ruleContext.targetPlatformHasConstraint(constraintValue)) { + isIncompatible = true; + } + } + } + + if (isIncompatible) { + // Create a dummy ConfiguredTarget that has the IncompatiblePlatformProvider set. + ImmutableList outputArtifacts = ruleContext.getOutputArtifacts(); + + if (ruleContext.isTestTarget() && outputArtifacts.size() == 0) { + // TODO(philsc): Figure out how to avoid this. Right now some test targets require the + // RunfilesSupport to be added to the RuleConfiguredTargetBuilder. E.g. sh_test requires + // this while cc_test does not. + outputArtifacts = ImmutableList.of(ruleContext.createOutputArtifact()); + } + + NestedSet filesToBuild = + NestedSetBuilder.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); + builder.add(IncompatiblePlatformProvider.class, IncompatiblePlatformProvider.simple()); + builder.add(RunfilesProvider.class, RunfilesProvider.simple(runfiles)); + if (outputArtifacts.size() > 0) { + 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.")); + } + return builder.build(); + } + } + + // No incompatibilities found. + return null; + } + private List> depAnalysisFailures(RuleContext ruleContext) { if (ruleContext.getConfiguration().allowAnalysisFailures()) { ImmutableList.Builder> analysisFailures = ImmutableList.builder(); 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 00000000000000..70cb92163397e2 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/IncompatiblePlatformProvider.java @@ -0,0 +1,29 @@ +// 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.devtools.build.lib.concurrent.ThreadSafety.Immutable; + +// TODO(phil): Document this. +@Immutable +public final class IncompatiblePlatformProvider implements TransitiveInfoProvider { + IncompatiblePlatformProvider() { + // Nothing to do. + } + + public static IncompatiblePlatformProvider simple() { + return new IncompatiblePlatformProvider(); + } +} 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 bbeffe7f2658bb..be83f24edea865 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 @@ -21,6 +21,7 @@ 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; @@ -57,6 +58,19 @@ public class TopLevelConstraintSemantics { private final Function configurationProvider; private final ExtendedEventHandler eventHandler; + // TODO(phil): Document this. + public class PlatformRestrictionsResult { + public ImmutableSet targetsToSkip; + public ImmutableSet targetsWithErrors; + + public PlatformRestrictionsResult( + ImmutableSet targetsToSkip, + ImmutableSet targetsWithErrors) { + this.targetsToSkip = targetsToSkip; + this.targetsWithErrors = targetsWithErrors; + } + }; + /** * Constructor with helper classes for loading targets. * @@ -85,6 +99,42 @@ private MissingEnvironment(Label environment, RemovedEnvironmentCulprit culprit) } } + // TODO(phil): Document this. + public PlatformRestrictionsResult checkPlatformRestrictions( + ImmutableList topLevelTargets, + List requestedTargetPatterns, + boolean keepGoing) + throws ViewCreationFailedException { + ImmutableSet.Builder incompatibleTargets = ImmutableSet.builder(); + ImmutableSet.Builder incompatibleButRequestedTargets = ImmutableSet.builder(); + + for (ConfiguredTarget target : topLevelTargets) { + if (target.getProvider(IncompatiblePlatformProvider.class) != null) { + String labelString = target.getLabel().toString(); + if (requestedTargetPatterns.contains(labelString)) { + if (!keepGoing) { + throw new ViewCreationFailedException( + "Target " + + labelString + + " is incompatible and cannot be built, but was explicitly requested."); + } + this.eventHandler.handle( + Event.warn( + "Incompatible target " + + labelString + + " specifically requested: it will not be built")); + incompatibleButRequestedTargets.add(target); + } else { + incompatibleTargets.add(target); + } + } + } + + return new PlatformRestrictionsResult( + ImmutableSet.copyOf(incompatibleTargets.build()), + ImmutableSet.copyOf(incompatibleButRequestedTargets.build())); + } + /** * 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 diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto index 29642df0b7f961..806fa51e4856bb 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto @@ -543,6 +543,7 @@ enum TestStatus { REMOTE_FAILURE = 6; FAILED_TO_BUILD = 7; TOOL_HALTED_BEFORE_TESTING = 8; + SKIPPED = 9; } // Payload on events reporting about individual test action. 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 5118fe42128145..9bcc21c236bb2e 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 @@ -213,6 +213,7 @@ private static AnalysisResult runAnalysisPhase( loadingResult, targetOptions, multiCpu, + request.getTargets(), request.getAspects(), request.getViewOptions(), request.getKeepGoing(), 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 d4a0e89ba553e8..f3bcdbd59f6ab7 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 @@ void showBuildResult( 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 succeeded = new ArrayList<>(); Collection failed = new ArrayList<>(); + Collection skipped = new ArrayList<>(); Collection 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/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index e2657e8c39f19d..be910ba9c9142b 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 @@ -264,6 +264,8 @@ public RuleErrorException(String message, Throwable cause) { */ public static final String COMPATIBLE_ENVIRONMENT_ATTR = "compatible_with"; + 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}. @@ -1411,6 +1413,7 @@ public Builder exemptFromConstraintChecking(String reason) { 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 62be54b78015d7..5d1b1321f9eae4 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 @@ -140,21 +140,40 @@ private void targetFailure(ConfiguredTargetKey configuredTargetKey) { } } + private void targetSkipped(ConfiguredTargetKey configuredTargetKey) { + TestResultAggregator aggregator = aggregators.get(configuredTargetKey); + if (aggregator != null) { + aggregator.targetSkipped(); + } + } + @VisibleForTesting void buildComplete( - Collection actualTargets, Collection successfulTargets) { + Collection actualTargets, + Collection skippedTargets, + Collection successfulTargets) { if (actualTargets == null || successfulTargets == null) { return; } + ImmutableSet 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 +183,8 @@ public void buildCompleteEvent(BuildCompleteEvent event) { 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/BuildEventStreamerUtils.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerUtils.java index fc0f161e5a145c..8b15084bb843e1 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerUtils.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerUtils.java @@ -45,6 +45,8 @@ public static TestStatus bepStatus(BlazeTestStatus status) { return BuildEventStreamProtos.TestStatus.REMOTE_FAILURE; case BLAZE_HALTED_BEFORE_TESTING: return BuildEventStreamProtos.TestStatus.TOOL_HALTED_BEFORE_TESTING; + case SKIPPED: + return BuildEventStreamProtos.TestStatus.SKIPPED; default: // Not used as the above is a complete case distinction; however, by the open // nature of protobuf enums, we need the clause to convice java, that we always 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 e05d7622a17136..bc6dba2acdcd4d 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 @@ -56,6 +56,7 @@ private static class TestResultStats { int failedRemotelyCount; int failedLocallyCount; int noStatusCount; + int skippedCount; int numberOfExecutedTargets; boolean wasUnreportedWrongSize; @@ -213,6 +214,8 @@ public void notify(Set summaries, int numberOfExecutedTargets) { stats.passCount++; } else if (summary.getStatus() == BlazeTestStatus.NO_STATUS) { stats.noStatusCount++; + } else if (summary.getStatus() == BlazeTestStatus.SKIPPED) { + stats.skippedCount++; } else if (summary.getStatus() == BlazeTestStatus.FAILED_TO_BUILD) { stats.failedToBuildCount++; } else if (summary.ranRemotely()) { @@ -311,8 +314,10 @@ private void printStats(TestResultStats stats) { addFailureToErrorList(results, "to build", stats.failedToBuildCount); 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", + addToErrorList(results, "was", "were", "skipped", stats.noStatusCount + stats.skippedCount); + 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 6650bcb2ec1a17..0d757b30ef2323 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 @@ -150,6 +150,21 @@ synchronized void targetFailure(boolean blazeHalted, boolean skipTargetsOnFailur 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.SKIPPED); + + // 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/UiEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java index ba1d490c98c607..2af4e55e8515b8 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java @@ -679,7 +679,8 @@ private boolean testSummaryProvidesNewInformation(TestSummary summary) { BlazeTestStatus.PASSED, BlazeTestStatus.FAILED_TO_BUILD, BlazeTestStatus.BLAZE_HALTED_BEFORE_TESTING, - BlazeTestStatus.NO_STATUS); + BlazeTestStatus.NO_STATUS, + BlazeTestStatus.SKIPPED); if (statusToIgnore.contains(summary.getStatus())) { return false; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index e542c3e7d33eb2..fd9e8f2f8321c3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -256,6 +256,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:dependency_kind", "//src/main/java/com/google/devtools/build/lib/analysis:duplicate_exception", "//src/main/java/com/google/devtools/build/lib/analysis:inconsistent_aspect_order_exception", + "//src/main/java/com/google/devtools/build/lib/analysis:incompatible_platform_provider", "//src/main/java/com/google/devtools/build/lib/analysis:platform_options", "//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context", "//src/main/java/com/google/devtools/build/lib/analysis:toolchain_collection", 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 47e88064df102f..c1cd9527c7bbb1 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 ImmutableMap getAspects() { public PackageRoots getPackageRoots() { return packageRoots; } + + /** + * Returns an equivalent {@link SkyframeAnalysisResult}, except with errored targets removed from + * the configured target list. + */ + public SkyframeAnalysisResult withAdditionalErroredTargets( + ImmutableSet erroredTargets) { + return new SkyframeAnalysisResult( + hasLoadingError, + /*hasAnalaysiError=*/ true, + hasActionConflicts, + Sets.difference(ImmutableSet.copyOf(configuredTargets), erroredTargets) + .immutableCopy() + .asList(), + walkableGraph, + aspects, + packageRoots); + } } diff --git a/src/main/protobuf/test_status.proto b/src/main/protobuf/test_status.proto index 72cb268607f0c0..95f9a2775aea63 100644 --- a/src/main/protobuf/test_status.proto +++ b/src/main/protobuf/test_status.proto @@ -41,6 +41,7 @@ enum BlazeTestStatus { REMOTE_FAILURE = 6; FAILED_TO_BUILD = 7; BLAZE_HALTED_BEFORE_TESTING = 8; + SKIPPED = 9; }; // TestCase contains detailed data about all tests (cases/suites/decorators) diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 6fa630566cda67..76e05b3e449655 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -1296,6 +1296,13 @@ sh_test( tags = ["no_windows"], ) +sh_test( + name = "target_compatible_with_test", + srcs = ["target_compatible_with_test.sh"], + data = [":test-deps"], + deps = ["@bazel_tools//tools/bash/runfiles"], +) + sh_test( name = "resource_compiler_toolchain_test", srcs = ["resource_compiler_toolchain_test.sh"], diff --git a/src/test/shell/bazel/target_compatible_with_test.sh b/src/test/shell/bazel/target_compatible_with_test.sh new file mode 100755 index 00000000000000..cd56353518090d --- /dev/null +++ b/src/test/shell/bazel/target_compatible_with_test.sh @@ -0,0 +1,390 @@ +#!/bin/bash + +# --- begin runfiles.bash initialization --- +set -euo pipefail +if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + if [[ -f "$0.runfiles_manifest" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" + elif [[ -f "$0.runfiles/MANIFEST" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + fi +fi +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" +elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ + "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" +else + echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash" + exit 1 +fi +# --- end runfiles.bash initialization --- + +source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +# `uname` returns the current platform, e.g "MSYS_NT-10.0" or "Linux". +# `tr` converts all upper case letters to lower case. +# `case` matches the result if the `uname | tr` expression to string prefixes +# that use the same wildcards as names do in Bash, i.e. "msys*" matches strings +# starting with "msys", and "*" matches everything (it's the default case). +case "$(uname -s | tr [:upper:] [:lower:])" in +msys*) + # As of 2019-01-15, Bazel on Windows only supports MSYS Bash. + declare -r is_windows=true + ;; +*) + declare -r is_windows=false + ;; +esac + +function set_up() { + mkdir -p target_skipping || fail "couldn't create directory" + cat < target_skipping/pass.sh || fail "couldn't create pass.sh" +#!/bin/bash +exit 0 +EOT + chmod +x target_skipping/pass.sh + + cat < target_skipping/fail.sh || fail "couldn't create fail.sh" +#!/bin/bash +exit 1 +EOT + chmod +x target_skipping/fail.sh + + cat < target_skipping/BUILD || fail "couldn't create BUILD file" +constraint_setting(name = "not_compatible_setting") + +constraint_value( + name = "not_compatible", + constraint_setting = ":not_compatible_setting", +) + +constraint_setting(name = "foo_version") + +constraint_value( + name = "foo1", + constraint_setting = ":foo_version", +) + +constraint_value( + name = "foo2", + constraint_setting = ":foo_version", +) + +constraint_value( + name = "foo3", + constraint_setting = ":foo_version", +) + +config_setting( + name = "foo1_selectable", + constraint_values = [":foo1"], +) + +config_setting( + name = "foo2_selectable", + constraint_values = [":foo2"], +) + +constraint_setting(name = "bar_version") + +constraint_value( + name = "bar1", + constraint_setting = "bar_version", +) + +constraint_value( + name = "bar2", + constraint_setting = "bar_version", +) + +# TODO(phil): Use more generic constraints for os and cpu. +platform( + name = "foo1_bar1_platform", + constraint_values = [ + "@platforms//os:linux", + "@platforms//cpu:x86_64", + ":foo1", + ":bar1", + ], +) + +platform( + name = "foo2_bar1_platform", + constraint_values = [ + "@platforms//os:linux", + "@platforms//cpu:x86_64", + ":foo2", + ":bar1", + ], +) + +platform( + name = "foo1_bar2_platform", + constraint_values = [ + "@platforms//os:linux", + "@platforms//cpu:x86_64", + ":foo1", + ":bar2", + ], +) + +platform( + name = "foo3_platform", + constraint_values = [ + "@platforms//os:linux", + "@platforms//cpu:x86_64", + ":foo3", + ], +) + +sh_test( + name = "pass_on_foo1", + srcs = [":pass.sh"], + target_compatible_with = [":foo1"], +) + +sh_test( + name = "fail_on_foo2", + srcs = [":fail.sh"], + target_compatible_with = [":foo2"], +) + +sh_test( + name = "pass_on_foo1_bar2", + srcs = [":pass.sh"], + target_compatible_with = [ + ":foo1", + ":bar2", + ], +) + +sh_binary( + name = "some_foo3_target", + srcs = [":pass.sh"], + target_compatible_with = [ + ":foo3", + ], +) +EOT + + cat < target_skipping/WORKSPACE || fail "couldn't create WORKSPACE" +EOT +} + +function test_console_log_for_builds() { + cd target_skipping || fail "couldn't cd into workspace" + + if ! bazel build --show_result=10 --host_platform=@//:foo3_platform \ + --platforms=@//:foo3_platform ... &> "${TEST_log}"; then + cat "${TEST_log}" + fail "Bazel failed the test." + fi + + cp "${TEST_log}" "${TEST_UNDECLARED_OUTPUTS_DIR}"/ + + expect_log 'Target //:some_foo3_target up-to-date' + expect_log 'Target //:fail_on_foo2 was skipped' + expect_log 'Target //:pass_on_foo1 was skipped' + expect_log 'Target //:pass_on_foo1_bar2 was skipped' + expect_not_log 'Target //:fail_on_foo2 failed to build' + expect_not_log 'Target //:pass_on_foo1 failed to build' + expect_not_log 'Target //:pass_on_foo1_bar2 failed to build' + expect_not_log 'Target //:fail_on_foo2 up-to-date' + expect_not_log 'Target //:pass_on_foo1 up-to-date' + expect_not_log 'Target //:pass_on_foo1_bar2 up-to-date' +} + +function test_console_log_for_tests() { + cd target_skipping || fail "couldn't cd into workspace" + + # Get repeatable results from this test. + bazel clean --expunge + + if ! bazel test --host_platform=@//:foo1_bar1_platform \ + --platforms=@//:foo1_bar1_platform ... &> "${TEST_log}"; then + cat "${TEST_log}" + fail "Bazel failed unexpectedly." + fi + expect_log 'Executed 1 out of 3 tests: 1 test passes and 2 were skipped' + expect_log '^//:pass_on_foo1 * PASSED in' + expect_log '^//:fail_on_foo2 * SKIPPED$' + expect_log '^//:pass_on_foo1_bar2 * SKIPPED$' + + if bazel test --host_platform=@//:foo2_bar1_platform \ + --platforms=@//:foo2_bar1_platform ... &> "${TEST_log}"; then + cat "${TEST_log}" + fail "Bazel passed unexpectedly." + fi + expect_log 'Executed 1 out of 3 tests: 1 fails locally and 2 were skipped.' + expect_log '^//:pass_on_foo1 * SKIPPED$' + expect_log '^//:fail_on_foo2 * FAILED in' + expect_log '^//:pass_on_foo1_bar2 * SKIPPED$' + + # Use :all for this one to validate similar behaviour. + if ! bazel test --host_platform=@//:foo1_bar2_platform \ + --platforms=@//:foo1_bar2_platform :all &> "${TEST_log}"; then + cat "${TEST_log}" + fail "Bazel failed unexpectedly." + fi + expect_log 'Executed 2 out of 3 tests: 2 tests pass and 1 was skipped' + expect_log '^//:pass_on_foo1 * PASSED in' + expect_log '^//:fail_on_foo2 * SKIPPED$' + expect_log '^//:pass_on_foo1_bar2 * PASSED in' +} + +function test_failure_on_incompatible_top_level_target() { + cd target_skipping || fail "couldn't cd into workspace" + + if bazel test --show_result=10 --host_platform=@//:foo1_bar1_platform \ + --platforms=@//:foo1_bar1_platform //:pass_on_foo1_bar2 \ + --build_event_text_file="${TEST_log}".build.json \ + &> "${TEST_log}"; then + fail "Bazel passed unexpectedly." + fi + + expect_log 'ERROR: Target //:pass_on_foo1_bar2 is incompatible and cannot be built' + expect_log '^FAILED: Build did NOT complete successfully' + + # Now look at the build event log. + mv "${TEST_log}".build.json "${TEST_log}" + + expect_log '^ name: "PARSING_FAILURE"$' + expect_log 'Target //:pass_on_foo1_bar2 is incompatible and cannot be built.' + + # Run an additional (passing) test and make sure we still fail the build. + if bazel test --show_result=10 --host_platform=@//:foo1_bar1_platform \ + --platforms=@//:foo1_bar1_platform //:pass_on_foo1_bar2 //:pass_on_foo1 \ + --keep_going \ + &> "${TEST_log}"; then + fail "Bazel passed unexpectedly." + fi + + expect_log '^//:pass_on_foo1 * PASSED in' + expect_log '^ERROR: command succeeded, but not all targets were analyzed' + expect_log '^FAILED: Build did NOT complete successfully' +} + +function test_build_event_protocol() { + cd target_skipping || fail "couldn't cd into workspace" + + if ! bazel test --show_result=10 --host_platform=@//:foo1_bar1_platform \ + --build_event_text_file=$TEST_log \ + --platforms=@//:foo1_bar1_platform ...; then + cat "${TEST_log}" + fail "Bazel failed unexpectedly." + fi + expect_not_log 'Target //:pass_on_foo1 build was skipped\.' + expect_log_n 'reason: SKIPPED\>' 3 + expect_log 'Target //:fail_on_foo2 build was skipped\.' + expect_log 'Target //:pass_on_foo1_bar2 build was skipped\.' +} + +function test_non_top_level_skipping() { + cat >> target_skipping/BUILD < "${TEST_log}"; then + fail "Bazel passed unexpectedly." + fi + expect_log 'ERROR: Target //:sh_foo2 is incompatible and cannot be built, but was explicitly requested' + expect_log 'FAILED: Build did NOT complete successfully' +} + +function test_py_test() { + cat > target_skipping/generator_tool.cc < +int main() { + printf("int main() { return 1; }\\n"); + return 0; +} +EOT + + cat >> target_skipping/BUILD < "${TEST_log}"; then + fail "Bazel passed unexpectedly." + fi + expect_log 'ERROR: Target //:generated_test is incompatible and cannot be built, but was explicitly requested' + expect_log 'FAILED: Build did NOT complete successfully' +} + +function test_or_logic() { + cat >> target_skipping/BUILD < "${TEST_log}"; then + fail "Bazel failed unexpectedly." + fi + expect_log '^//:pass_on_foo1_or_foo2_but_not_on_foo3 * PASSED in' + + if ! bazel test --show_result=10 --host_platform=@//:foo2_bar1_platform \ + --platforms=@//:foo2_bar1_platform //:pass_on_foo1_or_foo2_but_not_on_foo3 \ + --nocache_test_results &> "${TEST_log}"; then + fail "Bazel failed unexpectedly." + fi + expect_log '^//:pass_on_foo1_or_foo2_but_not_on_foo3 * PASSED in' + + if bazel test --show_result=10 --host_platform=@//:foo3_platform \ + --platforms=@//:foo3_platform //:pass_on_foo1_or_foo2_but_not_on_foo3 \ + &> "${TEST_log}"; then + fail "Bazel passed unexpectedly." + fi + + expect_log 'ERROR: Target //:pass_on_foo1_or_foo2_but_not_on_foo3 is incompatible and cannot be built, but was explicitly requested' + expect_log 'FAILED: Build did NOT complete successfully' +} + +run_suite "target_compatible_with tests"