Skip to content

Commit

Permalink
chore(filter): add additional logs for scm based filters (#8850)
Browse files Browse the repository at this point in the history
### Description

Adding a bunch of debug logs to help identify why a package was marked
as changed from a `--filter=[sha]`.

### Testing Instructions

👀
  • Loading branch information
chris-olszewski committed Jul 30, 2024
1 parent deee36b commit 34c9a58
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 13 deletions.
28 changes: 19 additions & 9 deletions crates/turborepo-lib/src/run/scope/change_detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ impl<'a> ScopeChangeDetector<'a> {
changed_files,
&lockfile_path,
) {
debug!("lockfile did not change");
return None;
}

Expand Down Expand Up @@ -99,15 +100,24 @@ impl<'a> GitChangeDetector for ScopeChangeDetector<'a> {
.change_mapper
.changed_packages(changed_files, lockfile_contents)?
{
PackageChanges::All => Ok(self
.pkg_graph
.packages()
.map(|(name, _)| name.to_owned())
.collect()),
PackageChanges::Some(packages) => Ok(packages
.iter()
.map(|package| package.name.to_owned())
.collect()),
PackageChanges::All => {
debug!("all packages changed");
Ok(self
.pkg_graph
.packages()
.map(|(name, _)| name.to_owned())
.collect())
}
PackageChanges::Some(packages) => {
debug!(
"determined {} packages changed: {packages:?}",
packages.len()
);
Ok(packages
.iter()
.map(|package| package.name.to_owned())
.collect())
}
}
}
}
13 changes: 12 additions & 1 deletion crates/turborepo-repository/src/change_mapper/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ impl<'a, PD: PackageChangeMapper> ChangeMapper<'a, PD> {
lockfile_change: Option<LockfileChange>,
) -> Result<PackageChanges, ChangeMapError> {
if Self::default_global_file_changed(&changed_files) {
debug!("global file changed");
return Ok(PackageChanges::All);
}

Expand All @@ -76,15 +77,23 @@ impl<'a, PD: PackageChangeMapper> ChangeMapper<'a, PD> {
Some(LockfileChange::WithContent(content)) => {
// if we run into issues, don't error, just assume all packages have changed
let Ok(lockfile_changes) = self.get_changed_packages_from_lockfile(content) else {
debug!("unable to determine lockfile changes, assuming all packages changed");
return Ok(PackageChanges::All);
};

debug!(
"found {} packages changed by lockfile",
lockfile_changes.len()
);
changed_pkgs.extend(lockfile_changes);

Ok(PackageChanges::Some(changed_pkgs))
}
// We don't have the actual contents, so just invalidate everything
Some(LockfileChange::Empty) => Ok(PackageChanges::All),
Some(LockfileChange::Empty) => {
debug!("no previous lockfile available, assuming all packages changed");
Ok(PackageChanges::All)
}
None => Ok(PackageChanges::Some(changed_pkgs)),
}
}
Expand Down Expand Up @@ -117,9 +126,11 @@ impl<'a, PD: PackageChangeMapper> ChangeMapper<'a, PD> {
return PackageChanges::All;
}
PackageMapping::Package(pkg) => {
debug!("package {pkg:?} changed due to {file:?}");
changed_packages.insert(pkg);
}
PackageMapping::All => {
debug!("all packages changed due to {file:?}");
return PackageChanges::All;
}
PackageMapping::None => {}
Expand Down
18 changes: 15 additions & 3 deletions crates/turborepo-repository/src/package_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{
use itertools::Itertools;
use petgraph::visit::{depth_first_search, Reversed};
use serde::Serialize;
use tracing::debug;
use turbopath::{
AbsoluteSystemPath, AbsoluteSystemPathBuf, AnchoredSystemPath, AnchoredSystemPathBuf,
};
Expand Down Expand Up @@ -424,9 +425,20 @@ impl PackageGraph {
} else {
self.packages
.iter()
.filter(|(_name, info)| {
closures.get(info.package_path().to_unix().as_str())
!= info.transitive_dependencies.as_ref()
.filter(|(name, info)| {
let previous_closure = closures.get(info.package_path().to_unix().as_str());
let not_equal = previous_closure != info.transitive_dependencies.as_ref();
if not_equal {
if let (Some(prev), Some(curr)) =
(previous_closure, info.transitive_dependencies.as_ref())
{
debug!(
"package {name} has differing closure: {:?}",
prev.symmetric_difference(curr)
);
}
}
not_equal
})
.map(|(name, info)| match name {
PackageName::Other(n) => {
Expand Down

0 comments on commit 34c9a58

Please sign in to comment.