Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --skip_incompatible_explicit_targets option #17404

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions site/en/extending/platforms.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ ERROR: Target //:target_incompatible_with_myplatform is incompatible and cannot
FAILED: Build did NOT complete successfully
```

Incompatible explicit targets are silently skipped if
`--skip_incompatible_explicit_targets` is enabled.

### More expressive constraints {:#expressive-constraints}

For more flexibility in expressing constraints, use the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@ public class AnalysisOptions extends OptionsBase {
)
public int maxConfigChangesToShow;

@Option(
name = "skip_incompatible_explicit_targets",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
help =
"Skip incompatible targets that are explicitly listed on the command line. "
+ "By default, building such targets results in an error but they are "
+ "silently skipped when this option is enabled. See: "
+ "https://bazel.build/extending/platforms#skipping-incompatible-targets"
)
public boolean skipIncompatibleExplicitTargets;

@Option(
name = "experimental_extra_action_filter",
defaultValue = "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ public AnalysisResult update(
ImmutableMap<String, String> aspectsParameters,
AnalysisOptions viewOptions,
boolean keepGoing,
boolean skipIncompatibleExplicitTargets,
boolean checkForActionConflicts,
QuiescingExecutors executors,
TopLevelArtifactContext topLevelOptions,
Expand Down Expand Up @@ -426,6 +427,7 @@ public AnalysisResult update(
getCoverageArtifactsHelper(
configuredTargets, allTargetsToTest, eventHandler, eventBus, loadingResult),
keepGoing,
skipIncompatibleExplicitTargets,
targetOptions.get(CoreOptions.class).strictConflictChecks,
checkForActionConflicts,
executors,
Expand Down Expand Up @@ -492,7 +494,11 @@ public AnalysisResult update(

PlatformRestrictionsResult platformRestrictions =
topLevelConstraintSemantics.checkPlatformRestrictions(
skyframeAnalysisResult.getConfiguredTargets(), explicitTargetPatterns, keepGoing);
skyframeAnalysisResult.getConfiguredTargets(),
explicitTargetPatterns,
keepGoing,
skipIncompatibleExplicitTargets
);

if (!platformRestrictions.targetsWithErrors().isEmpty()) {
// If there are any errored targets (e.g. incompatible targets that are explicitly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ public static PlatformCompatibility compatibilityWithPlatformRestrictions(
ConfiguredTarget configuredTarget,
ExtendedEventHandler eventHandler,
boolean eagerlyThrowError,
boolean explicitlyRequested)
boolean explicitlyRequested,
boolean skipIncompatibleExplicitTargets)
throws TargetCompatibilityCheckException {

RuleContextConstraintSemantics.IncompatibleCheckResult incompatibleCheckResult =
Expand All @@ -124,7 +125,7 @@ public static PlatformCompatibility compatibilityWithPlatformRestrictions(
// 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.
if (explicitlyRequested) {
if (!skipIncompatibleExplicitTargets && explicitlyRequested) {
if (eagerlyThrowError) {
// Use the slightly simpler form for printing error messages. I.e. no "@" prefix for
// targets in the main repository.
Expand All @@ -136,7 +137,8 @@ public static PlatformCompatibility compatibilityWithPlatformRestrictions(
String.format(TARGET_INCOMPATIBLE_ERROR_TEMPLATE, configuredTarget.getLabel(), "")));
return PlatformCompatibility.INCOMPATIBLE_EXPLICIT;
}
// If this is not an explicitly requested target we can safely skip it.
// We can safely skip this target if it wasn't explicitly requested or we've been instructed
// to skip explicitly requested targets.
return PlatformCompatibility.INCOMPATIBLE_IMPLICIT;
}

Expand Down Expand Up @@ -240,8 +242,8 @@ public static EnvironmentCompatibility compatibilityWithTargetEnvironment(
* 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.
* command line are errored unless --skip_incompatible_explicit_targets is enabled. 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
Expand All @@ -254,7 +256,8 @@ public static EnvironmentCompatibility compatibilityWithTargetEnvironment(
public PlatformRestrictionsResult checkPlatformRestrictions(
ImmutableSet<ConfiguredTarget> topLevelTargets,
ImmutableSet<Label> explicitTargetPatterns,
boolean keepGoing)
boolean keepGoing,
boolean skipIncompatibleExplicitTargets)
throws ViewCreationFailedException {
ImmutableSet.Builder<ConfiguredTarget> incompatibleTargets = ImmutableSet.builder();
ImmutableSet.Builder<ConfiguredTarget> incompatibleButRequestedTargets = ImmutableSet.builder();
Expand All @@ -266,7 +269,8 @@ public PlatformRestrictionsResult checkPlatformRestrictions(
target,
eventHandler,
/*eagerlyThrowError=*/ !keepGoing,
explicitTargetPatterns.contains(target.getLabel()));
explicitTargetPatterns.contains(target.getLabel()),
skipIncompatibleExplicitTargets);
if (PlatformCompatibility.INCOMPATIBLE_EXPLICIT.equals(platformCompatibility)) {
incompatibleButRequestedTargets.add(target);
} else if (PlatformCompatibility.INCOMPATIBLE_IMPLICIT.equals(platformCompatibility)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ private static AnalysisAndExecutionResult runAnalysisAndExecutionPhase(
request.getAspectsParameters(),
request.getViewOptions(),
request.getKeepGoing(),
request.getViewOptions().skipIncompatibleExplicitTargets,
request.getCheckForActionConflicts(),
env.getQuiescingExecutors(),
request.getTopLevelArtifactContext(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ private static AnalysisResult runAnalysisPhase(
request.getAspectsParameters(),
request.getViewOptions(),
request.getKeepGoing(),
request.getViewOptions().skipIncompatibleExplicitTargets,
request.getCheckForActionConflicts(),
env.getQuiescingExecutors(),
request.getTopLevelArtifactContext(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
state,
configuredTarget,
buildConfigurationValue,
buildDriverKey.isExplicitlyRequested());
buildDriverKey.isExplicitlyRequested(),
buildDriverKey.shouldSkipIncompatibleExplicitTargets());
if (isConfiguredTargetCompatible == null) {
return null;
}
Expand Down Expand Up @@ -273,7 +274,8 @@ private Boolean isConfiguredTargetCompatible(
State state,
ConfiguredTarget configuredTarget,
BuildConfigurationValue buildConfigurationValue,
boolean isExplicitlyRequested)
boolean isExplicitlyRequested,
boolean skipIncompatibleExplicitTargets)
throws InterruptedException, TargetCompatibilityCheckException {

if (!state.checkedForPlatformCompatibility) {
Expand All @@ -282,7 +284,8 @@ private Boolean isConfiguredTargetCompatible(
configuredTarget,
env.getListener(),
/*eagerlyThrowError=*/ true,
isExplicitlyRequested);
isExplicitlyRequested,
skipIncompatibleExplicitTargets);
state.checkedForPlatformCompatibility = true;
switch (platformCompatibility) {
case INCOMPATIBLE_EXPLICIT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,22 @@ public final class BuildDriverKey implements CPUHeavySkyKey {
private final TestType testType;
private final boolean strictActionConflictCheck;
private final boolean explicitlyRequested;
private final boolean skipIncompatibleExplicitTargets;
private final boolean isTopLevelAspectDriver;

private BuildDriverKey(
ActionLookupKey actionLookupKey,
TopLevelArtifactContext topLevelArtifactContext,
boolean strictActionConflictCheck,
boolean explicitlyRequested,
boolean skipIncompatibleExplicitTargets,
boolean isTopLevelAspectDriver,
TestType testType) {
this.actionLookupKey = actionLookupKey;
this.topLevelArtifactContext = topLevelArtifactContext;
this.strictActionConflictCheck = strictActionConflictCheck;
this.explicitlyRequested = explicitlyRequested;
this.skipIncompatibleExplicitTargets = skipIncompatibleExplicitTargets;
this.isTopLevelAspectDriver = isTopLevelAspectDriver;
this.testType = testType;
}
Expand All @@ -50,12 +53,14 @@ public static BuildDriverKey ofTopLevelAspect(
ActionLookupKey actionLookupKey,
TopLevelArtifactContext topLevelArtifactContext,
boolean strictActionConflictCheck,
boolean explicitlyRequested) {
boolean explicitlyRequested,
boolean skipIncompatibleExplicitTargets) {
return new BuildDriverKey(
actionLookupKey,
topLevelArtifactContext,
strictActionConflictCheck,
explicitlyRequested,
skipIncompatibleExplicitTargets,
/*isTopLevelAspectDriver=*/ true,
TestType.NOT_TEST);
}
Expand All @@ -65,12 +70,14 @@ public static BuildDriverKey ofConfiguredTarget(
TopLevelArtifactContext topLevelArtifactContext,
boolean strictActionConflictCheck,
boolean explicitlyRequested,
boolean skipIncompatibleExplicitTargets,
TestType testType) {
return new BuildDriverKey(
actionLookupKey,
topLevelArtifactContext,
strictActionConflictCheck,
explicitlyRequested,
skipIncompatibleExplicitTargets,
/*isTopLevelAspectDriver=*/ false,
testType);
}
Expand Down Expand Up @@ -99,6 +106,10 @@ public boolean isExplicitlyRequested() {
return explicitlyRequested;
}

public boolean shouldSkipIncompatibleExplicitTargets() {
return skipIncompatibleExplicitTargets;
}

public boolean isTopLevelAspectDriver() {
return isTopLevelAspectDriver;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
BuildResultListener buildResultListener,
CoverageReportActionsWrapperSupplier coverageReportActionsWrapperSupplier,
boolean keepGoing,
boolean skipIncompatibleExplicitTargets,
boolean strictConflictCheck,
boolean checkForActionConflicts,
QuiescingExecutors executors,
Expand Down Expand Up @@ -573,6 +574,7 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
strictConflictCheck,
/* explicitlyRequested= */ explicitTargetPatterns.contains(
ctKey.getLabel()),
skipIncompatibleExplicitTargets,
determineTestType(
testsToRun,
labelTargetMap,
Expand All @@ -589,7 +591,8 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets(
k,
topLevelArtifactContext,
strictConflictCheck,
/*explicitlyRequested=*/ explicitTargetPatterns.contains(k.getLabel())))
/*explicitlyRequested=*/ explicitTargetPatterns.contains(k.getLabel()),
skipIncompatibleExplicitTargets))
.collect(ImmutableSet.toImmutableSet());
List<DetailedExitCode> detailedExitCodes = new ArrayList<>();
MultiThreadPoolsQuiescingExecutor executor =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ public AnalysisResult update(
aspectsParameters,
viewOptions,
keepGoing,
/* skipIncompatibleExplicitTargets= */ false,
/* checkForActionConflicts= */ true,
QuiescingExecutorsImpl.forTesting(),
topLevelOptions,
Expand Down
28 changes: 28 additions & 0 deletions src/test/shell/integration/target_compatible_with_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,34 @@ EOF
expect_log 'Target //target_skipping:always_incompatible build was skipped'
}

# Validates that incompatible target skipping works with top level targets when
# --skip_incompatible_explicit_targets is enabled.
function test_success_on_incompatible_top_level_target_with_skipping() {
cd target_skipping || fail "couldn't cd into workspace"

# Validate a variety of ways to refer to the same target.
local -r -a incompatible_targets=(
:pass_on_foo1_bar2
//target_skipping:pass_on_foo1_bar2
@//target_skipping:pass_on_foo1_bar2
)

for incompatible_target in "${incompatible_targets[@]}"; do
echo "Testing ${incompatible_target}"

bazel test \
--show_result=10 \
--host_platform=@//target_skipping:foo1_bar1_platform \
--platforms=@//target_skipping:foo1_bar1_platform \
--build_event_text_file="${TEST_log}".build.json \
--skip_incompatible_explicit_targets \
"${incompatible_target}" &> "${TEST_log}" \
|| fail "Bazel failed unexpectedly."

expect_log '^//target_skipping:pass_on_foo1_bar2 * SKIPPED$'
done
}

# Crudely validates that the build event protocol contains useful information
# when targets are skipped due to incompatibilities.
function test_build_event_protocol() {
Expand Down