Skip to content

Commit 4c9a002

Browse files
authored
Watch folders based on BuildState (#8219)
* Watch folders based on BuildState * Add additional test cases * Code review feedback * Make test cross plat (I hope) * Avoids sleeps in test * Log error when source folder does not exist. * Add changelog entry * Code review feedback * Avoid race condition (I hope) * Trigger CI
1 parent dafd2d2 commit 4c9a002

12 files changed

+531
-39
lines changed

AGENTS.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,20 @@ make test-rewatch # Run integration tests
389389

390390
**Integration Tests**: The `make test-rewatch` command runs bash-based integration tests located in `rewatch/tests/suite.sh`. These tests use the `rewatch/testrepo/` directory as a test workspace with various package configurations to verify rewatch's behavior across different scenarios.
391391

392+
**Running Individual Integration Tests**: You can run individual test scripts directly by setting up the environment manually:
393+
394+
```bash
395+
cd rewatch/tests
396+
export REWATCH_EXECUTABLE="$(realpath ../target/debug/rescript)"
397+
eval $(node ./get_bin_paths.js)
398+
export RESCRIPT_BSC_EXE
399+
export RESCRIPT_RUNTIME
400+
source ./utils.sh
401+
bash ./watch/06-watch-missing-source-folder.sh
402+
```
403+
404+
This is useful for iterating on a specific test without running the full suite.
405+
392406
#### Debugging
393407

394408
- **Build State**: Use `log::debug!` to inspect `BuildState` contents
@@ -411,8 +425,16 @@ export RESCRIPT_RUNTIME=$(realpath packages/@rescript/runtime)
411425
cargo run --manifest-path rewatch/Cargo.toml -- build
412426
```
413427

428+
Note that the dev binary is `./rewatch/target/debug/rescript`, not `rewatch`. The binary name is `rescript` because that's the package name in `Cargo.toml`.
429+
414430
This is useful when testing rewatch changes against local compiler modifications without running a full `make` build cycle.
415431

432+
Use `-v` for info-level logging or `-vv` for debug-level logging (e.g., to see which folders are being watched in watch mode):
433+
434+
```bash
435+
cargo run --manifest-path rewatch/Cargo.toml -- -vv watch <folder>
436+
```
437+
416438
#### Performance Considerations
417439

418440
- **Incremental Builds**: Only recompile dirty modules
@@ -467,3 +489,9 @@ When clippy suggests refactoring that could impact performance, consider the tra
467489
2. Update `AsyncWatchArgs` for new parameters
468490
3. Handle different file types (`.res`, `.resi`, etc.)
469491
4. Consider performance impact of watching many files
492+
493+
## CI Gotchas
494+
495+
- **`sleep` is fragile** — Prefer polling (e.g., `wait_for_file`) over fixed sleeps. CI runners are slower than local machines.
496+
- **`exit_watcher` is async** — It only signals the watcher to stop (removes the lock file), it doesn't wait for the process to exit. Avoid triggering config-change events before exiting, as the watcher may start a concurrent rebuild.
497+
- **`sed -i` differs across platforms** — macOS requires `sed -i '' ...`, Linux does not. Use the `replace` / `normalize_paths` helpers from `rewatch/tests/utils.sh` instead of raw `sed`.

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
#### :nail_care: Polish
2626

27+
- Build system: Watch only source folders from build state instead of the entire project directory, and report missing configured source folders. https://github.com/rescript-lang/rescript/pull/8219
28+
2729
#### :house: Internal
2830

2931
# 13.0.0-alpha.1

rewatch/src/watcher.rs

Lines changed: 130 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use crate::build;
2-
use crate::build::build_types::SourceType;
2+
use crate::build::build_types::{BuildCommandState, SourceType};
33
use crate::build::clean;
44
use crate::cmd;
5+
use crate::config;
56
use crate::helpers;
67
use crate::helpers::StrippedVerbatimPath;
78
use crate::helpers::emojis::*;
@@ -12,7 +13,7 @@ use anyhow::{Context, Result};
1213
use futures_timer::Delay;
1314
use notify::event::ModifyKind;
1415
use notify::{Config, Error, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher};
15-
use std::path::Path;
16+
use std::path::{Path, PathBuf};
1617
use std::sync::Arc;
1718
use std::sync::Mutex;
1819
use std::time::{Duration, Instant};
@@ -57,32 +58,116 @@ fn matches_filter(path_buf: &Path, filter: &Option<regex::Regex>) -> bool {
5758
filter.as_ref().map(|re| !re.is_match(&name)).unwrap_or(true)
5859
}
5960

61+
/// Computes the list of paths to watch based on the build state.
62+
/// Returns tuples of (path, recursive_mode) for each watch target.
63+
fn compute_watch_paths(build_state: &BuildCommandState, root: &Path) -> Vec<(PathBuf, RecursiveMode)> {
64+
// Use a HashMap to deduplicate paths, giving precedence to Recursive mode
65+
// when the same path appears with different modes (e.g. package root watched
66+
// NonRecursively for rescript.json changes, but also as a source folder with
67+
// Recursive mode).
68+
let mut watch_paths: std::collections::HashMap<PathBuf, RecursiveMode> = std::collections::HashMap::new();
69+
70+
let mut insert = |path: PathBuf, mode: RecursiveMode| {
71+
watch_paths
72+
.entry(path)
73+
.and_modify(|existing| {
74+
if mode == RecursiveMode::Recursive {
75+
*existing = RecursiveMode::Recursive;
76+
}
77+
})
78+
.or_insert(mode);
79+
};
80+
81+
for (_, package) in build_state.build_state.packages.iter() {
82+
if !package.is_local_dep {
83+
continue;
84+
}
85+
86+
// Watch the package root non-recursively to detect rescript.json changes.
87+
// We watch the directory rather than the file directly because many editors
88+
// use atomic writes (delete + recreate or write to temp + rename) which would
89+
// cause a direct file watch to be lost after the first edit.
90+
insert(package.path.clone(), RecursiveMode::NonRecursive);
91+
92+
// Watch each source folder
93+
for source in &package.source_folders {
94+
let dir = package.path.join(&source.dir);
95+
if !dir.exists() {
96+
log::error!(
97+
"Could not read folder: {:?}. Specified in dependency: {}, located {:?}...",
98+
source.dir,
99+
package.name,
100+
package.path
101+
);
102+
continue;
103+
}
104+
let mode = match &source.subdirs {
105+
Some(config::Subdirs::Recurse(true)) => RecursiveMode::Recursive,
106+
_ => RecursiveMode::NonRecursive,
107+
};
108+
insert(dir, mode);
109+
}
110+
}
111+
112+
// Watch the lib/ directory for the lockfile (rescript.lock lives in lib/)
113+
let lib_dir = root.join("lib");
114+
if lib_dir.exists() {
115+
insert(lib_dir, RecursiveMode::NonRecursive);
116+
}
117+
118+
watch_paths.into_iter().collect()
119+
}
120+
121+
/// Registers all watch paths with the given watcher.
122+
fn register_watches(watcher: &mut RecommendedWatcher, watch_paths: &[(PathBuf, RecursiveMode)]) {
123+
for (path, mode) in watch_paths {
124+
let mode_str = if *mode == RecursiveMode::Recursive {
125+
"recursive"
126+
} else {
127+
"non-recursive"
128+
};
129+
log::debug!(" watching ({mode_str}): {}", path.display());
130+
if let Err(e) = watcher.watch(path, *mode) {
131+
log::error!("Could not watch {}: {}", path.display(), e);
132+
}
133+
}
134+
}
135+
136+
/// Unregisters all watch paths from the given watcher.
137+
fn unregister_watches(watcher: &mut RecommendedWatcher, watch_paths: &[(PathBuf, RecursiveMode)]) {
138+
for (path, _) in watch_paths {
139+
let _ = watcher.unwatch(path);
140+
}
141+
}
142+
60143
struct AsyncWatchArgs<'a> {
144+
watcher: &'a mut RecommendedWatcher,
145+
current_watch_paths: Vec<(PathBuf, RecursiveMode)>,
146+
initial_build_state: BuildCommandState,
61147
q: Arc<FifoQueue<Result<Event, Error>>>,
62148
path: &'a Path,
63149
show_progress: bool,
64150
filter: &'a Option<regex::Regex>,
65151
after_build: Option<String>,
66152
create_sourcedirs: bool,
67153
plain_output: bool,
68-
warn_error: Option<String>,
69154
}
70155

71156
async fn async_watch(
72157
AsyncWatchArgs {
158+
watcher,
159+
mut current_watch_paths,
160+
initial_build_state,
73161
q,
74162
path,
75163
show_progress,
76164
filter,
77165
after_build,
78166
create_sourcedirs,
79167
plain_output,
80-
warn_error,
81168
}: AsyncWatchArgs<'_>,
82169
) -> Result<()> {
83-
let mut build_state: build::build_types::BuildCommandState =
84-
build::initialize_build(None, filter, show_progress, path, plain_output, warn_error)
85-
.with_context(|| "Could not initialize build")?;
170+
let mut build_state = initial_build_state;
86171
let mut needs_compile_type = CompileType::Incremental;
87172
// create a mutex to capture if ctrl-c was pressed
88173
let ctrlc_pressed = Arc::new(Mutex::new(false));
@@ -128,6 +213,21 @@ async fn async_watch(
128213
return Ok(());
129214
}
130215

216+
// Detect rescript.json changes and trigger a full rebuild
217+
if event
218+
.paths
219+
.iter()
220+
.any(|p| p.file_name().map(|name| name == "rescript.json").unwrap_or(false))
221+
&& matches!(
222+
event.kind,
223+
EventKind::Modify(_) | EventKind::Create(_) | EventKind::Remove(_)
224+
)
225+
{
226+
log::debug!("rescript.json changed -> full compile");
227+
needs_compile_type = CompileType::Full;
228+
continue;
229+
}
230+
131231
let paths = event
132232
.paths
133233
.iter()
@@ -280,6 +380,12 @@ async fn async_watch(
280380
build_state.get_warn_error_override(),
281381
)
282382
.expect("Could not initialize build");
383+
384+
// Re-register watches based on the new build state
385+
unregister_watches(watcher, &current_watch_paths);
386+
current_watch_paths = compute_watch_paths(&build_state, path);
387+
register_watches(watcher, &current_watch_paths);
388+
283389
let _ = build::incremental_build(
284390
&mut build_state,
285391
None,
@@ -334,23 +440,34 @@ pub fn start(
334440
let mut watcher = RecommendedWatcher::new(move |res| producer.push(res), Config::default())
335441
.expect("Could not create watcher");
336442

337-
log::debug!("watching {folder}");
443+
let path = Path::new(folder);
338444

339-
watcher
340-
.watch(Path::new(folder), RecursiveMode::Recursive)
341-
.expect("Could not start watcher");
445+
// Do an initial build to discover packages and source folders
446+
let build_state: BuildCommandState = build::initialize_build(
447+
None,
448+
filter,
449+
show_progress,
450+
path,
451+
plain_output,
452+
warn_error.clone(),
453+
)
454+
.with_context(|| "Could not initialize build")?;
342455

343-
let path = Path::new(folder);
456+
// Compute and register targeted watches based on source folders
457+
let current_watch_paths = compute_watch_paths(&build_state, path);
458+
register_watches(&mut watcher, &current_watch_paths);
344459

345460
async_watch(AsyncWatchArgs {
461+
watcher: &mut watcher,
462+
current_watch_paths,
463+
initial_build_state: build_state,
346464
q: consumer,
347465
path,
348466
show_progress,
349467
filter,
350468
after_build,
351469
create_sourcedirs,
352470
plain_output,
353-
warn_error: warn_error.clone(),
354471
})
355472
.await
356473
})

rewatch/tests/lock/01-lock-when-watching.sh

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,6 @@ else
1717
exit 1
1818
fi
1919

20-
exit_watcher() {
21-
# kill watcher by removing lock file
22-
rm lib/rescript.lock
23-
}
24-
2520
rewatch_bg watch > /dev/null 2>&1 &
2621
success "Watcher Started"
2722

rewatch/tests/suite.sh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ fi
2828

2929
source ./utils.sh
3030

31+
bold "Check for stale rescript processes"
32+
STALE_PIDS=$(ps aux | grep "$REWATCH_EXECUTABLE" | grep -v grep | awk '{print $2}')
33+
if [ -n "$STALE_PIDS" ]; then
34+
error "Found stale rescript processes using this executable:"
35+
ps aux | grep "$REWATCH_EXECUTABLE" | grep -v grep
36+
exit 1
37+
fi
38+
success "No stale rescript processes found"
39+
3140
bold "Yarn install"
3241
(cd ../testrepo && yarn && cp node_modules/rescript-nodejs/bsconfig.json node_modules/rescript-nodejs/rescript.json)
3342

@@ -77,6 +86,10 @@ fi
7786
# Watch tests
7887
./watch/01-watch-recompile.sh &&
7988
./watch/02-watch-warnings-persist.sh &&
89+
./watch/03-watch-new-file.sh &&
90+
./watch/04-watch-config-change.sh &&
91+
./watch/05-watch-ignores-non-source.sh &&
92+
./watch/06-watch-missing-source-folder.sh &&
8093

8194
# Lock tests
8295
./lock/01-lock-when-watching.sh &&

rewatch/tests/utils.sh

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,27 @@ replace() {
5050
sed -i $1 $2;
5151
fi
5252
}
53+
54+
exit_watcher() {
55+
rm -f lib/rescript.lock
56+
}
57+
58+
wait_for_file() {
59+
local file="$1"; local timeout="${2:-30}"
60+
while [ "$timeout" -gt 0 ]; do
61+
[ -f "$file" ] && return 0
62+
sleep 1
63+
timeout=$((timeout - 1))
64+
done
65+
return 1
66+
}
67+
68+
wait_for_file_gone() {
69+
local file="$1"; local timeout="${2:-30}"
70+
while [ "$timeout" -gt 0 ]; do
71+
[ ! -f "$file" ] && return 0
72+
sleep 1
73+
timeout=$((timeout - 1))
74+
done
75+
return 1
76+
}

rewatch/tests/watch/01-watch-recompile.sh

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,6 @@ else
1515
exit 1
1616
fi
1717

18-
exit_watcher() {
19-
# kill watcher by removing lock file
20-
rm lib/rescript.lock
21-
}
22-
23-
# Wait until a file exists (with timeout in seconds, default 30)
24-
wait_for_file() {
25-
local file="$1"; local timeout="${2:-30}"
26-
while [ "$timeout" -gt 0 ]; do
27-
[ -f "$file" ] && return 0
28-
sleep 1
29-
timeout=$((timeout - 1))
30-
done
31-
return 1
32-
}
33-
3418
# Start watcher and capture logs for debugging
3519
rewatch_bg watch > rewatch.log 2>&1 &
3620
success "Watcher Started"

rewatch/tests/watch/02-watch-warnings-persist.sh

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,6 @@ else
1515
exit 1
1616
fi
1717

18-
exit_watcher() {
19-
# kill watcher by removing lock file
20-
rm -f lib/rescript.lock
21-
}
22-
2318
# Wait until a pattern appears in a file (with timeout in seconds, default 30)
2419
wait_for_pattern() {
2520
local file="$1"; local pattern="$2"; local timeout="${3:-30}"

0 commit comments

Comments
 (0)