mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-20 17:41:37 +00:00
Avoid watchers for one-shot builds and preserve watcher errors (AI-assisted)
This commit is contained in:
parent
7e60406373
commit
691efad330
7 changed files with 160 additions and 14 deletions
1
AI-canary.txt
Normal file
1
AI-canary.txt
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
This pull request is AI generated
|
||||||
|
|
@ -31,6 +31,7 @@ Making a new release? Simply add the new header with the version and date undern
|
||||||
|
|
||||||
## Unreleased
|
## 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)]
|
* LocalizationTables no longer report as changed when nothing changes. [(#1333)]
|
||||||
|
|
||||||
[#1333]: https://github.com/rojo-rbx/rojo/pull/1333
|
[#1333]: https://github.com/rojo-rbx/rojo/pull/1333
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,9 @@
|
||||||
|
|
||||||
## Unreleased Changes
|
## 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)
|
# 0.4.0 (2026-07-01)
|
||||||
* Added `Vfs::canonicalize`. [#1201]
|
* 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]
|
* **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]
|
||||||
|
|
|
||||||
|
|
@ -262,6 +262,16 @@ impl Vfs {
|
||||||
Ok(Self::new(StdBackend::new()?))
|
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.
|
/// Creates a new `Vfs` with the given backend.
|
||||||
pub fn new<B: VfsBackend>(backend: B) -> Self {
|
pub fn new<B: VfsBackend>(backend: B) -> Self {
|
||||||
let lock = VfsInner {
|
let lock = VfsInner {
|
||||||
|
|
@ -588,6 +598,30 @@ mod test {
|
||||||
use std::io;
|
use std::io;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn watching_can_be_enabled_after_starting_without_watcher(
|
||||||
|
) -> Result<(), Box<dyn std::error::Error>> {
|
||||||
|
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
|
/// https://github.com/rojo-rbx/rojo/issues/899
|
||||||
#[test]
|
#[test]
|
||||||
fn read_to_string_lf_normalized_keeps_trailing_newline() {
|
fn read_to_string_lf_normalized_keeps_trailing_newline() {
|
||||||
|
|
|
||||||
|
|
@ -4,24 +4,45 @@ use std::thread;
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
use std::{collections::HashSet, io};
|
use std::{collections::HashSet, io};
|
||||||
|
|
||||||
use crossbeam_channel::Receiver;
|
use crossbeam_channel::{Receiver, Sender};
|
||||||
use notify::{watcher, DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher};
|
use notify::{watcher, DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher};
|
||||||
|
|
||||||
use crate::{DirEntry, Metadata, ReadDir, VfsBackend, VfsEvent};
|
use crate::{DirEntry, Metadata, ReadDir, VfsBackend, VfsEvent};
|
||||||
|
|
||||||
/// `VfsBackend` that uses `std::fs` and the `notify` crate.
|
/// `VfsBackend` that uses `std::fs` and the `notify` crate.
|
||||||
pub struct StdBackend {
|
pub struct StdBackend {
|
||||||
watcher: RecommendedWatcher,
|
watcher: Option<RecommendedWatcher>,
|
||||||
|
watcher_sender: Sender<VfsEvent>,
|
||||||
watcher_receiver: Receiver<VfsEvent>,
|
watcher_receiver: Receiver<VfsEvent>,
|
||||||
watches: HashSet<PathBuf>,
|
watches: HashSet<PathBuf>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl StdBackend {
|
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<StdBackend> {
|
pub fn new() -> io::Result<StdBackend> {
|
||||||
let (notify_tx, notify_rx) = mpsc::channel();
|
let mut backend = Self::new_without_watcher();
|
||||||
let watcher = watcher(notify_tx, Duration::from_millis(50)).map_err(io::Error::other)?;
|
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();
|
let (tx, rx) = crossbeam_channel::unbounded();
|
||||||
|
Self {
|
||||||
|
watcher: None,
|
||||||
|
watcher_sender: tx,
|
||||||
|
watcher_receiver: rx,
|
||||||
|
watches: HashSet::new(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn create_watcher(tx: Sender<VfsEvent>) -> io::Result<RecommendedWatcher> {
|
||||||
|
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 || {
|
thread::spawn(move || {
|
||||||
for event in notify_rx {
|
for event in notify_rx {
|
||||||
|
|
@ -46,11 +67,7 @@ impl StdBackend {
|
||||||
Result::<(), crossbeam_channel::SendError<VfsEvent>>::Ok(())
|
Result::<(), crossbeam_channel::SendError<VfsEvent>>::Ok(())
|
||||||
});
|
});
|
||||||
|
|
||||||
Ok(Self {
|
Ok(watcher)
|
||||||
watcher,
|
|
||||||
watcher_receiver: rx,
|
|
||||||
watches: HashSet::new(),
|
|
||||||
})
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -122,15 +139,47 @@ impl VfsBackend for StdBackend {
|
||||||
{
|
{
|
||||||
Ok(())
|
Ok(())
|
||||||
} else {
|
} 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.watches.insert(path.to_path_buf());
|
||||||
self.watcher
|
watcher
|
||||||
.watch(path, RecursiveMode::Recursive)
|
.watch(path, RecursiveMode::Recursive)
|
||||||
.map_err(io::Error::other)
|
.map_err(notify_error_to_io)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn unwatch(&mut self, path: &Path) -> io::Result<()> {
|
fn unwatch(&mut self, path: &Path) -> io::Result<()> {
|
||||||
self.watches.remove(path);
|
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);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -78,8 +78,11 @@ impl BuildCommand {
|
||||||
let project_path = resolve_path(&self.project)?;
|
let project_path = resolve_path(&self.project)?;
|
||||||
|
|
||||||
log::trace!("Constructing in-memory filesystem");
|
log::trace!("Constructing in-memory filesystem");
|
||||||
let vfs = Vfs::new_default()?;
|
let vfs = if self.watch {
|
||||||
vfs.set_watch_enabled(self.watch);
|
Vfs::new_default()?
|
||||||
|
} else {
|
||||||
|
Vfs::new_default_without_watcher()
|
||||||
|
};
|
||||||
|
|
||||||
let session = ServeSession::new(vfs, project_path)?;
|
let session = ServeSession::new(vfs, project_path)?;
|
||||||
let mut cursor = session.message_queue().cursor();
|
let mut cursor = session.message_queue().cursor();
|
||||||
|
|
|
||||||
|
|
@ -108,3 +108,58 @@ fn run_build_test(test_name: &str) {
|
||||||
assert_snapshot!(test_name, contents);
|
assert_snapshot!(test_name, contents);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(target_os = "linux")]
|
||||||
|
#[test]
|
||||||
|
fn build_without_watch_does_not_create_inotify_instance() -> Result<(), Box<dyn std::error::Error>>
|
||||||
|
{
|
||||||
|
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(())
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue