Skip to content

Fix watch startup build locking #8413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cknitt wants to merge 2 commits into
base: master
Choose a base branch
from
+115 −80
Open

Fix watch startup build locking #8413

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c247a8
Fix watch startup build locking
cknitt May 6, 2026
bdc7a2f
Fix
cknitt May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 115 additions & 80 deletions rewatch/src/watcher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ enum CompileType {
None,
}

type WatchPaths = Vec<(PathBuf, RecursiveMode)>;
type StartupBuildResult = (
BuildCommandState,
WatchPaths,
Option<(Instant, build::CompilationOutcome)>,
);

fn is_rescript_file(path_buf: &Path) -> bool {
let extension = path_buf.extension().and_then(|ext| ext.to_str());

Expand Down Expand Up @@ -58,6 +65,31 @@ fn matches_filter(path_buf: &Path, filter: &Option<regex::Regex>) -> bool {
filter.as_ref().map(|re| !re.is_match(&name)).unwrap_or(true)
}

fn finish_successful_watch_compile(
after_build: Option<String>,
timing_total: Instant,
show_progress: bool,
plain_output: bool,
finished_message: &str,
compilation_kind: Option<&str>,
outcome: build::CompilationOutcome,
) {
if let Some(a) = after_build {
cmd::run(a)
}
let timing_total_elapsed = timing_total.elapsed();
if show_progress {
if plain_output {
println!("{finished_message}")
} else {
println!(
"\n{}\n",
build::format_finished_compilation_message(compilation_kind, outcome, timing_total_elapsed)
);
}
}
}

/// Computes the list of paths to watch based on the build state.
/// Returns tuples of (path, recursive_mode) for each watch target.
fn compute_watch_paths(build_state: &BuildCommandState, root: &Path) -> Vec<(PathBuf, RecursiveMode)> {
Expand Down Expand Up @@ -178,13 +210,8 @@ fn carry_forward_compile_warnings(previous: &BuildCommandState, next: &mut Build
}
}

fn should_clear_screen(
clear_screen: bool,
show_progress: bool,
plain_output: bool,
initial_build: bool,
) -> bool {
clear_screen && show_progress && !plain_output && !initial_build
fn should_clear_screen(clear_screen: bool, show_progress: bool, plain_output: bool) -> bool {
clear_screen && show_progress && !plain_output
}

fn clear_terminal_screen() {
Expand Down Expand Up @@ -237,7 +264,7 @@ async fn async_watch(
}: AsyncWatchArgs<'_>,
) -> Result<()> {
let mut build_state = initial_build_state;
let mut needs_compile_type = CompileType::Incremental;
let mut needs_compile_type = CompileType::None;
// create a mutex to capture if ctrl-c was pressed
let ctrlc_pressed = Arc::new(Mutex::new(false));
let ctrlc_pressed_clone = Arc::clone(&ctrlc_pressed);
Expand All @@ -249,8 +276,6 @@ async fn async_watch(
})
.expect("Error setting Ctrl-C handler");

let mut initial_build = true;

loop {
if *ctrlc_pressed_clone.lock().unwrap() {
if show_progress {
Expand Down Expand Up @@ -413,7 +438,7 @@ async fn async_watch(

match needs_compile_type {
CompileType::Incremental => {
if should_clear_screen(clear_screen, show_progress, plain_output, initial_build) {
if should_clear_screen(clear_screen, show_progress, plain_output) {
clear_terminal_screen();
print_rebuild_header(CompileType::Incremental);
}
Expand All @@ -422,47 +447,36 @@ async fn async_watch(
let result = build::incremental_build(
&mut build_state,
None,
initial_build,
false,
show_progress,
!initial_build,
true,
create_sourcedirs,
plain_output,
);

match result {
Ok(result) => {
if let Some(a) = after_build.clone() {
cmd::run(a)
}
let timing_total_elapsed = timing_total.elapsed();
if show_progress {
let compilation_type = if initial_build { "initial" } else { "incremental" };
if plain_output {
println!("Finished {compilation_type} compilation")
} else {
println!(
"\n{}\n",
build::format_finished_compilation_message(
Some(compilation_type),
result,
timing_total_elapsed,
)
);
}
}
finish_successful_watch_compile(
after_build.clone(),
timing_total,
show_progress,
plain_output,
"Finished incremental compilation",
Some("incremental"),
result,
);
}
Err(_) => {
if should_clear_screen(clear_screen, show_progress, plain_output, initial_build) {
if should_clear_screen(clear_screen, show_progress, plain_output) {
print_build_failed_footer();
}
}
}

needs_compile_type = CompileType::None;
initial_build = false;
}
CompileType::Full => {
if should_clear_screen(clear_screen, show_progress, plain_output, initial_build) {
if should_clear_screen(clear_screen, show_progress, plain_output) {
clear_terminal_screen();
print_rebuild_header(CompileType::Full);
}
Expand Down Expand Up @@ -497,7 +511,7 @@ async fn async_watch(
let result = build::incremental_build_without_lock(
&mut build_state,
None,
initial_build,
false,
show_progress,
false,
create_sourcedirs,
Expand All @@ -508,34 +522,23 @@ async fn async_watch(
});
match result {
Ok(result) => {
if let Some(a) = after_build.clone() {
cmd::run(a)
}

let timing_total_elapsed = timing_total.elapsed();
if show_progress {
if plain_output {
println!("Finished compilation")
} else {
println!(
"\n{}\n",
build::format_finished_compilation_message(
None,
result,
timing_total_elapsed,
)
);
}
}
finish_successful_watch_compile(
after_build.clone(),
timing_total,
show_progress,
plain_output,
"Finished compilation",
None,
result,
);
}
Err(_) => {
if should_clear_screen(clear_screen, show_progress, plain_output, initial_build) {
if should_clear_screen(clear_screen, show_progress, plain_output) {
print_build_failed_footer();
}
}
}
needs_compile_type = CompileType::None;
initial_build = false;
}
CompileType::None => {
// We want to sleep for a little while so the CPU can schedule other work. That way we end
Expand Down Expand Up @@ -569,25 +572,58 @@ pub fn start(

let path = Path::new(folder);

// Do an initial build to discover packages and source folders. Initialization can clean
// previous build artifacts, so it has to be serialized with other build operations.
let build_state: BuildCommandState = build::with_build_lock(path, || {
build::initialize_build(
None,
filter,
// Initialization can clean previous build artifacts, so it has to be serialized
// with the initial compile too.
let (build_state, current_watch_paths, initial_compile_result): StartupBuildResult =
build::with_build_lock(path, || {
let mut build_state = build::initialize_build(
None,
filter,
show_progress,
path,
plain_output,
warn_error.clone(),
prod,
features.clone(),
)
.with_context(|| "Could not initialize build")?;

// Compute and register targeted watches based on source folders.
let current_watch_paths = compute_watch_paths(&build_state, path);
register_watches(&mut watcher, &current_watch_paths);

let timing_total = Instant::now();
let initial_compile_result = build::incremental_build_without_lock(
&mut build_state,
None,
true,
show_progress,
false,
create_sourcedirs,
plain_output,
)
.ok()
.map(|result| (timing_total, result));

Ok::<StartupBuildResult, anyhow::Error>((
build_state,
current_watch_paths,
initial_compile_result,
))
})?;

// Run after-build outside build.lock. Hooks may invoke ReScript commands that need the same lock.
if let Some((timing_total, result)) = initial_compile_result {
finish_successful_watch_compile(
after_build.clone(),
timing_total,
show_progress,
path,
plain_output,
warn_error.clone(),
prod,
features.clone(),
)
.with_context(|| "Could not initialize build")
})?;

// Compute and register targeted watches based on source folders
let current_watch_paths = compute_watch_paths(&build_state, path);
register_watches(&mut watcher, &current_watch_paths);
"Finished initial compilation",
Some("initial"),
result,
);
}

async_watch(AsyncWatchArgs {
watcher: &mut watcher,
Expand Down Expand Up @@ -735,12 +771,11 @@ mod tests {
}

#[test]
fn clears_screen_only_for_non_initial_interactive_rebuilds() {
assert!(should_clear_screen(true, true, false, false));
assert!(!should_clear_screen(true, true, false, true));
assert!(!should_clear_screen(true, true, true, false));
assert!(!should_clear_screen(true, false, false, false));
assert!(!should_clear_screen(false, true, false, false));
fn clears_screen_only_for_interactive_rebuilds() {
assert!(should_clear_screen(true, true, false));
assert!(!should_clear_screen(true, true, true));
assert!(!should_clear_screen(true, false, false));
assert!(!should_clear_screen(false, true, false));
}

#[test]
Expand Down
Loading