Skip to content

Commit 5e9fa39

Browse files
authored
Add attribute validation to IncompatibleTargetChecker. (bazelbuild#18135)
This causes errors with configurable `target_compatible_with` to be reported as errors, rather than causing a Bazel crash. Fixes bazelbuild#18021. (This is functionally the same change as ebfb529, but applied to the release-6.2.0 branch)
1 parent 1a60fad commit 5e9fa39

File tree

5 files changed

+55
-10
lines changed

5 files changed

+55
-10
lines changed

src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public static Optional<RuleConfiguredTargetValue> createDirectlyIncompatibleTarg
107107
Environment env,
108108
@Nullable PlatformInfo platformInfo,
109109
NestedSetBuilder<Package> transitivePackages)
110-
throws InterruptedException {
110+
throws ConfiguredAttributeMapper.ValidationException, InterruptedException {
111111
Target target = targetAndConfiguration.getTarget();
112112
Rule rule = target.getAssociatedRule();
113113

@@ -124,7 +124,7 @@ public static Optional<RuleConfiguredTargetValue> createDirectlyIncompatibleTarg
124124
}
125125

126126
// Resolve the constraint labels.
127-
List<Label> labels = attrs.get("target_compatible_with", BuildType.LABEL_LIST);
127+
List<Label> labels = attrs.getAndValidate("target_compatible_with", BuildType.LABEL_LIST);
128128
ImmutableList<ConfiguredTargetKey> constraintKeys =
129129
labels.stream()
130130
.map(

src/main/java/com/google/devtools/build/lib/packages/ConfiguredAttributeMapper.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ private ValidationException(String message) {
127127
* Variation of {@link #get} that throws an informative exception if the attribute can't be
128128
* resolved due to intrinsic contradictions in the configuration.
129129
*/
130-
private <T> T getAndValidate(String attributeName, Type<T> type) throws ValidationException {
130+
public <T> T getAndValidate(String attributeName, Type<T> type) throws ValidationException {
131131
SelectorList<T> selectorList = getSelectorList(attributeName, type);
132132
if (selectorList == null) {
133133
// This is a normal attribute.

src/main/java/com/google/devtools/build/lib/skyframe/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ java_library(
307307
"//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception",
308308
"//src/main/java/com/google/devtools/build/lib/io:process_package_directory_exception",
309309
"//src/main/java/com/google/devtools/build/lib/packages",
310+
"//src/main/java/com/google/devtools/build/lib/packages:configured_attribute_mapper",
310311
"//src/main/java/com/google/devtools/build/lib/packages:exec_group",
311312
"//src/main/java/com/google/devtools/build/lib/packages:globber",
312313
"//src/main/java/com/google/devtools/build/lib/packages:globber_utils",

src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java

+24-7
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
import com.google.devtools.build.lib.events.StoredEventHandler;
7272
import com.google.devtools.build.lib.packages.Aspect;
7373
import com.google.devtools.build.lib.packages.BuildType;
74+
import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
7475
import com.google.devtools.build.lib.packages.ExecGroup;
7576
import com.google.devtools.build.lib.packages.NoSuchTargetException;
7677
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
@@ -108,6 +109,7 @@
108109
import java.util.function.Predicate;
109110
import java.util.stream.Collectors;
110111
import javax.annotation.Nullable;
112+
import net.starlark.java.syntax.Location;
111113

112114
/**
113115
* SkyFunction for {@link ConfiguredTargetValue}s.
@@ -335,13 +337,28 @@ public SkyValue compute(SkyKey key, Environment env)
335337
getPrioritizedDetailedExitCode(causes)));
336338
}
337339

338-
Optional<RuleConfiguredTargetValue> incompatibleTarget =
339-
IncompatibleTargetChecker.createDirectlyIncompatibleTarget(
340-
targetAndConfiguration,
341-
configConditions,
342-
env,
343-
platformInfo,
344-
state.transitivePackages);
340+
Optional<RuleConfiguredTargetValue> incompatibleTarget;
341+
try {
342+
incompatibleTarget =
343+
IncompatibleTargetChecker.createDirectlyIncompatibleTarget(
344+
targetAndConfiguration,
345+
configConditions,
346+
env,
347+
platformInfo,
348+
state.transitivePackages);
349+
} catch (ConfiguredAttributeMapper.ValidationException e) {
350+
BuildConfigurationValue configuration = targetAndConfiguration.getConfiguration();
351+
Label label = targetAndConfiguration.getLabel();
352+
Location location = targetAndConfiguration.getTarget().getLocation();
353+
env.getListener().post(new AnalysisRootCauseEvent(configuration, label, e.getMessage()));
354+
throw new DependencyEvaluationException(
355+
new ConfiguredValueCreationException(
356+
location, e.getMessage(), label, configuration.getEventId(), null, null),
357+
// These errors occur within DependencyResolver, which is attached to the current
358+
// target. i.e. no dependent ConfiguredTargetFunction call happens to report its own
359+
// error.
360+
/* depReportedOwnError= */ false);
361+
}
345362
if (incompatibleTarget == null) {
346363
return null;
347364
}

src/test/shell/integration/target_compatible_with_test.sh

+27
Original file line numberDiff line numberDiff line change
@@ -1551,4 +1551,31 @@ EOF
15511551
expect_not_log "${debug_message5}"
15521552
}
15531553

1554+
# Regression test for b/277371822.
1555+
function test_missing_default() {
1556+
cat >> target_skipping/BUILD <<'EOF'
1557+
sh_test(
1558+
name = "pass_on_foo1_or_foo2_but_not_on_foo3",
1559+
srcs = [":pass.sh"],
1560+
target_compatible_with = select({
1561+
":foo1": [],
1562+
# No default branch.
1563+
}),
1564+
)
1565+
EOF
1566+
1567+
cd target_skipping || fail "couldn't cd into workspace"
1568+
1569+
bazel test \
1570+
--show_result=10 \
1571+
--host_platform=@//target_skipping:foo3_platform \
1572+
--platforms=@//target_skipping:foo3_platform \
1573+
//target_skipping:pass_on_foo1_or_foo2_but_not_on_foo3 &> "${TEST_log}" \
1574+
&& fail "Bazel passed unexpectedly."
1575+
1576+
expect_log 'ERROR:.*configurable attribute "target_compatible_with" in //target_skipping:pass_on_foo1_or_foo2_but_not_on_foo3'
1577+
expect_log 'FAILED: Build did NOT complete successfully'
1578+
expect_not_log 'FATAL: bazel crashed'
1579+
}
1580+
15541581
run_suite "target_compatible_with tests"

0 commit comments

Comments
 (0)