Implement exclusions for custom soft errors

Summary:
Context: As part of "safer compatible with", we are rolling out enforcements of `dep_only_incompatible` errors to targets in the repo to prevent them from losing CI coverage due to a bad compatible_with definition change. Unfortunately, sometimes these rollout diffs break things and need to be reverted. I recently implemented custom soft errors as a way to more effectively source these issues before we roll things out. Each soft error has a set of target patterns attached and if any of those patterns violates `dep_only_incompatible`, then we fire the corresponding soft error that we can read in Scuba.

This diff adds exclusions to custom soft errors to exclude a subset of target patterns specified from having soft errors. The exclusions help us get more targeted soft errors and match what we do for rollouts of `dep_only_incompatible`  in having both inclusion and exclusion patterns.

To support exclusions, I added a new provider `DepOnlyIncompatibleRollout` that supports a `target_patterns` field and an `exclusions` field. To make this backwards compatible with existing definitions, `DepOnlyIncompatibleInfo` now accepts either a list of strings or a `DepOnlyIncompatibleRollout` type in the `custom_soft_errors` field. Once version bump is out and the provider is migrated over, I will delete the existing support for list of target patterns.

Reviewed By: Nero5023

Differential Revision: D71592839

fbshipit-source-id: 9c8e26cf4d92128f12b60f3a2253af03830b0ab2

Author: Scott Cao

app/buck2_build_api/src/interpreter/rule_defs/provider/builtin.rs

@@ -14,6 +14,7 @@ pub mod constraint_setting_info;
 pub mod constraint_value_info;
 pub mod default_info;
 pub mod dep_only_incompatible_info;
+pub mod dep_only_incompatible_rollout;
 pub mod execution_platform_info;
 pub mod execution_platform_registration_info;
 pub mod external_runner_test_info;

app/buck2_build_api/src/interpreter/rule_defs/provider/builtin/dep_only_incompatible_info.rs

@@ -15,25 +15,34 @@ use buck2_core::cells::CellResolver;
 use buck2_core::error::validate_logview_category;
 use buck2_core::pattern::pattern::ParsedPattern;
 use buck2_core::pattern::pattern_type::TargetPatternExtra;
+use buck2_core::target::label::label::TargetLabel;
 use buck2_util::arc_str::ArcStr;
 use dupe::Dupe;
+use either::Either;
 use starlark::any::ProvidesStaticType;
 use starlark::collections::SmallMap;
 use starlark::environment::GlobalsBuilder;
+use starlark::eval::Evaluator;
 use starlark::values::dict::DictRef;
 use starlark::values::dict::DictType;
+use starlark::values::dict::UnpackDictEntries;
 use starlark::values::list::ListRef;
 use starlark::values::list::ListType;
 use starlark::values::Coerce;
 use starlark::values::Freeze;
 use starlark::values::FreezeResult;
 use starlark::values::Trace;
+use starlark::values::UnpackValue;
+use starlark::values::Value;
 use starlark::values::ValueLifetimeless;
 use starlark::values::ValueLike;
 use starlark::values::ValueOf;
+use starlark::values::ValueOfUnchecked;
 use starlark::values::ValueOfUncheckedGeneric;
 
 use crate as buck2_build_api;
+use crate::interpreter::rule_defs::provider::builtin::dep_only_incompatible_rollout::DepOnlyIncompatibleRollout;
+use crate::interpreter::rule_defs::provider::builtin::dep_only_incompatible_rollout::FrozenDepOnlyIncompatibleRollout;
 
 /// A provider for defining custom soft error categories for dep-only incompatible targets.
 /// This can be used to get finer-grained data on whether it is safe to enable
@@ -45,8 +54,14 @@ use crate as buck2_build_api;
 ///    return [
 ///       DefaultInfo(),
 ///       DepOnlyIncompatibleInfo(custom_soft_errors = {
-///          "dep_only_incompatible_foo": ["root//foo/..."],
-///          "dep_only_incompatible_bar": ["root//bar/..."],
+///          "dep_only_incompatible_foo": DepOnlyIncompatibleRollout(
+///             target_patterns = ["root//foo/..."],
+///             exclusions = ["root//foo/excluded/..."],
+///          ),
+///          "dep_only_incompatible_bar": DepOnlyIncompatibleRollout(
+///             target_patterns = ["root//bar/..."],
+///             exclusions = [],
+///          ),
 ///       })
 ///    ]
 /// ```
@@ -59,38 +74,65 @@ use crate as buck2_build_api;
 #[derive(Clone, Debug, Trace, Coerce, Freeze, ProvidesStaticType, Allocative)]
 #[repr(C)]
 pub struct DepOnlyIncompatibleInfoGen<V: ValueLifetimeless> {
-    pub custom_soft_errors: ValueOfUncheckedGeneric<V, DictType<String, ListType<String>>>,
+    pub custom_soft_errors: ValueOfUncheckedGeneric<
+        V,
+        DictType<
+            String,
+            // We are in process of migrating from list[str] to DepOnlyIncompatibleRollout provider,
+            // so for now accept both. Once migration is done, we can remove ListType<String> variant.
+            Either<ListType<String>, FrozenDepOnlyIncompatibleRollout>,
+        >,
+    >,
 }
 
 #[starlark_module]
 fn dep_only_incompatible_info_creator(globals: &mut GlobalsBuilder) {
     #[starlark(as_type = FrozenDepOnlyIncompatibleInfo)]
     fn DepOnlyIncompatibleInfo<'v>(
-        #[starlark(require = named)] custom_soft_errors: ValueOf<
-            'v,
-            DictType<&'v str, ListType<&'v str>>,
+        #[starlark(require = named)] custom_soft_errors: UnpackDictEntries<
+            ValueOf<'v, &'v str>,
+            ValueOf<'v, Either<ListType<&'v str>, &'v DepOnlyIncompatibleRollout<'v>>>,
         >,
+        eval: &mut Evaluator<'v, '_, '_>,
     ) -> starlark::Result<DepOnlyIncompatibleInfo<'v>> {
-        let custom_soft_errors_dict = DictRef::from_value(custom_soft_errors.to_value())
-            .ok_or_else(|| {
-                starlark::Error::from(anyhow::anyhow!("Expected dict via type checking"))
-                    .into_internal_error()
-            })?;
-        for category in custom_soft_errors_dict.keys() {
-            let category = category.to_value().unpack_str().ok_or_else(|| {
+        let mut result = SmallMap::with_capacity(custom_soft_errors.entries.len());
+        for (category, value) in custom_soft_errors.entries {
+            let category_str = category.to_value().unpack_str().ok_or_else(|| {
                 starlark::Error::from(anyhow::anyhow!("Expected string via type checking"))
                     .into_internal_error()
             })?;
-            validate_logview_category(category)?;
+            validate_logview_category(category_str)?;
+            result.insert(category_str, value.value);
         }
         Ok(DepOnlyIncompatibleInfo {
-            custom_soft_errors: custom_soft_errors.as_unchecked().cast(),
+            custom_soft_errors: ValueOfUnchecked::new(eval.heap().alloc(result)),
         })
     }
 }
 
-pub type DepOnlyIncompatibleCustomSoftErrors =
-    SmallMap<ArcStr, Box<[ParsedPattern<TargetPatternExtra>]>>;
+#[derive(Debug, Clone, PartialEq, Eq, Allocative)]
+pub struct DepOnlyIncompatibleRolloutPatterns {
+    target_patterns: Box<[ParsedPattern<TargetPatternExtra>]>,
+    exclusions: Box<[ParsedPattern<TargetPatternExtra>]>,
+}
+
+impl DepOnlyIncompatibleRolloutPatterns {
+    pub fn matches(&self, target: &TargetLabel) -> bool {
+        for exclusion in &self.exclusions {
+            if exclusion.matches(target) {
+                return false;
+            }
+        }
+        for target_pattern in &self.target_patterns {
+            if target_pattern.matches(target) {
+                return true;
+            }
+        }
+        false
+    }
+}
+
+pub type DepOnlyIncompatibleCustomSoftErrors = SmallMap<ArcStr, DepOnlyIncompatibleRolloutPatterns>;
 
 impl FrozenDepOnlyIncompatibleInfo {
     pub fn custom_soft_errors(
@@ -99,27 +141,60 @@ impl FrozenDepOnlyIncompatibleInfo {
         cell_resolver: &CellResolver,
         cell_alias_resolver: &CellAliasResolver,
     ) -> buck2_error::Result<DepOnlyIncompatibleCustomSoftErrors> {
-        let custom_soft_errors: SmallMap<_, Box<[_]>> =
+        let value_to_target_pattern =
+            |target_pattern: Value<'_>| -> buck2_error::Result<ParsedPattern<TargetPatternExtra>> {
+                ParsedPattern::parse_precise(
+                    &target_pattern.to_value().to_str(),
+                    root_cell.dupe(),
+                    cell_resolver,
+                    cell_alias_resolver,
+                )
+            };
+
+        let custom_soft_errors: SmallMap<_, _> =
             DictRef::from_value(self.custom_soft_errors.get().to_value())
                 .expect("Internal error: expected to be a dict")
                 .iter()
                 .map(|(k, v)| {
-                    let k = ArcStr::from(k.to_value().to_str());
-                    let mut target_patterns = Vec::new();
-                    for target_pattern in ListRef::from_value(v)
-                        .expect("Internal error: expected to be a list")
-                        .iter()
+                    let k = ArcStr::from(k.unpack_str().expect("Type checked to be string"));
+                    let (target_patterns, exclusions) = match Either::<
+                        &ListRef<'_>,
+                        &DepOnlyIncompatibleRollout<'_>,
+                    >::unpack_value(v)?
+                    .expect("Internal error: type checked to be list or provider")
                     {
-                        let target_pattern = target_pattern.to_value().to_str();
-                        target_patterns.push(ParsedPattern::parse_precise(
-                            &target_pattern,
-                            root_cell.dupe(),
-                            &cell_resolver,
-                            &cell_alias_resolver,
-                        )?);
-                    }
-                    let v: Box<[_]> = target_patterns.into();
-                    Ok((k, v.into()))
+                        Either::Left(v) => {
+                            let target_patterns = v
+                                .iter()
+                                .map(value_to_target_pattern)
+                                .collect::<buck2_error::Result<Vec<_>>>()?;
+                            (
+                                target_patterns.into_boxed_slice(),
+                                vec![].into_boxed_slice(),
+                            )
+                        }
+                        Either::Right(v) => {
+                            let target_patterns = v
+                                .target_patterns()
+                                .iter()
+                                .map(value_to_target_pattern)
+                                .collect::<buck2_error::Result<Vec<_>>>()?;
+                            let exclusions = v
+                                .exclusions()
+                                .iter()
+                                .map(value_to_target_pattern)
+                                .collect::<buck2_error::Result<Vec<_>>>()?;
+                            (
+                                target_patterns.into_boxed_slice(),
+                                exclusions.into_boxed_slice(),
+                            )
+                        }
+                    };
+                    let v = DepOnlyIncompatibleRolloutPatterns {
+                        target_patterns,
+                        exclusions,
+                    };
+                    Ok((k, v))
                 })
                 .collect::<buck2_error::Result<_>>()?;
 

app/buck2_build_api/src/interpreter/rule_defs/provider/builtin/dep_only_incompatible_rollout.rs

@@ -0,0 +1,61 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under both the MIT license found in the
+ * LICENSE-MIT file in the root directory of this source tree and the Apache
+ * License, Version 2.0 found in the LICENSE-APACHE file in the root directory
+ * of this source tree.
+ */
+
+use allocative::Allocative;
+use buck2_build_api_derive::internal_provider;
+use starlark::any::ProvidesStaticType;
+use starlark::environment::GlobalsBuilder;
+use starlark::values::list::ListRef;
+use starlark::values::list::ListType;
+use starlark::values::Coerce;
+use starlark::values::Freeze;
+use starlark::values::FreezeResult;
+use starlark::values::Trace;
+use starlark::values::ValueLifetimeless;
+use starlark::values::ValueOf;
+use starlark::values::ValueOfUncheckedGeneric;
+
+use crate as buck2_build_api;
+
+/// A provider that is used to signal the targets that we want to soft error on
+/// within the `DepOnlyIncompatibleInfo` provider.
+#[internal_provider(dep_only_incompatible_custom_soft_error_creator)]
+#[derive(Clone, Debug, Trace, Coerce, Freeze, ProvidesStaticType, Allocative)]
+#[repr(C)]
+pub struct DepOnlyIncompatibleRolloutGen<V: ValueLifetimeless> {
+    /// A list of target patterns (minus exclusions) that we want to soft error on.
+    pub target_patterns: ValueOfUncheckedGeneric<V, ListType<String>>,
+    /// A list of target patterns that we want to exclude from the soft error.
+    pub exclusions: ValueOfUncheckedGeneric<V, ListType<String>>,
+}
+
+impl<'v> DepOnlyIncompatibleRollout<'v> {
+    pub fn target_patterns(&'v self) -> &'v ListRef<'v> {
+        ListRef::from_value(self.target_patterns.get())
+            .expect("internal error: type checked as list")
+    }
+
+    pub fn exclusions(&'v self) -> &'v ListRef<'v> {
+        ListRef::from_value(self.exclusions.get()).expect("internal error: type checked as list")
+    }
+}
+
+#[starlark_module]
+fn dep_only_incompatible_custom_soft_error_creator(globals: &mut GlobalsBuilder) {
+    #[starlark(as_type = FrozenDepOnlyIncompatibleRollout)]
+    fn DepOnlyIncompatibleRollout<'v>(
+        #[starlark(require = named)] target_patterns: ValueOf<'v, ListType<&'v str>>,
+        #[starlark(require = named)] exclusions: ValueOf<'v, ListType<&'v str>>,
+    ) -> starlark::Result<DepOnlyIncompatibleRollout<'v>> {
+        Ok(DepOnlyIncompatibleRollout {
+            target_patterns: target_patterns.as_unchecked().cast(),
+            exclusions: exclusions.as_unchecked().cast(),
+        })
+    }
+}

app/buck2_configured/src/nodes.rs

@@ -1325,14 +1325,16 @@ async fn get_dep_only_incompatible_custom_soft_error(
     let Some(custom_soft_errors) = ctx.compute(&GetDepOnlyIncompatibleInfo).await?? else {
         return Ok(None);
     };
-    let mut soft_error_categories = Vec::new();
-    for (soft_error_category, patterns) in custom_soft_errors.iter() {
-        for pattern in patterns.iter() {
-            if pattern.matches(target_label) {
-                soft_error_categories.push(soft_error_category.dupe());
+    let soft_error_categories: Vec<_> = custom_soft_errors
+        .iter()
+        .filter_map(|(soft_error_category, rollout_patterns)| {
+            if rollout_patterns.matches(target_label) {
+                Some(soft_error_category.dupe())
+            } else {
+                None
             }
-        }
-    }
+        })
+        .collect();
     Ok(Some(soft_error_categories))
 }
 

tests/core/configurations/configurations/test_target_incompatible.py

@@ -161,3 +161,21 @@ async def test_dep_only_incompatible_custom_soft_errors(buck: Buck) -> None:
     await buck.cquery("//dep_incompatible:transitive_dep_incompatible", *args)
     result = await buck.log("show")
     assert "Soft Error: soft_error_two" in result.stdout
+
+
+@buck_test(allow_soft_errors=True)
+async def test_dep_only_incompatible_custom_soft_errors_with_exclusions(
+    buck: Buck,
+) -> None:
+    args = [
+        "-c",
+        "buck2.dep_only_incompatible_info=//dep_incompatible/dep_only_incompatible_info:dep_only_incompatible_info_with_exclusions",
+    ]
+    await buck.cquery("//dep_incompatible:dep_incompatible", *args)
+    result = await buck.log("show")
+    assert "Soft Error: soft_error_one" in result.stdout
+    assert "Soft Error: soft_error_three" in result.stdout
+
+    await buck.cquery("//dep_incompatible:transitive_dep_incompatible", *args)
+    result = await buck.log("show")
+    assert "Soft Error: soft_error_two" in result.stdout

tests/core/configurations/configurations/test_target_incompatible_data/dep_incompatible/dep_only_incompatible_info/TARGETS.fixture

@@ -1,4 +1,4 @@
-load(":defs.bzl", "dep_only_incompatible_info")
+load(":defs.bzl", "dep_only_incompatible_info", "dep_only_incompatible_info_with_exclusions")
 
 dep_only_incompatible_info(
     name = "dep_only_incompatible_info",
@@ -11,3 +11,29 @@ dep_only_incompatible_info(
         ],
     },
 )
+
+dep_only_incompatible_info_with_exclusions(
+    name = "dep_only_incompatible_info_with_exclusions",
+    custom_soft_errors = {
+        "soft_error_one": {
+            "exclusions": [],
+            "target_patterns": [
+                "root//dep_incompatible:dep_incompatible",
+            ],
+        },
+        "soft_error_three": {
+            "exclusions": [
+                "root//dep_incompatible:transitive_dep_incompatible",
+            ],
+            "target_patterns": [
+                "root//dep_incompatible:",
+            ],
+        },
+        "soft_error_two": {
+            "exclusions": [],
+            "target_patterns": [
+                "root//dep_incompatible:transitive_dep_incompatible",
+            ],
+        },
+    },
+)

tests/core/configurations/configurations/test_target_incompatible_data/dep_incompatible/dep_only_incompatible_info/defs.bzl

@@ -18,3 +18,26 @@ dep_only_incompatible_info = rule(
     },
     is_configuration_rule = True,
 )
+
+def _impl_with_exclusions(ctx: AnalysisContext) -> list[Provider]:
+    custom_soft_errors = {
+        category: DepOnlyIncompatibleRollout(
+            target_patterns = v["target_patterns"],
+            exclusions = v.get("exclusions", []),
+        )
+        for category, v in ctx.attrs.custom_soft_errors.items()
+    }
+    return [
+        DefaultInfo(),
+        DepOnlyIncompatibleInfo(
+            custom_soft_errors = custom_soft_errors,
+        ),
+    ]
+
+dep_only_incompatible_info_with_exclusions = rule(
+    impl = _impl_with_exclusions,
+    attrs = {
+        "custom_soft_errors": attrs.dict(attrs.string(), attrs.dict(attrs.string(), attrs.list(attrs.string()))),
+    },
+    is_configuration_rule = True,
+)

tests/core/docs/test_builtin_docs_data/buck2-golden-docs/build/DepOnlyIncompatibleRollout.md

@@ -0,0 +1,5 @@
+# This file is @generated, regenerate by re-running test with `-- --env BUCK2_UPDATE_GOLDEN=1` appended to the test command
+
+# DepOnlyIncompatibleRollout
+## DepOnlyIncompatibleRollout.exclusions
+## DepOnlyIncompatibleRollout.target\_patterns