Skip to content

Commit

Permalink
Fix builds for filegroup targets with incompatible dependencies
Browse files Browse the repository at this point in the history
When building a `filegroup` with incompatible dependencies, the logic
was such that the incompatible dependencies would actually be built by
bazel. These dependencies only expose `FailAction`s so the build
failed.

This is caused by bazel skipping the incompatibility check for any
rules that don't participate in toolchain resolution.

For example, without the fix in `RuleContextConstraintSemantics.java`,
the added unit test fails with:

    ERROR: /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10: Reporting failed target //target_skipping:binary located at /home/phil/.cache/bazel/_bazel_phil/b4868b516be4e3bea4d221937a55ced4/sandbox/linux-sandbox/46/execroot/io_bazel/_tmp/c749fd38ac9a4ad6bd41e9653bbceab5/workspace/target_skipping/BUILD:103:10 failed: Can't build this. This target is incompatible. Please file a bug upstream. caused by //target_skipping:binary
    Target //target_skipping:filegroup failed to build

This patch makes it so targets that don't use toolchain resolution
skip the check for the `target_compatible_with` attribute, but still
check for incompatible dependencies.

Closes #12601.

PiperOrigin-RevId: 345263391
  • Loading branch information
philsc authored and copybara-github committed Dec 2, 2020
1 parent 65e9732 commit 84fadcf
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -854,12 +854,9 @@ public static ConfiguredTarget incompatibleConfiguredTarget(
RuleContext ruleContext,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> prerequisiteMap)
throws ActionConflictException, InterruptedException {
if (!ruleContext.getRule().getRuleClassObject().useToolchainResolution()) {
return null;
}

// This is incompatible if explicitly specified to be.
if (ruleContext.attributes().has("target_compatible_with")) {
// The target (ruleContext) is incompatible if explicitly specified to be.
if (ruleContext.getRule().getRuleClassObject().useToolchainResolution()
&& ruleContext.attributes().has("target_compatible_with")) {
ImmutableList<ConstraintValueInfo> invalidConstraintValues =
stream(
PlatformProviderUtils.constraintValues(
Expand Down
45 changes: 45 additions & 0 deletions src/test/shell/integration/target_compatible_with_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,51 @@ function test_console_log_for_tests() {
expect_log '^//target_skipping:pass_on_foo1_bar2 * PASSED in'
}
# Validates that filegroups (i.e. a rule that doesn't use toolchain resolution)
# is correctly skipped when it depends on an incompatible target. This is a
# regression test for https://github.com/bazelbuild/bazel/issues/12582.
function test_filegroup() {
cat > target_skipping/binary.cc <<EOF
#include <cstdio>
int main() {
return 0;
}
EOF
cat >> target_skipping/BUILD <<EOF
cc_binary(
name = "binary",
srcs = ["binary.cc"],
target_compatible_with = [
":foo3",
],
)
filegroup(
name = "filegroup",
srcs = [
":binary",
],
)
EOF
cd target_skipping || fail "couldn't cd into workspace"
bazel build \
--show_result=10 \
--host_platform=@//target_skipping:foo1_bar1_platform \
--platforms=@//target_skipping:foo1_bar1_platform \
:all &> "${TEST_log}" || fail "Bazel failed unexpectedly."
expect_log 'Target //target_skipping:filegroup was skipped'
bazel build \
--show_result=10 \
--host_platform=@//target_skipping:foo1_bar1_platform \
--platforms=@//target_skipping:foo1_bar1_platform \
:filegroup &> "${TEST_log}" && fail "Bazel passed unexpectedly."
expect_log 'Target //target_skipping:filegroup is incompatible and cannot be built'
}
# Validates that incompatible target skipping errors behave nicely with
# --keep_going. In other words, even if there's an error in the target skipping
# (e.g. because the user explicitly requested an incompatible target) we still
Expand Down

0 comments on commit 84fadcf

Please sign in to comment.