From 7280da86da8a2b6644a87e1148b81c3466af9071 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 16 Oct 2023 10:32:25 -0700 Subject: [PATCH 1/5] Fix filewatching setup cookie path --- crates/turborepo-filewatch/src/lib.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/turborepo-filewatch/src/lib.rs b/crates/turborepo-filewatch/src/lib.rs index e88ed35f72939..0df4c456a4121 100644 --- a/crates/turborepo-filewatch/src/lib.rs +++ b/crates/turborepo-filewatch/src/lib.rs @@ -336,7 +336,16 @@ async fn wait_for_cookie( root: &AbsoluteSystemPath, recv: &mut mpsc::Receiver, ) -> Result<(), WatchError> { - let cookie_path = root.join_component(".turbo-cookie"); + // TODO: should this be passed in? Currently the caller guarantees that the + // directory is empty, but it could be the responsibility of the + // filewatcher... + let cookie_path = root.join_components(&[".turbo", "cookies", ".turbo-cookie"]); + cookie_path.ensure_dir().map_err(|e| { + WatchError::Setup(format!( + "failed to create filesystem cookie dir for {}: {}", + cookie_path, e + )) + })?; cookie_path.create_with_contents("cookie").map_err(|e| { WatchError::Setup(format!("failed to write cookie to {}: {}", cookie_path, e)) })?; From c0c95e5fc22e01f8a3b04a8d96bddf60e3719a3b Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 16 Oct 2023 15:24:26 -0700 Subject: [PATCH 2/5] Move some cookie dir handling internal to filewatcher --- crates/turborepo-filewatch/src/globwatcher.rs | 12 ++- crates/turborepo-filewatch/src/lib.rs | 83 ++++++++++++++----- crates/turborepo-lib/src/daemon/server.rs | 8 +- 3 files changed, 77 insertions(+), 26 deletions(-) diff --git a/crates/turborepo-filewatch/src/globwatcher.rs b/crates/turborepo-filewatch/src/globwatcher.rs index fb05222813df5..d60d085c36253 100644 --- a/crates/turborepo-filewatch/src/globwatcher.rs +++ b/crates/turborepo-filewatch/src/globwatcher.rs @@ -422,7 +422,9 @@ mod test { setup(&repo_root); let cookie_dir = repo_root.join_component(".git"); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let cookie_jar = CookieJar::new(&cookie_dir, Duration::from_secs(2), watcher.subscribe()); let glob_watcher = GlobWatcher::new(&repo_root, cookie_jar, watcher.subscribe()); @@ -500,7 +502,9 @@ mod test { setup(&repo_root); let cookie_dir = repo_root.join_component(".git"); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let cookie_jar = CookieJar::new(&cookie_dir, Duration::from_secs(2), watcher.subscribe()); let glob_watcher = GlobWatcher::new(&repo_root, cookie_jar, watcher.subscribe()); @@ -588,7 +592,9 @@ mod test { setup(&repo_root); let cookie_dir = repo_root.join_component(".git"); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let cookie_jar = CookieJar::new(&cookie_dir, Duration::from_secs(2), watcher.subscribe()); let glob_watcher = GlobWatcher::new(&repo_root, cookie_jar, watcher.subscribe()); diff --git a/crates/turborepo-filewatch/src/lib.rs b/crates/turborepo-filewatch/src/lib.rs index 0df4c456a4121..7ec883790c485 100644 --- a/crates/turborepo-filewatch/src/lib.rs +++ b/crates/turborepo-filewatch/src/lib.rs @@ -86,10 +86,27 @@ pub struct FileSystemWatcher { // dropping the other sender for the broadcast channel, causing all receivers // to be notified of a close. _exit_ch: tokio::sync::oneshot::Sender<()>, + pub cookie_dir: AbsoluteSystemPathBuf, } impl FileSystemWatcher { - pub async fn new(root: &AbsoluteSystemPath) -> Result { + pub async fn new_with_default_cookie_dir( + root: &AbsoluteSystemPath, + ) -> Result { + Self::new(root, &root.join_components(&[".turbo", "cookies"])).await + } + + pub async fn new( + root: &AbsoluteSystemPath, + cookie_dir: &AbsoluteSystemPath, + ) -> Result { + if root.relation_to_path(cookie_dir) != PathRelation::Parent { + return Err(WatchError::Setup(format!( + "Invalid cookie directory: {} does not contain {}", + root, cookie_dir + ))); + } + setup_cookie_dir(cookie_dir)?; let (sender, _) = broadcast::channel(1024); let (send_file_events, mut recv_file_events) = mpsc::channel(1024); let watch_root = root.to_owned(); @@ -99,7 +116,7 @@ impl FileSystemWatcher { let (exit_ch, exit_signal) = tokio::sync::oneshot::channel(); // Ensure we are ready to receive new events, not events for existing state debug!("waiting for initial filesystem cookie"); - wait_for_cookie(root, &mut recv_file_events).await?; + wait_for_cookie(cookie_dir, &mut recv_file_events).await?; tokio::task::spawn(watch_events( watcher, watch_root, @@ -111,6 +128,7 @@ impl FileSystemWatcher { Ok(Self { sender, _exit_ch: exit_ch, + cookie_dir: cookie_dir.to_owned(), }) } @@ -119,6 +137,13 @@ impl FileSystemWatcher { } } +fn setup_cookie_dir(cookie_dir: &AbsoluteSystemPath) -> Result<(), WatchError> { + cookie_dir.create_dir_all().map_err(|e| { + WatchError::Setup(format!("failed to setup cookie dir {}: {}", cookie_dir, e)) + })?; + Ok(()) +} + #[cfg(not(any(feature = "watch_ancestors", feature = "manual_recursive_watch")))] async fn watch_events( _watcher: Backend, @@ -333,19 +358,13 @@ fn make_watcher(event_handler: F) -> Result, ) -> Result<(), WatchError> { // TODO: should this be passed in? Currently the caller guarantees that the // directory is empty, but it could be the responsibility of the // filewatcher... - let cookie_path = root.join_components(&[".turbo", "cookies", ".turbo-cookie"]); - cookie_path.ensure_dir().map_err(|e| { - WatchError::Setup(format!( - "failed to create filesystem cookie dir for {}: {}", - cookie_path, e - )) - })?; + let cookie_path = cookie_dir.join_component(".turbo-cookie"); cookie_path.create_with_contents("cookie").map_err(|e| { WatchError::Setup(format!("failed to write cookie to {}: {}", cookie_path, e)) })?; @@ -450,7 +469,9 @@ mod test { let sibling_path = parent_path.join_component("sibling"); sibling_path.create_dir_all().unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -508,7 +529,9 @@ mod test { let child_path = parent_path.join_component("child"); child_path.create_dir_all().unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -550,7 +573,9 @@ mod test { let child_path = parent_path.join_component("child"); child_path.create_dir_all().unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -579,7 +604,9 @@ mod test { let child_path = parent_path.join_component("child"); child_path.create_dir_all().unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -611,7 +638,9 @@ mod test { let child_path = parent_path.join_component("child"); child_path.create_dir_all().unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -642,7 +671,9 @@ mod test { let child_path = parent_path.join_component("child"); child_path.create_dir_all().unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -683,7 +714,9 @@ mod test { let symlink_path = repo_root.join_component("symlink"); symlink_path.symlink_to_dir(child_path.as_str()).unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -722,7 +755,9 @@ mod test { let symlink_path = repo_root.join_component("symlink"); symlink_path.symlink_to_dir(child_path.as_str()).unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -766,7 +801,9 @@ mod test { let child_path = parent_path.join_component("child"); child_path.create_dir_all().unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -804,7 +841,9 @@ mod test { let child_path = parent_path.join_component("child"); child_path.create_dir_all().unwrap(); - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); let mut recv = watcher.subscribe(); expect_watching(&mut recv, &[&repo_root, &parent_path, &child_path]).await; @@ -825,7 +864,9 @@ mod test { let mut recv = { // create and immediately drop the watcher, which should trigger the exit // channel - let watcher = FileSystemWatcher::new(&repo_root).await.unwrap(); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root) + .await + .unwrap(); watcher.subscribe() }; diff --git a/crates/turborepo-lib/src/daemon/server.rs b/crates/turborepo-lib/src/daemon/server.rs index 17be4752294b3..4df5071490d34 100644 --- a/crates/turborepo-lib/src/daemon/server.rs +++ b/crates/turborepo-lib/src/daemon/server.rs @@ -93,8 +93,12 @@ async fn start_filewatching( cookie_dir: AbsoluteSystemPathBuf, watcher_tx: watch::Sender>>, ) -> Result<(), WatchError> { - let watcher = FileSystemWatcher::new(&repo_root).await?; - let cookie_jar = CookieJar::new(&cookie_dir, Duration::from_millis(100), watcher.subscribe()); + let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root).await?; + let cookie_jar = CookieJar::new( + &watcher.cookie_dir, + Duration::from_millis(100), + watcher.subscribe(), + ); let glob_watcher = GlobWatcher::new(&repo_root, cookie_jar, watcher.subscribe()); // We can ignore failures here, it means the server is shutting down and // receivers have gone out of scope. From 2b5da144714f7ab8abff6fb5c975e6520baa82da Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 16 Oct 2023 16:14:44 -0700 Subject: [PATCH 3/5] Unwind unused cookie_dir --- crates/turborepo-filewatch/src/lib.rs | 12 +++++++++++- crates/turborepo-lib/src/commands/daemon.rs | 15 --------------- crates/turborepo-lib/src/daemon/server.rs | 13 +------------ .../turborepo-paths/src/absolute_system_path.rs | 4 ++++ .../src/absolute_system_path_buf.rs | 4 ---- 5 files changed, 16 insertions(+), 32 deletions(-) diff --git a/crates/turborepo-filewatch/src/lib.rs b/crates/turborepo-filewatch/src/lib.rs index 7ec883790c485..3c62a081b46ae 100644 --- a/crates/turborepo-filewatch/src/lib.rs +++ b/crates/turborepo-filewatch/src/lib.rs @@ -25,7 +25,7 @@ use tracing::{debug, warn}; // linux -> recursive watch, watch ancestors #[cfg(feature = "watch_ancestors")] use turbopath::PathRelation; -use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf}; +use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, PathRelation}; #[cfg(feature = "manual_recursive_watch")] use { notify::{ @@ -93,6 +93,9 @@ impl FileSystemWatcher { pub async fn new_with_default_cookie_dir( root: &AbsoluteSystemPath, ) -> Result { + // We already store logs in .turbo and recommend it be gitignore'd. + // Watchman uses .git, but we can't guarantee that git is present _or_ + // that the turbo root is the same as the git root. Self::new(root, &root.join_components(&[".turbo", "cookies"])).await } @@ -138,6 +141,13 @@ impl FileSystemWatcher { } fn setup_cookie_dir(cookie_dir: &AbsoluteSystemPath) -> Result<(), WatchError> { + // We need to ensure that the cookie directory is cleared out first so + // that we can start over with cookies. + if cookie_dir.exists() { + cookie_dir.remove_dir_all().map_err(|e| { + WatchError::Setup(format!("failed to clear cookie dir {}: {}", cookie_dir, e)) + })?; + } cookie_dir.create_dir_all().map_err(|e| { WatchError::Setup(format!("failed to setup cookie dir {}: {}", cookie_dir, e)) })?; diff --git a/crates/turborepo-lib/src/commands/daemon.rs b/crates/turborepo-lib/src/commands/daemon.rs index dbc605c78cdda..eac4f4abe56d8 100644 --- a/crates/turborepo-lib/src/commands/daemon.rs +++ b/crates/turborepo-lib/src/commands/daemon.rs @@ -174,23 +174,8 @@ pub async fn daemon_server( } CloseReason::Interrupt }); - // We already store logs in .turbo and recommend it be gitignore'd. - // Watchman uses .git, but we can't guarantee that git is present _or_ - // that the turbo root is the same as the git root. - let cookie_dir = base.repo_root.join_components(&[".turbo", "cookies"]); - // We need to ensure that the cookie directory is cleared out first so - // that we can start over with cookies. - if cookie_dir.exists() { - cookie_dir - .remove_dir_all() - .map_err(|e| DaemonError::CookieDir(e, cookie_dir.clone()))?; - } - cookie_dir - .create_dir_all() - .map_err(|e| DaemonError::CookieDir(e, cookie_dir.clone()))?; let reason = crate::daemon::serve( &base.repo_root, - cookie_dir, &daemon_root, log_file, timeout, diff --git a/crates/turborepo-lib/src/daemon/server.rs b/crates/turborepo-lib/src/daemon/server.rs index 4df5071490d34..4d93de435eb3f 100644 --- a/crates/turborepo-lib/src/daemon/server.rs +++ b/crates/turborepo-lib/src/daemon/server.rs @@ -90,7 +90,6 @@ impl From for tonic::Status { async fn start_filewatching( repo_root: AbsoluteSystemPathBuf, - cookie_dir: AbsoluteSystemPathBuf, watcher_tx: watch::Sender>>, ) -> Result<(), WatchError> { let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root).await?; @@ -117,7 +116,6 @@ const REQUEST_TIMEOUT: Duration = Duration::from_millis(100); /// to be wired to signal handling. pub async fn serve( repo_root: &AbsoluteSystemPath, - cookie_dir: AbsoluteSystemPathBuf, daemon_root: &AbsoluteSystemPath, log_file: AbsoluteSystemPathBuf, timeout: Duration, @@ -147,7 +145,7 @@ where // all references are dropped. let fw_shutdown = trigger_shutdown.clone(); let fw_handle = tokio::task::spawn(async move { - if let Err(e) = start_filewatching(watcher_repo_root, cookie_dir, watcher_tx).await { + if let Err(e) = start_filewatching(watcher_repo_root, watcher_tx).await { error!("filewatching failed to start: {}", e); let _ = fw_shutdown.send(()).await; } @@ -440,8 +438,6 @@ mod test { .unwrap(); let repo_root = path.join_component("repo"); - let cookie_dir = repo_root.join_component(".git"); - cookie_dir.create_dir_all().unwrap(); let daemon_root = path.join_component("daemon"); let log_file = daemon_root.join_component("log"); tracing::info!("start"); @@ -453,7 +449,6 @@ mod test { let handle = tokio::task::spawn(async move { serve( &repo_root, - cookie_dir, &daemon_root, log_file, Duration::from_secs(60 * 60), @@ -488,8 +483,6 @@ mod test { .unwrap(); let repo_root = path.join_component("repo"); - let cookie_dir = repo_root.join_component(".git"); - cookie_dir.create_dir_all().unwrap(); let daemon_root = path.join_component("daemon"); let log_file = daemon_root.join_component("log"); @@ -500,7 +493,6 @@ mod test { let exit_signal = rx.map(|_result| CloseReason::Interrupt); let close_reason = serve( &repo_root, - cookie_dir, &daemon_root, log_file, Duration::from_millis(5), @@ -530,8 +522,6 @@ mod test { .unwrap(); let repo_root = path.join_component("repo"); - let cookie_dir = repo_root.join_component(".git"); - cookie_dir.create_dir_all().unwrap(); let daemon_root = path.join_component("daemon"); daemon_root.create_dir_all().unwrap(); let log_file = daemon_root.join_component("log"); @@ -544,7 +534,6 @@ mod test { let repo_root = server_repo_root; serve( &repo_root, - cookie_dir, &daemon_root, log_file, Duration::from_secs(60 * 60), diff --git a/crates/turborepo-paths/src/absolute_system_path.rs b/crates/turborepo-paths/src/absolute_system_path.rs index 47aa697dbcd85..0e23e676e55b0 100644 --- a/crates/turborepo-paths/src/absolute_system_path.rs +++ b/crates/turborepo-paths/src/absolute_system_path.rs @@ -122,6 +122,10 @@ impl AbsoluteSystemPath { self.0.as_str().as_bytes() } + pub fn exists(&self) -> bool { + self.0.exists() + } + pub fn ancestors(&self) -> impl Iterator { self.0.ancestors().map(Self::new_unchecked) } diff --git a/crates/turborepo-paths/src/absolute_system_path_buf.rs b/crates/turborepo-paths/src/absolute_system_path_buf.rs index b481b73b8628e..985d3b2cf3652 100644 --- a/crates/turborepo-paths/src/absolute_system_path_buf.rs +++ b/crates/turborepo-paths/src/absolute_system_path_buf.rs @@ -192,10 +192,6 @@ impl AbsoluteSystemPathBuf { self.0.file_name() } - pub fn exists(&self) -> bool { - self.0.exists() - } - pub fn try_exists(&self) -> Result { // try_exists is an experimental API and not yet in fs_err Ok(std::fs::try_exists(&self.0)?) From bb61470888cc17e91da181381082c7c2bc531340 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 16 Oct 2023 16:22:20 -0700 Subject: [PATCH 4/5] Fix use statements --- crates/turborepo-filewatch/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/turborepo-filewatch/src/lib.rs b/crates/turborepo-filewatch/src/lib.rs index 3c62a081b46ae..c4a1e56718edf 100644 --- a/crates/turborepo-filewatch/src/lib.rs +++ b/crates/turborepo-filewatch/src/lib.rs @@ -10,6 +10,8 @@ use std::{ time::Duration, }; +// windows -> no recursive watch, watch ancestors +// linux -> recursive watch, watch ancestors // macos -> custom watcher impl in fsevents, no recursive watch, no watching ancestors #[cfg(target_os = "macos")] use fsevent::FsEventWatcher; @@ -21,10 +23,6 @@ use notify::{Event, EventHandler, RecursiveMode, Watcher}; use thiserror::Error; use tokio::sync::{broadcast, mpsc}; use tracing::{debug, warn}; -// windows -> no recursive watch, watch ancestors -// linux -> recursive watch, watch ancestors -#[cfg(feature = "watch_ancestors")] -use turbopath::PathRelation; use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, PathRelation}; #[cfg(feature = "manual_recursive_watch")] use { From d265e0023be0c6470d9721ca318559340a893b69 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Tue, 17 Oct 2023 11:26:04 -0700 Subject: [PATCH 5/5] cooke_dir -> cookie_dir() --- crates/turborepo-filewatch/src/lib.rs | 6 +++++- crates/turborepo-lib/src/daemon/server.rs | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/turborepo-filewatch/src/lib.rs b/crates/turborepo-filewatch/src/lib.rs index c4a1e56718edf..d4ccfd7e40025 100644 --- a/crates/turborepo-filewatch/src/lib.rs +++ b/crates/turborepo-filewatch/src/lib.rs @@ -84,7 +84,7 @@ pub struct FileSystemWatcher { // dropping the other sender for the broadcast channel, causing all receivers // to be notified of a close. _exit_ch: tokio::sync::oneshot::Sender<()>, - pub cookie_dir: AbsoluteSystemPathBuf, + cookie_dir: AbsoluteSystemPathBuf, } impl FileSystemWatcher { @@ -136,6 +136,10 @@ impl FileSystemWatcher { pub fn subscribe(&self) -> broadcast::Receiver> { self.sender.subscribe() } + + pub fn cookie_dir(&self) -> &AbsoluteSystemPath { + &self.cookie_dir + } } fn setup_cookie_dir(cookie_dir: &AbsoluteSystemPath) -> Result<(), WatchError> { diff --git a/crates/turborepo-lib/src/daemon/server.rs b/crates/turborepo-lib/src/daemon/server.rs index 4d93de435eb3f..20341804c6132 100644 --- a/crates/turborepo-lib/src/daemon/server.rs +++ b/crates/turborepo-lib/src/daemon/server.rs @@ -94,7 +94,7 @@ async fn start_filewatching( ) -> Result<(), WatchError> { let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root).await?; let cookie_jar = CookieJar::new( - &watcher.cookie_dir, + watcher.cookie_dir(), Duration::from_millis(100), watcher.subscribe(), );