btd: Rename GraphImpact -> ImpactTraceData

Summary: Want to include other data here as well, e.g. whether the node is terminal or not. Differential Revision: D60877326 fbshipit-source-id: fd97f696925b7c7b95a7244808894f11883ded1e
facebookincubator · Aug 7, 2024 · 93a793f · 93a793f
1 parent 7f7330c
commit 93a793f
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 53 deletions.
diff --git a/btd/src/check.rs b/btd/src/check.rs
@@ -23,7 +23,7 @@ use crate::buck::types::Package;
 use crate::buck::types::TargetLabel;
 use crate::buck::types::TargetPattern;
 use crate::changes::Changes;
-use crate::diff::ImpactReason;
+use crate::diff::ImpactTraceData;
 
 #[derive(Debug, Error, Serialize)]
 pub enum ValidationError {
@@ -142,7 +142,7 @@ pub fn check_errors(base: &Targets, diff: &Targets, changes: &Changes) -> Vec<Va
 pub fn check_dangling(
     base: &Targets,
     diff: &Targets,
-    immediate_changes: &[(&BuckTarget, ImpactReason)],
+    immediate_changes: &[(&BuckTarget, ImpactTraceData)],
     universe: &[TargetPattern],
 ) -> Vec<ValidationError> {
     let exists_after = diff.targets_by_label_key();
@@ -359,7 +359,7 @@ mod tests {
                 ]),
                 &[(
                     &modified_target,
-                    ImpactReason {
+                    ImpactTraceData {
                         affected_dep: "".to_owned(),
                         root_cause: ("".to_owned(), RootImpactKind::Inputs),
                     },
@@ -384,7 +384,7 @@ mod tests {
                 ]),
                 &[(
                     &modified_target,
-                    ImpactReason {
+                    ImpactTraceData {
                         affected_dep: "".to_owned(),
                         root_cause: ("".to_owned(), RootImpactKind::Inputs),
                     }
@@ -409,7 +409,7 @@ mod tests {
                 ]),
                 &[(
                     &modified_target,
-                    ImpactReason {
+                    ImpactTraceData {
                         affected_dep: "".to_owned(),
                         root_cause: ("".to_owned(), RootImpactKind::Inputs),
                     }
@@ -433,7 +433,7 @@ mod tests {
                 ]),
                 &[(
                     &modified_target,
-                    ImpactReason {
+                    ImpactTraceData {
                         affected_dep: "".to_owned(),
                         root_cause: ("".to_owned(), RootImpactKind::Inputs),
                     }
@@ -457,7 +457,7 @@ mod tests {
                 ]),
                 &[(
                     &modified_target,
-                    ImpactReason {
+                    ImpactTraceData {
                         affected_dep: "".to_owned(),
                         root_cause: ("".to_owned(), RootImpactKind::Inputs),
                     }

diff --git a/btd/src/diff.rs b/btd/src/diff.rs
@@ -87,23 +87,23 @@ fn is_changed_ci_srcs(file_deps: &[Glob], changes: &Changes) -> bool {
 pub struct GraphImpact<'a> {
     /// Targets which changed, and whose change is expected to impact
     /// things that depend on them (they changed recursively).
-    recursive: Vec<(&'a BuckTarget, ImpactReason)>,
+    recursive: Vec<(&'a BuckTarget, ImpactTraceData)>,
     /// Targets which changed in a way that won't impact things recursively.
     /// Currently only package value changes.
-    non_recursive: Vec<(&'a BuckTarget, ImpactReason)>,
+    non_recursive: Vec<(&'a BuckTarget, ImpactTraceData)>,
     /// Targets which are removed.
-    removed: Vec<(&'a BuckTarget, ImpactReason)>,
+    removed: Vec<(&'a BuckTarget, ImpactTraceData)>,
 }
 
 impl<'a> GraphImpact<'a> {
-    pub fn from_recursive(recursive: Vec<(&'a BuckTarget, ImpactReason)>) -> Self {
+    pub fn from_recursive(recursive: Vec<(&'a BuckTarget, ImpactTraceData)>) -> Self {
         Self {
             recursive,
             ..Default::default()
         }
     }
 
-    pub fn from_non_recursive(non_recursive: Vec<(&'a BuckTarget, ImpactReason)>) -> Self {
+    pub fn from_non_recursive(non_recursive: Vec<(&'a BuckTarget, ImpactTraceData)>) -> Self {
         Self {
             non_recursive,
             ..Default::default()
@@ -114,7 +114,7 @@ impl<'a> GraphImpact<'a> {
         self.recursive.len() + self.non_recursive.len()
     }
 
-    pub fn iter(&'a self) -> impl Iterator<Item = (&'a BuckTarget, ImpactReason)> {
+    pub fn iter(&'a self) -> impl Iterator<Item = (&'a BuckTarget, ImpactTraceData)> {
         self.recursive
             .iter()
             .chain(self.non_recursive.iter())
@@ -129,9 +129,11 @@ impl<'a> GraphImpact<'a> {
     }
 }
 
+/// Contains metadata about why a target was impacted and information on the
+/// targets place in the dependency graph.
 #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd)]
 #[derive(serde::Serialize, serde::Deserialize)]
-pub struct ImpactReason {
+pub struct ImpactTraceData {
     /// The target name of the direct dependency which
     /// caused this target to be impacted.
     pub affected_dep: String, // parent_target_name
@@ -140,9 +142,9 @@ pub struct ImpactReason {
     pub root_cause: (String, RootImpactKind), // root_target_name, reason
 }
 
-impl ImpactReason {
+impl ImpactTraceData {
     pub fn new(target: &BuckTarget, kind: RootImpactKind) -> Self {
-        ImpactReason {
+        ImpactTraceData {
             affected_dep: String::new(),
             root_cause: (
                 format!("{}:{}", target.package.as_str(), target.name.as_str()),
@@ -192,7 +194,7 @@ pub fn immediate_target_changes<'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)))
+                .map(|t| (t, ImpactTraceData::new(t, RootImpactKind::UniversalFile)))
                 .collect(),
         );
         ret.sort();
@@ -214,7 +216,7 @@ pub fn immediate_target_changes<'a>(
             Some(x) => x,
             None => {
                 res.recursive
-                    .push((target, ImpactReason::new(target, RootImpactKind::New)));
+                    .push((target, ImpactTraceData::new(target, RootImpactKind::New)));
                 continue;
             }
         };
@@ -263,18 +265,18 @@ pub fn immediate_target_changes<'a>(
             .or_else(change_rule)
         {
             res.recursive
-                .push((target, ImpactReason::new(target, reason)));
+                .push((target, ImpactTraceData::new(target, reason)));
         } else if let Some(reason) = change_package_values() {
             res.non_recursive
-                .push((target, ImpactReason::new(target, reason)));
+                .push((target, ImpactTraceData::new(target, reason)));
         }
     }
 
     // We remove targets from `old` when iterating `diff` above.
     // At this point, only removed targets are left in `old`.
     res.removed = old
         .into_values()
-        .map(|target| (target, ImpactReason::new(target, RootImpactKind::Remove)))
+        .map(|target| (target, ImpactTraceData::new(target, RootImpactKind::Remove)))
         .collect();
 
     // Sort to ensure deterministic output
@@ -296,7 +298,7 @@ pub fn recursive_target_changes<'a>(
     changes: &GraphImpact<'a>,
     depth: Option<usize>,
     follow_rule_type: impl Fn(&RuleType) -> bool,
-) -> Vec<Vec<(&'a BuckTarget, ImpactReason)>> {
+) -> Vec<Vec<(&'a BuckTarget, ImpactTraceData)>> {
     // Just an optimisation, but saves building the reverse mapping
     if changes.recursive.is_empty() && changes.removed.is_empty() {
         let mut res = if changes.non_recursive.is_empty() {
@@ -379,12 +381,12 @@ pub fn recursive_target_changes<'a>(
 
     // Track targets depending on removed targets, but we don't add removed targets
     // to results
-    let mut todo_silent: Vec<(&BuckTarget, ImpactReason)> = changes.removed.clone();
-    let mut next_silent: Vec<(&BuckTarget, ImpactReason)> = Vec::new();
+    let mut todo_silent: Vec<(&BuckTarget, ImpactTraceData)> = changes.removed.clone();
+    let mut next_silent: Vec<(&BuckTarget, ImpactTraceData)> = Vec::new();
 
     fn add_result<'a>(
-        results: &mut Vec<Vec<(&'a BuckTarget, ImpactReason)>>,
-        mut items: Vec<(&'a BuckTarget, ImpactReason)>,
+        results: &mut Vec<Vec<(&'a BuckTarget, ImpactTraceData)>>,
+        mut items: Vec<(&'a BuckTarget, ImpactTraceData)>,
     ) {
         // Sort to ensure deterministic output
         items.sort_by_key(|(x, _)| x.label_key());
@@ -403,7 +405,7 @@ pub fn recursive_target_changes<'a>(
 
         for (lbl, reason) in todo.iter().chain(todo_silent.iter()) {
             if follow_rule_type(&lbl.rule_type) {
-                let updated_reason = ImpactReason {
+                let updated_reason = ImpactTraceData {
                     affected_dep: lbl.label().to_string(),
                     root_cause: reason.root_cause.clone(),
                 };
@@ -738,7 +740,7 @@ mod tests {
             recursive: Vec::new(),
             non_recursive: vec![(
                 diff.targets().next().unwrap(),
-                ImpactReason {
+                ImpactTraceData {
                     affected_dep: "".to_owned(),
                     root_cause: ("".to_owned(), RootImpactKind::Inputs),
                 },
@@ -786,14 +788,14 @@ mod tests {
         let changes = GraphImpact {
             recursive: vec![(
                 diff.targets().next().unwrap(),
-                ImpactReason {
+                ImpactTraceData {
                     affected_dep: "".to_owned(),
                     root_cause: ("".to_owned(), RootImpactKind::Inputs),
                 },
             )],
             non_recursive: vec![(
                 diff.targets().nth(1).unwrap(),
-                ImpactReason {
+                ImpactTraceData {
                     affected_dep: "".to_owned(),
                     root_cause: ("".to_owned(), RootImpactKind::Inputs),
                 },
@@ -834,7 +836,7 @@ mod tests {
 
         let changes = GraphImpact::from_recursive(vec![(
             diff.targets().next().unwrap(),
-            ImpactReason {
+            ImpactTraceData {
                 affected_dep: "".to_owned(),
                 root_cause: ("".to_owned(), RootImpactKind::Inputs),
             },
@@ -875,11 +877,11 @@ mod tests {
         let changes = GraphImpact {
             recursive: vec![(
                 changed_target,
-                ImpactReason::new(changed_target, RootImpactKind::Inputs),
+                ImpactTraceData::new(changed_target, RootImpactKind::Inputs),
             )],
             removed: vec![(
                 &removed,
-                ImpactReason::new(&removed, RootImpactKind::Remove),
+                ImpactTraceData::new(&removed, RootImpactKind::Remove),
             )],
             ..Default::default()
         };
@@ -915,7 +917,7 @@ mod tests {
             BuckTarget::testing("dep", "code//foo", "prelude//rules.bzl:cxx_library");
         let changes = GraphImpact::from_recursive(vec![(
             &change_target,
-            ImpactReason {
+            ImpactTraceData {
                 affected_dep: "".to_owned(),
                 root_cause: ("".to_owned(), RootImpactKind::Inputs),
             },
@@ -952,7 +954,7 @@ mod tests {
                 .map(|x| {
                     (
                         x,
-                        ImpactReason {
+                        ImpactTraceData {
                             affected_dep: "".to_owned(),
                             root_cause: ("".to_owned(), RootImpactKind::Inputs),
                         },
@@ -1138,7 +1140,7 @@ mod tests {
         );
         impact.recursive.push((
             targets.targets().nth(1).unwrap(),
-            ImpactReason {
+            ImpactTraceData {
                 affected_dep: "".to_owned(),
                 root_cause: ("".to_owned(), RootImpactKind::Inputs),
             },
@@ -1170,7 +1172,7 @@ mod tests {
 
         let changes = GraphImpact::from_recursive(vec![(
             diff.targets().next().unwrap(),
-            ImpactReason {
+            ImpactTraceData {
                 affected_dep: "".to_owned(),
                 root_cause: ("".to_owned(), RootImpactKind::Inputs),
             },

diff --git a/btd/src/glean.rs b/btd/src/glean.rs
@@ -18,7 +18,7 @@ use crate::buck::types::TargetLabelKeyRef;
 use crate::changes::Changes;
 use crate::diff::immediate_target_changes;
 use crate::diff::recursive_target_changes;
-use crate::diff::ImpactReason;
+use crate::diff::ImpactTraceData;
 
 fn cxx_rule_type(typ: &RuleType) -> bool {
     let short = typ.short();
@@ -35,7 +35,7 @@ pub fn glean_changes<'a>(
     diff: &'a Targets,
     changes: &Changes,
     depth: Option<usize>,
-) -> Vec<Vec<(&'a BuckTarget, ImpactReason)>> {
+) -> Vec<Vec<(&'a BuckTarget, ImpactTraceData)>> {
     let header = immediate_target_changes(
         base,
         diff,
@@ -49,9 +49,9 @@ pub fn glean_changes<'a>(
 }
 
 fn merge<'a>(
-    a: Vec<Vec<(&'a BuckTarget, ImpactReason)>>,
-    b: Vec<Vec<(&'a BuckTarget, ImpactReason)>>,
-) -> Vec<Vec<(&'a BuckTarget, ImpactReason)>> {
+    a: Vec<Vec<(&'a BuckTarget, ImpactTraceData)>>,
+    b: Vec<Vec<(&'a BuckTarget, ImpactTraceData)>>,
+) -> Vec<Vec<(&'a BuckTarget, ImpactTraceData)>> {
     let mut seen: HashSet<TargetLabelKeyRef> = HashSet::new();
     let mut res = Vec::new();
     for layer in a.into_iter().zip_longest(b) {

diff --git a/btd/src/graph_size.rs b/btd/src/graph_size.rs
@@ -20,7 +20,7 @@ use crate::buck::targets::BuckTarget;
 use crate::buck::targets::Targets;
 use crate::buck::types::TargetLabel;
 use crate::buck::types::TargetLabelKeyRef;
-use crate::diff::ImpactReason;
+use crate::diff::ImpactTraceData;
 use crate::output::Output;
 use crate::output::OutputFormat;
 
@@ -80,7 +80,7 @@ impl GraphSize {
 
     pub fn print_recursive_changes(
         &mut self,
-        changes: &[Vec<(&BuckTarget, ImpactReason)>],
+        changes: &[Vec<(&BuckTarget, ImpactTraceData)>],
         sudos: &HashSet<TargetLabelKeyRef>,
         output: OutputFormat,
     ) {

diff --git a/btd/src/lib.rs b/btd/src/lib.rs
@@ -55,7 +55,7 @@ use crate::buck::types::TargetLabelKeyRef;
 use crate::buck::types::TargetPattern;
 use crate::changes::Changes;
 use crate::check::ValidationError;
-use crate::diff::ImpactReason;
+use crate::diff::ImpactTraceData;
 use crate::diff::RootImpactKind;
 use crate::graph_size::GraphSize;
 use crate::output::Output;
@@ -459,7 +459,7 @@ impl OutputFormat {
 }
 
 fn print_recursive_changes<'a, T: Serialize + 'a>(
-    changes: &[Vec<(&'a BuckTarget, ImpactReason)>],
+    changes: &[Vec<(&'a BuckTarget, ImpactTraceData)>],
     sudos: &HashSet<TargetLabelKeyRef>,
     output: OutputFormat,
     mut augment: impl FnMut(&'a BuckTarget, Output<'a>) -> T,