diff --git a/CHANGELOG.md b/CHANGELOG.md index b2692a2..0327601 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `scontrol` values containing a space are no longer cut at the space, so a job whose working directory, script or log path has a space in it now opens the right file instead of waiting forever on a path that does not exist. + ## [0.2.0] - 2026-08-28 ### Added diff --git a/src/backend/commands.rs b/src/backend/commands.rs index 8684fbe..181626a 100644 --- a/src/backend/commands.rs +++ b/src/backend/commands.rs @@ -1,6 +1,13 @@ use async_process::{Command, Output}; use color_eyre::Result; +use regex::Regex; use std::collections::HashMap; +use std::sync::LazyLock; + +/// A key in `scontrol` output: a word ending in `=`, at the start of the text +/// or after whitespace. Everything up to the next key is the value. +static SCONTROL_KEY: LazyLock = + LazyLock::new(|| Regex::new(r"(?:^|\s)([A-Za-z][A-Za-z0-9_:]*)=").unwrap()); pub fn check_slurm_available() -> Result<()> { use std::process::Command as StdCommand; @@ -58,13 +65,27 @@ pub fn scontrol_show_job(job_id: &str) -> Option { } /// Parse scontrol's space-separated `Key=Value` output into a map. +/// +/// Values run from the `=` to the start of the next key rather than to the +/// next space, so paths containing spaces survive. All four values sqwatch +/// keeps are user-controlled paths. pub fn parse_scontrol_kv(text: &str) -> HashMap { - let mut out = HashMap::new(); - for token in text.split_whitespace() { - if let Some(eq) = token.find('=') { - out.insert(token[..eq].to_string(), token[eq + 1..].to_string()); - } + let keys: Vec<_> = SCONTROL_KEY.captures_iter(text).collect(); + let mut out = HashMap::with_capacity(keys.len()); + + for (i, cap) in keys.iter().enumerate() { + let name = cap.get(1).expect("key group always matches"); + let value_start = cap.get(0).expect("whole match").end(); + let value_end = match keys.get(i + 1) { + Some(next) => next.get(1).expect("key group always matches").start(), + None => text.len(), + }; + out.insert( + name.as_str().to_string(), + text[value_start..value_end].trim().to_string(), + ); } + out } @@ -148,3 +169,67 @@ pub async fn list_qos() -> Vec { .filter(|l| !l.is_empty()) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + + /// One line of `scontrol show job -o`, with a space in three of the paths. + const SPACED: &str = "JobId=42 JobName=my job UserId=me(1000) JobState=RUNNING \ +StdOut=/home/me/my logs/out.txt StdErr=/home/me/my logs/err.txt \ +WorkDir=/scratch/Smith Lab Command=/home/me/run.sh"; + + #[test] + fn keeps_paths_that_contain_spaces() { + let kv = parse_scontrol_kv(SPACED); + assert_eq!(kv.get("StdOut").unwrap(), "/home/me/my logs/out.txt"); + assert_eq!(kv.get("StdErr").unwrap(), "/home/me/my logs/err.txt"); + assert_eq!(kv.get("WorkDir").unwrap(), "/scratch/Smith Lab"); + assert_eq!(kv.get("JobName").unwrap(), "my job"); + } + + #[test] + fn keeps_reading_after_a_value_with_a_space() { + let kv = parse_scontrol_kv(SPACED); + assert_eq!(kv.get("JobId").unwrap(), "42"); + assert_eq!(kv.get("JobState").unwrap(), "RUNNING"); + assert_eq!(kv.get("Command").unwrap(), "/home/me/run.sh"); + } + + #[test] + fn parses_ordinary_output_unchanged() { + let kv = parse_scontrol_kv("JobId=7 Partition=gpu NumCPUs=4 Priority=41823"); + assert_eq!(kv.get("JobId").unwrap(), "7"); + assert_eq!(kv.get("Partition").unwrap(), "gpu"); + assert_eq!(kv.get("NumCPUs").unwrap(), "4"); + assert_eq!(kv.get("Priority").unwrap(), "41823"); + assert_eq!(kv.len(), 4); + } + + #[test] + fn keeps_keys_that_carry_a_colon() { + let kv = parse_scontrol_kv("TresPerNode=gres:gpu:2 MinMemoryNode=8G"); + assert_eq!(kv.get("TresPerNode").unwrap(), "gres:gpu:2"); + assert_eq!(kv.get("MinMemoryNode").unwrap(), "8G"); + } + + #[test] + fn an_empty_value_stays_empty() { + let kv = parse_scontrol_kv("Comment= JobId=1"); + assert_eq!(kv.get("Comment").unwrap(), ""); + assert_eq!(kv.get("JobId").unwrap(), "1"); + } + + #[test] + fn a_multi_line_response_is_read_to_the_end() { + let kv = parse_scontrol_kv("JobId=1 WorkDir=/a b\nJobId=2 WorkDir=/c d\n"); + assert_eq!(kv.get("JobId").unwrap(), "2"); + assert_eq!(kv.get("WorkDir").unwrap(), "/c d"); + } + + #[test] + fn no_keys_gives_an_empty_map() { + assert!(parse_scontrol_kv("").is_empty()); + assert!(parse_scontrol_kv("slurm_load_jobs error: Invalid job id").is_empty()); + } +}