Skip to content

Commit 2100788

Browse files
committed
Close remaining rewatch build.lock gaps (#8410)
* Close remaining rewatch build.lock gaps * CHANGELOG # Conflicts: # CHANGELOG.md # rewatch/src/build.rs # rewatch/src/watcher.rs
1 parent 0607044 commit 2100788

3 files changed

Lines changed: 70 additions & 60 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
- Fix type lowering for `dict{}` and `async`, so you don't need to annotate one extra time when the type is known. https://github.com/rescript-lang/rescript/pull/8359
3232
- Fix issue where warning 56 would blow up with `dict{}` patterns. https://github.com/rescript-lang/rescript/pull/8403
3333
- Acquire rewatch build lock before initialization. https://github.com/rescript-lang/rescript/pull/8409
34+
- Close remaining rewatch build.lock gaps. https://github.com/rescript-lang/rescript/pull/8410
3435

3536
#### :nail_care: Polish
3637

rewatch/src/build.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ impl fmt::Display for IncrementalBuildError {
236236
}
237237
}
238238

239-
fn with_build_lock<T>(path: &Path, f: impl FnOnce() -> T) -> T {
239+
pub fn with_build_lock<T>(path: &Path, f: impl FnOnce() -> T) -> T {
240240
let build_folder = path.to_string_lossy().to_string();
241241
let _lock = get_lock_or_exit(LockKind::Build, &build_folder);
242242
let result = f();
@@ -269,7 +269,7 @@ pub fn incremental_build(
269269

270270
// `build` needs to lock before initialization because initialization mutates previous
271271
// build artifacts. Keep the compile body separate so it can run under that wider lock.
272-
fn incremental_build_without_lock(
272+
pub fn incremental_build_without_lock(
273273
build_state: &mut BuildCommandState,
274274
default_timing: Option<Duration>,
275275
initial_build: bool,

rewatch/src/watcher.rs

Lines changed: 67 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ async fn async_watch(
225225
if show_progress {
226226
println!("\nExiting...");
227227
}
228-
clean::cleanup_after_build(&build_state);
228+
build::with_build_lock(path, || clean::cleanup_after_build(&build_state));
229229
break Ok(());
230230
}
231231
let mut events: Vec<Event> = vec![];
@@ -250,7 +250,7 @@ async fn async_watch(
250250
if show_progress {
251251
println!("\nExiting... (lockfile removed)");
252252
}
253-
clean::cleanup_after_build(&build_state);
253+
build::with_build_lock(path, || clean::cleanup_after_build(&build_state));
254254
return Ok(());
255255
}
256256

@@ -412,53 +412,59 @@ async fn async_watch(
412412
}
413413
CompileType::Full => {
414414
let timing_total = Instant::now();
415-
let mut next_build_state = build::initialize_build(
416-
None,
417-
filter,
418-
show_progress,
419-
path,
420-
plain_output,
421-
build_state.get_warn_error_override(),
422-
)
423-
.expect("Could not initialize build");
424-
425-
// Full rebuilds can be triggered by editor atomic saves that surface as rename events.
426-
// Preserve warning state for unchanged modules so their warnings are re-emitted after the
427-
// fresh build state replaces the previous one.
428-
carry_forward_compile_warnings(&build_state, &mut next_build_state);
429-
build_state = next_build_state;
430-
431-
// Re-register watches based on the new build state
432-
unregister_watches(watcher, &current_watch_paths);
433-
current_watch_paths = compute_watch_paths(&build_state, path);
434-
register_watches(watcher, &current_watch_paths);
435-
436-
let _ = build::incremental_build(
437-
&mut build_state,
438-
None,
439-
initial_build,
440-
show_progress,
441-
false,
442-
create_sourcedirs,
443-
plain_output,
444-
);
445-
if let Some(a) = after_build.clone() {
446-
cmd::run(a)
447-
}
448-
449-
build::write_build_ninja(&build_state);
415+
// Reinitialization runs cleanup for previous build artifacts, so full rebuilds need
416+
// the same build lock boundary as regular `rescript build`.
417+
let result = build::with_build_lock(path, || {
418+
let mut next_build_state = build::initialize_build(
419+
None,
420+
filter,
421+
show_progress,
422+
path,
423+
plain_output,
424+
build_state.get_warn_error_override(),
425+
)
426+
.expect("Could not initialize build");
427+
428+
// Full rebuilds can be triggered by editor atomic saves that surface as rename events.
429+
// Preserve warning state for unchanged modules so their warnings are re-emitted after the
430+
// fresh build state replaces the previous one.
431+
carry_forward_compile_warnings(&build_state, &mut next_build_state);
432+
build_state = next_build_state;
433+
434+
// Re-register watches based on the new build state
435+
unregister_watches(watcher, &current_watch_paths);
436+
current_watch_paths = compute_watch_paths(&build_state, path);
437+
register_watches(watcher, &current_watch_paths);
438+
439+
let result = build::incremental_build_without_lock(
440+
&mut build_state,
441+
None,
442+
initial_build,
443+
show_progress,
444+
false,
445+
create_sourcedirs,
446+
plain_output,
447+
);
448+
build::write_build_ninja(&build_state);
449+
result
450+
});
451+
if result.is_ok() {
452+
if let Some(a) = after_build.clone() {
453+
cmd::run(a)
454+
}
450455

451-
let timing_total_elapsed = timing_total.elapsed();
452-
if show_progress {
453-
if plain_output {
454-
println!("Finished compilation")
455-
} else {
456-
println!(
457-
"\n{}{}Finished compilation in {:.2}s\n",
458-
LINE_CLEAR,
459-
SPARKLES,
460-
timing_total_elapsed.as_secs_f64()
461-
);
456+
let timing_total_elapsed = timing_total.elapsed();
457+
if show_progress {
458+
if plain_output {
459+
println!("Finished compilation")
460+
} else {
461+
println!(
462+
"\n{}{}Finished compilation in {:.2}s\n",
463+
LINE_CLEAR,
464+
SPARKLES,
465+
timing_total_elapsed.as_secs_f64()
466+
);
467+
}
462468
}
463469
}
464470
needs_compile_type = CompileType::None;
@@ -493,16 +499,19 @@ pub fn start(
493499

494500
let path = Path::new(folder);
495501

496-
// Do an initial build to discover packages and source folders
497-
let build_state: BuildCommandState = build::initialize_build(
498-
None,
499-
filter,
500-
show_progress,
501-
path,
502-
plain_output,
503-
warn_error.clone(),
504-
)
505-
.with_context(|| "Could not initialize build")?;
502+
// Do an initial build to discover packages and source folders. Initialization can clean
503+
// previous build artifacts, so it has to be serialized with other build operations.
504+
let build_state: BuildCommandState = build::with_build_lock(path, || {
505+
build::initialize_build(
506+
None,
507+
filter,
508+
show_progress,
509+
path,
510+
plain_output,
511+
warn_error.clone(),
512+
)
513+
.with_context(|| "Could not initialize build")
514+
})?;
506515

507516
// Compute and register targeted watches based on source folders
508517
let current_watch_paths = compute_watch_paths(&build_state, path);

0 commit comments

Comments
 (0)