Skip to content

Commit a5af682

Browse files
cknittclaude
andcommitted
Re-register all watches outside macOS
Unwatching one path can drop an overlapping watch with inotify, so only macOS, where FSEvents restarts its stream on every change, updates watches incrementally. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BC5F5QPeQh5cHBjGBnvJ2T Signed-off-by: Christoph Knittel <ck@cca.io>
1 parent 93a6095 commit a5af682

2 files changed

Lines changed: 144 additions & 5 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Rewatch: keep watching a source folder on Linux when an overlapping recursive source folder is removed from `rescript.json`. https://github.com/rescript-lang/rescript/pull/8839

‎rewatch/src/watcher.rs‎

Lines changed: 143 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -216,19 +216,33 @@ fn register_watches(
216216
registered
217217
}
218218

219-
/// Brings the registered watches from `current` to `next`, touching only the paths that changed.
219+
/// Whether watches are updated by touching only the paths that changed (see `update_watches`).
220+
const INCREMENTAL_WATCH_UPDATES: bool = cfg!(target_os = "macos");
221+
222+
/// Brings the registered watches from `current` to `next`.
223+
///
224+
/// FSEvents on macOS restarts its event stream on every `watch`/`unwatch` call, and events that
225+
/// happen during the restart are lost. There, only the paths that changed are touched, so that a
226+
/// full rebuild keeping the same paths never restarts the stream. FSEvents watches are
227+
/// independent paths, so unwatching one doesn't affect the others. Watches whose directory was
228+
/// replaced since registration are registered again.
220229
///
221-
/// Some backends restart their event stream on every `watch`/`unwatch` call (FSEvents on macOS
222-
/// does), and events that happen during the restart are lost. Leaving unchanged watches alone
223-
/// avoids those gaps entirely in the common case where a full rebuild keeps the same paths.
224-
/// Watches whose directory was replaced since registration are registered again.
230+
/// Elsewhere, every watch is unwatched and registered again. Watches there can depend on each
231+
/// other: inotify on Linux keeps one watch per directory, shared by every registered path that
232+
/// reaches it (overlapping source folders, symlinks), so unwatching one path can drop another.
225233
///
226234
/// Returns the watches that are registered afterwards, which excludes paths that failed to register.
227235
fn update_watches(
228236
watcher: &mut RecommendedWatcher,
229237
current: Vec<RegisteredWatch>,
230238
next: &[(PathBuf, RecursiveMode)],
231239
) -> Vec<RegisteredWatch> {
240+
if !INCREMENTAL_WATCH_UPDATES {
241+
for watch in current {
242+
let _ = watcher.unwatch(&watch.path);
243+
}
244+
return register_watches(watcher, next);
245+
}
232246
let (mut kept, stale): (Vec<_>, Vec<_>) = current.into_iter().partition(|watch| {
233247
next.iter()
234248
.any(|(path, mode)| watch.is_still_valid_for(path, *mode))
@@ -999,4 +1013,128 @@ mod tests {
9991013
std::fs::create_dir(&dir).expect("dir should be recreated");
10001014
assert!(!watch.is_still_valid_for(&dir, RecursiveMode::Recursive));
10011015
}
1016+
1017+
/// Creates the symlinks `(link, target)` of `links`, registers `before` with a real watcher,
1018+
/// updates the watches to `after`, and checks that a file written to `write_dir` afterwards is
1019+
/// reported. Paths are relative to a temp dir, and `""` is the temp dir itself.
1020+
#[cfg(target_os = "linux")]
1021+
fn assert_update_keeps_watching(
1022+
before: &[(&str, RecursiveMode)],
1023+
after: &[(&str, RecursiveMode)],
1024+
write_dir: &str,
1025+
links: &[(&str, &str)],
1026+
) {
1027+
use std::sync::mpsc;
1028+
1029+
let temp_dir = tempfile::TempDir::new().expect("temp dir should be created");
1030+
let root = temp_dir.path();
1031+
for (link, target) in links {
1032+
std::fs::create_dir_all(root.join(target)).expect("dir should be created");
1033+
if let Some(parent) = root.join(link).parent() {
1034+
std::fs::create_dir_all(parent).expect("dir should be created");
1035+
}
1036+
std::os::unix::fs::symlink(root.join(target), root.join(link))
1037+
.expect("symlink should be created");
1038+
}
1039+
let to_watch_paths = |paths: &[(&str, RecursiveMode)]| -> WatchPaths {
1040+
paths
1041+
.iter()
1042+
.map(|(path, mode)| (root.join(path), *mode))
1043+
.collect()
1044+
};
1045+
for (path, _) in to_watch_paths(before).iter().chain(&to_watch_paths(after)) {
1046+
std::fs::create_dir_all(path).expect("dir should be created");
1047+
}
1048+
std::fs::create_dir_all(root.join(write_dir)).expect("dir should be created");
1049+
1050+
let (tx, rx) = mpsc::channel();
1051+
let mut watcher = RecommendedWatcher::new(tx, Config::default()).expect("watcher should start");
1052+
let current = register_watches(&mut watcher, &to_watch_paths(before));
1053+
let registered = update_watches(&mut watcher, current, &to_watch_paths(after));
1054+
assert_eq!(registered.len(), after.len());
1055+
1056+
let file = root.join(write_dir).join("Demo.res");
1057+
std::fs::write(&file, "let x = 2").expect("file should be written");
1058+
// Events name the file through whichever spelling of its directory notify registered
1059+
let resolved_file = std::fs::canonicalize(&file).expect("file should resolve");
1060+
let is_file = |path: &PathBuf| std::fs::canonicalize(path).ok().as_ref() == Some(&resolved_file);
1061+
let deadline = Instant::now() + Duration::from_secs(5);
1062+
loop {
1063+
let remaining = deadline.saturating_duration_since(Instant::now());
1064+
match rx.recv_timeout(remaining) {
1065+
Ok(Ok(event)) if event.paths.iter().any(is_file) => return,
1066+
Ok(_) => continue,
1067+
Err(_) => panic!(
1068+
"no event for {} after updating watches from {before:?} to {after:?}",
1069+
file.display()
1070+
),
1071+
}
1072+
}
1073+
}
1074+
1075+
/// inotify shares one watch per directory between overlapping paths, so unwatching one path
1076+
/// can drop another (#8825). Watches that remain configured must keep reporting events.
1077+
#[cfg(target_os = "linux")]
1078+
#[test]
1079+
fn keeps_watching_after_an_overlapping_watch_is_removed() {
1080+
use RecursiveMode::{NonRecursive, Recursive};
1081+
1082+
// A folder kept below a removed recursive folder
1083+
assert_update_keeps_watching(
1084+
&[
1085+
("", NonRecursive),
1086+
("src", Recursive),
1087+
("src/retained", NonRecursive),
1088+
],
1089+
&[("", NonRecursive), ("src/retained", NonRecursive)],
1090+
"src/retained",
1091+
&[],
1092+
);
1093+
assert_update_keeps_watching(
1094+
&[("src", Recursive), ("src/retained", Recursive)],
1095+
&[("src/retained", Recursive)],
1096+
"src/retained/nested",
1097+
&[],
1098+
);
1099+
// A folder removed below a kept recursive folder
1100+
assert_update_keeps_watching(
1101+
&[("src", Recursive), ("src/retained", NonRecursive)],
1102+
&[("src", Recursive)],
1103+
"src/retained",
1104+
&[],
1105+
);
1106+
// A folder that stops being recursive
1107+
assert_update_keeps_watching(
1108+
&[("", Recursive), ("src", NonRecursive)],
1109+
&[("", NonRecursive), ("src", NonRecursive)],
1110+
"src",
1111+
&[],
1112+
);
1113+
// Other spellings of a folder below the removed one
1114+
assert_update_keeps_watching(
1115+
&[("other/../src/retained", NonRecursive), ("src", Recursive)],
1116+
&[("other/../src/retained", NonRecursive)],
1117+
"src/retained",
1118+
&[],
1119+
);
1120+
assert_update_keeps_watching(
1121+
&[("link", NonRecursive), ("src", Recursive)],
1122+
&[("link", NonRecursive)],
1123+
"src/retained",
1124+
&[("link", "src/retained")],
1125+
);
1126+
// A recursive folder reaching a separately watched one through a symlink
1127+
assert_update_keeps_watching(
1128+
&[("shared", NonRecursive), ("src", Recursive)],
1129+
&[("src", Recursive)],
1130+
"shared",
1131+
&[("src/link", "shared")],
1132+
);
1133+
assert_update_keeps_watching(
1134+
&[("shared", NonRecursive), ("src", Recursive)],
1135+
&[("shared", NonRecursive)],
1136+
"shared",
1137+
&[("src/link", "shared")],
1138+
);
1139+
}
10021140
}

0 commit comments

Comments
 (0)