Treat buckconfig change as universal change

Summary:
(reported from [a few posts](https://fb.workplace.com/groups/td.users/permalink/3347502112212451/))
Let BTD return everything if certain "universal" file is touched. This list of file matcher is more strict than the condition to "rerun" everything, as we will skip checking hash etc. Usually this condition is met when diff touches buck configs which implicitly used by certain mode, and it not directly reflected in targets themselves (only take effect when we try to execute some target with these configs).

Around 15 such changes every day: https://fburl.com/daiquery/9yzyej76 , hopefully we have enough filtering in later stages (Verse) to avoid flooding sandcastle fleet.

An alternative is to let diff author have more control (eg. through an explicit directive) to tell BTD to treat everything as affected. With a very restricted list, this "pessimistic" approach will not require diff author add additional directives, while may cause unnecessary cost. (diff author may still use other directives to focusing on a subset of targets)

Reviewed By: ndmitchell, aniketmathur, 8Keep

Differential Revision: D58345671

fbshipit-source-id: ee44a2bc15f8ad6e6cf38d872d5011eed56b4895

btd/src/buck/config.rs

@@ -25,6 +25,17 @@ pub fn is_buck_deployment(path: &CellPath) -> bool {
     path.as_str().starts_with("fbsource//tools/buck2-versions/")
 }
 
+// Touching these files will let btd treat everything as affected.
+pub fn is_buckconfig_change(path: &CellPath) -> bool {
+    let ext = path.extension();
+    let str = path.as_str();
+    ext == Some("buckconfig")
+        || str.starts_with("fbsource//tools/buckconfigs/")
+        || (ext.is_none()
+            && (str.starts_with("fbsource//arvr/mode/")
+                || str.starts_with("fbsource//fbcode/mode/")))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -38,4 +49,38 @@ mod tests {
             "fbsource//tools/buck2-versions/stable"
         )));
     }
+
+    #[test]
+    fn test_is_buckconfig_change() {
+        // random buckconfigs
+        assert!(!is_buckconfig_change(&CellPath::new(
+            "fbcode//some_config.bcfg"
+        )));
+        // buckconfigs
+        assert!(is_buckconfig_change(&CellPath::new(
+            "fbsource//.buckconfig"
+        )));
+        // bcfg
+        assert!(is_buckconfig_change(&CellPath::new(
+            "fbsource//tools/buckconfigs/abc/xyz.bcfg"
+        )));
+        assert!(!is_buckconfig_change(&CellPath::new(
+            "fbcode//buck2/TARGETS"
+        )));
+        assert!(!is_buckconfig_change(&CellPath::new(
+            "fbcode//buck2/src/file.rs"
+        )));
+        assert!(is_buckconfig_change(&CellPath::new(
+            "fbsource//tools/buckconfigs/cxx/windows/clang.inc"
+        )));
+        assert!(is_buckconfig_change(&CellPath::new(
+            "fbsource//arvr/mode/dv/dev.buckconfig"
+        )));
+        assert!(is_buckconfig_change(&CellPath::new(
+            "fbsource//tools/buckconfigs/fbsource-specific.bcfg"
+        )));
+        assert!(is_buckconfig_change(&CellPath::new(
+            "fbsource//.buckconfig"
+        )));
+    }
 }

btd/src/diff.rs

@@ -14,6 +14,7 @@ use std::mem;
 
 use tracing::warn;
 
+use crate::buck::config::is_buckconfig_change;
 use crate::buck::config::should_exclude_bzl_file_from_transitive_impact_tracing;
 use crate::buck::glob::GlobSpec;
 use crate::buck::target_map::TargetMap;
@@ -102,6 +103,13 @@ impl<'a> GraphImpact<'a> {
         }
     }
 
+    pub fn from_non_recursive(non_recursive: Vec<(&'a BuckTarget, ImpactReason)>) -> Self {
+        Self {
+            non_recursive,
+            ..Default::default()
+        }
+    }
+
     pub fn len(&self) -> usize {
         self.recursive.len() + self.non_recursive.len()
     }
@@ -171,6 +179,8 @@ pub enum RootImpactKind {
     Remove,
     /// When we want to manually rerun the target.
     ManualForRerun,
+    /// Universal file is touched
+    UniversalFile,
 }
 
 pub fn immediate_target_changes<'a>(
@@ -179,6 +189,16 @@ pub fn immediate_target_changes<'a>(
     changes: &Changes,
     track_prelude_changes: bool,
 ) -> GraphImpact<'a> {
+    if changes.cell_paths().any(is_buckconfig_change) {
+        let mut ret = GraphImpact::from_non_recursive(
+            diff.targets()
+                .map(|t| (t, ImpactReason::new(t, RootImpactKind::UniversalFile)))
+                .collect(),
+        );
+        ret.sort();
+        return ret;
+    }
+
     // Find those targets which are different
     let mut old = base.targets_by_label_key();
 
@@ -530,6 +550,47 @@ mod tests {
         assert_eq!(non_recursive.map(|x| x.as_str()), &["foo//bar:zzz",]);
     }
 
+    #[test]
+    fn test_everything_is_immediate_on_universal_changes() {
+        fn target(
+            pkg: &str,
+            name: &str,
+            inputs: &[&CellPath],
+            hash: &str,
+            package_values: &PackageValues,
+        ) -> TargetsEntry {
+            TargetsEntry::Target(BuckTarget {
+                inputs: inputs.iter().map(|x| (*x).clone()).collect(),
+                hash: TargetHash::new(hash),
+                package_values: package_values.clone(),
+                ..BuckTarget::testing(name, pkg, "prelude//rules.bzl:cxx_library")
+            })
+        }
+
+        let file1 = CellPath::new("fbsource//tools/buckconfigs/file1.bcfg");
+        let file2 = CellPath::new("foo//bar/file2.txt");
+        let file3 = CellPath::new("foo//bar/file3.txt");
+
+        let default_pacakge_value = PackageValues::new(&["default"], serde_json::Value::Null);
+        let base = Targets::new(vec![
+            target("foo//bar", "aaa", &[], "123", &default_pacakge_value),
+            target("foo//baz", "bbb", &[&file2], "123", &default_pacakge_value),
+            target("foo//bar", "ccc", &[&file3], "123", &default_pacakge_value),
+        ]);
+        let res = immediate_target_changes(
+            &base,
+            &base,
+            &Changes::testing(&[Status::Modified(file1), Status::Removed(file2)]),
+            false,
+        );
+        assert!(res.recursive.is_empty());
+        let non_recursive = res.non_recursive.map(|(x, _)| x.label().to_string());
+        assert_eq!(
+            non_recursive.map(|x| x.as_str()),
+            &["foo//bar:aaa", "foo//bar:ccc", "foo//baz:bbb"]
+        );
+    }
+
     #[test]
     fn test_immediate_changes_with_removed() {
         fn target(

btd/src/rerun.rs

@@ -13,6 +13,7 @@ use std::path::Path;
 
 use crate::buck::cells::CellInfo;
 use crate::buck::config::is_buck_deployment;
+use crate::buck::config::is_buckconfig_change;
 use crate::buck::package_resolver::PackageResolver;
 use crate::buck::targets::Targets;
 use crate::buck::types::CellName;
@@ -28,24 +29,10 @@ pub enum PackageStatus {
     Unknown,
 }
 
-fn is_buckconfig(path: &CellPath) -> bool {
-    // Need to match .buckconfig and .bcfg suffix
-    // There are also configs from chef etc (e.g. /etc/buckconfig.d/fb_chef.ini)
-    // but they won't show up in the change list anyway since they aren't version controlled.
-    //
-    // If the user passes `@mode/dev` that will change everything.
-    // There are also files in `arvr/mode/**/*.inc` that get pulled in to config and places like
-    // `tools/buckconfigs/cxx/windows/clang.inc`.
-    let ext = path.extension();
-    let str = path.as_str();
-    ext == Some("bcfg")
-        || ext == Some("buckconfig")
-        || str.contains("/mode/")
-        || str.contains("/buckconfigs/")
-}
-
+// Whether the change invalidates the graph, and we will rerun everything
+// for a updated graph.
 fn invalidates_graph(path: &CellPath) -> bool {
-    is_buckconfig(path) || is_buck_deployment(path)
+    is_buckconfig_change(path) || is_buck_deployment(path)
 }
 
 /// Compute the targets we should rerun, or None if we should do everything.
@@ -253,22 +240,6 @@ mod tests {
     use crate::buck::types::TargetHash;
     use crate::buck::types::TargetLabel;
 
-    #[test]
-    fn test_is_buckconfig() {
-        assert!(!is_buckconfig(&CellPath::new("fbcode//buck2/TARGETS")));
-        assert!(!is_buckconfig(&CellPath::new("fbcode//buck2/src/file.rs")));
-        assert!(is_buckconfig(&CellPath::new(
-            "fbsource//tools/buckconfigs/cxx/windows/clang.inc"
-        )));
-        assert!(is_buckconfig(&CellPath::new(
-            "fbsource//arvr/mode/dv/dev.buckconfig"
-        )));
-        assert!(is_buckconfig(&CellPath::new(
-            "fbsource//tools/buckconfigs/fbsource-specific.bcfg"
-        )));
-        assert!(is_buckconfig(&CellPath::new("fbsource//.buckconfig")));
-    }
-
     #[test]
     fn test_rerun_globs() {
         let target_entries = vec![

btd/test/test_e2e.py

@@ -221,7 +221,7 @@ def check_properties(patch, rdeps):
         } in rdeps
         assert len(rdeps) == 2
     elif patch == "buckconfig":
-        assert len(rdeps) == 1
+        assert len(rdeps) == 3
     elif patch == "cfg_modifiers":
         assert rdeps == [
             {