Skip to content

Commit

Permalink
feat(turborepo): Log reason why all packages were considered changed
Browse files Browse the repository at this point in the history
  • Loading branch information
mehulkar committed Jul 30, 2024
1 parent 30a8bb8 commit b353351
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 32 deletions.
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/package_changes_watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ impl Subscriber {
tracing::warn!("changed_packages: {:?}", changed_packages);

match changed_packages {
Ok(PackageChanges::All) => {
Ok(PackageChanges::All(_)) => {
// We tell the client that we need to rediscover the packages, i.e.
// all bets are off, just re-run everything
let _ = self
Expand Down
13 changes: 8 additions & 5 deletions crates/turborepo-lib/src/run/scope/change_detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,14 @@ 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::All(reason) => {
debug!("All packages changed: {:?}", reason);
Ok(self
.pkg_graph
.packages()
.map(|(name, _)| name.to_owned())
.collect())
}
PackageChanges::Some(packages) => Ok(packages
.iter()
.map(|package| package.name.to_owned())
Expand Down
69 changes: 46 additions & 23 deletions crates/turborepo-repository/src/change_mapper/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub use package::{
};
use tracing::debug;
use turbopath::{AbsoluteSystemPath, AnchoredSystemPathBuf};
use turborepo_lockfiles::Package;

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / @turbo/repository (ubuntu, Node 20)

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Turborepo rust check

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Build Turborepo (ubuntu, self-hosted, linux, x64, metal)

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / @turbo/repository (macos, Node 18)

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / @turbo/repository (macos, Node 20)

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Turborepo rust clippy

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on ubuntu

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Build Turborepo (macos, macos-12)

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Build Turborepo (windows, windows-latest)

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on macos

unused import: `turborepo_lockfiles::Package`

Check warning on line 11 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Turborepo Integration (ubuntu-latest)

unused import: `turborepo_lockfiles::Package`
use wax::Program;

use crate::package_graph::{ChangedPackagesError, PackageGraph, WorkspacePackage};
Expand All @@ -23,9 +24,18 @@ pub enum LockfileChange {
WithContent(Vec<u8>),
}

#[derive(Debug, PartialEq, Eq)]
pub enum AllPackageChangeReason {
DefaultGlobalFileChanged,
LockfileChangeDetectionFailed,
LockfileChangedWithoutDetails,
RootInternalDepChanged,
NonPackageFileChanged,
}

#[derive(Debug, PartialEq, Eq)]
pub enum PackageChanges {
All,
All(AllPackageChangeReason),
Some(HashSet<WorkspacePackage>),
}

Expand Down Expand Up @@ -61,32 +71,43 @@ impl<'a, PD: PackageChangeMapper> ChangeMapper<'a, PD> {
lockfile_change: Option<LockfileChange>,
) -> Result<PackageChanges, ChangeMapError> {
if Self::default_global_file_changed(&changed_files) {
return Ok(PackageChanges::All);
return Ok(PackageChanges::All(
AllPackageChangeReason::DefaultGlobalFileChanged,
));
}

// get filtered files and add the packages that contain them
let filtered_changed_files = self.filter_ignored_files(changed_files.iter())?;
let PackageChanges::Some(mut changed_pkgs) =
self.get_changed_packages(filtered_changed_files.into_iter())
else {
return Ok(PackageChanges::All);
};

match lockfile_change {
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 {
return Ok(PackageChanges::All);
};

changed_pkgs.extend(lockfile_changes);
match self.get_changed_packages(filtered_changed_files.into_iter()) {
PackageChanges::All(reason) => {
return Ok(PackageChanges::All(reason));

Check failure on line 84 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Turborepo rust clippy

unneeded `return` statement
}

Ok(PackageChanges::Some(changed_pkgs))
PackageChanges::Some(mut changed_pkgs) => {
return match lockfile_change {

Check failure on line 88 in crates/turborepo-repository/src/change_mapper/mod.rs

View workflow job for this annotation

GitHub Actions / Turborepo rust clippy

unneeded `return` statement
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 {
return Ok(PackageChanges::All(
AllPackageChangeReason::LockfileChangeDetectionFailed,
));
};

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(
AllPackageChangeReason::LockfileChangedWithoutDetails,
)),
None => Ok(PackageChanges::Some(changed_pkgs)),
};
}
// We don't have the actual contents, so just invalidate everything
Some(LockfileChange::Empty) => Ok(PackageChanges::All),
None => Ok(PackageChanges::Some(changed_pkgs)),
}
};
}

fn filter_ignored_files<'b>(
Expand All @@ -111,16 +132,18 @@ impl<'a, PD: PackageChangeMapper> ChangeMapper<'a, PD> {
// Internal root dependency changed so global hash has changed
PackageMapping::Package(pkg) if root_internal_deps.contains(&pkg) => {
debug!(
"root internal dependency \"{}\" changed due to: {file:?}",
"{} changes root internal dependency: \"{}\"",
file.to_string(),
pkg.name
);
return PackageChanges::All;
return PackageChanges::All(AllPackageChangeReason::RootInternalDepChanged);
}
PackageMapping::Package(pkg) => {
debug!("{} changes \"{}\"", file.to_string(), pkg.name);
changed_packages.insert(pkg);
}
PackageMapping::All => {
return PackageChanges::All;
return PackageChanges::All(AllPackageChangeReason::NonPackageFileChanged);
}
PackageMapping::None => {}
}
Expand Down
5 changes: 4 additions & 1 deletion crates/turborepo-repository/src/change_mapper/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,10 @@ mod tests {

// We should return All because we don't have global deps and
// therefore must be conservative about changes
assert_eq!(package_changes, PackageChanges::All);
assert_eq!(
package_changes,
PackageChanges::All(AllPackageChangeReason::DefaultGlobalFileChanged)

Check failure on line 177 in crates/turborepo-repository/src/change_mapper/package.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on ubuntu

failed to resolve: use of undeclared type `AllPackageChangeReason`

Check failure on line 177 in crates/turborepo-repository/src/change_mapper/package.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on macos

failed to resolve: use of undeclared type `AllPackageChangeReason`
);

let turbo_package_detector =
GlobalDepsPackageChangeMapper::new(&pkg_graph, std::iter::empty::<&str>())?;
Expand Down
6 changes: 4 additions & 2 deletions packages/turbo-repository/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use napi::Error;
use napi_derive::napi;
use turbopath::{AbsoluteSystemPath, AnchoredSystemPathBuf};
use turborepo_repository::{
change_mapper::{ChangeMapper, DefaultPackageChangeMapper, PackageChanges},
change_mapper::{
AllPackageChangeReason, ChangeMapper, DefaultPackageChangeMapper, PackageChanges,

Check warning on line 11 in packages/turbo-repository/rust/src/lib.rs

View workflow job for this annotation

GitHub Actions / @turbo/repository (ubuntu, Node 20)

unused import: `AllPackageChangeReason`

Check warning on line 11 in packages/turbo-repository/rust/src/lib.rs

View workflow job for this annotation

GitHub Actions / @turbo/repository (macos, Node 18)

unused import: `AllPackageChangeReason`

Check warning on line 11 in packages/turbo-repository/rust/src/lib.rs

View workflow job for this annotation

GitHub Actions / @turbo/repository (macos, Node 20)

unused import: `AllPackageChangeReason`
},
inference::RepoState as WorkspaceState,
package_graph::{PackageGraph, PackageName, PackageNode, WorkspacePackage, ROOT_PKG_NAME},
};
Expand Down Expand Up @@ -211,7 +213,7 @@ impl Workspace {
};

let packages = match package_changes {
PackageChanges::All => self
PackageChanges::All(_) => self
.graph
.packages()
.map(|(name, info)| WorkspacePackage {
Expand Down

0 comments on commit b353351

Please sign in to comment.