Skip to content

Commit bdc4367

Browse files
maybeharshitharssrivcursoragent
authored
fix(spur-cli): honor SPUR_* env-var twins in sbatch (#526)
sbatch resolved env defaults through clap's `env` attribute, which takes a single variable name per flag, so the SPUR_* twins that srun and salloc accept were silently ignored. Route sbatch through the shared env_defaults resolver instead, applying env after the directive merge and only to flags the CLI did not set, which preserves the CLI > env > #SBATCH precedence. SPUR_JOB_NAME and SPUR_NODELIST stay unread: both are injected into every running job, so honoring them would make an sbatch submitted from inside a job inherit the enclosing job's name and node placement. Co-authored-by: harssriv <harssriv@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent dd5f6e3 commit bdc4367

2 files changed

Lines changed: 289 additions & 112 deletions

File tree

crates/spur-cli/src/env_defaults.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) 2026 Advanced Micro Devices, Inc. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
//! Environment-variable defaults for `srun` and `salloc` flags.
4+
//! Environment-variable defaults for `srun`, `salloc`, and `sbatch` flags.
55
//!
66
//! clap's `#[arg(env = ...)]` accepts a single variable name, but Slurm
77
//! defines several aliases per flag (e.g. `--nodes` reads `SLURM_NNODES` and
@@ -169,10 +169,10 @@ where
169169

170170
/// Test-only guard that isolates env-var tests from the runner's environment.
171171
///
172-
/// Clears every `SPUR_/SLURM_/SALLOC_/SRUN_`-prefixed variable on construction
173-
/// and on drop, so a CI runner that injects `SLURM_*` vars cannot perturb the
174-
/// resolvers and a panicking test cannot leak state into the next one. Tests
175-
/// using it must be `#[serial]`.
172+
/// Clears every `SPUR_/SLURM_/SALLOC_/SRUN_/SBATCH_`-prefixed variable on
173+
/// construction and on drop, so a CI runner that injects `SLURM_*` vars cannot
174+
/// perturb the resolvers and a panicking test cannot leak state into the next
175+
/// one. Tests using it must be `#[serial]`.
176176
#[cfg(test)]
177177
pub(crate) struct EnvGuard;
178178

@@ -195,6 +195,7 @@ impl EnvGuard {
195195
|| k.starts_with("SLURM_")
196196
|| k.starts_with("SALLOC_")
197197
|| k.starts_with("SRUN_")
198+
|| k.starts_with("SBATCH_")
198199
})
199200
.collect();
200201
for k in stale {

0 commit comments

Comments
 (0)