Merge rule and aspect validation output groups

By merging the special `_validation` output groups across a rule and its attached aspects, aspects and rules can simultaneously use validation actions.

Fixes #19624

Closes #19630.

PiperOrigin-RevId: 571084964
Change-Id: I3135ce1f9e61124e96a3b7d8e0ea0c3a3cdf526d

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<OutputGroupInfo> providers) throws DuplicateException {
@@ -189,11 +189,25 @@ public static OutputGroupInfo merge(List<OutputGroupInfo> providers) throws Dupl
  Map<String, NestedSet<Artifact>> 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<Artifact> 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));
  }
 

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"