don't cache external job failures if they could be caused by the user killing processes
All checks were successful
/ test (pull_request) Successful in 4m51s
/ test (push) Successful in 5m30s

This commit is contained in:
Jacob Lifshay 2025-10-24 02:27:20 -07:00
parent 7dc4417874
commit edcc5927a5
Signed by: programmerjake
SSH key fingerprint: SHA256:HnFTLGpSm4Q4Fj502oCFisjZSoakwEuTsJJMSke63RQ

View file

@ -12,7 +12,7 @@ use crate::{
}; };
use base64::{Engine, prelude::BASE64_URL_SAFE_NO_PAD}; use base64::{Engine, prelude::BASE64_URL_SAFE_NO_PAD};
use clap::builder::OsStringValueParser; use clap::builder::OsStringValueParser;
use eyre::{Context, bail, ensure, eyre}; use eyre::{Context, ensure, eyre};
use serde::{ use serde::{
Deserialize, Deserializer, Serialize, Serializer, Deserialize, Deserializer, Serialize, Serializer,
de::{DeserializeOwned, Error}, de::{DeserializeOwned, Error},
@ -26,6 +26,7 @@ use std::{
io::Write, io::Write,
marker::PhantomData, marker::PhantomData,
path::{Path, PathBuf}, path::{Path, PathBuf},
process::ExitStatus,
sync::OnceLock, sync::OnceLock,
}; };
@ -365,13 +366,17 @@ impl ExternalJobCaching {
.stdin(std::process::Stdio::null()); .stdin(std::process::Stdio::null());
Ok(cmd) Ok(cmd)
} }
pub fn run<F: FnOnce(std::process::Command) -> eyre::Result<()>>( pub fn run<F>(
self, self,
command_line: Interned<[Interned<OsStr>]>, command_line: Interned<[Interned<OsStr>]>,
input_file_paths: impl IntoIterator<Item = Interned<Path>>, input_file_paths: impl IntoIterator<Item = Interned<Path>>,
output_file_paths: impl IntoIterator<Item = Interned<Path>> + Clone, output_file_paths: impl IntoIterator<Item = Interned<Path>> + Clone,
run_fn: F, run_fn: F,
) -> eyre::Result<()> { exit_status_to_error: impl FnOnce(ExitStatus) -> eyre::Report,
) -> eyre::Result<()>
where
F: FnOnce(std::process::Command) -> eyre::Result<Result<(), ExitStatus>>,
{
let mut hasher = JobCacheHasher::default(); let mut hasher = JobCacheHasher::default();
hasher.hash_iter(command_line.iter(), |hasher, arg| { hasher.hash_iter(command_line.iter(), |hasher, arg| {
hasher.hash_sized_os_str(arg) hasher.hash_sized_os_str(arg)
@ -419,7 +424,26 @@ impl ExternalJobCaching {
}) })
.expect("spawn shouldn't fail"); .expect("spawn shouldn't fail");
run_fn(cmd) run_fn(cmd)
}); })?;
if let Err(exit_status) = result {
// check if the user may have terminated it or something, don't cache the failure
let user_maybe_terminated;
#[cfg(unix)]
{
user_maybe_terminated = std::os::unix::process::ExitStatusExt::signal(&exit_status)
.is_some()
|| exit_status.code().is_none_or(|code| code > 1);
}
#[cfg(not(unix))]
{
user_maybe_terminated = !exit_status.success();
}
if user_maybe_terminated {
let _ = std::fs::remove_file(self.cache_json_path);
return Err(exit_status_to_error(exit_status));
}
}
let result = result.map_err(exit_status_to_error);
ExternalJobCacheV2 { ExternalJobCacheV2 {
version: ExternalJobCacheVersion::CURRENT, version: ExternalJobCacheVersion::CURRENT,
inputs_hash, inputs_hash,
@ -444,16 +468,26 @@ impl ExternalJobCaching {
.write_to_file(self.cache_json_path)?; .write_to_file(self.cache_json_path)?;
result result
} }
pub fn run_maybe_cached<F: FnOnce(std::process::Command) -> eyre::Result<()>>( pub fn run_maybe_cached<F>(
this: Option<Self>, this: Option<Self>,
command_line: Interned<[Interned<OsStr>]>, command_line: Interned<[Interned<OsStr>]>,
input_file_paths: impl IntoIterator<Item = Interned<Path>>, input_file_paths: impl IntoIterator<Item = Interned<Path>>,
output_file_paths: impl IntoIterator<Item = Interned<Path>> + Clone, output_file_paths: impl IntoIterator<Item = Interned<Path>> + Clone,
run_fn: F, run_fn: F,
) -> eyre::Result<()> { exit_status_to_error: impl FnOnce(ExitStatus) -> eyre::Report,
) -> eyre::Result<()>
where
F: FnOnce(std::process::Command) -> eyre::Result<Result<(), ExitStatus>>,
{
match this { match this {
Some(this) => this.run(command_line, input_file_paths, output_file_paths, run_fn), Some(this) => this.run(
None => run_fn(Self::make_command(command_line)?), command_line,
input_file_paths,
output_file_paths,
run_fn,
exit_status_to_error,
),
None => run_fn(Self::make_command(command_line)?)?.map_err(exit_status_to_error),
} }
} }
} }
@ -1119,10 +1153,12 @@ impl<T: ExternalCommand> JobKind for ExternalCommandJobKind<T> {
} }
let status = acquired_job.run_command(cmd, |cmd| cmd.status())?; let status = acquired_job.run_command(cmd, |cmd| cmd.status())?;
if !status.success() { if !status.success() {
bail!("running {command_line:?} failed: {status}") Ok(Err(status))
} else {
Ok(Ok(()))
} }
Ok(())
}, },
|status| eyre!("running {command_line:?} failed: {status}"),
)?; )?;
Ok(job Ok(job
.output_paths() .output_paths()