Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(Turborepo): Handle directory moves in package discovery #7700

Merged
merged 1 commit into from Mar 13, 2024
Merged

fix(Turborepo): Handle directory moves in package discovery #7700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
229 changes: 138 additions & 91 deletions crates/turborepo-filewatch/src/package_watcher.rs
View file
Open in desktop
Expand Up @@ -468,69 +468,72 @@ impl<T: PackageDiscovery + Send + Sync + 'static> Subscriber<T> {
.filter_map(|p| p.as_os_str().to_str())
{
let path_file = AbsoluteSystemPathBuf::new(path).expect("watched paths are absolute");

// the path to the workspace this file is in is the parent
let path_workspace = path_file
.parent()
.expect("watched paths will not be at the root")
.to_owned();

let is_workspace = match state
.filter
.target_is_workspace(&self.repo_root, &path_workspace)
{
Ok(is_workspace) => is_workspace,
Err(e) => {
// this will only error if `repo_root` is not an anchor of `path_workspace`.
// if we hit this case, we can safely ignore it
tracing::debug!("yielded path not in workspace: {:?}", e);
let path_workspace: &AbsoluteSystemPath =
if path_file.file_name() == Some("package.json") {
// The file event is for a package.json file. Check if the parent is a workspace
let path_parent = path_file
.parent()
.expect("watched paths will not be at the root");
if state
.filter
.target_is_workspace(&self.repo_root, path_parent)
.unwrap_or(false)
{
path_parent
} else {
// irrelevant package.json file update, it's not in a directory
// matching workspace globs
continue;
}
} else if state
.filter
.target_is_workspace(&self.repo_root, &path_file)
.unwrap_or(false)
{
// The file event is for a workspace directory itself
&path_file
} else {
// irrelevant file update, it's not a package.json file or a workspace directory
continue;
}
};

if is_workspace {
tracing::debug!("tracing file in package: {:?}", path_file);
let package_json = path_workspace.join_component("package.json");
let turbo_json = path_workspace.join_component("turbo.json");

let (package_exists, turbo_exists) = join!(
tokio::fs::try_exists(&package_json),
tokio::fs::try_exists(&turbo_json)
);
};

self.package_data_tx
.send_modify(|mut data| match (&mut data, package_exists) {
(Some(data), Ok(true)) => {
data.insert(
path_workspace,
WorkspaceData {
package_json,
turbo_json: turbo_exists
.unwrap_or_default()
.then_some(turbo_json),
},
);
}
(Some(data), Ok(false)) => {
data.remove(&path_workspace);
}
(None, Ok(true)) => {
let mut map = HashMap::new();
map.insert(
path_workspace,
WorkspaceData {
package_json,
turbo_json: turbo_exists
.unwrap_or_default()
.then_some(turbo_json),
},
);
*data = Some(map);
}
(None, Ok(false)) => {} // do nothing
(_, Err(_)) => todo!(),
});
}
tracing::debug!("handling change to workspace {path_workspace}");
let package_json = path_workspace.join_component("package.json");
let turbo_json = path_workspace.join_component("turbo.json");

let (package_exists, turbo_exists) = join!(
tokio::fs::try_exists(&package_json),
tokio::fs::try_exists(&turbo_json)
);

self.package_data_tx
.send_modify(|mut data| match (&mut data, package_exists) {
(Some(data), Ok(true)) => {
data.insert(
path_workspace.to_owned(),
WorkspaceData {
package_json,
turbo_json: turbo_exists.unwrap_or_default().then_some(turbo_json),
},
);
}
(Some(data), Ok(false)) => {
data.remove(path_workspace);
}
(None, Ok(true)) => {
let mut map = HashMap::new();
map.insert(
path_workspace.to_owned(),
WorkspaceData {
package_json,
turbo_json: turbo_exists.unwrap_or_default().then_some(turbo_json),
},
);
*data = Some(map);
}
(None, Ok(false)) => {} // do nothing
(_, Err(_)) => todo!(),
});
}

Ok(())
Expand Down Expand Up @@ -734,29 +737,25 @@ mod test {
turbo_json: None,
},
WorkspaceData {
package_json: root
.join_component("packages")
.join_component("foo")
.join_component("package.json"),
package_json: root.join_components(&["packages", "foo", "package.json"]),
turbo_json: None,
},
WorkspaceData {
package_json: root.join_components(&["packages", "bar", "package.json"]),
turbo_json: None,
},
];

// create folders and files
for data in &package_data {
tokio::fs::create_dir_all(data.package_json.parent().unwrap())
.await
.unwrap();
tokio::fs::write(&data.package_json, b"{}").await.unwrap();
data.package_json.ensure_dir().unwrap();
data.package_json.create_with_contents("{}").unwrap();
}

// write workspaces to root
tokio::fs::write(
root.join_component("package.json"),
r#"{"workspaces":["packages/*"]}"#,
)
.await
.unwrap();
root.join_component("package.json")
.create_with_contents(r#"{"workspaces":["packages/*"]}"#)
.unwrap();

let mock_discovery = MockDiscovery {
manager,
Expand Down Expand Up @@ -807,10 +806,11 @@ mod test {
turbo_json: None,
},
WorkspaceData {
package_json: root
.join_component("packages")
.join_component("foo")
.join_component("package.json"),
package_json: root.join_components(&["packages", "bar", "package.json",]),
turbo_json: None,
},
WorkspaceData {
package_json: root.join_components(&["packages", "foo", "package.json",]),
turbo_json: None,
},
]
Expand All @@ -819,20 +819,14 @@ mod test {
tracing::info!("removing subpackage");

// delete package.json in foo
tokio::fs::remove_file(
root.join_component("packages")
.join_component("foo")
.join_component("package.json"),
)
.await
.unwrap();
root.join_components(&["packages", "foo", "package.json"])
.remove_file()
.unwrap();

tx.send(Ok(notify::Event {
kind: notify::EventKind::Remove(notify::event::RemoveKind::File),
paths: vec![root
.join_component("packages")
.join_component("foo")
.join_component("package.json")
.join_components(&["packages", "foo", "package.json"])
.as_std_path()
.to_owned()],
..Default::default()
Expand All @@ -856,11 +850,64 @@ mod test {
};

assert_eq!(
data.unwrap().values().cloned().collect::<Vec<_>>(),
data.unwrap()
.values()
.cloned()
.sorted_by_key(|f| f.package_json.clone())
.collect::<Vec<_>>(),
vec![
WorkspaceData {
package_json: root.join_component("package.json"),
turbo_json: None,
},
WorkspaceData {
package_json: root.join_components(&["packages", "bar", "package.json"]),
turbo_json: None,
}
]
);

// move package bar
root.join_components(&["packages", "bar"])
.rename(&root.join_component("bar"))
.unwrap();

tx.send(Ok(notify::Event {
kind: notify::EventKind::Modify(notify::event::ModifyKind::Any),
paths: vec![root
.join_components(&["packages", "bar"])
.as_std_path()
.to_owned()],
..Default::default()
}))
.unwrap();

let (data, _) = join! {
package_data.get(),
async {
// simulate fs round trip
tokio::time::sleep(std::time::Duration::from_millis(100)).await;

let path = cookie_writer.cookie_dir().join_component("3.cookie").as_std_path().to_owned();
tracing::info!("writing cookie at {}", path.to_string_lossy());
tx.send(Ok(notify::Event {
kind: notify::EventKind::Create(notify::event::CreateKind::File),
paths: vec![path],
..Default::default()
})).unwrap();
}
};

assert_eq!(
data.unwrap()
.values()
.cloned()
.sorted_by_key(|f| f.package_json.clone())
.collect::<Vec<_>>(),
vec![WorkspaceData {
package_json: root.join_component("package.json"),
turbo_json: None,
},]
}]
);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/daemon/client.rs
View file
Open in desktop
Expand Up @@ -127,7 +127,7 @@ impl<T> DaemonClient<T> {
pub async fn discover_packages(&mut self) -> Result<DiscoverPackagesResponse, DaemonError> {
let req = proto::DiscoverPackagesRequest {};
let mut req = req.into_request();
req.set_timeout(Duration::from_millis(10));
req.set_timeout(Duration::from_millis(30));
let response = self.client.discover_packages(req).await?.into_inner();

Ok(response)
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-paths/src/absolute_system_path.rs
View file
Open in desktop
Expand Up @@ -31,7 +31,7 @@ pub enum PathRelation {
Child,
}

#[derive(Debug, PartialEq, Eq)]
#[derive(Debug, PartialEq, Eq, Hash)]
pub struct AbsoluteSystemPath(Utf8Path);

impl ToOwned for AbsoluteSystemPath {
Expand Down