diff --git a/crates/memofs/src/lib.rs b/crates/memofs/src/lib.rs index bb6ae5a1..e9ce26c4 100644 --- a/crates/memofs/src/lib.rs +++ b/crates/memofs/src/lib.rs @@ -584,9 +584,10 @@ impl VfsLock<'_> { #[cfg(test)] mod test { - use crate::{InMemoryFs, StdBackend, Vfs, VfsSnapshot}; + use crate::{InMemoryFs, StdBackend, Vfs, VfsEvent, VfsSnapshot}; use std::io; - use std::path::PathBuf; + use std::path::{Path, PathBuf}; + use std::time::{Duration, Instant}; /// https://github.com/rojo-rbx/rojo/issues/899 #[test] @@ -687,6 +688,73 @@ mod test { ); } + fn saw_event_for(vfs: &Vfs, path: &Path, timeout: Duration) -> bool { + let receiver = vfs.event_receiver(); + let deadline = Instant::now() + timeout; + loop { + match receiver.recv_deadline(deadline) { + Ok(VfsEvent::Create(event_path) | VfsEvent::Write(event_path)) + if event_path == path => + { + return true + } + Ok(_) => continue, + Err(_) => return false, + } + } + } + + /// Package managers replace a package by moving its directory aside and + /// creating a new one at the same path. The move reaches the change + /// processor as a `Remove` for that path, which it commits after the new + /// directory already exists and is being watched. Only notify's inotify + /// backend splits a recursive watch into per-directory entries shared by + /// every root, so this fails before the fix on Linux alone. + fn assert_watches_directory_recreated_at_removed_path(watch_package_first: bool) { + let dir = tempfile::tempdir().unwrap(); + let root = dunce::canonicalize(dir.path()).unwrap(); + let package = root.join("package"); + fs_err::create_dir_all(package.join("src")).unwrap(); + + let vfs = Vfs::new(StdBackend::new().unwrap()); + if watch_package_first { + vfs.read_dir(&package).unwrap(); + } + vfs.read_dir(&root).unwrap(); + + fs_err::rename(&package, root.join(".package-old")).unwrap(); + fs_err::create_dir_all(package.join("src")).unwrap(); + // Files written before the watcher reaches a new directory raise no + // events, so rewrite a probe until one arrives. + let probe = package.join("src").join("probe.luau"); + let watched = (0..50).any(|_| { + fs_err::write(&probe, "return {}").unwrap(); + saw_event_for(&vfs, &probe, Duration::from_millis(100)) + }); + assert!(watched, "the recreated directory was never watched"); + + vfs.commit_event(&VfsEvent::Remove(package.clone())) + .unwrap(); + + let module = package.join("src").join("module.luau"); + fs_err::write(&module, "return {}").unwrap(); + assert!( + saw_event_for(&vfs, &module, Duration::from_secs(5)), + "no event for {} after the removal of its old directory was committed", + module.display() + ); + } + + #[test] + fn std_backend_watches_directory_recreated_at_removed_path() { + assert_watches_directory_recreated_at_removed_path(false); + } + + #[test] + fn std_backend_watches_recreated_directory_watched_before_its_parent() { + assert_watches_directory_recreated_at_removed_path(true); + } + #[test] fn canonicalize_std_backend_missing_errors() { let dir = tempfile::tempdir().unwrap(); diff --git a/rojo-test/serve-tests/recreated_directory/default.project.json b/rojo-test/serve-tests/recreated_directory/default.project.json new file mode 100644 index 00000000..deaceca2 --- /dev/null +++ b/rojo-test/serve-tests/recreated_directory/default.project.json @@ -0,0 +1,12 @@ +{ + "name": "recreated_directory", + "tree": { + "$className": "Folder", + "Loader": { + "$path": "deps/package" + }, + "Packages": { + "$path": "deps" + } + } +} diff --git a/rojo-test/serve-tests/recreated_directory/deps/package/Old.luau b/rojo-test/serve-tests/recreated_directory/deps/package/Old.luau new file mode 100644 index 00000000..bf545338 --- /dev/null +++ b/rojo-test/serve-tests/recreated_directory/deps/package/Old.luau @@ -0,0 +1 @@ +return "old" diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index 0d6f75cc..f8910766 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -1,13 +1,20 @@ -use std::fs; +use std::{ + collections::HashMap, + fs, thread, + time::{Duration, Instant}, +}; use insta::{assert_snapshot, assert_yaml_snapshot, with_settings}; -use rbx_dom_weak::types::Ref; +use rbx_dom_weak::{ + types::{Ref, Variant}, + ustr, +}; use reqwest::StatusCode; use tempfile::tempdir; use crate::rojo_test::{ internable::InternAndRedact, - serve_util::{deserialize_msgpack, run_serve_test, serialize_to_xml_model}, + serve_util::{deserialize_msgpack, run_serve_test, serialize_to_xml_model, TestServeSession}, }; use librojo::web_api::{SerializeResponse, SocketPacketType}; @@ -276,6 +283,90 @@ fn move_folder_of_stuff() { }); } +#[test] +fn recreated_directory() { + run_serve_test("recreated_directory", |session, _redactions| { + let root_id = session.get_api_rojo().unwrap().root_instance_id; + let package = session.path().join("deps/package"); + + fs::write(package.join("Old.luau"), "return 0").unwrap(); + wait_for_sources(&session, root_id, |sources| { + sources["Loader/Old"] == "return 0" && sources["Packages/package/Old"] == "return 0" + }); + + // Package managers replace a package by moving its directory aside and + // creating a new one at the same path. `Loader` sorts before + // `Packages`, so the package directory is watched before its parent. + // The moved copy is kept, because deleting it hits a separate panic in + // the change processor. + fs::rename(&package, session.path().join("deps/.package-old")).unwrap(); + fs::create_dir(&package).unwrap(); + wait_for_sources(&session, root_id, |sources| { + sources.contains_key("Packages/.package-old/Old") + }); + + let module = package.join("New.luau"); + fs::write(&module, "return 1").unwrap(); + wait_for_sources(&session, root_id, |sources| { + !sources.contains_key("Loader/Old") + && sources.contains_key("Loader/New") + && sources.contains_key("Packages/package/New") + }); + + // Creating the directory can still be pending when `New` is written, + // and handling it rereads the directory. Only a live watch reports an + // edit to a file that already exists. + fs::write(&module, "return 2").unwrap(); + wait_for_sources(&session, root_id, |sources| { + sources["Loader/New"] == "return 2" && sources["Packages/package/New"] == "return 2" + }); + }); +} + +/// Polls the served tree until `ready` accepts it. The tree is given as a map +/// from each instance's path below `root_id`, written as names joined by `/`, +/// to its `Source`, or an empty string if it has none. +fn wait_for_sources( + session: &TestServeSession, + root_id: Ref, + ready: impl Fn(&HashMap) -> bool, +) { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + let read_response = session.get_api_read(root_id).unwrap(); + let instances = &read_response.instances; + let sources: HashMap = instances + .values() + .filter(|instance| instance.id != root_id) + .map(|instance| { + let mut names = vec![instance.name.to_string()]; + let mut parent = instance.parent; + while parent != root_id { + let ancestor = &instances[&parent]; + names.push(ancestor.name.to_string()); + parent = ancestor.parent; + } + names.reverse(); + + let source = match instance.properties.get(&ustr("Source")).map(AsRef::as_ref) { + Some(Variant::String(source)) => source.clone(), + _ => String::new(), + }; + (names.join("/"), source) + }) + .collect(); + + if ready(&sources) { + return; + } + assert!( + Instant::now() < deadline, + "served tree never reached the expected state: {sources:?}" + ); + thread::sleep(Duration::from_millis(100)); + } +} + #[test] fn empty_json_model() { run_serve_test("empty_json_model", |session, mut redactions| {