diff --git a/eden/scm/lib/workingcopy/src/watchmanfs/treestate.rs b/eden/scm/lib/workingcopy/src/watchmanfs/treestate.rs index b66426143f..cc68c2f9d1 100644 --- a/eden/scm/lib/workingcopy/src/watchmanfs/treestate.rs +++ b/eden/scm/lib/workingcopy/src/watchmanfs/treestate.rs @@ -10,7 +10,6 @@ use std::sync::Arc; use anyhow::anyhow; use anyhow::Result; -use parking_lot::Mutex; use pathmatcher::Matcher; use repolock::RepoLocker; use treestate::dirstate; @@ -26,147 +25,110 @@ use watchman_client::prelude::*; use crate::util::walk_treestate; -pub trait WatchmanTreeStateWrite { - fn mark_needs_check(&mut self, path: &RepoPathBuf) -> Result; - - fn clear_needs_check(&mut self, path: &RepoPathBuf) -> Result; - - fn set_clock(&mut self, clock: Clock) -> Result<()>; - - fn flush(self, locker: &RepoLocker) -> Result<()>; -} - -pub trait WatchmanTreeStateRead { - fn list_needs_check( - &mut self, - matcher: Arc, - ) -> Result<(Vec, Vec)>; - - fn get_clock(&self) -> Result>; -} - -#[derive(Clone)] -pub struct WatchmanTreeState<'a> { - pub treestate: Arc>, - pub root: &'a Path, -} - -impl WatchmanTreeStateWrite for WatchmanTreeState<'_> { - fn mark_needs_check(&mut self, path: &RepoPathBuf) -> Result { - let mut treestate = self.treestate.lock(); - - let state = treestate.get(path)?; - let filestate = match state { - Some(filestate) => { - let filestate = filestate.clone(); - if filestate.state.intersects(StateFlags::NEED_CHECK) { - // It's already marked need_check, so return early so we don't mutate the - // treestate. - return Ok(false); - } - FileStateV2 { - state: filestate.state | StateFlags::NEED_CHECK, - ..filestate - } - } - // The file is currently untracked - None => FileStateV2 { - state: StateFlags::NEED_CHECK, - mode: 0o666, - size: -1, - mtime: -1, - copied: None, - }, - }; - treestate.insert(path, &filestate)?; - Ok(true) - } - - fn clear_needs_check(&mut self, path: &RepoPathBuf) -> Result { - let mut treestate = self.treestate.lock(); - - let state = treestate.get(path)?; - if let Some(filestate) = state { +pub fn mark_needs_check(ts: &mut TreeState, path: &RepoPathBuf) -> Result { + let state = ts.get(path)?; + let filestate = match state { + Some(filestate) => { let filestate = filestate.clone(); - if !filestate.state.intersects(StateFlags::NEED_CHECK) { - // It's already clear. + if filestate.state.intersects(StateFlags::NEED_CHECK) { + // It's already marked need_check, so return early so we don't mutate the + // treestate. return Ok(false); } - let filestate = FileStateV2 { - state: filestate.state & !StateFlags::NEED_CHECK, + FileStateV2 { + state: filestate.state | StateFlags::NEED_CHECK, ..filestate - }; - treestate.insert(path, &filestate)?; - return Ok(true); + } } - Ok(false) - } + // The file is currently untracked + None => FileStateV2 { + state: StateFlags::NEED_CHECK, + mode: 0o666, + size: -1, + mtime: -1, + copied: None, + }, + }; + ts.insert(path, &filestate)?; + Ok(true) +} - fn set_clock(&mut self, clock: Clock) -> Result<()> { - let mut treestate = self.treestate.lock(); - - let clock_string = match clock { - Clock::Spec(ClockSpec::StringClock(string)) => Ok(string), - clock => Err(anyhow!( - "Watchman implementation only handles opaque string type. Got the following clock instead: {:?}", - clock - )), - }?; - - let mut metadata_buf = treestate.get_metadata(); - let mut metadata = Metadata::deserialize(&mut metadata_buf)?; - metadata.0.insert("clock".to_string(), clock_string); - let mut metadata_buf = vec![]; - metadata.serialize(&mut metadata_buf)?; - treestate.set_metadata(&metadata_buf); - - Ok(()) - } - - fn flush(self, locker: &RepoLocker) -> Result<()> { - match dirstate::flush(&self.root, &mut self.treestate.lock(), locker) { - Ok(()) => Ok(()), - // If the dirstate was changed before we flushed, that's ok. Let the other write win - // since writes during status are just optimizations. - Err(e) => match e.downcast_ref::() { - Some(e) if *e == ErrorKind::TreestateOutOfDate => Ok(()), - _ => Err(e), - }, +pub fn clear_needs_check(ts: &mut TreeState, path: &RepoPathBuf) -> Result { + let state = ts.get(path)?; + if let Some(filestate) = state { + let filestate = filestate.clone(); + if !filestate.state.intersects(StateFlags::NEED_CHECK) { + // It's already clear. + return Ok(false); } + let filestate = FileStateV2 { + state: filestate.state & !StateFlags::NEED_CHECK, + ..filestate + }; + ts.insert(path, &filestate)?; + return Ok(true); + } + Ok(false) +} + +pub fn set_clock(ts: &mut TreeState, clock: Clock) -> Result<()> { + let clock_string = match clock { + Clock::Spec(ClockSpec::StringClock(string)) => Ok(string), + clock => Err(anyhow!( + "Watchman implementation only handles opaque string type. Got the following clock instead: {:?}", + clock + )), + }?; + + let mut metadata_buf = ts.get_metadata(); + let mut metadata = Metadata::deserialize(&mut metadata_buf)?; + metadata.0.insert("clock".to_string(), clock_string); + let mut metadata_buf = vec![]; + metadata.serialize(&mut metadata_buf)?; + ts.set_metadata(&metadata_buf); + + Ok(()) +} + +pub fn flush(root: &Path, ts: &mut TreeState, locker: &RepoLocker) -> Result<()> { + match dirstate::flush(root, ts, locker) { + Ok(()) => Ok(()), + // If the dirstate was changed before we flushed, that's ok. Let the other write win + // since writes during status are just optimizations. + Err(e) => match e.downcast_ref::() { + Some(e) if *e == ErrorKind::TreestateOutOfDate => Ok(()), + _ => Err(e), + }, } } -impl WatchmanTreeStateRead for WatchmanTreeState<'_> { - fn list_needs_check( - &mut self, - matcher: Arc, - ) -> Result<(Vec, Vec)> { - let mut needs_check = Vec::new(); +pub fn list_needs_check( + ts: &mut TreeState, + matcher: Arc, +) -> Result<(Vec, Vec)> { + let mut needs_check = Vec::new(); - let parse_errs = walk_treestate( - &mut self.treestate.lock(), - matcher, - StateFlags::NEED_CHECK, - StateFlags::empty(), - |path, _state| { - needs_check.push(path); - Ok(()) - }, - )?; + let parse_errs = walk_treestate( + ts, + matcher, + StateFlags::NEED_CHECK, + StateFlags::empty(), + |path, _state| { + needs_check.push(path); + Ok(()) + }, + )?; - Ok((needs_check, parse_errs)) - } + Ok((needs_check, parse_errs)) +} - fn get_clock(&self) -> Result> { - let treestate = self.treestate.lock(); - - let mut metadata_buf = treestate.get_metadata(); - let metadata = Metadata::deserialize(&mut metadata_buf)?; - Ok(metadata - .0 - .get(&"clock".to_string()) - .map(|clock| Clock::Spec(ClockSpec::StringClock(clock.clone())))) - } +pub fn get_clock(ts: &mut TreeState) -> Result> { + let mut metadata_buf = ts.get_metadata(); + let metadata = Metadata::deserialize(&mut metadata_buf)?; + Ok(metadata + .0 + .get(&"clock".to_string()) + .map(|clock| Clock::Spec(ClockSpec::StringClock(clock.clone())))) } #[cfg(test)] @@ -199,12 +161,7 @@ mod tests { false, )); - let mut wm_ts = WatchmanTreeState { - treestate: Arc::new(Mutex::new(ts)), - root: "/dev/null".as_ref(), - }; - - let (needs_check, _) = wm_ts.list_needs_check(matcher)?; + let (needs_check, _) = list_needs_check(&mut ts, matcher)?; assert_eq!( needs_check, vec![RepoPathBuf::from_string("include_me".to_string())?], diff --git a/eden/scm/lib/workingcopy/src/watchmanfs/watchmanfs.rs b/eden/scm/lib/workingcopy/src/watchmanfs/watchmanfs.rs index 6efcecfee2..b243061587 100644 --- a/eden/scm/lib/workingcopy/src/watchmanfs/watchmanfs.rs +++ b/eden/scm/lib/workingcopy/src/watchmanfs/watchmanfs.rs @@ -7,6 +7,7 @@ use std::collections::HashSet; use std::io::Write; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -29,8 +30,10 @@ use types::RepoPathBuf; use vfs::VFS; use watchman_client::prelude::*; -use super::treestate::WatchmanTreeState; -use super::treestate::WatchmanTreeStateWrite; +use super::treestate::clear_needs_check; +use super::treestate::flush; +use super::treestate::mark_needs_check; +use super::treestate::set_clock; use crate::filechangedetector::ArcReadFileContents; use crate::filechangedetector::FileChangeDetector; use crate::filechangedetector::FileChangeDetectorTrait; @@ -38,7 +41,8 @@ use crate::filechangedetector::FileChangeResult; use crate::filechangedetector::ResolvedFileChangeResult; use crate::filesystem::PendingChangeResult; use crate::filesystem::PendingChanges; -use crate::watchmanfs::treestate::WatchmanTreeStateRead; +use crate::watchmanfs::treestate::get_clock; +use crate::watchmanfs::treestate::list_needs_check; use crate::workingcopy::WorkingCopy; type ArcReadTreeManifest = Arc; @@ -124,12 +128,9 @@ impl PendingChanges for WatchmanFileSystem { config: &dyn Config, io: &IO, ) -> Result>>> { - let mut watchman_ts = WatchmanTreeState { - treestate: self.treestate.clone(), - root: self.vfs.root(), - }; + let ts = &mut *self.treestate.lock(); - let prev_clock = watchman_ts.get_clock()?; + let prev_clock = get_clock(ts)?; let result = async_runtime::block_on(self.query_result(WatchmanConfig { clock: prev_clock.clone(), @@ -160,8 +161,7 @@ impl PendingChanges for WatchmanFileSystem { .as_ref() .map_or(false, |f| f.len() > file_change_threshold); - let manifests = - WorkingCopy::current_manifests(&self.treestate.lock(), &self.tree_resolver)?; + let manifests = WorkingCopy::current_manifests(ts, &self.tree_resolver)?; let file_change_detector = FileChangeDetector::new( self.vfs.clone(), @@ -170,7 +170,7 @@ impl PendingChanges for WatchmanFileSystem { self.store.clone(), ); - let (ts_needs_check, ts_errors) = watchman_ts.list_needs_check(matcher)?; + let (ts_needs_check, ts_errors) = list_needs_check(ts, matcher)?; let mut wm_errors: Vec = Vec::new(); let wm_needs_check: Vec = result @@ -190,7 +190,7 @@ impl PendingChanges for WatchmanFileSystem { let mut pending_changes = detect_changes( file_change_detector, - &mut self.treestate.lock(), + ts, ts_needs_check, wm_needs_check, result.clock, @@ -205,7 +205,7 @@ impl PendingChanges for WatchmanFileSystem { .map(|e| Err(anyhow!(e))), ); - pending_changes.persist(watchman_ts, should_update_clock, &self.locker)?; + pending_changes.persist(self.vfs.root(), ts, should_update_clock, &self.locker)?; Ok(Box::new(pending_changes.into_iter())) } @@ -317,13 +317,14 @@ impl WatchmanPendingChanges { #[tracing::instrument(skip_all)] pub fn persist( &mut self, - mut treestate: impl WatchmanTreeStateWrite, + root: &Path, + ts: &mut TreeState, should_update_clock: bool, locker: &RepoLocker, ) -> Result<()> { let mut wrote = false; for path in self.needs_clear.iter() { - match treestate.clear_needs_check(path) { + match clear_needs_check(ts, path) { Ok(v) => wrote |= v, Err(e) => // We can still build a valid result if we fail to clear the @@ -336,16 +337,16 @@ impl WatchmanPendingChanges { } for path in self.needs_mark.iter() { - wrote |= treestate.mark_needs_check(path)?; + wrote |= mark_needs_check(ts, path)?; } // If the treestate is already dirty, we're going to write it anyway, so let's go ahead and // update the clock while we're at it. if should_update_clock || wrote { - treestate.set_clock(self.clock.clone())?; + set_clock(ts, self.clock.clone())?; } - treestate.flush(locker) + flush(root, ts, locker) } }