diff --git a/docs/site/data/schema/ferrflow-package.json b/docs/site/data/schema/ferrflow-package.json index ebad62cc..416c71d0 100644 --- a/docs/site/data/schema/ferrflow-package.json +++ b/docs/site/data/schema/ferrflow-package.json @@ -692,6 +692,12 @@ "continue" ], "default": "abort" + }, + "timeout": { + "type": "integer", + "minimum": 1, + "description": "Seconds a hook may run before FerrFlow kills it and its child processes, then applies onFailure. Defaults to 900 (15 minutes).", + "default": 900 } }, "additionalProperties": false diff --git a/docs/site/data/schema/ferrflow.json b/docs/site/data/schema/ferrflow.json index a47d4c10..db42ee6a 100644 --- a/docs/site/data/schema/ferrflow.json +++ b/docs/site/data/schema/ferrflow.json @@ -1167,6 +1167,12 @@ "continue" ], "default": "abort" + }, + "timeout": { + "type": "integer", + "minimum": 1, + "description": "Seconds a hook may run before FerrFlow kills it and its child processes, then applies onFailure. Defaults to 900 (15 minutes).", + "default": 900 } }, "additionalProperties": false diff --git a/docs/site/docs-en/configuration/config-file.md b/docs/site/docs-en/configuration/config-file.md index e867b212..eb263401 100644 --- a/docs/site/docs-en/configuration/config-file.md +++ b/docs/site/docs-en/configuration/config-file.md @@ -778,6 +778,7 @@ on_failure = "abort" | `onSuccess` | string | — | Run once after the whole release completes cleanly | | `onError` | string | — | Run once when the release fails; sets `FERRFLOW_ERROR_CODE` (once per run) | | `onFailure` | string | `"abort"` | Strategy — `"abort"` cancels the release on hook failure, `"continue"` prints a warning | +| `timeout` | number | `900` | Seconds a hook may run before it is killed and `onFailure` applies | ### Environment variables @@ -807,6 +808,8 @@ For the once-per-run hooks (`preRelease`, `onSuccess`, `onError`) the per-packag `onFailure` is the failure **strategy** (`abort` / `continue`), not a command. The command that runs _when_ a release fails is `onError`, which receives the failing `FERRFLOW_ERROR_CODE`. +Hooks run with stdin closed, so a command that would prompt (`npm login`, a `gpg` passphrase, a git credential prompt) fails at once instead of waiting forever. A hook still running after `timeout` seconds is killed along with any process it started, and counts as a failed hook: `abort` stops the release, `continue` warns and moves on. Every minute a long hook prints that it is still running. Set `timeout` per package to override the workspace value. On Linux and macOS a hook runs in its own process group so it can be killed with everything it started, which also means a Ctrl+C on an interactive `ferrflow release` stops FerrFlow but not a hook that is already running; the timeout still bounds it. + ### Per-package hooks Package-level hooks **replace** workspace-level hooks for that package (they are not merged). diff --git a/docs/site/docs-fr/configuration/config-file.md b/docs/site/docs-fr/configuration/config-file.md index c7a53a4c..a867d87b 100644 --- a/docs/site/docs-fr/configuration/config-file.md +++ b/docs/site/docs-fr/configuration/config-file.md @@ -637,6 +637,7 @@ on_failure = "abort" | `onSuccess` | string | — | Exécuté une fois après une release entièrement réussie | | `onError` | string | — | Exécuté une fois quand la release échoue ; définit `FERRFLOW_ERROR_CODE` (une fois) | | `onFailure` | string | `"abort"` | Stratégie — `"abort"` annule la release en cas d'échec de hook, `"continue"` affiche un avertissement | +| `timeout` | number | `900` | Secondes qu'un hook peut tourner avant d'être tué et que `onFailure` s'applique | ### Variables d'environnement @@ -666,6 +667,8 @@ Pour les hooks exécutés une seule fois par run (`preRelease`, `onSuccess`, `on `onFailure` est la **stratégie** d'échec (`abort` / `continue`), pas une commande. La commande exécutée _quand_ une release échoue est `onError`, qui reçoit le `FERRFLOW_ERROR_CODE` fautif. +Les hooks s'exécutent avec stdin fermé : une commande qui poserait une question (`npm login`, une passphrase `gpg`, une demande d'identifiants git) échoue tout de suite au lieu d'attendre indéfiniment. Un hook encore en cours après `timeout` secondes est tué avec les processus qu'il a lancés, et compte comme un hook en échec : `abort` arrête la release, `continue` affiche un avertissement et poursuit. Chaque minute, un hook long indique qu'il tourne toujours. `timeout` défini sur un package remplace la valeur du workspace. Sous Linux et macOS, un hook tourne dans son propre groupe de processus pour pouvoir être tué avec tout ce qu'il a lancé : un Ctrl+C sur un `ferrflow release` interactif arrête donc FerrFlow mais pas un hook déjà en cours, qui reste borné par son timeout. + ### Hooks par package Les hooks au niveau du package **remplacent** les hooks du workspace pour ce package (ils ne sont pas fusionnés). diff --git a/schema/ferrflow-package.json b/schema/ferrflow-package.json index ebad62cc..416c71d0 100644 --- a/schema/ferrflow-package.json +++ b/schema/ferrflow-package.json @@ -692,6 +692,12 @@ "continue" ], "default": "abort" + }, + "timeout": { + "type": "integer", + "minimum": 1, + "description": "Seconds a hook may run before FerrFlow kills it and its child processes, then applies onFailure. Defaults to 900 (15 minutes).", + "default": 900 } }, "additionalProperties": false diff --git a/schema/ferrflow.json b/schema/ferrflow.json index a47d4c10..db42ee6a 100644 --- a/schema/ferrflow.json +++ b/schema/ferrflow.json @@ -1167,6 +1167,12 @@ "continue" ], "default": "abort" + }, + "timeout": { + "type": "integer", + "minimum": 1, + "description": "Seconds a hook may run before FerrFlow kills it and its child processes, then applies onFailure. Defaults to 900 (15 minutes).", + "default": 900 } }, "additionalProperties": false diff --git a/src/config/types.rs b/src/config/types.rs index e45ba098..efb17457 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -65,6 +65,8 @@ pub struct HooksConfig { pub on_error: Option, #[serde(default, alias = "onFailure")] pub on_failure: Option, + #[serde(default)] + pub timeout: Option, } #[derive(Debug, Deserialize, Serialize, Clone, Copy, PartialEq, Default)] diff --git a/src/hooks/mod.rs b/src/hooks/mod.rs index b86c14b9..31fd3e65 100644 --- a/src/hooks/mod.rs +++ b/src/hooks/mod.rs @@ -5,5 +5,5 @@ mod runner; pub use context::{HookCommit, HookContext, HookFile, HookPackage}; pub use point::HookPoint; -pub use resolve::{resolve_hook, resolve_on_failure}; +pub use resolve::{HookPolicy, resolve_hook, resolve_policy}; pub use runner::{capture_build_metadata, run_hook}; diff --git a/src/hooks/resolve.rs b/src/hooks/resolve.rs index d5ac4a8e..9dc125a9 100644 --- a/src/hooks/resolve.rs +++ b/src/hooks/resolve.rs @@ -1,7 +1,31 @@ +use std::time::Duration; + use crate::config::{HooksConfig, OnFailure}; use super::HookPoint; +pub const DEFAULT_HOOK_TIMEOUT: Duration = Duration::from_secs(15 * 60); + +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct HookPolicy { + pub on_failure: OnFailure, + pub timeout: Duration, +} + +pub fn resolve_policy( + pkg_hooks: Option<&HooksConfig>, + ws_hooks: Option<&HooksConfig>, +) -> HookPolicy { + let timeout = pkg_hooks + .and_then(|h| h.timeout) + .or_else(|| ws_hooks.and_then(|h| h.timeout)) + .map_or(DEFAULT_HOOK_TIMEOUT, |secs| Duration::from_secs(secs.get())); + HookPolicy { + on_failure: resolve_on_failure(pkg_hooks, ws_hooks), + timeout, + } +} + pub fn resolve_hook( pkg_hooks: Option<&HooksConfig>, ws_hooks: Option<&HooksConfig>, @@ -58,6 +82,7 @@ pub fn resolve_on_failure( #[cfg(test)] mod tests { use super::*; + use std::num::NonZeroU64; fn ws_hooks(pre_bump: Option<&str>, post_publish: Option<&str>) -> HooksConfig { HooksConfig { @@ -98,6 +123,45 @@ mod tests { assert!(result.is_none()); } + #[test] + fn a_zero_timeout_is_rejected_when_the_config_is_read() { + let zero = serde_json::from_str::(r#"{"timeout": 0}"#); + assert!(zero.is_err()); + let ten = serde_json::from_str::(r#"{"timeout": 10}"#).unwrap(); + assert_eq!(ten.timeout, NonZeroU64::new(10)); + } + + #[test] + fn timeout_defaults_to_fifteen_minutes() { + assert_eq!(resolve_policy(None, None).timeout, DEFAULT_HOOK_TIMEOUT); + assert_eq!(DEFAULT_HOOK_TIMEOUT, Duration::from_secs(900)); + } + + #[test] + fn timeout_comes_from_the_package_then_the_workspace() { + let ws = HooksConfig { + timeout: NonZeroU64::new(120), + ..Default::default() + }; + let pkg = HooksConfig { + timeout: NonZeroU64::new(30), + ..Default::default() + }; + + assert_eq!( + resolve_policy(None, Some(&ws)).timeout, + Duration::from_secs(120) + ); + assert_eq!( + resolve_policy(Some(&pkg), Some(&ws)).timeout, + Duration::from_secs(30) + ); + assert_eq!( + resolve_policy(Some(&HooksConfig::default()), Some(&ws)).timeout, + Duration::from_secs(120) + ); + } + #[test] fn on_failure_defaults_to_abort() { assert_eq!(resolve_on_failure(None, None), OnFailure::Abort); @@ -140,6 +204,7 @@ mod tests { on_success: Some("j".into()), on_error: Some("k".into()), on_failure: None, + timeout: None, }; for (point, expected) in [ (HookPoint::PreBump, "a"), diff --git a/src/hooks/runner.rs b/src/hooks/runner.rs index e6725db0..924dd7c2 100644 --- a/src/hooks/runner.rs +++ b/src/hooks/runner.rs @@ -1,18 +1,21 @@ +mod process; + use crate::config::OnFailure; use crate::error_code::{self, ErrorCodeExt}; use anyhow::Context as _; use anyhow::Result; use colored::Colorize; use std::path::Path; -use std::process::{Command, Stdio}; +use std::process::Command; -use super::{HookContext, HookPoint}; +use super::{HookContext, HookPoint, HookPolicy}; +use process::{Finish, run_bounded}; pub fn run_hook( point: HookPoint, command: &str, ctx: &HookContext, - on_failure: OnFailure, + policy: HookPolicy, dry_run: bool, verbose: bool, working_dir: &Path, @@ -34,34 +37,35 @@ pub fn run_hook( command ); - let mut cmd = build_hook_command(command, ctx, working_dir); - - if verbose { - let status = cmd - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .status()?; - - if !status.success() { - return handle_failure(point, command, status.code(), on_failure); - } - } else { - let output = cmd.output()?; - - if !output.status.success() { - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - if !stdout.is_empty() { - eprint!("{stdout}"); - } - if !stderr.is_empty() { - eprint!("{stderr}"); - } - return handle_failure(point, command, output.status.code(), on_failure); + let cmd = build_hook_command(command, ctx, working_dir); + + match run_bounded(cmd, !verbose, policy.timeout, point.label())? { + Finish::Exited { status, .. } if status.success() => Ok(()), + Finish::Exited { + status, + stdout, + stderr, + } => { + echo_captured(&stdout); + echo_captured(&stderr); + let code = status + .code() + .map_or_else(|| "signal".to_string(), |c| c.to_string()); + handle_failure(point, command, &format!("exit {code}"), policy.on_failure) } + Finish::TimedOut => handle_failure( + point, + command, + &format!("timed out after {}s", policy.timeout.as_secs()), + policy.on_failure, + ), } +} - Ok(()) +fn echo_captured(output: &[u8]) { + if !output.is_empty() { + eprint!("{}", String::from_utf8_lossy(output)); + } } fn build_hook_command(command: &str, ctx: &HookContext, working_dir: &Path) -> Command { @@ -124,29 +128,21 @@ fn build_command(command: &str) -> Command { fn handle_failure( point: HookPoint, command: &str, - code: Option, + reason: &str, on_failure: OnFailure, ) -> Result<()> { - let code_str = code - .map(|c| c.to_string()) - .unwrap_or_else(|| "signal".to_string()); - match on_failure { OnFailure::Abort => Err(anyhow::anyhow!( - "hook [{}] failed (exit {}): {}", - point.label(), - code_str, - command + "hook [{}] failed ({reason}): {command}", + point.label() )) .error_code(error_code::HOOK_FAILED)?, OnFailure::Continue => { tracing::warn!( "{}", format!( - " Warning: hook [{}] failed (exit {}): {}", - point.label(), - code_str, - command + " Warning: hook [{}] failed ({reason}): {command}", + point.label() ) .yellow() ); @@ -181,7 +177,7 @@ mod tests { #[test] fn handle_failure_abort_returns_error() { - let result = handle_failure(HookPoint::PreBump, "echo fail", Some(1), OnFailure::Abort); + let result = handle_failure(HookPoint::PreBump, "echo fail", "exit 1", OnFailure::Abort); assert!(result.is_err()); let msg = format!("{:?}", result.unwrap_err()); assert!(msg.contains("pre_bump")); @@ -194,18 +190,11 @@ mod tests { let result = handle_failure( HookPoint::PostBump, "echo fail", - Some(42), + "exit 42", OnFailure::Continue, ); assert!(result.is_ok()); } - - #[test] - fn handle_failure_signal_no_exit_code() { - let result = handle_failure(HookPoint::PreCommit, "killed", None, OnFailure::Abort); - assert!(result.is_err()); - assert!(format!("{:?}", result.unwrap_err()).contains("signal")); - } } #[cfg(test)] diff --git a/src/hooks/runner/process.rs b/src/hooks/runner/process.rs new file mode 100644 index 00000000..4c690c71 --- /dev/null +++ b/src/hooks/runner/process.rs @@ -0,0 +1,123 @@ +use anyhow::{Context, Result}; +use std::io::Read; +use std::process::{Child, Command, ExitStatus, Stdio}; +use std::sync::mpsc::{self, Receiver}; +use std::time::{Duration, Instant}; + +const POLL_INTERVAL: Duration = Duration::from_millis(50); +const HEARTBEAT_INTERVAL: Duration = Duration::from_secs(60); + +pub(super) enum Finish { + Exited { + status: ExitStatus, + stdout: Vec, + stderr: Vec, + }, + TimedOut, +} + +pub(super) fn run_bounded( + mut cmd: Command, + capture: bool, + timeout: Duration, + label: &str, +) -> Result { + cmd.stdin(Stdio::null()); + if capture { + cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); + } else { + cmd.stdout(Stdio::inherit()).stderr(Stdio::inherit()); + } + own_process_group(&mut cmd); + + let deadline = Instant::now() + timeout; + let mut child = cmd.spawn().context("failed to start the hook")?; + let stdout = child.stdout.take().map(drain); + let stderr = child.stderr.take().map(drain); + + let Some(status) = wait_until(&mut child, deadline, label)? else { + return Ok(time_out(&mut child)); + }; + match (collect(stdout, deadline), collect(stderr, deadline)) { + (Some(stdout), Some(stderr)) => Ok(Finish::Exited { + status, + stdout, + stderr, + }), + _ => Ok(time_out(&mut child)), + } +} + +fn time_out(child: &mut Child) -> Finish { + kill_tree(child); + let _ = child.wait(); + Finish::TimedOut +} + +fn wait_until(child: &mut Child, deadline: Instant, label: &str) -> Result> { + let start = Instant::now(); + let mut next_heartbeat = start + HEARTBEAT_INTERVAL; + loop { + if let Some(status) = child.try_wait()? { + return Ok(Some(status)); + } + let now = Instant::now(); + if now >= deadline { + return Ok(None); + } + if now >= next_heartbeat { + tracing::info!( + " [{label}] still running after {}s", + (now - start).as_secs() + ); + next_heartbeat += HEARTBEAT_INTERVAL; + } + std::thread::sleep(POLL_INTERVAL); + } +} + +fn drain(mut pipe: impl Read + Send + 'static) -> Receiver> { + let (tx, rx) = mpsc::channel(); + std::thread::spawn(move || { + let mut buf = Vec::new(); + let _ = pipe.read_to_end(&mut buf); + let _ = tx.send(buf); + }); + rx +} + +fn collect(reader: Option>>, deadline: Instant) -> Option> { + let Some(reader) = reader else { + return Some(Vec::new()); + }; + reader + .recv_timeout(deadline.saturating_duration_since(Instant::now())) + .ok() +} + +#[cfg(unix)] +fn own_process_group(cmd: &mut Command) { + use std::os::unix::process::CommandExt; + cmd.process_group(0); +} + +#[cfg(windows)] +fn own_process_group(_cmd: &mut Command) {} + +#[cfg(unix)] +fn kill_tree(child: &mut Child) { + if let Ok(group) = libc::pid_t::try_from(child.id()) { + unsafe { libc::kill(-group, libc::SIGKILL) }; + } + let _ = child.kill(); +} + +#[cfg(windows)] +fn kill_tree(child: &mut Child) { + let _ = Command::new("taskkill") + .args(["/T", "/F", "/PID", &child.id().to_string()]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + let _ = child.kill(); +} diff --git a/src/hooks/runner/process_tests.rs b/src/hooks/runner/process_tests.rs index 01a965e9..c761f90c 100644 --- a/src/hooks/runner/process_tests.rs +++ b/src/hooks/runner/process_tests.rs @@ -50,12 +50,19 @@ fn read(dir: &Path, file: &str) -> String { .to_string() } +fn policy(on_failure: OnFailure) -> HookPolicy { + HookPolicy { + on_failure, + timeout: crate::hooks::resolve::DEFAULT_HOOK_TIMEOUT, + } +} + fn run(command: &str, dir: &Path, on_failure: OnFailure, dry_run: bool) -> Result<()> { run_hook( HookPoint::PreBump, command, &ctx(), - on_failure, + policy(on_failure), dry_run, false, dir, @@ -115,7 +122,7 @@ fn a_release_summary_hook_sees_every_tag_of_the_run() { HookPoint::OnSuccess, &write_var("FERRFLOW_TAG", "tags.txt"), &summary, - OnFailure::Abort, + policy(OnFailure::Abort), false, false, dir.path(), @@ -168,7 +175,7 @@ fn a_failing_hook_aborts_in_verbose_mode_too() { HookPoint::PostTag, "exit 5", &ctx(), - OnFailure::Abort, + policy(OnFailure::Abort), false, true, dir.path(), @@ -224,3 +231,100 @@ fn build_metadata_rejects_output_semver_would_refuse() { "{err:#}" ); } + +fn sleep_for(seconds: u32) -> String { + if cfg!(windows) { + format!("ping -n {} 127.0.0.1 >NUL", seconds + 1) + } else { + format!("sleep {seconds}") + } +} + +fn bounded(on_failure: OnFailure, seconds: u64) -> HookPolicy { + HookPolicy { + on_failure, + timeout: std::time::Duration::from_secs(seconds), + } +} + +fn run_with(command: &str, dir: &Path, policy: HookPolicy) -> Result<()> { + run_hook( + HookPoint::PreBump, + command, + &ctx(), + policy, + false, + false, + dir, + ) +} + +#[test] +fn a_hook_past_its_timeout_is_killed_and_reported() { + let dir = tempfile::tempdir().unwrap(); + let started = std::time::Instant::now(); + + let err = run_with(&sleep_for(30), dir.path(), bounded(OnFailure::Abort, 1)).unwrap_err(); + + assert!(started.elapsed() < std::time::Duration::from_secs(15)); + let msg = format!("{err:#}"); + assert!(msg.contains("timed out after 1s"), "{msg}"); + assert_eq!( + crate::error_code::code_from_error(&err), + Some(error_code::HOOK_FAILED.to_string()) + ); +} + +#[test] +fn a_timed_out_hook_with_continue_does_not_stop_the_release() { + let dir = tempfile::tempdir().unwrap(); + run_with(&sleep_for(30), dir.path(), bounded(OnFailure::Continue, 1)).unwrap(); +} + +#[test] +fn a_hook_reading_stdin_gets_end_of_file_instead_of_blocking() { + let dir = tempfile::tempdir().unwrap(); + let command = if cfg!(windows) { + "sort > stdin.txt" + } else { + "cat > stdin.txt" + }; + + run_with(command, dir.path(), bounded(OnFailure::Abort, 10)).unwrap(); + + assert_eq!(read(dir.path(), "stdin.txt"), ""); +} + +#[cfg(unix)] +#[test] +fn a_timeout_also_kills_what_the_hook_started_in_the_background() { + let dir = tempfile::tempdir().unwrap(); + + run_with( + "sleep 30 & echo $! > child.pid; sleep 30", + dir.path(), + bounded(OnFailure::Continue, 1), + ) + .unwrap(); + + let pid = read(dir.path(), "child.pid"); + std::thread::sleep(std::time::Duration::from_millis(200)); + let alive = std::process::Command::new("kill") + .args(["-0", &pid]) + .status() + .unwrap() + .success(); + assert!(!alive, "background child {pid} outlived the hook"); +} + +#[cfg(unix)] +#[test] +fn a_background_process_holding_the_output_open_cannot_outlast_the_timeout() { + let dir = tempfile::tempdir().unwrap(); + let started = std::time::Instant::now(); + + let err = run_with("sleep 30 &", dir.path(), bounded(OnFailure::Abort, 1)).unwrap_err(); + + assert!(started.elapsed() < std::time::Duration::from_secs(15)); + assert!(format!("{err:#}").contains("timed out after 1s"), "{err:#}"); +} diff --git a/src/monorepo/run/execute.rs b/src/monorepo/run/execute.rs index 7e802392..3b442424 100644 --- a/src/monorepo/run/execute.rs +++ b/src/monorepo/run/execute.rs @@ -10,7 +10,7 @@ use std::path::Path; use crate::config::{Config, ReleaseCommitMode}; use crate::error_code::{self, ErrorCodeExt}; use crate::git::{Remote, Repository, create_tag, force_push_tags, push, push_tags}; -use crate::hooks::{HookContext, HookPoint, resolve_hook, resolve_on_failure, run_hook}; +use crate::hooks::{HookContext, HookPoint, resolve_hook, resolve_policy, run_hook}; use super::checkpoint::{Checkpoint, Phase}; use super::summary::{PlannedTag, write_github_step_summary}; @@ -205,14 +205,14 @@ fn run_pre_commit_hooks(plan: &mut ReleasePlan<'_>) -> Result<()> { let pkg = &plan.config.packages[*pkg_idx]; let ws_hooks = plan.config.workspace.hooks.as_ref(); let pkg_hooks = pkg.hooks.as_ref(); - let on_failure = resolve_on_failure(pkg_hooks, ws_hooks); + let policy = resolve_policy(pkg_hooks, ws_hooks); if let Some(cmd) = resolve_hook(pkg_hooks, ws_hooks, HookPoint::PreCommit) { let before = collect_dirty_files(plan.repo); run_hook( HookPoint::PreCommit, &cmd, ctx, - on_failure, + policy, plan.dry_run, plan.verbose, plan.root, @@ -291,13 +291,13 @@ fn run_package_hooks(plan: &ReleasePlan<'_>, point: HookPoint) -> Result<()> { let pkg = &plan.config.packages[*pkg_idx]; let ws_hooks = plan.config.workspace.hooks.as_ref(); let pkg_hooks = pkg.hooks.as_ref(); - let on_failure = resolve_on_failure(pkg_hooks, ws_hooks); + let policy = resolve_policy(pkg_hooks, ws_hooks); if let Some(cmd) = resolve_hook(pkg_hooks, ws_hooks, point) { run_hook( point, &cmd, ctx, - on_failure, + policy, plan.dry_run, plan.verbose, plan.root, @@ -310,7 +310,7 @@ fn run_package_hooks(plan: &ReleasePlan<'_>, point: HookPoint) -> Result<()> { fn run_release_summary_hook(plan: &ReleasePlan<'_>, point: HookPoint) -> Result<()> { let ws_hooks = plan.config.workspace.hooks.as_ref(); if let Some(cmd) = resolve_hook(None, ws_hooks, point) { - let on_failure = resolve_on_failure(None, ws_hooks); + let policy = resolve_policy(None, ws_hooks); let tags: Vec = plan.tags_to_create.iter().map(|t| t.tag.clone()).collect(); let mut ctx = HookContext::release_summary(plan.root, &tags, plan.dry_run, plan.config.is_monorepo()); @@ -321,7 +321,7 @@ fn run_release_summary_hook(plan: &ReleasePlan<'_>, point: HookPoint) -> Result< point, &cmd, &ctx, - on_failure, + policy, plan.dry_run, plan.verbose, plan.root, @@ -521,7 +521,7 @@ fn run_post_publish_hooks(plan: &mut ReleasePlan<'_>) -> Result<()> { let pkg = &plan.config.packages[*pkg_idx]; let ws_hooks = plan.config.workspace.hooks.as_ref(); let pkg_hooks = pkg.hooks.as_ref(); - let on_failure = resolve_on_failure(pkg_hooks, ws_hooks); + let policy = resolve_policy(pkg_hooks, ws_hooks); if let Some(cmd) = resolve_hook(pkg_hooks, ws_hooks, HookPoint::PostPublish) { let mut ctx = ctx.clone(); ctx.release_url = release_url_for_tag(plan.forge_results, &ctx.tag); @@ -529,7 +529,7 @@ fn run_post_publish_hooks(plan: &mut ReleasePlan<'_>) -> Result<()> { HookPoint::PostPublish, &cmd, &ctx, - on_failure, + policy, plan.dry_run, plan.verbose, plan.root, @@ -589,7 +589,7 @@ pub(super) fn print_dry_run_hooks(plan: &mut ReleasePlan<'_>) -> Result<()> { let pkg = &plan.config.packages[*pkg_idx]; let ws_hooks = plan.config.workspace.hooks.as_ref(); let pkg_hooks = pkg.hooks.as_ref(); - let on_failure = resolve_on_failure(pkg_hooks, ws_hooks); + let policy = resolve_policy(pkg_hooks, ws_hooks); for point in [ HookPoint::PreCommit, HookPoint::PostCommit, @@ -599,7 +599,7 @@ pub(super) fn print_dry_run_hooks(plan: &mut ReleasePlan<'_>) -> Result<()> { HookPoint::PostPublish, ] { if let Some(cmd) = resolve_hook(pkg_hooks, ws_hooks, point) { - run_hook(point, &cmd, ctx, on_failure, true, plan.verbose, plan.root)?; + run_hook(point, &cmd, ctx, policy, true, plan.verbose, plan.root)?; } } run_publishers_for_package(plan, pkg, &ctx.package, &ctx.new_version, &ctx.tag)?; diff --git a/src/monorepo/run/package.rs b/src/monorepo/run/package.rs index bc16eaf0..2c68c174 100644 --- a/src/monorepo/run/package.rs +++ b/src/monorepo/run/package.rs @@ -8,7 +8,7 @@ use crate::config::{Config, PackageConfig}; use crate::formats::get_handler; use crate::git::tag_exists; use crate::hooks::{ - HookCommit, HookContext, HookFile, HookPackage, HookPoint, resolve_hook, resolve_on_failure, + HookCommit, HookContext, HookFile, HookPackage, HookPoint, resolve_hook, resolve_policy, run_hook, }; use crate::versioning::truncate_version; @@ -284,13 +284,12 @@ impl PackageRun<'_> { hook_ctx: &HookContext, dry_run: bool, ) -> Result<()> { - let on_failure = - resolve_on_failure(pkg.hooks.as_ref(), self.config.workspace.hooks.as_ref()); + let policy = resolve_policy(pkg.hooks.as_ref(), self.config.workspace.hooks.as_ref()); run_hook( point, cmd, hook_ctx, - on_failure, + policy, dry_run, self.flags.verbose, self.root, diff --git a/src/monorepo/run/publish.rs b/src/monorepo/run/publish.rs index 81a9ade6..030d0ea8 100644 --- a/src/monorepo/run/publish.rs +++ b/src/monorepo/run/publish.rs @@ -4,7 +4,7 @@ use std::path::Path; use crate::config::{Config, OnFailure}; use crate::git::Repository; use crate::hooks::{ - HookContext, HookPackage, HookPoint, resolve_hook, resolve_on_failure, run_hook, + HookContext, HookPackage, HookPoint, HookPolicy, resolve_hook, resolve_policy, run_hook, }; use crate::timing::Timing; @@ -156,12 +156,12 @@ impl Publisher<'_> { return Ok(()); }; let ctx = self.summary_context(released_tags); - let on_failure = resolve_on_failure(None, ws_hooks); + let policy = resolve_policy(None, ws_hooks); run_hook( HookPoint::OnSuccess, &cmd, &ctx, - on_failure, + policy, self.flags.dry_run, self.flags.verbose, self.root, @@ -175,11 +175,15 @@ impl Publisher<'_> { }; let mut ctx = self.summary_context(released_tags); ctx.error_code = crate::error_code::code_from_error(err); + let policy = HookPolicy { + on_failure: OnFailure::Continue, + ..resolve_policy(None, ws_hooks) + }; let _ = run_hook( HookPoint::OnError, &cmd, &ctx, - OnFailure::Continue, + policy, self.flags.dry_run, self.flags.verbose, self.root,