From d1204e33627e5bef56589e29c4a3af1837b55d9e Mon Sep 17 00:00:00 2001 From: RickyYii <237135932+RickyYii@users.noreply.github.com> Date: Sun, 30 Aug 2026 16:50:41 +0000 Subject: [PATCH] fix(dev): make the documented checks pass on a fresh clone CONTRIBUTING tells contributors to run `cargo test`, `cargo fmt --check` and `cargo clippy -- -D warnings`. None of the three passed on a clean checkout. `cargo test` never reached a test on Linux or macOS. The Tauri build script aborts when a bundle resource path is missing, and both `engines/linux/brush/` and `engines/macos/arm64/` are gitignored, so they do not exist until `setup:engines` downloads binaries into them. CI never saw this because it installs the engines first. Track a README in each directory, following the convention already used for engines/ffmpeg, engines/colmap and engines/brush, so the directories survive a clone while the binaries stay out of Git. Two tests then failed for reasons unrelated to the code under test: - `linux_root_is_flat_and_discovery_can_fall_back_to_path` asserted `discovered.ffmpeg.is_file()`, which requires FFmpeg on PATH. Assert the resolver contract instead - override wins, then PATH, otherwise the managed path is kept - so the test covers the same behaviour on a host with or without FFmpeg. - `cancellation_terminates_descendant_processes` asserted that `/proc/{pid}` no longer exists. A killed descendant is reparented to PID 1, and PID 1 does not reap in every container, so the process correctly stops running but stays visible as a zombie and the assertion fails. Assert termination instead: on Linux the entry is gone or the state is `Z`, on other Unix targets signal 0 reports no such process. The old assertion was also vacuous on macOS, where `/proc` never exists; the new one is not. The fixed 50ms sleep becomes a bounded poll so the check does not race the reaper. `cargo clippy -- -D warnings` failed on current stable Rust at `reserve_gaussian_video_export` (`clippy::nonminimal_bool`). CI installs Rust with an unpinned `rustup update stable`, so this turns the Ubuntu and macOS workflows red as runners pick the toolchain up. Verified on Ubuntu 24.04 x86_64: cargo test 68 passed (was 66 passed / 2 failed), cargo fmt --check clean, cargo clippy --all-targets -D warnings clean, npm test 48 passed, tsc clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012vBtVi2gXiSpUVoBeFuvu2 --- .gitignore | 2 ++ engines/linux/brush/README.md | 7 ++++++ engines/macos/arm64/README.md | 8 +++++++ src-tauri/src/commands/mod.rs | 4 ++-- src-tauri/src/engines/health.rs | 10 +++++++- src-tauri/src/process/mod.rs | 41 +++++++++++++++++++++++++++++++-- 6 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 engines/linux/brush/README.md create mode 100644 engines/macos/arm64/README.md diff --git a/.gitignore b/.gitignore index fc7eca3e..219336a2 100644 --- a/.gitignore +++ b/.gitignore @@ -24,4 +24,6 @@ engines/colmap/** engines/brush/** !engines/brush/README.md engines/linux/brush/** +!engines/linux/brush/README.md engines/macos/arm64/** +!engines/macos/arm64/README.md diff --git a/engines/linux/brush/README.md b/engines/linux/brush/README.md new file mode 100644 index 00000000..4aa27eea --- /dev/null +++ b/engines/linux/brush/README.md @@ -0,0 +1,7 @@ +Brush v0.3.0 Ubuntu x86_64 runtime. `npm run setup:engines:linux` downloads and +verifies `brush_app` into this directory; the binary itself is never committed. + +This README is tracked so the directory exists in a fresh clone. +`src-tauri/tauri.linux.conf.json` declares the directory as a bundle resource, +and the Tauri build script aborts when a declared resource path is missing -- +which would otherwise make `cargo test` fail before compiling any test. diff --git a/engines/macos/arm64/README.md b/engines/macos/arm64/README.md new file mode 100644 index 00000000..88ade42f --- /dev/null +++ b/engines/macos/arm64/README.md @@ -0,0 +1,8 @@ +Apple Silicon macOS runtime. `npm run setup:engines:macos` downloads and verifies +the arm64 FFmpeg/FFprobe/COLMAP/Brush closure into `bin/` and `lib/`; the +binaries themselves are never committed. + +This README is tracked so the directory exists in a fresh clone. +`src-tauri/tauri.macos.conf.json` declares the directory as a bundle resource, +and the Tauri build script aborts when a declared resource path is missing -- +which would otherwise make `cargo test` fail before compiling any test. diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index 3f0fcf80..7ab4532e 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -461,9 +461,9 @@ pub async fn begin_gaussian_video_export( let (root, _, _) = catalog::registered_final_ply_for_project(project_id).await?; let active = state.active.lock().await; - if !active + if active .as_ref() - .is_some_and(|session| session.project_id == project_id) + .is_none_or(|session| session.project_id != project_id) { return Err(SplatError::Process( "该项目当前未在 OOOSplat 预览中打开，无法导出视频。".into(), diff --git a/src-tauri/src/engines/health.rs b/src-tauri/src/engines/health.rs index d5506c3e..0e8d4848 100644 --- a/src-tauri/src/engines/health.rs +++ b/src-tauri/src/engines/health.rs @@ -834,7 +834,15 @@ mod tests { let paths = EnginePaths::from_root("/opt/ooosplat-engines"); assert_eq!(paths.colmap, PathBuf::from("/opt/ooosplat-engines/colmap")); let discovered = EnginePaths::from_candidates(PathBuf::from("/missing/engines")); - assert!(discovered.ffmpeg.is_file()); + // FFmpeg is not installed on every contributor machine or CI runner, so assert + // the resolver contract instead of requiring the binary: an explicit override + // wins, then PATH, and otherwise the managed path is kept so engine health can + // report the exact file it expected. + let expected = std::env::var_os("OOOSPLAT_FFMPEG") + .map(PathBuf::from) + .or_else(|| find_on_path("ffmpeg")) + .unwrap_or_else(|| PathBuf::from("/missing/engines/ffmpeg")); + assert_eq!(discovered.ffmpeg, expected); } #[cfg(target_os = "macos")] diff --git a/src-tauri/src/process/mod.rs b/src-tauri/src/process/mod.rs index 98cf03a4..3aeb51c8 100644 --- a/src-tauri/src/process/mod.rs +++ b/src-tauri/src/process/mod.rs @@ -475,7 +475,44 @@ mod tests { let pid = descendant_pid.lock().unwrap().expect("descendant PID"); manager.cancel(); assert!(matches!(run.await.unwrap(), Err(SplatError::Cancelled))); - tokio::time::sleep(Duration::from_millis(50)).await; - assert!(!PathBuf::from(format!("/proc/{pid}")).exists()); + + let mut terminated = false; + for _ in 0..150 { + if descendant_is_terminated(pid) { + terminated = true; + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + assert!( + terminated, + "descendant {pid} was still running after cancellation" + ); + } + + /// A killed descendant is reparented to PID 1, and PID 1 does not reap in every + /// container, so the process can linger as a zombie with its `/proc` entry intact. + /// The entry being gone and the entry being a zombie both mean the process stopped + /// running, which is what cancellation has to guarantee. + #[cfg(target_os = "linux")] + fn descendant_is_terminated(pid: u32) -> bool { + let Ok(status) = std::fs::read_to_string(format!("/proc/{pid}/status")) else { + return true; + }; + status + .lines() + .find_map(|line| line.strip_prefix("State:")) + .map(|state| state.trim_start().starts_with('Z')) + .unwrap_or(true) + } + + /// Unix targets without `/proc` reap orphans through PID 1, so a terminated + /// descendant disappears outright rather than lingering as a visible zombie. + #[cfg(all(unix, not(target_os = "linux")))] + fn descendant_is_terminated(pid: u32) -> bool { + // SAFETY: signal 0 only runs the existence and permission checks for the PID + // and delivers nothing, so no process state is observed or modified. + let alive = unsafe { libc::kill(pid as libc::pid_t, 0) } == 0; + !alive } }