From 3377441a3deb7fb4b3b1ceeaacccaabfb10d5f70 Mon Sep 17 00:00:00 2001 From: Florian Hammerschmidt Date: Fri, 9 Oct 2026 14:53:50 +0200 Subject: [PATCH 1/5] Keep watcher running after full rebuild initialization errors Signed-off-by: Florian Hammerschmidt --- CHANGELOG.md | 1 + rewatch/src/watcher.rs | 16 ++- rewatch/tests/suite.sh | 1 + .../07-watch-recovers-from-invalid-config.sh | 103 ++++++++++++++++++ 4 files changed, 116 insertions(+), 5 deletions(-) create mode 100755 rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a9390cd84b..483281dac95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ #### :bug: Bug fix +- Keep watch mode running after full rebuild initialization fails so it can recover after a later valid change. https://github.com/rescript-lang/rescript/pull/8641 - Fix compiler crashes when wrapping values in `Some` or passing optional arguments whose types come from a transitive dependency. https://github.com/rescript-lang/rescript/pull/8719 - Fix unused-pattern warnings for variant spreads reported without a location. https://github.com/rescript-lang/rescript/pull/8744 - Keep `@warning("-4")` on `if` expressions when formatting; the formatter dropped it. https://github.com/rescript-lang/rescript/pull/8741 diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index 9984d6908b4..a34489b7e97 100644 --- a/rewatch/src/watcher.rs +++ b/rewatch/src/watcher.rs @@ -569,7 +569,7 @@ async fn async_watch( let timing_total = Instant::now(); // Reinitialization runs cleanup for previous build artifacts, so full rebuilds need // the same build lock boundary as regular `rescript build`. - let result = build::with_build_lock(path, || { + let result = build::with_build_lock(path, || -> Result<_> { let mut next_build_state = build::initialize_build( None, filter, @@ -581,7 +581,7 @@ async fn async_watch( features.clone(), SourceMapCommand::Watch, ) - .expect("Could not initialize build"); + .context("Could not initialize build")?; // Full rebuilds can be triggered by editor atomic saves that surface as rename events. // Preserve warnings and blocked dirty modules when fresh state replaces the previous one. @@ -603,10 +603,10 @@ async fn async_watch( plain_output, ); build::write_build_ninja(&build_state); - result + Ok(result) }); match result { - Ok(result) => { + Ok(Ok(result)) => { finish_successful_watch_compile( after_build.clone(), timing_total, @@ -617,7 +617,13 @@ async fn async_watch( result, ); } - Err(_) => { + Ok(Err(_)) => { + if should_clear_screen(clear_screen, show_progress, plain_output) { + print_build_failed_footer(); + } + } + Err(error) => { + eprintln!("{error:#}"); if should_clear_screen(clear_screen, show_progress, plain_output) { print_build_failed_footer(); } diff --git a/rewatch/tests/suite.sh b/rewatch/tests/suite.sh index 3209eae41d8..0efc9e26469 100755 --- a/rewatch/tests/suite.sh +++ b/rewatch/tests/suite.sh @@ -145,6 +145,7 @@ fi ./watch/05-watch-ignores-non-source.sh && ./watch/06-watch-missing-source-folder.sh && ./watch/07-changed-interface-after-failed-implementation.sh && +./watch/07-watch-recovers-from-invalid-config.sh && # Lock tests ./lock/01-lock-when-watching.sh && diff --git a/rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh b/rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh new file mode 100755 index 00000000000..775e54e0be1 --- /dev/null +++ b/rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh @@ -0,0 +1,103 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: Watcher recovers from an invalid config change" + +wait_for_pattern() { + local file="$1" pattern="$2" timeout="${3:-30}" + while [ "$timeout" -gt 0 ]; do + grep -q "$pattern" "$file" 2> /dev/null && return 0 + sleep 1 + timeout=$((timeout - 1)) + done + return 1 +} + +wait_for_pattern_count() { + local file="$1" pattern="$2" expected="$3" timeout="${4:-30}" + while [ "$timeout" -gt 0 ]; do + local count + count=$(grep -c "$pattern" "$file" 2> /dev/null || true) + [ "$count" -ge "$expected" ] && return 0 + sleep 1 + timeout=$((timeout - 1)) + done + return 1 +} + +rewatch clean > /dev/null 2>&1 +cp rescript.json rescript.json.watch-recovery.bak +rewatch_bg watch > rewatch.log 2>&1 & + +if ! wait_for_file "./src/Test.mjs" 20 || ! wait_for_file "lib/watch.lock" 20; then + error "Initial watch build did not complete" + cat rewatch.log + exit_watcher + mv rescript.json.watch-recovery.bak rescript.json + exit 1 +fi + +if ! wait_for_pattern rewatch.log "Finished initial compilation" 60; then + error "Initial watch build did not finish" + cat rewatch.log + exit_watcher + mv rescript.json.watch-recovery.bak rescript.json + exit 1 +fi + +initial_completed=$(grep -c "Finished compilation" rewatch.log 2> /dev/null || true) +printf '{"name":' > rescript.json.watch-recovery.next +mv rescript.json.watch-recovery.next rescript.json + +if ! wait_for_pattern rewatch.log "Could not initialize build" 30; then + error "Watcher did not report the invalid config" + cat rewatch.log + exit_watcher + mv rescript.json.watch-recovery.bak rescript.json + exit 1 +fi + +node -e ' + const fs = require("fs"); + const config = JSON.parse(fs.readFileSync(process.argv[1], "utf8")); + config.suffix = ".recovered.mjs"; + fs.writeFileSync(process.argv[2], `${JSON.stringify(config, null, 2)}\n`); +' rescript.json.watch-recovery.bak rescript.json.watch-recovery.next +mv rescript.json.watch-recovery.next rescript.json + +expected_completed=$((initial_completed + 1)) +if ! wait_for_pattern_count rewatch.log "Finished compilation" "$expected_completed" 30; then + error "Watcher did not recover after restoring a valid config" + cat rewatch.log + exit_watcher + mv rescript.json.watch-recovery.bak rescript.json + exit 1 +fi + +echo '// watch-recovery-test' >> src/Test.res +if ! wait_for_file "./src/Test.recovered.mjs" 20; then + error "Recovered watcher did not compile with the restored config" + cat rewatch.log + exit_watcher + mv rescript.json.watch-recovery.bak rescript.json + git checkout -- src/Test.res + exit 1 +fi + +exit_watcher +rewatch clean > /dev/null 2>&1 +mv rescript.json.watch-recovery.bak rescript.json +git checkout -- src/Test.res +rm -f rewatch.log +rewatch build > /dev/null 2>&1 + +if git diff --exit-code . > /dev/null 2>&1 && [ -z "$(git ls-files --others --exclude-standard .)" ]; then + success "Watcher recovered from invalid config" +else + error "Watcher recovery test left repository changes" + git diff . + git ls-files --others --exclude-standard . + exit 1 +fi From f83cd4ffd86edb9d53890bdd86255bdc4534c3c7 Mon Sep 17 00:00:00 2001 From: Florian Hammerschmidt Date: Fri, 9 Oct 2026 15:55:07 +0200 Subject: [PATCH 2/5] Retry watcher initialization and watch invalid dependencies Signed-off-by: Florian Hammerschmidt --- CHANGELOG.md | 2 +- rewatch/src/build/packages.rs | 94 ++++----- rewatch/src/watcher.rs | 186 ++++++++++++++---- rewatch/tests/suite.sh | 1 + .../07-watch-recovers-from-invalid-config.sh | 78 ++++---- ...08-watch-recovers-new-dependency-config.sh | 83 ++++++++ 6 files changed, 329 insertions(+), 115 deletions(-) create mode 100755 rewatch/tests/watch/08-watch-recovers-new-dependency-config.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 483281dac95..9efec79b99f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ #### :bug: Bug fix -- Keep watch mode running after full rebuild initialization fails so it can recover after a later valid change. https://github.com/rescript-lang/rescript/pull/8641 +- Keep watch mode running after full rebuild initialization fails, retry initialization on source edits, and watch newly added dependencies for configuration fixes. https://github.com/rescript-lang/rescript/pull/8641 - Fix compiler crashes when wrapping values in `Some` or passing optional arguments whose types come from a transitive dependency. https://github.com/rescript-lang/rescript/pull/8719 - Fix unused-pattern warnings for variant spreads reported without a location. https://github.com/rescript-lang/rescript/pull/8744 - Keep `@warning("-4")` on `if` expressions when formatting; the formatter dropped it. https://github.com/rescript-lang/rescript/pull/8741 diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 7a820b9b557..c544d738de0 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -8,12 +8,13 @@ use crate::helpers::StrippedVerbatimPath; use crate::helpers::emojis::*; use crate::project_context::{MonoRepoContext, ProjectContext}; use ahash::{AHashMap, AHashSet}; -use anyhow::{Result, anyhow}; +use anyhow::{Context, Result, anyhow}; use console::style; use log::debug; use rayon::prelude::*; use std::collections::hash_map::Entry; use std::error; +use std::fmt; use std::fs::{self}; use std::hash::{Hash, Hasher}; use std::path::{Path, PathBuf}; @@ -282,6 +283,30 @@ pub fn read_dependency( Ok(canonical_path) } +/// Retain the resolved directory even when its config cannot be read, so watch mode can +/// observe a correction to a dependency that was absent from the previous build state. +#[derive(Debug)] +pub struct DependencyConfigError { + pub package_path: PathBuf, + source: anyhow::Error, +} + +impl fmt::Display for DependencyConfigError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "Could not read dependency config at {}", + self.package_path.display() + ) + } +} + +impl error::Error for DependencyConfigError { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + Some(self.source.as_ref()) + } +} + /// Given a config, recursively finds all dependencies. /// 1. It starts with registering dependencies and /// prevents the operation for the ones which are already @@ -296,7 +321,7 @@ fn read_dependencies( show_progress: bool, is_local_dep: bool, prod: bool, -) -> Vec { +) -> Result> { let mut dependencies: Vec = package_config.get_dependency_names(); // Concatenate dev dependencies if is_local_dep is true and not in prod mode @@ -311,17 +336,14 @@ fn read_dependencies( // Package already registered - check for duplicate (different path) // Re-resolve from current package and from root to compare paths if let Ok(current_path) = read_dependency(package_name, package_config, project_context) - && let Ok(chosen_path) = read_dependency(package_name, &project_context.current_config, project_context) + && let Ok(chosen_path) = + read_dependency(package_name, &project_context.current_config, project_context) && current_path != chosen_path { // Different paths - this is a duplicate let root_path = project_context.get_root_path(); - let chosen_relative = chosen_path - .strip_prefix(root_path) - .unwrap_or(&chosen_path); - let duplicate_relative = current_path - .strip_prefix(root_path) - .unwrap_or(¤t_path); + let chosen_relative = chosen_path.strip_prefix(root_path).unwrap_or(&chosen_path); + let duplicate_relative = current_path.strip_prefix(root_path).unwrap_or(¤t_path); let current_package_path = package_config .path .parent() @@ -348,39 +370,22 @@ fn read_dependencies( .collect::>() // Read all config files in parallel instead of blocking .par_iter() - .map(|package_name| { - let (config, canonical_path) = - match read_dependency(package_name, package_config, project_context) { - Err(error) => { - if show_progress { - println!( - "{} {} Error building package tree. {}", - style("[1/2]").bold().dim(), - CROSS, - error - ); - } - - let parent_path_str = project_context.get_root_path().to_string_lossy(); - log::error!( - "Could not build package tree reading dependency '{package_name}' at path '{parent_path_str}'. Error: {error}", + .map(|package_name| -> Result { + let canonical_path = + read_dependency(package_name, package_config, project_context).inspect_err(|error| { + if show_progress { + println!( + "{} {} Error building package tree. {}", + style("[1/2]").bold().dim(), + CROSS, + error ); - - std::process::exit(2) - } - Ok(canonical_path) => { - match read_config(&canonical_path) { - Ok(config) => (config, canonical_path), - Err(error) => { - let parent_path_str = project_context.get_root_path().to_string_lossy(); - log::error!( - "Could not build package tree for '{package_name}' at path '{parent_path_str}'. Error: {error}", - ); - std::process::exit(2) - } - } } - }; + })?; + let config = read_config(&canonical_path).map_err(|source| DependencyConfigError { + package_path: canonical_path.clone(), + source, + })?; let is_local_dep = is_local_dependency(project_context, package_name, &canonical_path); @@ -391,15 +396,16 @@ fn read_dependencies( show_progress, is_local_dep, prod, - ); + ) + .with_context(|| format!("Could not read dependencies of '{package_name}'"))?; - Dependency { + Ok(Dependency { name: package_name.to_owned(), config, path: canonical_path, dependencies, is_local_dep, - } + }) }) .collect() } @@ -585,7 +591,7 @@ fn read_packages( show_progress, /* is local dep */ true, prod, - )); + )?); for d in dependencies.iter() { if !map.contains_key(&d.name) { diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index a34489b7e97..f8b5c83ed7e 100644 --- a/rewatch/src/watcher.rs +++ b/rewatch/src/watcher.rs @@ -25,6 +25,25 @@ enum CompileType { None, } +fn compile_type_for_source_event(kind: EventKind, initialization_failed: bool) -> CompileType { + match kind { + EventKind::Remove(_) + | EventKind::Any + | EventKind::Create(_) + | EventKind::Modify(ModifyKind::Name(_)) => CompileType::Full, + EventKind::Modify(ModifyKind::Data(_) | ModifyKind::Any) => { + if initialization_failed { + CompileType::Full + } else { + CompileType::Incremental + } + } + EventKind::Access(_) + | EventKind::Other + | EventKind::Modify(ModifyKind::Metadata(_) | ModifyKind::Other) => CompileType::None, + } +} + type WatchPaths = Vec<(PathBuf, RecursiveMode)>; type StartupBuildResult = ( BuildCommandState, @@ -249,6 +268,19 @@ fn update_watches( kept } +fn watch_paths_after_initialization_error(current: &[RegisteredWatch], error: &anyhow::Error) -> WatchPaths { + let mut next: WatchPaths = current + .iter() + .map(|watch| (watch.path.clone(), watch.mode)) + .collect(); + if let Some(error) = error.downcast_ref::() + && !next.iter().any(|(path, _)| *path == error.package_path) + { + next.push((error.package_path.clone(), RecursiveMode::NonRecursive)); + } + next +} + fn carry_forward_compile_state(previous: &BuildCommandState, next: &mut BuildCommandState) { for (module_name, next_module) in next.build_state.modules.iter_mut() { let Some(previous_module) = previous.build_state.modules.get(module_name) else { @@ -362,6 +394,7 @@ async fn async_watch( ) -> Result<()> { let mut build_state = initial_build_state; let mut needs_compile_type = CompileType::None; + let mut initialization_failed = false; loop { if ctrlc_pressed.load(Ordering::SeqCst) { cleanup_before_watch_exit(path, &build_state, show_progress, "\nExiting..."); @@ -425,31 +458,35 @@ async fn async_watch( for path in paths { let path_buf = path.to_path_buf(); - match (needs_compile_type, event.kind) { + match ( + needs_compile_type, + compile_type_for_source_event(event.kind, initialization_failed), + ) { ( CompileType::Incremental | CompileType::None, - // when we have a name change, create or remove event we need to do a full compile - EventKind::Remove(_) - | EventKind::Any - | EventKind::Create(_) - | EventKind::Modify(ModifyKind::Name(_)), + // Structural changes and edits after failed initialization need a full scan. + CompileType::Full, ) => { // if we are going to do a full compile, we don't need to bother marking // files dirty because we do a full scan anyway - log::debug!("received {:?} while needs_compile_type was {needs_compile_type:?} -> full compile", event.kind); + log::debug!( + "received {:?} while needs_compile_type was {needs_compile_type:?} -> full compile", + event.kind + ); needs_compile_type = CompileType::Full; } ( CompileType::None | CompileType::Incremental, - // when we have a data change event, we can do an incremental compile - EventKind::Modify(ModifyKind::Data(_)) | - // windows sends ModifyKind::Any on file content changes - EventKind::Modify(ModifyKind::Any), + // Content edits can compile incrementally while initialization is valid. + CompileType::Incremental, ) => { // if we are going to compile incrementally, we need to mark the exact files // dirty - log::debug!("received {:?} while needs_compile_type was {needs_compile_type:?} -> incremental compile", event.kind); + log::debug!( + "received {:?} while needs_compile_type was {needs_compile_type:?} -> incremental compile", + event.kind + ); if let Ok(canonicalized_path_buf) = path_buf .canonicalize() .map(StrippedVerbatimPath::to_stripped_verbatim_path) @@ -467,33 +504,34 @@ async fn async_watch( if let Some(module) = build_state.build_state.modules.get_mut(&module_name) { match module.source_type { SourceType::SourceFile(ref mut source_file) => { - let canonicalized_implementation_file = - package.path.join(&source_file.implementation.path); - if canonicalized_path_buf == canonicalized_implementation_file { - if let Ok(modified) = - canonicalized_path_buf.metadata().and_then(|x| x.modified()) - { - source_file.implementation.last_modified = modified; - }; - source_file.implementation.parse_dirty = true; - break; - } - - // mark the interface file dirty - if let Some(ref mut interface) = source_file.interface { - let canonicalized_interface_file = - package.path.join(&interface.path); - if canonicalized_path_buf == canonicalized_interface_file { + let canonicalized_implementation_file = + package.path.join(&source_file.implementation.path); + if canonicalized_path_buf == canonicalized_implementation_file { if let Ok(modified) = canonicalized_path_buf .metadata() .and_then(|x| x.modified()) { - interface.last_modified = modified; - } - interface.parse_dirty = true; + source_file.implementation.last_modified = modified; + }; + source_file.implementation.parse_dirty = true; break; } - } + + // mark the interface file dirty + if let Some(ref mut interface) = source_file.interface { + let canonicalized_interface_file = + package.path.join(&interface.path); + if canonicalized_path_buf == canonicalized_interface_file { + if let Ok(modified) = canonicalized_path_buf + .metadata() + .and_then(|x| x.modified()) + { + interface.last_modified = modified; + } + interface.parse_dirty = true; + break; + } + } } SourceType::MlMap(_) => (), } @@ -506,10 +544,7 @@ async fn async_watch( ( CompileType::None | CompileType::Incremental, // these are not relevant events for compilation - EventKind::Access(_) - | EventKind::Other - | EventKind::Modify(ModifyKind::Metadata(_)) - | EventKind::Modify(ModifyKind::Other), + CompileType::None, ) => (), // if we already need a full compile, we don't need to check for other events (CompileType::Full, _) => (), @@ -582,6 +617,7 @@ async fn async_watch( SourceMapCommand::Watch, ) .context("Could not initialize build")?; + initialization_failed = false; // Full rebuilds can be triggered by editor atomic saves that surface as rename events. // Preserve warnings and blocked dirty modules when fresh state replaces the previous one. @@ -623,6 +659,16 @@ async fn async_watch( } } Err(error) => { + // Keep source edits on the full initialization path until the current + // configuration can be loaded. Never compile against the stale state. + initialization_failed = true; + let recovery_watch_paths = + watch_paths_after_initialization_error(¤t_watches, &error); + current_watches = update_watches( + watcher, + std::mem::take(&mut current_watches), + &recovery_watch_paths, + ); eprintln!("{error:#}"); if should_clear_screen(clear_screen, show_progress, plain_output) { print_build_failed_footer(); @@ -887,6 +933,72 @@ mod tests { } } + #[test] + fn source_edits_retry_failed_initialization() { + for kind in [ + EventKind::Modify(ModifyKind::Data(notify::event::DataChange::Content)), + EventKind::Modify(ModifyKind::Any), + ] { + assert_eq!( + compile_type_for_source_event(kind, false), + CompileType::Incremental + ); + assert_eq!(compile_type_for_source_event(kind, true), CompileType::Full); + } + + let metadata = EventKind::Modify(ModifyKind::Metadata(notify::event::MetadataKind::Any)); + assert_eq!(compile_type_for_source_event(metadata, true), CompileType::None); + } + + #[test] + fn watches_new_dependency_after_config_read_fails() { + let temp = tempfile::TempDir::new().unwrap(); + let root = temp.path().canonicalize().unwrap().to_stripped_verbatim_path(); + let dependency = root.join("node_modules/parent/node_modules/new-dependency"); + std::fs::create_dir_all(&dependency).unwrap(); + std::fs::write( + root.join("rescript.json"), + r#"{"name":"root","dependencies":["parent"]}"#, + ) + .unwrap(); + std::fs::write( + root.join("node_modules/parent/rescript.json"), + r#"{"name":"parent","dependencies":["new-dependency"]}"#, + ) + .unwrap(); + std::fs::write(dependency.join("rescript.json"), r#"{"name":"#).unwrap(); + + let context = ProjectContext::new(&root).unwrap(); + let error = build::packages::make(&None, &context, false, false, None) + .unwrap_err() + .context("Could not initialize build"); + assert!(format!("{error:#}").contains("Failed to parse rescript.json")); + + let mut current = vec![RegisteredWatch { + path: root.clone(), + mode: RecursiveMode::NonRecursive, + identity: dir_identity(&root), + }]; + let paths = watch_paths_after_initialization_error(¤t, &error); + assert_eq!(paths.len(), 2); + assert!(paths.contains(&(root, RecursiveMode::NonRecursive))); + assert!(paths.contains(&(dependency.clone(), RecursiveMode::NonRecursive))); + + // A config error in an already watched source directory must retain its recursive mode. + current.push(RegisteredWatch { + path: dependency.clone(), + mode: RecursiveMode::Recursive, + identity: dir_identity(&dependency), + }); + let paths = watch_paths_after_initialization_error(¤t, &error); + assert_eq!(paths.len(), 2); + assert!(paths.contains(&(dependency.clone(), RecursiveMode::Recursive))); + + std::fs::write(dependency.join("rescript.json"), r#"{"name":"new-dependency"}"#).unwrap(); + let recovered = build::packages::make(&None, &context, false, false, None).unwrap(); + assert!(recovered.contains_key("new-dependency")); + } + #[test] fn clears_screen_only_for_interactive_rebuilds() { assert!(should_clear_screen(true, true, false)); diff --git a/rewatch/tests/suite.sh b/rewatch/tests/suite.sh index 0efc9e26469..245c9dbafc9 100755 --- a/rewatch/tests/suite.sh +++ b/rewatch/tests/suite.sh @@ -146,6 +146,7 @@ fi ./watch/06-watch-missing-source-folder.sh && ./watch/07-changed-interface-after-failed-implementation.sh && ./watch/07-watch-recovers-from-invalid-config.sh && +./watch/08-watch-recovers-new-dependency-config.sh && # Lock tests ./lock/01-lock-when-watching.sh && diff --git a/rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh b/rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh index 775e54e0be1..2fd76136830 100755 --- a/rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh +++ b/rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh @@ -5,36 +5,23 @@ cd ../../testrepo bold "Test: Watcher recovers from an invalid config change" -wait_for_pattern() { - local file="$1" pattern="$2" timeout="${3:-30}" - while [ "$timeout" -gt 0 ]; do - grep -q "$pattern" "$file" 2> /dev/null && return 0 - sleep 1 - timeout=$((timeout - 1)) - done - return 1 +stop_watcher() { + if ! exit_watcher; then + exit 1 + fi } -wait_for_pattern_count() { - local file="$1" pattern="$2" expected="$3" timeout="${4:-30}" - while [ "$timeout" -gt 0 ]; do - local count - count=$(grep -c "$pattern" "$file" 2> /dev/null || true) - [ "$count" -ge "$expected" ] && return 0 - sleep 1 - timeout=$((timeout - 1)) - done - return 1 -} - -rewatch clean > /dev/null 2>&1 +if ! rewatch clean > /dev/null 2>&1; then + error "Could not clean before watcher recovery test" + exit 1 +fi cp rescript.json rescript.json.watch-recovery.bak rewatch_bg watch > rewatch.log 2>&1 & if ! wait_for_file "./src/Test.mjs" 20 || ! wait_for_file "lib/watch.lock" 20; then error "Initial watch build did not complete" cat rewatch.log - exit_watcher + stop_watcher mv rescript.json.watch-recovery.bak rescript.json exit 1 fi @@ -42,7 +29,7 @@ fi if ! wait_for_pattern rewatch.log "Finished initial compilation" 60; then error "Initial watch build did not finish" cat rewatch.log - exit_watcher + stop_watcher mv rescript.json.watch-recovery.bak rescript.json exit 1 fi @@ -54,8 +41,27 @@ mv rescript.json.watch-recovery.next rescript.json if ! wait_for_pattern rewatch.log "Could not initialize build" 30; then error "Watcher did not report the invalid config" cat rewatch.log - exit_watcher + stop_watcher + mv rescript.json.watch-recovery.bak rescript.json + exit 1 +fi + +# A content-only edit must retry initialization instead of compiling with the old config. +initial_errors=$(grep -c "Could not initialize build" rewatch.log 2> /dev/null || true) +echo 'let watchRecoveryMarker = "watch-recovery-marker"' >> src/Test.res +if ! wait_for_pattern_count rewatch.log "Could not initialize build" "$((initial_errors + 1))" 30; then + error "Source edit did not retry failed initialization" + cat rewatch.log + stop_watcher + mv rescript.json.watch-recovery.bak rescript.json + git restore --worktree -- src/Test.res + exit 1 +fi +if grep -q "watch-recovery-marker" src/Test.mjs 2> /dev/null; then + error "Watcher compiled the source using stale configuration" + stop_watcher mv rescript.json.watch-recovery.bak rescript.json + git restore --worktree -- src/Test.res exit 1 fi @@ -71,27 +77,33 @@ expected_completed=$((initial_completed + 1)) if ! wait_for_pattern_count rewatch.log "Finished compilation" "$expected_completed" 30; then error "Watcher did not recover after restoring a valid config" cat rewatch.log - exit_watcher + stop_watcher mv rescript.json.watch-recovery.bak rescript.json + git restore --worktree -- src/Test.res exit 1 fi -echo '// watch-recovery-test' >> src/Test.res -if ! wait_for_file "./src/Test.recovered.mjs" 20; then +if ! wait_for_pattern "./src/Test.recovered.mjs" "watch-recovery-marker" 20; then error "Recovered watcher did not compile with the restored config" cat rewatch.log - exit_watcher + stop_watcher mv rescript.json.watch-recovery.bak rescript.json - git checkout -- src/Test.res + git restore --worktree -- src/Test.res exit 1 fi -exit_watcher -rewatch clean > /dev/null 2>&1 +stop_watcher +if ! rewatch clean > /dev/null 2>&1; then + error "Could not clean after watcher recovery test" + exit 1 +fi mv rescript.json.watch-recovery.bak rescript.json -git checkout -- src/Test.res +git restore --worktree -- src/Test.res rm -f rewatch.log -rewatch build > /dev/null 2>&1 +if ! rewatch build > /dev/null 2>&1; then + error "Could not rebuild after watcher recovery test" + exit 1 +fi if git diff --exit-code . > /dev/null 2>&1 && [ -z "$(git ls-files --others --exclude-standard .)" ]; then success "Watcher recovered from invalid config" diff --git a/rewatch/tests/watch/08-watch-recovers-new-dependency-config.sh b/rewatch/tests/watch/08-watch-recovers-new-dependency-config.sh new file mode 100755 index 00000000000..84532e28efe --- /dev/null +++ b/rewatch/tests/watch/08-watch-recovers-new-dependency-config.sh @@ -0,0 +1,83 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: Watcher recovers when only a newly added dependency config is fixed" + +dependency_config=packages/standalone/rescript.json + +stop_and_restore() { + if ! exit_watcher; then + return 1 + fi + mv rescript.json.watch-dependency.bak rescript.json + mv "$dependency_config.watch-dependency.bak" "$dependency_config" + rm -f rewatch.log + if ! rewatch build > /dev/null 2>&1; then + error "Could not rebuild testrepo after dependency recovery test" + return 1 + fi + # The root no longer depends on standalone; restore its artifacts with its own build. + if ! (cd packages/standalone && rewatch build > /dev/null 2>&1); then + error "Could not rebuild standalone after dependency recovery test" + return 1 + fi +} + +fail_test() { + error "$1" + cat rewatch.log + if ! stop_and_restore; then + exit 1 + fi + exit 1 +} + +if ! rewatch clean > /dev/null 2>&1; then + error "Could not clean before dependency recovery test" + exit 1 +fi +cp rescript.json rescript.json.watch-dependency.bak +cp "$dependency_config" "$dependency_config.watch-dependency.bak" +rewatch_bg watch > rewatch.log 2>&1 & + +if ! wait_for_file "lib/watch.lock" 20 || ! wait_for_pattern rewatch.log "Finished initial compilation" 60; then + fail_test "Initial watch build did not finish" +fi +initial_completed=$(grep -c "Finished compilation" rewatch.log 2> /dev/null || true) + +# Standalone is installed but absent from the root's original ReScript dependency graph. +printf '{"name":' > "$dependency_config.watch-dependency.next" +mv "$dependency_config.watch-dependency.next" "$dependency_config" +node -e ' + const fs = require("fs"); + const config = JSON.parse(fs.readFileSync(process.argv[1], "utf8")); + config.dependencies.push("@testrepo/standalone"); + fs.writeFileSync(process.argv[2], `${JSON.stringify(config, null, 2)}\n`); +' rescript.json rescript.json.watch-dependency.next +mv rescript.json.watch-dependency.next rescript.json + +if ! wait_for_pattern rewatch.log "Could not read dependency config.*standalone" 30; then + fail_test "Watcher did not report the newly added dependency's invalid config" +fi + +# Only fix the new dependency. Its directory must have a recovery watch by this point. +cp "$dependency_config.watch-dependency.bak" "$dependency_config.watch-dependency.next" +mv "$dependency_config.watch-dependency.next" "$dependency_config" +if ! wait_for_pattern_count rewatch.log "Finished compilation" "$((initial_completed + 1))" 30; then + fail_test "Watcher did not recover after fixing only the new dependency's config" +fi + +if ! stop_and_restore; then + exit 1 +fi + +if git diff --exit-code . > /dev/null 2>&1 && [ -z "$(git ls-files --others --exclude-standard .)" ]; then + success "Watcher recovered after the new dependency config was fixed" +else + error "Dependency recovery test left repository changes" + git diff . + git ls-files --others --exclude-standard . + exit 1 +fi From b7ff3d53c2fdae9b948e867a416dfb1333126879 Mon Sep 17 00:00:00 2001 From: Florian Hammerschmidt Date: Mon, 12 Oct 2026 22:53:10 +0200 Subject: [PATCH 3/5] Restore dependency resolution error context Signed-off-by: Florian Hammerschmidt --- rewatch/src/build/packages.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index c544d738de0..4e266a3ccf4 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -371,8 +371,8 @@ fn read_dependencies( // Read all config files in parallel instead of blocking .par_iter() .map(|package_name| -> Result { - let canonical_path = - read_dependency(package_name, package_config, project_context).inspect_err(|error| { + let canonical_path = read_dependency(package_name, package_config, project_context) + .inspect_err(|error| { if show_progress { println!( "{} {} Error building package tree. {}", @@ -381,6 +381,9 @@ fn read_dependencies( error ); } + }) + .with_context(|| { + format!("Could not build package tree reading dependency '{package_name}'") })?; let config = read_config(&canonical_path).map_err(|source| DependencyConfigError { package_path: canonical_path.clone(), From ff35b9a9045622b8f38f8a44ec119dcd9bd7e6a0 Mon Sep 17 00:00:00 2001 From: Florian Hammerschmidt Date: Tue, 13 Oct 2026 14:13:35 +0200 Subject: [PATCH 4/5] Address watch recovery documentation and output nits Signed-off-by: Florian Hammerschmidt --- changes/watch-rebuild-recovery.fix.md | 1 + rewatch/src/build.rs | 2 +- rewatch/src/build/clean.rs | 2 +- rewatch/src/build/packages.rs | 23 ++----------------- rewatch/src/format.rs | 2 +- rewatch/src/watcher.rs | 4 ++-- rewatch/tests/suite.sh | 4 ++-- ... 08-watch-recovers-from-invalid-config.sh} | 0 ...9-watch-recovers-new-dependency-config.sh} | 0 tests/build_tests/weird_deps/input.js | 1 + tests/build_tests/weird_devdeps/input.js | 1 + 11 files changed, 12 insertions(+), 28 deletions(-) rename rewatch/tests/watch/{07-watch-recovers-from-invalid-config.sh => 08-watch-recovers-from-invalid-config.sh} (100%) rename rewatch/tests/watch/{08-watch-recovers-new-dependency-config.sh => 09-watch-recovers-new-dependency-config.sh} (100%) diff --git a/changes/watch-rebuild-recovery.fix.md b/changes/watch-rebuild-recovery.fix.md index 89152cd15cb..9ea4c0feded 100644 --- a/changes/watch-rebuild-recovery.fix.md +++ b/changes/watch-rebuild-recovery.fix.md @@ -1 +1,2 @@ - Keep watch mode running after full rebuild initialization fails, retry initialization on source edits, and watch newly added dependencies for configuration fixes. https://github.com/rescript-lang/rescript/pull/8641 +- Dependency resolution and configuration errors now return exit code `1` instead of `2` from `rescript build`, `clean`, and `format`. https://github.com/rescript-lang/rescript/pull/8641 diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index 1db5dbc218a..4dcc23bc0bb 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -186,7 +186,7 @@ pub fn initialize_build( let compiler = get_compiler_info(&project_context)?; let timing_clean_start = Instant::now(); - let packages = packages::make(filter, &project_context, show_progress, prod, features.as_ref())?; + let packages = packages::make(filter, &project_context, prod, features.as_ref())?; let source_map_args = project_context .get_root_config() diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 9661f2911d2..67591925394 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -346,7 +346,7 @@ pub fn clean(path: &Path, show_progress: bool, plain_output: bool, prod: bool) - // `clean` always acts on the full set of source directories regardless of which features are // active. We explicitly pass `None` so every tagged source folder is included and its // artifacts can be removed, even for features the user hasn't enabled for this build. - let packages = packages::make(&None, &project_context, show_progress, prod, None)?; + let packages = packages::make(&None, &project_context, prod, None)?; let timing_clean_compiler_assets = Instant::now(); if !plain_output && show_progress { diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 4e266a3ccf4..7f71c145c58 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -9,7 +9,6 @@ use crate::helpers::emojis::*; use crate::project_context::{MonoRepoContext, ProjectContext}; use ahash::{AHashMap, AHashSet}; use anyhow::{Context, Result, anyhow}; -use console::style; use log::debug; use rayon::prelude::*; use std::collections::hash_map::Entry; @@ -318,7 +317,6 @@ fn read_dependencies( registered_dependencies_set: &mut AHashSet, project_context: &ProjectContext, package_config: &Config, - show_progress: bool, is_local_dep: bool, prod: bool, ) -> Result> { @@ -372,16 +370,6 @@ fn read_dependencies( .par_iter() .map(|package_name| -> Result { let canonical_path = read_dependency(package_name, package_config, project_context) - .inspect_err(|error| { - if show_progress { - println!( - "{} {} Error building package tree. {}", - style("[1/2]").bold().dim(), - CROSS, - error - ); - } - }) .with_context(|| { format!("Could not build package tree reading dependency '{package_name}'") })?; @@ -396,7 +384,6 @@ fn read_dependencies( &mut registered_dependencies_set.to_owned(), project_context, &config, - show_progress, is_local_dep, prod, ) @@ -567,11 +554,7 @@ This inconsistency will cause issues with package resolution.\n", }) } -fn read_packages( - project_context: &ProjectContext, - show_progress: bool, - prod: bool, -) -> Result> { +fn read_packages(project_context: &ProjectContext, prod: bool) -> Result> { // Store all packages and completely deduplicate them let mut map: AHashMap = AHashMap::new(); @@ -591,7 +574,6 @@ fn read_packages( &mut registered_dependencies_set, project_context, &project_context.current_config, - show_progress, /* is local dep */ true, prod, )?); @@ -854,11 +836,10 @@ fn compute_active_features( pub fn make( filter: &Option, project_context: &ProjectContext, - show_progress: bool, prod: bool, cli_features: Option<&Vec>, ) -> Result> { - let mut map = read_packages(project_context, show_progress, prod)?; + let mut map = read_packages(project_context, prod)?; let active_features = compute_active_features(&map, cli_features, prod)?; diff --git a/rewatch/src/format.rs b/rewatch/src/format.rs index eed70f0812f..ff0b897fba0 100644 --- a/rewatch/src/format.rs +++ b/rewatch/src/format.rs @@ -39,7 +39,7 @@ fn get_files_in_scope() -> Result> { let project_context = project_context::ProjectContext::new(¤t_dir)?; // Format walks all source files regardless of feature selection. - let packages = packages::make(&None, &project_context, false, false, None)?; + let packages = packages::make(&None, &project_context, false, None)?; let mut files: Vec = Vec::new(); let packages_to_format = project_context.get_scoped_local_packages(); diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index d7e2ec3dc23..00d5d34cefd 100644 --- a/rewatch/src/watcher.rs +++ b/rewatch/src/watcher.rs @@ -969,7 +969,7 @@ mod tests { std::fs::write(dependency.join("rescript.json"), r#"{"name":"#).unwrap(); let context = ProjectContext::new(&root).unwrap(); - let error = build::packages::make(&None, &context, false, false, None) + let error = build::packages::make(&None, &context, false, None) .unwrap_err() .context("Could not initialize build"); assert!(format!("{error:#}").contains("Failed to parse rescript.json")); @@ -995,7 +995,7 @@ mod tests { assert!(paths.contains(&(dependency.clone(), RecursiveMode::Recursive))); std::fs::write(dependency.join("rescript.json"), r#"{"name":"new-dependency"}"#).unwrap(); - let recovered = build::packages::make(&None, &context, false, false, None).unwrap(); + let recovered = build::packages::make(&None, &context, false, None).unwrap(); assert!(recovered.contains_key("new-dependency")); } diff --git a/rewatch/tests/suite.sh b/rewatch/tests/suite.sh index 245c9dbafc9..43467d69315 100755 --- a/rewatch/tests/suite.sh +++ b/rewatch/tests/suite.sh @@ -145,8 +145,8 @@ fi ./watch/05-watch-ignores-non-source.sh && ./watch/06-watch-missing-source-folder.sh && ./watch/07-changed-interface-after-failed-implementation.sh && -./watch/07-watch-recovers-from-invalid-config.sh && -./watch/08-watch-recovers-new-dependency-config.sh && +./watch/08-watch-recovers-from-invalid-config.sh && +./watch/09-watch-recovers-new-dependency-config.sh && # Lock tests ./lock/01-lock-when-watching.sh && diff --git a/rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh b/rewatch/tests/watch/08-watch-recovers-from-invalid-config.sh similarity index 100% rename from rewatch/tests/watch/07-watch-recovers-from-invalid-config.sh rename to rewatch/tests/watch/08-watch-recovers-from-invalid-config.sh diff --git a/rewatch/tests/watch/08-watch-recovers-new-dependency-config.sh b/rewatch/tests/watch/09-watch-recovers-new-dependency-config.sh similarity index 100% rename from rewatch/tests/watch/08-watch-recovers-new-dependency-config.sh rename to rewatch/tests/watch/09-watch-recovers-new-dependency-config.sh diff --git a/tests/build_tests/weird_deps/input.js b/tests/build_tests/weird_deps/input.js index d98217346bb..145846d0a21 100644 --- a/tests/build_tests/weird_deps/input.js +++ b/tests/build_tests/weird_deps/input.js @@ -9,4 +9,5 @@ const out = await execBuild(); assert.notEqual(out.status, 0); assert.match(out.stderr, /could not build package tree/i); assert.match(out.stderr, /dependency 'weird'/i); +assert.doesNotMatch(out.stdout, /error building package tree/i); await execClean(); diff --git a/tests/build_tests/weird_devdeps/input.js b/tests/build_tests/weird_devdeps/input.js index d98217346bb..145846d0a21 100644 --- a/tests/build_tests/weird_devdeps/input.js +++ b/tests/build_tests/weird_devdeps/input.js @@ -9,4 +9,5 @@ const out = await execBuild(); assert.notEqual(out.status, 0); assert.match(out.stderr, /could not build package tree/i); assert.match(out.stderr, /dependency 'weird'/i); +assert.doesNotMatch(out.stdout, /error building package tree/i); await execClean(); From fef2886a0a06f3dc3f773960b39f4906919d273d Mon Sep 17 00:00:00 2001 From: Florian Hammerschmidt Date: Wed, 14 Oct 2026 21:54:11 +0200 Subject: [PATCH 5/5] Tie watcher recovery regression to the source edit Log the source event path and its requested compile type once, then wait for the edited source to request full initialization and for an error after that event. Delayed config failures can no longer satisfy the recovery assertion before the edit is processed. Signed-off-by: Florian Hammerschmidt --- rewatch/src/watcher.rs | 18 ++++++------------ .../08-watch-recovers-from-invalid-config.sh | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index 00d5d34cefd..602d648e05b 100644 --- a/rewatch/src/watcher.rs +++ b/rewatch/src/watcher.rs @@ -462,11 +462,13 @@ async fn async_watch( .filter(|path| matches_filter(path, filter)); for path in paths { let path_buf = path.to_path_buf(); + let source_compile_type = compile_type_for_source_event(event.kind, initialization_failed); + log::debug!( + "received {:?} for {path:?} -> {source_compile_type:?} (pending: {needs_compile_type:?})", + event.kind + ); - match ( - needs_compile_type, - compile_type_for_source_event(event.kind, initialization_failed), - ) { + match (needs_compile_type, source_compile_type) { ( CompileType::Incremental | CompileType::None, // Structural changes and edits after failed initialization need a full scan. @@ -474,10 +476,6 @@ async fn async_watch( ) => { // if we are going to do a full compile, we don't need to bother marking // files dirty because we do a full scan anyway - log::debug!( - "received {:?} while needs_compile_type was {needs_compile_type:?} -> full compile", - event.kind - ); needs_compile_type = CompileType::Full; } @@ -488,10 +486,6 @@ async fn async_watch( ) => { // if we are going to compile incrementally, we need to mark the exact files // dirty - log::debug!( - "received {:?} while needs_compile_type was {needs_compile_type:?} -> incremental compile", - event.kind - ); if let Ok(canonicalized_path_buf) = path_buf .canonicalize() .map(StrippedVerbatimPath::to_stripped_verbatim_path) diff --git a/rewatch/tests/watch/08-watch-recovers-from-invalid-config.sh b/rewatch/tests/watch/08-watch-recovers-from-invalid-config.sh index 2fd76136830..94ce1d6ac92 100755 --- a/rewatch/tests/watch/08-watch-recovers-from-invalid-config.sh +++ b/rewatch/tests/watch/08-watch-recovers-from-invalid-config.sh @@ -11,12 +11,25 @@ stop_watcher() { fi } +wait_for_source_retry() { + local marker='received Modify.*Test[.]res.* -> Full' + wait_for_pattern rewatch.log "$marker" 30 || return 1 + # Require an initialization failure after the source event, not another config event before it. + local preceding_errors + preceding_errors=$(awk -v marker="$marker" ' + $0 ~ marker {exit} + /Could not initialize build/ {count++} + END {print count + 0} + ' rewatch.log) + wait_for_pattern_count rewatch.log "Could not initialize build" "$((preceding_errors + 1))" 30 +} + if ! rewatch clean > /dev/null 2>&1; then error "Could not clean before watcher recovery test" exit 1 fi cp rescript.json rescript.json.watch-recovery.bak -rewatch_bg watch > rewatch.log 2>&1 & +rewatch_bg -vv watch > rewatch.log 2>&1 & if ! wait_for_file "./src/Test.mjs" 20 || ! wait_for_file "lib/watch.lock" 20; then error "Initial watch build did not complete" @@ -47,9 +60,8 @@ if ! wait_for_pattern rewatch.log "Could not initialize build" 30; then fi # A content-only edit must retry initialization instead of compiling with the old config. -initial_errors=$(grep -c "Could not initialize build" rewatch.log 2> /dev/null || true) echo 'let watchRecoveryMarker = "watch-recovery-marker"' >> src/Test.res -if ! wait_for_pattern_count rewatch.log "Could not initialize build" "$((initial_errors + 1))" 30; then +if ! wait_for_source_retry; then error "Source edit did not retry failed initialization" cat rewatch.log stop_watcher