Merge rule and aspect validation output groups

src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java

@@ -189,7 +189,8 @@ public NestedSet<Artifact> getOutputGroup(String outputGroupName) {
   }
 
   /**
-   * Merges output groups from two output providers. The set of output groups must be disjoint.
+   * Merges output groups from two output providers. The set of output groups must be disjoint,
+   * except for the special validation output group, which is always merged.
    *
    * @param providers providers to merge {@code this} with.
    */
@@ -207,6 +208,9 @@ public static OutputGroupInfo merge(List<OutputGroupInfo> providers)
     Set<String> seenGroups = new HashSet<>();
     for (OutputGroupInfo provider : providers) {
       for (String outputGroup : provider.outputGroups.keySet()) {
+        if (outputGroup.equals(VALIDATION)) {
+          continue;
+        }
         if (!seenGroups.add(outputGroup)) {
           throw new DuplicateException(
               "Output group " + outputGroup + " provided twice");
@@ -215,6 +219,17 @@ public static OutputGroupInfo merge(List<OutputGroupInfo> providers)
         resultBuilder.put(outputGroup, provider.getOutputGroup(outputGroup));
       }
     }
+    // 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 new OutputGroupInfo(resultBuilder.buildOrThrow());
   }
 

src/test/shell/integration/validation_actions_test.sh

@@ -508,4 +508,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"