diff --git a/.gitignore b/.gitignore index f6839452..d77fcf54 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ /site /target -/server/scratch +/scratch-project **/*.rs.bk /generate-docs.run \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d5894d2..b15b44eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Rojo Changelog ## [Unreleased] +* Fixed in-memory filesystem not handling out-of-order filesystem change events ## [0.5.0 Alpha 1](https://github.com/LPGhatguy/rojo/releases/tag/v0.5.0-alpha.1) (January 14, 2019) * Changed plugin UI to be way prettier diff --git a/server/src/fs_watcher.rs b/server/src/fs_watcher.rs index 84741941..178994d2 100644 --- a/server/src/fs_watcher.rs +++ b/server/src/fs_watcher.rs @@ -4,7 +4,7 @@ use std::{ thread, }; -use log::info; +use log::trace; use notify::{ self, DebouncedEvent, @@ -23,6 +23,8 @@ const WATCH_TIMEOUT: Duration = Duration::from_millis(100); fn handle_event(imfs: &Mutex, rbx_session: &Mutex, event: DebouncedEvent) { match event { DebouncedEvent::Create(path) => { + trace!("Path created: {}", path.display()); + { let mut imfs = imfs.lock().unwrap(); imfs.path_created(&path).unwrap(); @@ -34,6 +36,8 @@ fn handle_event(imfs: &Mutex, rbx_session: &Mutex, event: Debo } }, DebouncedEvent::Write(path) => { + trace!("Path created: {}", path.display()); + { let mut imfs = imfs.lock().unwrap(); imfs.path_updated(&path).unwrap(); @@ -45,6 +49,8 @@ fn handle_event(imfs: &Mutex, rbx_session: &Mutex, event: Debo } }, DebouncedEvent::Remove(path) => { + trace!("Path removed: {}", path.display()); + { let mut imfs = imfs.lock().unwrap(); imfs.path_removed(&path).unwrap(); @@ -56,6 +62,8 @@ fn handle_event(imfs: &Mutex, rbx_session: &Mutex, event: Debo } }, DebouncedEvent::Rename(from_path, to_path) => { + trace!("Path rename: {} to {}", from_path.display(), to_path.display()); + { let mut imfs = imfs.lock().unwrap(); imfs.path_moved(&from_path, &to_path).unwrap(); @@ -66,7 +74,9 @@ fn handle_event(imfs: &Mutex, rbx_session: &Mutex, event: Debo rbx_session.path_renamed(&from_path, &to_path); } }, - _ => {}, + other => { + trace!("Unhandled FS event: {:?}", other); + }, } } @@ -100,11 +110,11 @@ impl FsWatcher { let root_path = root_path.clone(); thread::spawn(move || { - info!("Watcher thread ({}) started", root_path.display()); + trace!("Watcher thread ({}) started", root_path.display()); while let Ok(event) = watch_rx.recv() { handle_event(&imfs, &rbx_session, event); } - info!("Watcher thread ({}) stopped", root_path.display()); + trace!("Watcher thread ({}) stopped", root_path.display()); }); } } diff --git a/server/src/imfs.rs b/server/src/imfs.rs index 08fe7157..f2c611e0 100644 --- a/server/src/imfs.rs +++ b/server/src/imfs.rs @@ -1,6 +1,6 @@ use std::{ collections::{HashMap, HashSet}, - path::{Path, PathBuf}, + path::{self, Path, PathBuf}, fs, io, }; @@ -24,9 +24,12 @@ fn add_sync_points(imfs: &mut Imfs, project_node: &ProjectNode) -> io::Result<() Ok(()) } -/// The in-memory filesystem keeps a mirror of all files being watcher by Rojo +/// The in-memory filesystem keeps a mirror of all files being watched by Rojo /// in order to deduplicate file changes in the case of bidirectional syncing /// from Roblox Studio. +/// +/// It also enables Rojo to quickly generate React-like snapshots to make +/// reasoning about instances and how they relate to files easier. #[derive(Debug, Clone)] pub struct Imfs { items: HashMap, @@ -66,21 +69,21 @@ impl Imfs { self.roots.insert(path.to_path_buf()); - self.read_from_disk(path) + self.descend_and_read_from_disk(path) } pub fn path_created(&mut self, path: &Path) -> io::Result<()> { debug_assert!(path.is_absolute()); debug_assert!(self.is_within_roots(path)); - self.read_from_disk(path) + self.descend_and_read_from_disk(path) } pub fn path_updated(&mut self, path: &Path) -> io::Result<()> { debug_assert!(path.is_absolute()); debug_assert!(self.is_within_roots(path)); - self.read_from_disk(path) + self.descend_and_read_from_disk(path) } pub fn path_removed(&mut self, path: &Path) -> io::Result<()> { @@ -132,9 +135,7 @@ impl Imfs { Some(ImfsItem::Directory(directory)) => { directory.children.remove(child); }, - _ => { - panic!("Tried to unlink child of path that wasn't a directory!"); - }, + _ => {}, } } @@ -153,6 +154,38 @@ impl Imfs { } } + fn descend_and_read_from_disk(&mut self, path: &Path) -> io::Result<()> { + let root_path = self.get_root_path(path) + .expect("Tried to mkdirp for path that wasn't within roots!"); + + // If this path is a root, we should read the entire thing. + if root_path == path { + self.read_from_disk(path)?; + return Ok(()); + } + + let relative_path = path.strip_prefix(root_path).unwrap(); + let mut current_path = root_path.to_path_buf(); + + for component in relative_path.components() { + match component { + path::Component::Normal(name) => { + let next_path = current_path.join(name); + + if self.items.contains_key(&next_path) { + current_path = next_path; + } else { + self.read_from_disk(¤t_path)?; + break; + } + }, + _ => unreachable!(), + } + } + + Ok(()) + } + fn read_from_disk(&mut self, path: &Path) -> io::Result<()> { let metadata = fs::metadata(path)?; @@ -195,6 +228,16 @@ impl Imfs { } } + fn get_root_path<'a>(&'a self, path: &Path) -> Option<&'a Path> { + for root_path in &self.roots { + if path.starts_with(root_path) { + return Some(root_path) + } + } + + None + } + fn is_within_roots(&self, path: &Path) -> bool { for root_path in &self.roots { if path.starts_with(root_path) { diff --git a/server/test-scratch-project b/server/test-scratch-project deleted file mode 100755 index f227ef13..00000000 --- a/server/test-scratch-project +++ /dev/null @@ -1,18 +0,0 @@ -#!/bin/sh - -set -e - -if [ ! -d "../test-projects/$1" ] -then - echo "Pick a project that exists!" - exit 1 -fi - -if [ -d "scratch" ] -then - rm -rf scratch -fi - -mkdir -p scratch -cp -r "../test-projects/$1" scratch -cargo run -- serve "scratch/$1" \ No newline at end of file diff --git a/server/tests/imfs.rs b/server/tests/imfs.rs index 39ffafc9..f9f55393 100644 --- a/server/tests/imfs.rs +++ b/server/tests/imfs.rs @@ -232,6 +232,16 @@ fn adding_folder() -> io::Result<()> { FsEvent::Created(file1_path.clone()), FsEvent::Created(file2_path.clone()), ], + vec![ + FsEvent::Created(file1_path.clone()), + FsEvent::Created(file2_path.clone()), + FsEvent::Created(folder_path.clone()), + ], + vec![ + FsEvent::Created(file1_path.clone()), + FsEvent::Created(folder_path.clone()), + FsEvent::Created(file2_path.clone()), + ], ]; for events in &possible_event_sequences { @@ -294,6 +304,10 @@ fn removing_folder() -> io::Result<()> { FsEvent::Removed(resources.baz_path.clone()), FsEvent::Removed(resources.foo_path.clone()), ], + vec![ + FsEvent::Removed(resources.foo_path.clone()), + FsEvent::Removed(resources.baz_path.clone()), + ], ]; for events in &possible_event_sequences { diff --git a/test-scratch-project b/test-scratch-project new file mode 100644 index 00000000..da79446a --- /dev/null +++ b/test-scratch-project @@ -0,0 +1,21 @@ +#!/bin/sh + +# Copies a project from 'test-projects' into a folder that can be messed with +# without accidentally checking the results into version control. + +set -e + +if [ ! -d "test-projects/$1" ] +then + echo "Pick a project that exists!" + exit 1 +fi + +if [ -d "scratch-project/$1" ] +then + rm -rf "scratch-project/$1" +fi + +mkdir -p scratch-project +cp -r "test-projects/$1" scratch-project +cargo run -- serve "scratch-project/$1" \ No newline at end of file