Add support for skipping validations in label and label_list attributes.

verification: cquery //:target --output=starlark --starlark:expr='providers(target)["OutputGroupInfo"]._validation' should be reduced or completely empty PiperOrigin-RevId: 608545504 Change-Id: I922e6ab23d3f999369e68db3a84a063c1517cc56
meteorcloudy · Feb 20, 2024 · 26eecb6 · 26eecb6
1 parent adaf299
commit 26eecb6
Show file tree

Hide file tree

Showing 6 changed files with 94 additions and 6 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -378,7 +378,8 @@ public static void collectTransitiveValidationOutputGroups(
       // not fail the overall build, since those dependencies should have their own builds
       // and tests that should surface any failing validations.
       Attribute attribute = ruleContext.attributes().getAttributeDefinition(attributeName);
-      if (!attribute.isToolDependency()
+      if (!attribute.skipValidations()
+          && !attribute.isToolDependency()
           && !attribute.isImplicit()
           && attribute.getType().getLabelClass() == LabelClass.DEPENDENCY) {
 

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java
@@ -206,6 +206,11 @@ private static Attribute.Builder<?> createAttribute(
       builder.setPropertyFlag("MANDATORY");
     }
 
+    if (containsNonNoneKey(arguments, SKIP_VALIDATIONS_ARG)
+        && (Boolean) arguments.get(SKIP_VALIDATIONS_ARG)) {
+      builder.setPropertyFlag("SKIP_VALIDATIONS");
+    }
+
     if (containsNonNoneKey(arguments, ALLOW_EMPTY_ARG)
         && !(Boolean) arguments.get(ALLOW_EMPTY_ARG)) {
       builder.setPropertyFlag("NON_EMPTY");
@@ -509,6 +514,7 @@ public Descriptor labelAttribute(
       Object allowFiles,
       Object allowSingleFile,
       Boolean mandatory,
+      Boolean skipValidations,
       Sequence<?> providers,
       Object allowRules,
       Object cfg,
@@ -542,6 +548,8 @@ public Descriptor labelAttribute(
                 allowSingleFile,
                 MANDATORY_ARG,
                 mandatory,
+                SKIP_VALIDATIONS_ARG,
+                skipValidations,
                 PROVIDERS_ARG,
                 providers,
                 ALLOW_RULES_ARG,
@@ -597,6 +605,7 @@ public Descriptor labelListAttribute(
       Sequence<?> providers,
       Sequence<?> flags,
       Boolean mandatory,
+      Boolean skipValidations,
       Object cfg,
       Sequence<?> aspects,
       StarlarkThread thread)
@@ -621,7 +630,9 @@ public Descriptor labelListAttribute(
             CONFIGURATION_ARG,
             cfg,
             ASPECTS_ARG,
-            aspects);
+            aspects,
+            SKIP_VALIDATIONS_ARG,
+            skipValidations);
     ImmutableAttributeFactory attribute =
         createAttributeFactory(
             BuildType.LABEL_LIST,

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -180,6 +180,9 @@ private enum PropertyFlag {
 
     /** Whether this attribute was defined using Starlark's {@code attrs} module. */
     STARLARK_DEFINED,
+
+    /** Whether to run the transitive validation actions from this attribute. */
+    SKIP_VALIDATIONS,
   }
 
   // TODO(bazel-team): modify this interface to extend Predicate and have an extra error
@@ -1946,6 +1949,10 @@ public boolean isSilentRuleClassFilter() {
     return getPropertyFlag(PropertyFlag.SILENT_RULECLASS_FILTER);
   }
 
+  public boolean skipValidations() {
+    return getPropertyFlag(PropertyFlag.SKIP_VALIDATIONS);
+  }
+
   /** Returns true if this label type parameter skips the analysis time filetype check. */
   public boolean isSkipAnalysisTimeFileTypeCheck() {
     return getPropertyFlag(PropertyFlag.SKIP_ANALYSIS_TIME_FILETYPE_CHECK);

diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkAttrModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkAttrModuleApi.java
@@ -89,6 +89,12 @@ public interface StarlarkAttrModuleApi extends StarlarkValue {
       "Aspects that should be applied to the dependency or dependencies specified by this "
           + "attribute.";
 
+  String SKIP_VALIDATIONS_ARG = "skip_validations";
+  String SKIP_VALIDATIONS_ARG_DOC =
+      "If true, validation actions of transitive dependencies from "
+          + "this attribute will not run. This is a temporary mitigation and WILL be removed in "
+          + "the future.";
+
   String CONFIGURATION_ARG = "cfg";
   // TODO(b/151742236): Update when new Starlark-based configuration framework is implemented.
   String CONFIGURATION_DOC =
@@ -309,6 +315,12 @@ Descriptor stringAttribute(
             named = true,
             positional = false,
             doc = MANDATORY_DOC),
+        @Param(
+            name = SKIP_VALIDATIONS_ARG,
+            defaultValue = "False",
+            named = true,
+            positional = false,
+            doc = SKIP_VALIDATIONS_ARG_DOC),
         @Param(
             name = PROVIDERS_ARG,
             defaultValue = "[]",
@@ -361,6 +373,7 @@ Descriptor labelAttribute(
       Object allowFiles,
       Object allowSingleFile,
       Boolean mandatory,
+      Boolean skipValidations,
       Sequence<?> providers,
       Object allowRules,
       Object cfg,
@@ -502,6 +515,12 @@ Descriptor intListAttribute(
             named = true,
             positional = false,
             doc = MANDATORY_DOC),
+        @Param(
+            name = SKIP_VALIDATIONS_ARG,
+            defaultValue = "False",
+            named = true,
+            positional = false,
+            doc = SKIP_VALIDATIONS_ARG_DOC),
         @Param(
             name = CONFIGURATION_ARG,
             defaultValue = "None",
@@ -526,6 +545,7 @@ Descriptor labelListAttribute(
       Sequence<?> providers,
       Sequence<?> flags,
       Boolean mandatory,
+      Boolean skipValidations,
       Object cfg,
       Sequence<?> aspects,
       StarlarkThread thread)

diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java
@@ -70,6 +70,7 @@ public Descriptor labelAttribute(
       Object allowFiles,
       Object allowSingleFile,
       Boolean mandatory,
+      Boolean skipValidations,
       Sequence<?> providers,
       Object allowRules,
       Object cfg,
@@ -119,6 +120,7 @@ public Descriptor labelListAttribute(
       Sequence<?> providers,
       Sequence<?> flags,
       Boolean mandatory,
+      Boolean skipValidations,
       Object cfg,
       Sequence<?> aspects,
       StarlarkThread thread)

diff --git a/src/test/shell/integration/validation_actions_test.sh b/src/test/shell/integration/validation_actions_test.sh
@@ -115,10 +115,10 @@ load(
     "rule_with_implicit_outs_and_validation",
     "rule_with_implicit_and_host_deps")
 
-rule_with_implicit_outs_and_validation(name = "foo0")
-rule_with_implicit_outs_and_validation(name = "foo1")
-rule_with_implicit_outs_and_validation(name = "foo2")
-rule_with_implicit_outs_and_validation(name = "foo3")
+rule_with_implicit_outs_and_validation(name = "foo0", visibility = ["//visibility:public"])
+rule_with_implicit_outs_and_validation(name = "foo1", visibility = ["//visibility:public"])
+rule_with_implicit_outs_and_validation(name = "foo2", visibility = ["//visibility:public"])
+rule_with_implicit_outs_and_validation(name = "foo3", visibility = ["//visibility:public"])
 
 filegroup(
   name = "foo2_filegroup",
@@ -229,6 +229,14 @@ function assert_exists() {
   return 1
 }
 
+function assert_not_exists() {
+  path="$1"
+  [ ! -f "$path" ] && return 0
+
+  fail "Expected file '$path' to not exist, but it did"
+  return 1
+}
+
 #### Tests #####################################################################
 
 function test_validation_actions() {
@@ -561,4 +569,43 @@ EOF
   expect_log "aspect validation failed!"
 }
 
+function test_skip_validation_action_with_attribute_flag() {
+  setup_test_project
+  setup_passing_validation_action
+  mkdir -p skip_validations
+  cat > skip_validations/defs.bzl <<'EOF'
+def _rule_with_attrs_that_skips_validation_impl(ctx):
+    return []
+
+rule_with_attrs_that_skips_validation = rule(
+    implementation = _rule_with_attrs_that_skips_validation_impl,
+    attrs = {
+        "run_validations_label": attr.label(),
+        "run_validations_label_list": attr.label_list(),
+        "skip_validations_label": attr.label(skip_validations = True),
+        "skip_validations_label_list": attr.label_list(skip_validations = True),
+    },
+)
+EOF
+  cat > skip_validations/BUILD <<'EOF'
+load( ":defs.bzl", "rule_with_attrs_that_skips_validation" )
+
+rule_with_attrs_that_skips_validation(
+    name = "target_with_attr_that_skips_validation",
+    run_validations_label = "//validation_actions:foo0",
+    run_validations_label_list = ["//validation_actions:foo1"],
+    skip_validations_label = "//validation_actions:foo2",
+    skip_validations_label_list =  ["//validation_actions:foo3"],
+)
+EOF
+
+  bazel build //skip_validations:target_with_attr_that_skips_validation \
+    >& $TEST_log || fail "Expected build to succeed"
+
+  assert_exists bazel-bin/validation_actions/foo0.validation
+  assert_exists bazel-bin/validation_actions/foo1.validation
+  assert_not_exists bazel-bin/validation_actions/foo2.validation
+  assert_not_exists bazel-bin/validation_actions/foo3.validation
+}
+
 run_suite "Validation actions integration tests"