Skip to content

Commit

Permalink
Implement the ci_hints mechanism
Browse files Browse the repository at this point in the history
Summary: Implements as per the spec in the diff before.

Reviewed By: pcerl017

Differential Revision: D49378988

fbshipit-source-id: bffe4d56704724071a481d8dd274c9e3d42ce164
  • Loading branch information
ndmitchell authored and facebook-github-bot committed Nov 15, 2023
1 parent 55ff5b6 commit 30f8d30
Showing 1 changed file with 62 additions and 0 deletions.
62 changes: 62 additions & 0 deletions btd/src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,19 @@ use std::collections::HashMap;
use std::collections::HashSet;
use std::mem;

use tracing::warn;

use crate::buck::glob::GlobSpec;
use crate::buck::target_map::TargetMap;
use crate::buck::targets::TargetLabelKey;
use crate::buck::targets::Targets;
use crate::buck::targets::TargetsTarget;
use crate::buck::types::CellPath;
use crate::buck::types::Glob;
use crate::buck::types::Package;
use crate::buck::types::RuleType;
use crate::buck::types::TargetLabel;
use crate::buck::types::TargetName;
use crate::changes::Changes;

/// Given the state, which .bzl files have changed, either directly or by transitive dependencies
Expand Down Expand Up @@ -139,6 +144,15 @@ pub fn immediate_target_changes<'a>(
res
}

fn hint_applies_to(target: &TargetsTarget) -> Option<(&Package, TargetName)> {
// for hints, the name will be `foo//bar:ci_hint@baz` which means
// we need to test `foo//bar:baz`.
Some((
&target.package,
TargetName::new(target.name.as_str().strip_prefix("ci_hint@")?),
))
}

pub fn recursive_target_changes<'a>(
diff: &'a Targets,
changes: &GraphImpact<'a>,
Expand All @@ -161,13 +175,34 @@ pub fn recursive_target_changes<'a>(
// We expect most things will have at least one dependency, so a reasonable approximate size
let mut rdeps: TargetMap<&TargetsTarget> =
TargetMap::with_capacity(diff.len_targets_upperbound());
let mut hints: HashMap<(&Package, TargetName), TargetLabel> = HashMap::new();
for target in diff.targets() {
for d in target.deps.iter() {
rdeps.insert(d, target)
}
for d in target.ci_deps.iter() {
rdeps.insert_pattern(d, target);
}
if target.rule_type.short() == "ci_hint" {
match hint_applies_to(target) {
Some(dest) => {
hints.insert(dest, target.label());
}
None => warn!("`ci_hint` target has invalid name: `{}`", target.label()),
}
}
}
// We record the hints going through (while we don't have the targets to hand),
// then fill them in later with this loop
if !hints.is_empty() {
for target in diff.targets() {
if let Some(hint) = hints.remove(&(&target.package, target.name.clone())) {
rdeps.insert(&hint, target);
if hints.is_empty() {
break;
}
}
}
}

// The code below is carefully optimised to avoid multiple lookups and reuse memory allocations.
Expand Down Expand Up @@ -694,4 +729,31 @@ mod test {
2
);
}

#[test]
fn test_recursive_changes_hint() {
// We should be able to deal with cycles, and pieces that aren't on the graph
let diff = Targets::new(vec![
TargetsEntry::Target(TargetsTarget {
..TargetsTarget::testing(
"ci_hint@baz",
"foo//bar",
"fbcode//target_determinator/macros/rules/ci_hint.bzl:ci_hint",
)
}),
TargetsEntry::Target(TargetsTarget {
..TargetsTarget::testing("baz", "foo//bar", "prelude//rules.bzl:cxx_library")
}),
]);

let changes = GraphImpact {
recursive: vec![diff.targets().next().unwrap()],
non_recursive: Vec::new(),
};
let res = recursive_target_changes(&diff, &changes, Some(3), |_| true);
assert_eq!(res[0].len(), 1);
assert_eq!(res[1].len(), 1);
assert_eq!(res[1][0].name, TargetName::new("baz"));
assert_eq!(res.iter().flatten().count(), 2);
}
}

0 comments on commit 30f8d30

Please sign in to comment.