From 691efad3300ac1f8f7fb49d91048f1440e220cb3 Mon Sep 17 00:00:00 2001 From: CheckPickerUpper <194722943+CheckPickerUpper@users.noreply.github.com> Date: Wed, 7 Oct 2026 16:53:22 -0400 Subject: [PATCH] Avoid watchers for one-shot builds and preserve watcher errors (AI-assisted) --- AI-canary.txt | 1 + CHANGELOG.md | 1 + crates/memofs/CHANGELOG.md | 3 ++ crates/memofs/src/lib.rs | 34 +++++++++++++++ crates/memofs/src/std_backend.rs | 73 ++++++++++++++++++++++++++------ src/cli/build.rs | 7 ++- tests/tests/build.rs | 55 ++++++++++++++++++++++++ 7 files changed, 160 insertions(+), 14 deletions(-) create mode 100644 AI-canary.txt diff --git a/AI-canary.txt b/AI-canary.txt new file mode 100644 index 00000000..780a5852 --- /dev/null +++ b/AI-canary.txt @@ -0,0 +1 @@ +This pull request is AI generated diff --git a/CHANGELOG.md b/CHANGELOG.md index 3943ae5f..83629238 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Making a new release? Simply add the new header with the version and date undern ## Unreleased +* Fixed `rojo build` allocating a filesystem watcher without `--watch`, which could fail when inotify instances were exhausted. Watcher errors now preserve the underlying I/O error. * LocalizationTables no longer report as changed when nothing changes. [(#1333)] [#1333]: https://github.com/rojo-rbx/rojo/pull/1333 diff --git a/crates/memofs/CHANGELOG.md b/crates/memofs/CHANGELOG.md index 3e841b2b..f378169f 100644 --- a/crates/memofs/CHANGELOG.md +++ b/crates/memofs/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased Changes +* Added `StdBackend::new_without_watcher` and `Vfs::new_default_without_watcher` for filesystem access without allocating a watcher. Watching can be enabled later. +* Watcher I/O errors now preserve their original message and error kind. + # 0.4.0 (2026-07-01) * Added `Vfs::canonicalize`. [#1201] * **Breaking:** `StdBackend::new` and `Vfs::new_default` now return `io::Result`, so a failure to create the filesystem watcher is reported as an error instead of panicking. The `Default` implementation for `StdBackend` has been removed as a result. [#1267] diff --git a/crates/memofs/src/lib.rs b/crates/memofs/src/lib.rs index bb6ae5a1..8222959b 100644 --- a/crates/memofs/src/lib.rs +++ b/crates/memofs/src/lib.rs @@ -262,6 +262,16 @@ impl Vfs { Ok(Self::new(StdBackend::new()?)) } + /// Creates a `Vfs` with `StdBackend` and automatic file watching disabled. + /// + /// No filesystem watcher is allocated unless automatic watching is later + /// enabled and a file or directory is read. + pub fn new_default_without_watcher() -> Self { + let vfs = Self::new(StdBackend::new_without_watcher()); + vfs.set_watch_enabled(false); + vfs + } + /// Creates a new `Vfs` with the given backend. pub fn new(backend: B) -> Self { let lock = VfsInner { @@ -588,6 +598,30 @@ mod test { use std::io; use std::path::PathBuf; + #[test] + fn watching_can_be_enabled_after_starting_without_watcher( + ) -> Result<(), Box> { + let dir = tempfile::tempdir()?; + let dir_path = dunce::canonicalize(dir.path())?; + let vfs = Vfs::new_default_without_watcher(); + let receiver = vfs.event_receiver(); + + assert_eq!(vfs.read_dir(&dir_path)?.count(), 0); + vfs.set_watch_enabled(true); + assert_eq!(vfs.read_dir(&dir_path)?.count(), 0); + + let file_path = dir_path.join("new.txt"); + fs_err::write(&file_path, "created after watching was enabled")?; + let event = receiver.recv_timeout(std::time::Duration::from_secs(5))?; + assert!( + matches!(&event, crate::VfsEvent::Create(path) if path == &file_path), + "expected creation of {}, got {:?}", + file_path.display(), + event + ); + Ok(()) + } + /// https://github.com/rojo-rbx/rojo/issues/899 #[test] fn read_to_string_lf_normalized_keeps_trailing_newline() { diff --git a/crates/memofs/src/std_backend.rs b/crates/memofs/src/std_backend.rs index 3f0ab2c1..a66a58ac 100644 --- a/crates/memofs/src/std_backend.rs +++ b/crates/memofs/src/std_backend.rs @@ -4,24 +4,45 @@ use std::thread; use std::time::Duration; use std::{collections::HashSet, io}; -use crossbeam_channel::Receiver; +use crossbeam_channel::{Receiver, Sender}; use notify::{watcher, DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher}; use crate::{DirEntry, Metadata, ReadDir, VfsBackend, VfsEvent}; /// `VfsBackend` that uses `std::fs` and the `notify` crate. pub struct StdBackend { - watcher: RecommendedWatcher, + watcher: Option, + watcher_sender: Sender, watcher_receiver: Receiver, watches: HashSet, } impl StdBackend { + /// Creates a filesystem backend with an initialized file watcher. + /// + /// Returns an error if the platform's watcher cannot be created. pub fn new() -> io::Result { - let (notify_tx, notify_rx) = mpsc::channel(); - let watcher = watcher(notify_tx, Duration::from_millis(50)).map_err(io::Error::other)?; + let mut backend = Self::new_without_watcher(); + backend.watcher = Some(Self::create_watcher(backend.watcher_sender.clone())?); + Ok(backend) + } + /// Creates a filesystem backend without allocating a file watcher. + /// + /// A watcher is initialized only if `watch` is subsequently called. + pub fn new_without_watcher() -> StdBackend { let (tx, rx) = crossbeam_channel::unbounded(); + Self { + watcher: None, + watcher_sender: tx, + watcher_receiver: rx, + watches: HashSet::new(), + } + } + + fn create_watcher(tx: Sender) -> io::Result { + let (notify_tx, notify_rx) = mpsc::channel(); + let watcher = watcher(notify_tx, Duration::from_millis(50)).map_err(notify_error_to_io)?; thread::spawn(move || { for event in notify_rx { @@ -46,11 +67,7 @@ impl StdBackend { Result::<(), crossbeam_channel::SendError>::Ok(()) }); - Ok(Self { - watcher, - watcher_receiver: rx, - watches: HashSet::new(), - }) + Ok(watcher) } } @@ -122,15 +139,47 @@ impl VfsBackend for StdBackend { { Ok(()) } else { + let watcher = match &mut self.watcher { + Some(watcher) => watcher, + slot @ None => slot.insert(Self::create_watcher(self.watcher_sender.clone())?), + }; self.watches.insert(path.to_path_buf()); - self.watcher + watcher .watch(path, RecursiveMode::Recursive) - .map_err(io::Error::other) + .map_err(notify_error_to_io) } } fn unwatch(&mut self, path: &Path) -> io::Result<()> { self.watches.remove(path); - self.watcher.unwatch(path).map_err(io::Error::other) + match &mut self.watcher { + Some(watcher) => watcher.unwatch(path).map_err(notify_error_to_io), + None => Ok(()), + } + } +} + +fn notify_error_to_io(error: notify::Error) -> io::Error { + match error { + // notify 4 formats I/O errors using the deprecated Error::description, + // which can hide the actual operating system error. + notify::Error::Io(error) => error, + error => io::Error::other(error), + } +} + +#[cfg(test)] +mod tests { + use super::notify_error_to_io; + use std::io; + + #[test] + fn notify_io_error_preserves_message_and_kind() { + let message = "filesystem watcher permission denied"; + let source = io::Error::new(io::ErrorKind::PermissionDenied, message); + let error = notify_error_to_io(notify::Error::Io(source)); + + assert_eq!(error.to_string(), message); + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied); } } diff --git a/src/cli/build.rs b/src/cli/build.rs index 82e81387..f89d90b4 100644 --- a/src/cli/build.rs +++ b/src/cli/build.rs @@ -78,8 +78,11 @@ impl BuildCommand { let project_path = resolve_path(&self.project)?; log::trace!("Constructing in-memory filesystem"); - let vfs = Vfs::new_default()?; - vfs.set_watch_enabled(self.watch); + let vfs = if self.watch { + Vfs::new_default()? + } else { + Vfs::new_default_without_watcher() + }; let session = ServeSession::new(vfs, project_path)?; let mut cursor = session.message_queue().cursor(); diff --git a/tests/tests/build.rs b/tests/tests/build.rs index c7873050..9b23cc33 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -108,3 +108,58 @@ fn run_build_test(test_name: &str) { assert_snapshot!(test_name, contents); }); } + +#[cfg(target_os = "linux")] +#[test] +fn build_without_watch_does_not_create_inotify_instance() -> Result<(), Box> +{ + const CHILD_ENV: &str = "ROJO_TEST_BUILD_WITHOUT_WATCH_CHILD"; + + // Isolate descriptor inspection from other tests that create file watchers. + if std::env::var_os(CHILD_ENV).is_none() { + let output = Command::new(std::env::current_exe()?) + .args([ + "--exact", + "tests::build::build_without_watch_does_not_create_inotify_instance", + "--nocapture", + ]) + .env(CHILD_ENV, "1") + .output()?; + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + return Ok(()); + } + + let project_dir = tempdir()?; + fs::write( + project_dir.path().join("default.project.json"), + r#"{"name":"WithoutWatcher","tree":{"$className":"Folder","$path":"src"}}"#, + )?; + fs::create_dir(project_dir.path().join("src"))?; + fs::write(project_dir.path().join("src/main.luau"), "return 42")?; + let output_path = project_dir.path().join("output.rbxmx"); + + librojo::cli::BuildCommand { + project: project_dir.path().to_path_buf(), + output: Some(output_path.clone()), + plugin: None, + watch: false, + } + .run()?; + + assert!(fs::read_to_string(output_path)?.contains("return 42")); + // BuildCommand keeps its session alive until process exit, so an eagerly + // allocated watcher remains observable after the build completes. + for entry in fs::read_dir("/proc/self/fd")? { + match fs::read_link(entry?.path()) { + Ok(target) => assert_ne!(target, Path::new("anon_inode:inotify")), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => return Err(err.into()), + } + } + Ok(()) +}