Skip to content

Commit fd0e206

Browse files
nbafrankclaude
andcommitted
Release v0.2.16: Sonoma URL, cache R-minor sentinel, install timeout, SIGINT cleanup, pin warning
- #51: macOS R install URL is now Sonoma-aware (sonoma-arm64 / sonoma-x86_64 for Darwin >= 14, with big-sur fallback for older R versions). Resolves the 4.6.0 404 reported by adamhsparks on Sonoma. - #66: Project library carries a .uvr-r-version sentinel. If the active R doesn't match the sentinel (R upgraded out from under the library), sync wipes and reinstalls instead of reusing ABI-incompatible builds. - #63/#64 phase 1: every library-affecting command warns loudly when the project pin (.r-version or [project] r_version) doesn't match the active R, including the "pinned but not installed" case. - #52: per-package install timeout (default 30m) via --timeout flag and UVR_INSTALL_TIMEOUT env. Stale 00LOCK-<pkg>/ dirs are cleaned up on any failure path (timeout, exit, error). - #58: Ctrl+C now kills in-flight R CMD INSTALL, removes its 00LOCK dir, and exits 130. Process-global registry tracks active installs. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent 160ae0f commit fd0e206

15 files changed

Lines changed: 740 additions & 60 deletions

File tree

Cargo.lock

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ members = ["crates/uvr", "crates/uvr-core"]
33
resolver = "2"
44

55
[workspace.package]
6-
version = "0.2.15"
6+
version = "0.2.16"
77
edition = "2021"
88
authors = ["uvr contributors"]
99
license = "MIT"

crates/uvr-core/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ urlencoding.workspace = true
3131
zip.workspace = true
3232
regex.workspace = true
3333

34+
[target.'cfg(unix)'.dependencies]
35+
libc = "0.2"
36+
3437
[build-dependencies]
3538
serde_json.workspace = true
3639
serde.workspace = true

crates/uvr-core/src/installer/r_cmd_install.rs

Lines changed: 251 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,86 @@
11
use std::io::{BufRead, BufReader};
22
use std::path::Path;
33
use std::process::{Command, Stdio};
4+
use std::sync::atomic::{AtomicBool, Ordering};
5+
use std::sync::Arc;
6+
use std::time::{Duration, Instant};
47

58
use crate::error::{Result, UvrError};
69

10+
/// Default per-package install timeout when neither `--timeout` nor the env var is set.
11+
pub const DEFAULT_INSTALL_TIMEOUT: Duration = Duration::from_secs(30 * 60);
12+
13+
/// Parse a duration like `30m`, `2h`, `90s`, or a bare number (interpreted as seconds).
14+
/// Returns `None` for unparseable input — callers fall back to the default.
15+
pub fn parse_install_timeout(s: &str) -> Option<Duration> {
16+
let s = s.trim();
17+
if s.is_empty() {
18+
return None;
19+
}
20+
let (num, suffix) = match s.find(|c: char| !c.is_ascii_digit()) {
21+
Some(idx) => (&s[..idx], &s[idx..]),
22+
None => (s, ""),
23+
};
24+
let n: u64 = num.parse().ok()?;
25+
let secs = match suffix.trim() {
26+
"" | "s" | "sec" | "secs" | "second" | "seconds" => n,
27+
"m" | "min" | "mins" | "minute" | "minutes" => n.checked_mul(60)?,
28+
"h" | "hr" | "hrs" | "hour" | "hours" => n.checked_mul(3600)?,
29+
_ => return None,
30+
};
31+
Some(Duration::from_secs(secs))
32+
}
33+
34+
/// Resolve the effective install timeout: explicit override > env var > default.
35+
pub fn effective_install_timeout(explicit: Option<Duration>) -> Duration {
36+
if let Some(d) = explicit {
37+
return d;
38+
}
39+
if let Ok(env) = std::env::var("UVR_INSTALL_TIMEOUT") {
40+
if let Some(d) = parse_install_timeout(&env) {
41+
return d;
42+
}
43+
}
44+
DEFAULT_INSTALL_TIMEOUT
45+
}
46+
47+
/// Remove a stale `00LOCK-<package>/` directory left behind by an aborted
48+
/// `R CMD INSTALL`. Best-effort: if removal fails the next install attempt
49+
/// will surface a clearer error than a silent skip would.
50+
pub fn cleanup_lock_dir(library: &Path, package_name: &str) {
51+
let lock_dir = library.join(format!("00LOCK-{package_name}"));
52+
if lock_dir.exists() {
53+
let _ = std::fs::remove_dir_all(&lock_dir);
54+
}
55+
}
56+
57+
/// Send a TERM-equivalent signal to a child process by PID. On Unix uses
58+
/// SIGTERM (graceful), on Windows uses TerminateProcess (immediate). Best-effort.
59+
fn kill_pid(pid: u32) {
60+
#[cfg(unix)]
61+
unsafe {
62+
// SIGTERM is graceful; the process gets a chance to clean up. If it
63+
// ignores TERM, the timeout watchdog can be hardened later with SIGKILL.
64+
libc::kill(pid as i32, libc::SIGTERM);
65+
}
66+
#[cfg(windows)]
67+
{
68+
use std::process::Command;
69+
// taskkill /F /T /PID <pid> kills the whole process tree. This catches
70+
// R's child Rscript / cc.exe sub-processes that would otherwise live on.
71+
let _ = Command::new("taskkill")
72+
.args(["/F", "/T", "/PID", &pid.to_string()])
73+
.stdin(std::process::Stdio::null())
74+
.stdout(std::process::Stdio::null())
75+
.stderr(std::process::Stdio::null())
76+
.status();
77+
}
78+
#[cfg(not(any(unix, windows)))]
79+
{
80+
let _ = pid; // suppress unused warning on exotic targets
81+
}
82+
}
83+
784
pub struct RCmdInstall {
885
pub r_binary: String,
986
}
@@ -17,32 +94,62 @@ impl RCmdInstall {
1794

1895
/// Run `R CMD INSTALL --library=<lib_path> --no-test-load <tarball>`.
1996
/// On failure, the captured stderr is included in the error message.
97+
/// On any failure (timeout, non-zero exit, parse error), the
98+
/// `00LOCK-<package>/` directory is removed from `library`.
2099
pub fn install(&self, tarball: &Path, library: &Path, package_name: &str) -> Result<()> {
21-
let mut cmd = self.build_cmd(tarball, library);
22-
cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
100+
let result: Result<()> = (|| {
101+
let mut cmd = self.build_cmd(tarball, library);
102+
cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
23103

24-
let output = cmd.output()?;
104+
let output = cmd.output()?;
25105

26-
if !output.status.success() {
27-
let code = output.status.code().unwrap_or(-1);
28-
let stderr = String::from_utf8_lossy(&output.stderr);
29-
let stdout = String::from_utf8_lossy(&output.stdout);
30-
let log = if stderr.is_empty() { stdout } else { stderr };
31-
return Err(UvrError::Other(format!(
32-
"R CMD INSTALL failed for '{package_name}' (exit {code}):\n{log}"
33-
)));
106+
if !output.status.success() {
107+
let code = output.status.code().unwrap_or(-1);
108+
let stderr = String::from_utf8_lossy(&output.stderr);
109+
let stdout = String::from_utf8_lossy(&output.stdout);
110+
let log = if stderr.is_empty() { stdout } else { stderr };
111+
return Err(UvrError::Other(format!(
112+
"R CMD INSTALL failed for '{package_name}' (exit {code}):\n{log}"
113+
)));
114+
}
115+
Ok(())
116+
})();
117+
if result.is_err() {
118+
cleanup_lock_dir(library, package_name);
34119
}
35-
36-
Ok(())
120+
result
37121
}
38122

39123
/// Like `install`, but streams stderr line-by-line to a callback so the
40-
/// caller can update a progress spinner with compilation output.
124+
/// caller can update a progress spinner with compilation output. The
125+
/// subprocess is killed if it runs longer than `timeout` (or
126+
/// `UVR_INSTALL_TIMEOUT`, or 30m default) — see #52. On any failure
127+
/// the `00LOCK-<package>/` dir is cleaned up.
41128
pub fn install_streaming<F>(
42129
&self,
43130
tarball: &Path,
44131
library: &Path,
45132
package_name: &str,
133+
timeout: Option<Duration>,
134+
on_line: F,
135+
) -> Result<()>
136+
where
137+
F: Fn(&str),
138+
{
139+
let timeout = effective_install_timeout(timeout);
140+
let result = self.install_streaming_inner(tarball, library, package_name, timeout, on_line);
141+
if result.is_err() {
142+
cleanup_lock_dir(library, package_name);
143+
}
144+
result
145+
}
146+
147+
fn install_streaming_inner<F>(
148+
&self,
149+
tarball: &Path,
150+
library: &Path,
151+
package_name: &str,
152+
timeout: Duration,
46153
on_line: F,
47154
) -> Result<()>
48155
where
@@ -52,11 +159,51 @@ impl RCmdInstall {
52159
cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
53160

54161
let mut child = cmd.spawn()?;
162+
let pid = child.id();
163+
164+
// Register this install so the SIGINT handler can kill the child + clean
165+
// up its 00LOCK on Ctrl+C (#58). Deregister at the end (success or fail).
166+
crate::signal::register(crate::signal::ActiveInstall {
167+
pid,
168+
library: library.to_path_buf(),
169+
package_name: package_name.to_string(),
170+
});
171+
struct Deregister(u32);
172+
impl Drop for Deregister {
173+
fn drop(&mut self) {
174+
crate::signal::unregister(self.0);
175+
}
176+
}
177+
let _deregister_guard = Deregister(pid);
178+
179+
// Watchdog: kill the child if `timeout` elapses before completion.
180+
// The flag tells the watchdog the install thread is done (success or
181+
// graceful failure), so the watchdog never kills a finished process.
182+
let completed = Arc::new(AtomicBool::new(false));
183+
let timed_out = Arc::new(AtomicBool::new(false));
184+
let watchdog = {
185+
let completed = Arc::clone(&completed);
186+
let timed_out = Arc::clone(&timed_out);
187+
std::thread::spawn(move || {
188+
let start = Instant::now();
189+
while start.elapsed() < timeout {
190+
if completed.load(Ordering::SeqCst) {
191+
return;
192+
}
193+
std::thread::sleep(Duration::from_millis(200));
194+
}
195+
if !completed.load(Ordering::SeqCst) {
196+
timed_out.store(true, Ordering::SeqCst);
197+
kill_pid(pid);
198+
}
199+
})
200+
};
55201

56202
// Collect all output for error reporting
57203
let mut all_stderr = String::new();
58204

59-
// Read stderr line-by-line to update progress
205+
// Read stderr line-by-line to update progress. When the watchdog kills
206+
// the child, the pipe closes and this loop exits naturally.
60207
if let Some(stderr) = child.stderr.take() {
61208
let reader = BufReader::new(stderr);
62209
for line in reader.lines().map_while(|l| l.ok()) {
@@ -70,6 +217,18 @@ impl RCmdInstall {
70217
}
71218

72219
let status = child.wait()?;
220+
completed.store(true, Ordering::SeqCst);
221+
let _ = watchdog.join();
222+
223+
if timed_out.load(Ordering::SeqCst) {
224+
return Err(UvrError::Other(format!(
225+
"Install of '{package_name}' timed out after {}s — killed by uvr (#52). \
226+
Override with `--timeout <duration>` or `UVR_INSTALL_TIMEOUT=<duration>` \
227+
(e.g. 1h).",
228+
timeout.as_secs()
229+
)));
230+
}
231+
73232
if !status.success() {
74233
let code = status.code().unwrap_or(-1);
75234
// Also grab stdout if stderr was empty
@@ -176,3 +335,80 @@ impl RCmdInstall {
176335
cmd
177336
}
178337
}
338+
339+
#[cfg(test)]
340+
mod tests {
341+
use super::*;
342+
343+
#[test]
344+
fn parse_seconds_default() {
345+
assert_eq!(parse_install_timeout("90"), Some(Duration::from_secs(90)));
346+
assert_eq!(parse_install_timeout("90s"), Some(Duration::from_secs(90)));
347+
assert_eq!(
348+
parse_install_timeout("90 sec"),
349+
Some(Duration::from_secs(90))
350+
);
351+
}
352+
353+
#[test]
354+
fn parse_minutes() {
355+
assert_eq!(
356+
parse_install_timeout("30m"),
357+
Some(Duration::from_secs(1800))
358+
);
359+
assert_eq!(
360+
parse_install_timeout("30 min"),
361+
Some(Duration::from_secs(1800))
362+
);
363+
}
364+
365+
#[test]
366+
fn parse_hours() {
367+
assert_eq!(parse_install_timeout("2h"), Some(Duration::from_secs(7200)));
368+
assert_eq!(
369+
parse_install_timeout("1 hour"),
370+
Some(Duration::from_secs(3600))
371+
);
372+
}
373+
374+
#[test]
375+
fn parse_invalid_returns_none() {
376+
assert!(parse_install_timeout("").is_none());
377+
assert!(parse_install_timeout("nope").is_none());
378+
assert!(parse_install_timeout("30x").is_none());
379+
assert!(parse_install_timeout("-5m").is_none());
380+
}
381+
382+
#[test]
383+
fn effective_prefers_explicit() {
384+
let d = effective_install_timeout(Some(Duration::from_secs(42)));
385+
assert_eq!(d, Duration::from_secs(42));
386+
}
387+
388+
#[test]
389+
fn effective_default_when_none() {
390+
// Don't tamper with env in this test — just ensure default kicks in
391+
// when neither explicit nor a parseable env var is set.
392+
std::env::remove_var("UVR_INSTALL_TIMEOUT");
393+
let d = effective_install_timeout(None);
394+
assert_eq!(d, DEFAULT_INSTALL_TIMEOUT);
395+
}
396+
397+
#[test]
398+
fn cleanup_lock_dir_removes_directory() {
399+
let tmp = tempfile::TempDir::new().unwrap();
400+
let lib = tmp.path();
401+
let lock = lib.join("00LOCK-foo");
402+
std::fs::create_dir_all(lock.join("nested")).unwrap();
403+
std::fs::write(lock.join("file"), "x").unwrap();
404+
cleanup_lock_dir(lib, "foo");
405+
assert!(!lock.exists());
406+
}
407+
408+
#[test]
409+
fn cleanup_lock_dir_handles_missing() {
410+
let tmp = tempfile::TempDir::new().unwrap();
411+
// Should not panic when nothing's there.
412+
cleanup_lock_dir(tmp.path(), "doesnotexist");
413+
}
414+
}

crates/uvr-core/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,6 @@ pub mod project;
88
pub mod r_version;
99
pub mod registry;
1010
pub mod resolver;
11+
pub mod signal;
1112
pub mod sysreqs;
1213
pub mod sysreqs_rules;

0 commit comments

Comments
 (0)