diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java index 626a9eb0bd97df..aa09d736d6b142 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java @@ -175,7 +175,7 @@ public static OutputGroupInfo get(ProviderCollection collection) { /** * Merges output groups from a list of output providers. The set of output groups must be - * disjoint. + * disjoint, except for the special validation output group, which is always merged. */ @Nullable public static OutputGroupInfo merge(List providers) throws DuplicateException { @@ -189,11 +189,25 @@ public static OutputGroupInfo merge(List providers) throws Dupl Map> outputGroups = new TreeMap<>(); for (OutputGroupInfo provider : providers) { for (String group : provider) { + if (group.equals(VALIDATION)) { + continue; + } if (outputGroups.put(group, provider.getOutputGroup(group)) != null) { throw new DuplicateException("Output group " + group + " provided twice"); } } } + // Allow both an aspect and the rule to use validation actions. + NestedSetBuilder validationOutputs = NestedSetBuilder.stableOrder(); + for (OutputGroupInfo provider : providers) { + try { + validationOutputs.addTransitive(provider.getOutputGroup(VALIDATION)); + } catch (IllegalArgumentException e) { + // Thrown if nested set orders aren't compatible. + throw new DuplicateException( + "Output group " + VALIDATION + " provided twice with incompatible depset orders"); + } + } return createInternal(ImmutableMap.copyOf(outputGroups)); } diff --git a/src/test/shell/integration/validation_actions_test.sh b/src/test/shell/integration/validation_actions_test.sh index 8050aa623ddbf3..88d193c034c7d5 100755 --- a/src/test/shell/integration/validation_actions_test.sh +++ b/src/test/shell/integration/validation_actions_test.sh @@ -511,4 +511,54 @@ function test_validation_actions_flags() { expect_log "Target //validation_actions:foo0 up-to-date:" } +function test_validation_actions_in_rule_and_aspect() { + setup_test_project + + mkdir -p aspect + cat > aspect/BUILD <<'EOF' +exports_files(["aspect_validation_tool"]) +EOF + cat > aspect/def.bzl <<'EOF' +def _validation_aspect_impl(target, ctx): + validation_output = ctx.actions.declare_file(ctx.rule.attr.name + ".aspect_validation") + ctx.actions.run( + outputs = [validation_output], + executable = ctx.executable._validation_tool, + arguments = [validation_output.path]) + return [ + OutputGroupInfo(_validation = depset([validation_output])), + ] + +validation_aspect = aspect( + implementation = _validation_aspect_impl, + attrs = { + "_validation_tool": attr.label( + allow_single_file = True, + default = Label(":aspect_validation_tool"), + executable = True, + cfg = "exec"), + }, +) +EOF + cat > aspect/aspect_validation_tool <<'EOF' +#!/bin/bash +echo "aspect validation output" > $1 +EOF + chmod +x aspect/aspect_validation_tool + setup_passing_validation_action + + bazel build --run_validations --aspects=//aspect:def.bzl%validation_aspect \ + //validation_actions:foo0 >& "$TEST_log" || fail "Expected build to succeed" + + cat > aspect/aspect_validation_tool <<'EOF' +#!/bin/bash +echo "aspect validation failed!" +exit 1 +EOF + + bazel build --run_validations --aspects=//aspect:def.bzl%validation_aspect \ + //validation_actions:foo0 >& "$TEST_log" && fail "Expected build to fail" + expect_log "aspect validation failed!" +} + run_suite "Validation actions integration tests"