Skip to content

Commit

Permalink
refactor(turborepo): Package Detection (#7549)
Browse files Browse the repository at this point in the history
### Description
Factored out package detection, i.e. the part where we take a file and
map it to the package into a trait. This allows us to have multiple
implementations, such as a default one that only uses the package graph,
and a more advanced implementation that uses the package graph and
turbo.json.

<!--
  ✍️ Write a short summary of your work.
  If necessary, include relevant screenshots.
-->

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->


Closes TURBO-2465

---------

Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
  • Loading branch information
NicholasLYang and chris-olszewski committed Mar 5, 2024
1 parent fb038a7 commit 95cbbb0
Show file tree
Hide file tree
Showing 10 changed files with 294 additions and 74 deletions.
142 changes: 142 additions & 0 deletions crates/turborepo-lib/src/global_deps_package_detector.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
use thiserror::Error;
use turbopath::AnchoredSystemPath;
use turborepo_repository::{
change_mapper::{DefaultPackageDetector, PackageChangeMapper, PackageMapping},
package_graph::{PackageGraph, WorkspacePackage},
};
use wax::{BuildError, Program};

#[derive(Error, Debug)]
pub enum Error {
#[error(transparent)]
InvalidFilter(#[from] BuildError),
}

/// A package detector that uses a global deps list to determine
/// if a file should cause all packages to be marked as changed.
/// This is less conservative than the `DefaultPackageDetector`
/// which assumes that any changed file that is not in a package
/// changes all packages. Since we have a list of global deps,
/// we can check against that and avoid invalidating in unnecessary cases.
pub struct GlobalDepsPackageDetector<'a> {
pkg_dep_graph: &'a PackageGraph,
global_deps_matcher: wax::Any<'a>,
}

impl<'a> GlobalDepsPackageDetector<'a> {
pub fn new<S: wax::Pattern<'a>, I: Iterator<Item = S>>(
pkg_dep_graph: &'a PackageGraph,
global_deps: I,
) -> Result<Self, Error> {
let global_deps_matcher = wax::any(global_deps)?;

Ok(Self {
pkg_dep_graph,
global_deps_matcher,
})
}
}

impl<'a> PackageChangeMapper for GlobalDepsPackageDetector<'a> {
fn detect_package(&self, path: &AnchoredSystemPath) -> PackageMapping {
match DefaultPackageDetector::new(self.pkg_dep_graph).detect_package(path) {
// Since `DefaultPackageDetector` is overly conservative, we can check here if
// the path is actually in globalDeps and if not, return it as
// PackageDetection::Package(WorkspacePackage::root()).
PackageMapping::All => {
let cleaned_path = path.clean();
let in_global_deps = self.global_deps_matcher.is_match(cleaned_path.as_str());

if in_global_deps {
PackageMapping::All
} else {
PackageMapping::Package(WorkspacePackage::root())
}
}
result => result,
}
}
}

#[cfg(test)]
mod tests {
use tempfile::tempdir;
use turbopath::{AbsoluteSystemPath, AnchoredSystemPathBuf};
use turborepo_repository::{
change_mapper::{ChangeMapper, DefaultPackageDetector, PackageChanges},
discovery,
discovery::PackageDiscovery,
package_graph::{PackageGraphBuilder, WorkspacePackage},
package_json::PackageJson,
};

use super::GlobalDepsPackageDetector;

#[allow(dead_code)]
pub struct MockDiscovery;

impl PackageDiscovery for MockDiscovery {
async fn discover_packages(
&self,
) -> Result<discovery::DiscoveryResponse, discovery::Error> {
Ok(discovery::DiscoveryResponse {
package_manager: turborepo_repository::package_manager::PackageManager::Npm,
workspaces: vec![],
})
}

async fn discover_packages_blocking(
&self,
) -> Result<discovery::DiscoveryResponse, discovery::Error> {
self.discover_packages().await
}
}

#[tokio::test]
async fn test_different_package_detectors() -> Result<(), anyhow::Error> {
let repo_root = tempdir()?;
let root_package_json = PackageJson::default();

let pkg_graph = PackageGraphBuilder::new(
AbsoluteSystemPath::from_std_path(repo_root.path())?,
root_package_json,
)
.with_package_discovery(MockDiscovery)
.build()
.await?;

let default_package_detector = DefaultPackageDetector::new(&pkg_graph);
let change_mapper = ChangeMapper::new(&pkg_graph, vec![], default_package_detector);

let package_changes = change_mapper.changed_packages(
[AnchoredSystemPathBuf::from_raw("README.md")?]
.into_iter()
.collect(),
None,
)?;

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

let turbo_package_detector =
GlobalDepsPackageDetector::new(&pkg_graph, std::iter::empty::<&str>())?;
let change_mapper = ChangeMapper::new(&pkg_graph, vec![], turbo_package_detector);

let package_changes = change_mapper.changed_packages(
[AnchoredSystemPathBuf::from_raw("README.md")?]
.into_iter()
.collect(),
None,
)?;

// We only get a root workspace change since we have global deps specified and
// README.md is not one of them
assert_eq!(
package_changes,
PackageChanges::Some([WorkspacePackage::root()].into_iter().collect())
);

Ok(())
}
}
1 change: 1 addition & 0 deletions crates/turborepo-lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ mod engine;

mod framework;
mod gitignore;
mod global_deps_package_detector;
pub(crate) mod globwatcher;
mod hash;
mod opts;
Expand Down
25 changes: 17 additions & 8 deletions crates/turborepo-lib/src/run/scope/change_detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@ use std::collections::HashSet;

use turbopath::{AbsoluteSystemPath, AnchoredSystemPathBuf};
use turborepo_repository::{
change_mapper::{ChangeMapError, ChangeMapper, LockfileChange, PackageChanges},
change_mapper::{
ChangeMapError, ChangeMapper, DefaultPackageDetector, LockfileChange, PackageChanges,
},
package_graph::{PackageGraph, PackageName},
};
use turborepo_scm::SCM;

use crate::global_deps_package_detector::{Error, GlobalDepsPackageDetector};

/// Given two git refs, determine which packages have changed between them.
pub trait GitChangeDetector {
fn changed_packages(
Expand All @@ -18,7 +22,7 @@ pub trait GitChangeDetector {

pub struct ScopeChangeDetector<'a> {
turbo_root: &'a AbsoluteSystemPath,
change_mapper: ChangeMapper<'a>,
change_mapper: ChangeMapper<'a, GlobalDepsPackageDetector<'a>>,
scm: &'a SCM,
pkg_graph: &'a PackageGraph,
}
Expand All @@ -28,17 +32,18 @@ impl<'a> ScopeChangeDetector<'a> {
turbo_root: &'a AbsoluteSystemPath,
scm: &'a SCM,
pkg_graph: &'a PackageGraph,
global_deps: Vec<String>,
global_deps: impl Iterator<Item = &'a str>,
ignore_patterns: Vec<String>,
) -> Self {
let change_mapper = ChangeMapper::new(pkg_graph, global_deps, ignore_patterns);
) -> Result<Self, Error> {
let pkg_detector = GlobalDepsPackageDetector::new(pkg_graph, global_deps)?;
let change_mapper = ChangeMapper::new(pkg_graph, ignore_patterns, pkg_detector);

Self {
Ok(Self {
turbo_root,
change_mapper,
scm,
pkg_graph,
}
})
}

/// Gets the lockfile content from SCM if it has changed.
Expand All @@ -54,7 +59,11 @@ impl<'a> ScopeChangeDetector<'a> {
.package_manager()
.lockfile_path(self.turbo_root);

if !ChangeMapper::lockfile_changed(self.turbo_root, changed_files, &lockfile_path) {
if !ChangeMapper::<DefaultPackageDetector>::lockfile_changed(
self.turbo_root,
changed_files,
&lockfile_path,
) {
return None;
}

Expand Down
29 changes: 22 additions & 7 deletions crates/turborepo-lib/src/run/scope/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ use super::{
simple_glob::{Match, SimpleGlob},
target_selector::{InvalidSelectorError, TargetSelector},
};
use crate::{run::scope::change_detector::ScopeChangeDetector, turbo_json::TurboJson};
use crate::{
global_deps_package_detector, run::scope::change_detector::ScopeChangeDetector,
turbo_json::TurboJson,
};

pub struct PackageInference {
package_name: Option<String>,
Expand Down Expand Up @@ -112,19 +115,29 @@ impl<'a> FilterResolver<'a, ScopeChangeDetector<'a>> {
turbo_root: &'a AbsoluteSystemPath,
inference: Option<PackageInference>,
scm: &'a SCM,
root_turbo_json: &TurboJson,
) -> Self {
let mut global_deps = opts.global_deps.clone();
global_deps.extend_from_slice(&root_turbo_json.global_deps);
root_turbo_json: &'a TurboJson,
) -> Result<Self, ResolutionError> {
let global_deps = opts
.global_deps
.iter()
.map(|s| s.as_str())
.chain(root_turbo_json.global_deps.iter().map(|s| s.as_str()));

let change_detector = ScopeChangeDetector::new(
turbo_root,
scm,
pkg_graph,
global_deps,
opts.ignore_patterns.clone(),
);
Self::new_with_change_detector(pkg_graph, turbo_root, inference, scm, change_detector)
)?;

Ok(Self::new_with_change_detector(
pkg_graph,
turbo_root,
inference,
scm,
change_detector,
))
}
}

Expand Down Expand Up @@ -585,6 +598,8 @@ pub enum ResolutionError {
glob: String,
err: Box<wax::BuildError>,
},
#[error("failed to construct glob for globalDependencies")]
GlobalDependenciesGlob(#[from] global_deps_package_detector::Error),
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/run/scope/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ pub fn resolve_packages(
pkg_inference,
scm,
root_turbo_json,
)
)?
.resolve(&opts.get_filters())
}
4 changes: 4 additions & 0 deletions crates/turborepo-paths/src/anchored_system_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,8 @@ impl AnchoredSystemPath {
.unwrap(),
)
}

pub fn clean(&self) -> AnchoredSystemPathBuf {
AnchoredSystemPathBuf(self.0.as_std_path().clean().try_into().unwrap())
}
}

0 comments on commit 95cbbb0

Please sign in to comment.