Keep watch alive after rebuild errors

changes/watch-rebuild-recovery.fix.md

@@ -0,0 +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

rewatch-ocaml/README.md

@@ -105,6 +105,10 @@ These are intentional:
   own process group.
 - `format -v` and `compiler-args -v` write their logs to stderr, so stdout
   remains formatted source or JSON. Rust writes them to stdout.
+- Dependency resolution failures and invalid dependency configurations from
+  `build`, `clean`, and `format` return exit code `2` here and `1` in Rust.
+- While the root configuration is invalid, only configuration changes trigger
+  recovery attempts here. Rust also retries initialization on watched source edits.
 - Watch mode also notices a missing dependency being installed into an
   existing `node_modules` of an ancestor directory, where the dependency
   lookup finds it.

rewatch-ocaml/tests/check_command_validation.sh

@@ -518,19 +518,19 @@ require_same_output() {
 
 require_both_errors_contain() {
   local name=$1
-  local fragment=$2
-  local native_fragment=${fragment/"$work"/"$native_work"}
-  local native_short_fragment=${fragment/"$work"/"$native_short_work"}
-  if ! { grep -F "$fragment" "$work/rust.err" >/dev/null || \
-      tr '\\' '/' <"$work/rust.err" | grep -F "$native_fragment" >/dev/null || \
-      tr '\\' '/' <"$work/rust.err" | grep -F "$native_short_fragment" >/dev/null; } || \
-    ! { grep -F "$fragment" "$work/ocaml.err" >/dev/null || \
-      tr '\\' '/' <"$work/ocaml.err" | grep -F "$native_fragment" >/dev/null || \
-      tr '\\' '/' <"$work/ocaml.err" | grep -F "$native_short_fragment" >/dev/null; }; then
-    printf '%s: expected both errors to contain %s\n' "$name" "$fragment" >&2
-    dump_both
-    exit 1
-  fi
+  local implementation fragment native_fragment native_short_fragment
+  for implementation in rust ocaml; do
+    if [ "$implementation" = rust ]; then fragment=$2; else fragment=${3:-$2}; fi
+    native_fragment=${fragment/"$work"/"$native_work"}
+    native_short_fragment=${fragment/"$work"/"$native_short_work"}
+    if ! { grep -F "$fragment" "$work/$implementation.err" >/dev/null || \
+        tr '\\' '/' <"$work/$implementation.err" | grep -F "$native_fragment" >/dev/null || \
+        tr '\\' '/' <"$work/$implementation.err" | grep -F "$native_short_fragment" >/dev/null; }; then
+      printf '%s: expected %s error to contain %s\n' "$name" "$implementation" "$fragment" >&2
+      dump_both
+      exit 1
+    fi
+  done
 }
 
 # Rust needs RESCRIPT_BSC_EXE to exist; OCaml builds and formats with the
@@ -1190,14 +1190,16 @@ if ! grep -F \
   cat "$work/ocaml.out" "$work/ocaml.err" >&2
   exit 1
 fi
-run_case build-missing-dependency exit2 exit2 build "$work/missing-dependency"
+run_case build-missing-dependency reject exit2 build "$work/missing-dependency"
 require_both_errors_contain build-missing-dependency \
-  "Could not build package tree reading dependency 'absent' at path '$work/missing-dependency'. Error:"
-run_case build-configless-dependency exit2 exit2 build "$work/configless-dependency"
+  "Could not build package tree reading dependency 'absent'"
+run_case build-configless-dependency reject exit2 build "$work/configless-dependency"
 require_both_errors_contain build-configless-dependency \
+  "Could not read dependency config at $work/configless-dependency/node_modules/no-config" \
   "Could not build package tree for 'no-config' at path '$work/configless-dependency'. Error:"
-run_case build-malformed-dependency exit2 exit2 build "$work/malformed-dependency"
+run_case build-malformed-dependency reject exit2 build "$work/malformed-dependency"
 require_both_errors_contain build-malformed-dependency \
+  "Could not read dependency config at $work/malformed-dependency/node_modules/bad-config" \
   "Could not build package tree for 'bad-config' at path '$work/malformed-dependency'. Error:"
 run_case build-duplicate-dependency accept accept build "$work/duplicate-dependency"
 duplicate_warning='Duplicated package: shared ./node_modules/shared (chosen) vs ./node_modules/a/node_modules/shared in ./node_modules/a'
@@ -1224,55 +1226,46 @@ if ! grep -F "$duplicate_warning" "$work/rust.err.norm" >/dev/null || \
   exit 1
 fi
 
-"$rust" watch "$work/watch-config-rust" \
-  >"$work/watch-rust.out" 2>"$work/watch-rust.err" &
-rust_watch_pid=$!
-background_pids="$background_pids $rust_watch_pid"
-wait_for_file "$work/watch-config-rust/src/A.js"
-wait_for_text "$work/watch-config-rust/lib/ocaml/.compiler.log" "#Done("
-printf '{ invalid json\n' >"$work/watch-config-rust/rescript.json"
-wait_for_text "$work/watch-rust.err" "Could not initialize build"
-wait_for_exit "$rust_watch_pid"
-set +e
-wait "$rust_watch_pid"
-rust_watch_status=$?
-set -e
-if [ "$rust_watch_status" -ne 101 ]; then
-  printf 'watch-invalid-config-rebuild: expected Rust panic exit 101, got %s\n' \
-    "$rust_watch_status" >&2
-  cat "$work/watch-rust.out" "$work/watch-rust.err" >&2
-  exit 1
-fi
-
-"$ocaml" watch "$work/watch-config-ocaml" \
-  >"$work/watch-ocaml.out" 2>"$work/watch-ocaml.err" &
-ocaml_watch_pid=$!
-background_pids="$background_pids $ocaml_watch_pid"
-wait_for_file "$work/watch-config-ocaml/src/A.js"
-wait_for_text "$work/watch-config-ocaml/lib/ocaml/.compiler.log" "#Done("
-printf '{ invalid json\n' >"$work/watch-config-ocaml/rescript.json"
-wait_for_text "$work/watch-ocaml.err" "invalid JSON"
-if ! kill -0 "$ocaml_watch_pid" 2>/dev/null; then
-  echo "OCaml watcher exited after a recoverable config error" >&2
-  cat "$work/watch-ocaml.out" "$work/watch-ocaml.err" >&2
-  exit 1
-fi
-printf '{"name":"watch-config","sources":["src"],"dependencies":["definitely-missing-dep"]}\n' \
-  >"$work/watch-config-ocaml/rescript.json"
-wait_for_text "$work/watch-ocaml.err" "definitely-missing-dep"
-if ! kill -0 "$ocaml_watch_pid" 2>/dev/null; then
-  echo "OCaml watcher exited after a recoverable dependency error" >&2
-  cat "$work/watch-ocaml.out" "$work/watch-ocaml.err" >&2
-  exit 1
-fi
-printf '{"name":"watch-config","sources":["src"],"package-specs":{"module":"esmodule","in-source":true,"suffix":".mjs"}}\n' \
-  >"$work/watch-config-ocaml/rescript.json"
-wait_for_file "$work/watch-config-ocaml/src/A.mjs"
-wait_for_file "$work/watch-config-ocaml/lib/bs/build.ninja"
-if ! terminate_and_wait "$ocaml_watch_pid" "recoverable-config watcher"; then
-  cat "$work/watch-ocaml.out" "$work/watch-ocaml.err" >&2
-  exit 1
-fi
+for implementation in rust ocaml; do
+  if [ "$implementation" = rust ]; then
+    executable=$rust
+    config_error="Could not initialize build"
+  else
+    executable=$ocaml
+    config_error="invalid JSON"
+  fi
+  watch_project="$work/watch-config-$implementation"
+  watch_out="$work/watch-$implementation.out"
+  watch_err="$work/watch-$implementation.err"
+  "$executable" watch "$watch_project" >"$watch_out" 2>"$watch_err" &
+  watch_pid=$!
+  background_pids="$background_pids $watch_pid"
+  wait_for_file "$watch_project/src/A.js"
+  wait_for_text "$watch_project/lib/ocaml/.compiler.log" "#Done("
+  printf '{ invalid json\n' >"$watch_project/rescript.json"
+  wait_for_text "$watch_err" "$config_error"
+  if ! kill -0 "$watch_pid" 2>/dev/null; then
+    echo "$implementation watcher exited after a recoverable config error" >&2
+    cat "$watch_out" "$watch_err" >&2
+    exit 1
+  fi
+  printf '{"name":"watch-config","sources":["src"],"dependencies":["definitely-missing-dep"]}\n' \
+    >"$watch_project/rescript.json"
+  wait_for_text "$watch_err" "definitely-missing-dep"
+  if ! kill -0 "$watch_pid" 2>/dev/null; then
+    echo "$implementation watcher exited after a recoverable dependency error" >&2
+    cat "$watch_out" "$watch_err" >&2
+    exit 1
+  fi
+  printf '{"name":"watch-config","sources":["src"],"package-specs":{"module":"esmodule","in-source":true,"suffix":".mjs"}}\n' \
+    >"$watch_project/rescript.json"
+  wait_for_file "$watch_project/src/A.mjs"
+  wait_for_file "$watch_project/lib/bs/build.ninja"
+  if ! terminate_and_wait "$watch_pid" "$implementation recoverable-config watcher"; then
+    cat "$watch_out" "$watch_err" >&2
+    exit 1
+  fi
+done
 checked=$((checked + 1))
 
 retained_graph_marker="$work/watch-retained-graph/after-build.log"
@@ -1529,9 +1522,9 @@ if ! cmp -s "$work/quiet-watch-rust.err.norm" \
 fi
 checked=$((checked + 1))
 
-run_case clean-missing-dependency exit2 exit2 clean "$work/missing-dependency"
-run_case clean-configless-dependency exit2 exit2 clean "$work/configless-dependency"
-run_case clean-malformed-dependency exit2 exit2 clean "$work/malformed-dependency"
+run_case clean-missing-dependency reject exit2 clean "$work/missing-dependency"
+run_case clean-configless-dependency reject exit2 clean "$work/configless-dependency"
+run_case clean-malformed-dependency reject exit2 clean "$work/malformed-dependency"
 run_both clean "$work/clean-duplicate-{impl}"
 if [ "$(classify "$rust_status")" != reject ] || [ "$ocaml_status" -ne 0 ] || \
   [ ! -e "$work/clean-duplicate-rust/src/one/A.js" ] || \
@@ -1549,8 +1542,8 @@ set +e
 "$rust" watch "$work/missing-dependency" >"$work/rust.out" 2>"$work/rust.err"
 rust_status=$?
 set -e
-if [ "$rust_status" -ne 2 ]; then
-  echo "watch-missing-dependency: expected Rust exit 2, got $rust_status" >&2
+if [ "$rust_status" -ne 1 ]; then
+  echo "watch-missing-dependency: expected Rust exit 1, got $rust_status" >&2
   cat "$work/rust.out" "$work/rust.err" >&2
   exit 1
 fi
@@ -1581,11 +1574,11 @@ printf '{"name":"missing-dependency","sources":["src"],"dependencies":["absent"]
   >"$work/missing-dependency/rescript.json"
 checked=$((checked + 1))
 run_case --cwd "$work/missing-dependency" \
-  format-missing-dependency exit2 exit2 format
+  format-missing-dependency reject exit2 format
 run_case --cwd "$work/configless-dependency" \
-  format-configless-dependency exit2 exit2 format
+  format-configless-dependency reject exit2 format
 run_case --cwd "$work/malformed-dependency" \
-  format-malformed-dependency exit2 exit2 format
+  format-malformed-dependency reject exit2 format
 if [ -e "$work/missing-dependency/lib/build.lock" ] || \
   [ -e "$work/missing-dependency/lib/watch.lock" ]; then
   echo "OCaml dependency failures left a build or watch lock behind" >&2

rewatch/src/build.rs

@@ -189,7 +189,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()

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 {