git-ents.gitmain
⌘K
foforge
commit 7bbeb66
feat: implement exec-local as an EffectExecutor over the docker sandbox

LocalExecutor adapts the existing docker-sandboxed local backend to git-backend’s executor seam: sync_tree, git_toolchain::export, and cache restore/snapshot are the same calls the engine’s backends make, so materialization stays one code path (scale-out rule 6); spawn hands the container run to a worker thread and wait joins it.

refactor: extract local::export_toolchains from resolve_toolchains Assisted-by: Claude:claude-sonnet-5

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

Cargo.lock @@ -1617,6 +1617,7 @@ version = "0.0.0" dependencies = [ "facet", + "git-backend", "git-store", "git-toolchain", "gix-hash",
crates/git-effect/Cargo.toml @@ -7,6 +7,7 @@ [dependencies] facet = { workspace = true } +git-backend = { workspace = true } git-store = { workspace = true } git-toolchain = { workspace = true } gix-hash = { workspace = true }
crates/git-effect/src/engine.rs @@ -95,7 +95,7 @@ /// panicking: a live buffer is best-effort output for a browser to look at, /// not something worth tearing the process down over if a prior panic /// poisoned it. -fn lock<T>(mutex: &StdMutex<T>) -> std::sync::MutexGuard<'_, T> { +pub(crate) fn lock<T>(mutex: &StdMutex<T>) -> std::sync::MutexGuard<'_, T> { mutex.lock().unwrap_or_else(PoisonError::into_inner) } @@ -945,7 +945,11 @@ /// ## Requirements /// /// @relation(checks.sandbox) -fn activate(command: &str, toolchains: &[String], dirs: &HashMap<String, String>) -> String { +pub(crate) fn activate( + command: &str, + toolchains: &[String], + dirs: &HashMap<String, String>, +) -> String { if toolchains.is_empty() { return command.to_owned(); } @@ -967,7 +971,7 @@ /// ## Requirements /// /// @relation(checks.cache) -fn with_cache_env(command: &str, cache_dir: Option<&str>) -> String { +pub(crate) fn with_cache_env(command: &str, cache_dir: Option<&str>) -> String { match cache_dir { Some(dir) => format!("export EFFECT_CACHE_DIR={dir}; {command}"), None => command.to_owned(), @@ -1158,11 +1162,11 @@ /// A finished effect run: its outcome, wall-clock duration, process exit code /// (when the command ran to completion), and the full terminal session as an /// asciicast v2 recording. -struct RunResult { - status: Status, - duration_secs: u64, - recording: String, - exit_code: Option<i32>, +pub(crate) struct RunResult { + pub(crate) status: Status, + pub(crate) duration_secs: u64, + pub(crate) recording: String, + pub(crate) exit_code: Option<i32>, } /// Run one effect in the Sprite's [`WORKDIR`], recording its terminal session — @@ -1247,7 +1251,7 @@ /// ## Requirements /// /// @relation(checks.sandbox) -fn run_one_docker( +pub(crate) fn run_one_docker( sandbox: &local::Sandbox, name: &str, command: &str,
crates/git-effect/src/lib.rs @@ -8,6 +8,9 @@ //! [`engine`] runs the effects a `post-receive` hook queues, in a Sprite //! sandbox, and records their outcomes through `results`. [`cache`] persists //! a read-write cache directory an effect's command can build up across runs. +//! [`executor`] adapts the Docker backend to [`git_backend::EffectExecutor`] +//! — `exec-local`, the executor seam's local half (`docs/scale-out.adoc`, +//! WS7). //! //! This crate used to be `checks` (definitions in `git-ents-core`, execution //! in `git-ents-server`) — see each module's migration note for the storage @@ -17,6 +20,7 @@ pub mod definition; pub mod docker; pub mod engine; +pub mod executor; pub mod local; pub mod results; #[cfg(test)] @@ -24,4 +28,5 @@ pub use cache::{CACHE_NS, cache_dir, cache_ref}; pub use definition::{EFFECTS_NS, Effect, effect_ref, load, load_all, order, store}; +pub use executor::LocalExecutor; pub use results::{CommitRuns, RESULTS_NS, Run, RunOutcome, Status, record, runs, update_run};
crates/git-effect/src/local.rs @@ -112,14 +112,24 @@ sandbox: &Sandbox, runnable: &[Effect], ) -> Result<Vec<String>, String> { - let mut names: Vec<&str> = runnable + let mut names: Vec<String> = runnable .iter() - .flat_map(|effect| effect.toolchains.iter().map(String::as_str)) + .flat_map(|effect| effect.toolchains.iter().cloned()) .collect(); names.sort_unstable(); names.dedup(); + export_toolchains(repo, sandbox, &names)?; + Ok(names) +} - for name in &names { +/// Extract each of `names` into `sandbox.toolchains_dir()/<name>` via +/// [`git_toolchain::export`], once — a name already exported into this +/// sandbox is left alone. The extraction half of [`resolve_toolchains`], +/// shared with the `exec-local` executor ([`crate::executor`]), which +/// receives its toolchain names pre-resolved rather than reading them off +/// an [`Effect`]. +pub fn export_toolchains(repo: &Path, sandbox: &Sandbox, names: &[String]) -> Result<(), String> { + for name in names { let dest = sandbox.toolchains_dir().join(name); if dest.exists() { continue; @@ -127,7 +137,7 @@ git_toolchain::export(repo, name, &dest) .map_err(|e| format!("could not resolve toolchain {name}: {e}"))?; } - Ok(names.into_iter().map(str::to_owned).collect()) + Ok(()) } /// The container/host-relative path a toolchain named `name` was exported to
crates/git-effect/src/executor.rs @@ -1,0 +1,237 @@ +//! `exec-local`: [`git_backend::EffectExecutor`] over this crate's existing +//! Docker-sandboxed local backend — the `exec-local` row of +//! `docs/scale-out.adoc`'s "EffectExecutor" table (WS7). +//! +//! A thin adapter, not a second execution path: the tree checkout goes +//! through [`local::sync_tree`], toolchains through the same +//! [`git_toolchain::export`] call every local backend uses (via +//! [`local::export_toolchains`]), caches through [`cache::restore_local`] / +//! [`cache::snapshot_local`], and the run itself through the engine's own +//! Docker backend. That is correctness rule 6 ("materialization is one code +//! path") applied to the local/remote split: `exec-local` and a +//! push-triggered engine run may differ in who orchestrates them, never in +//! the code that materializes and runs an effect. + +use std::collections::HashMap; +use std::path::PathBuf; +use std::sync::{Arc, Mutex as StdMutex, PoisonError}; +use std::thread::JoinHandle; + +use git_backend::{ + EffectDef, EffectExecutor, EffectHandle, EffectStatus, Error, MaterializedInputs, +}; + +use crate::results::Status; +use crate::{cache, docker, engine, local}; + +/// [`EffectExecutor`] running each effect in a throwaway local Docker +/// container (see [`crate::docker`]), materialized from one repository. +/// `spawn` prepares the sandbox and hands the run to a worker thread; +/// `wait` joins it. +/// +/// ## Requirements +/// +/// @relation(checks.sandbox) +pub struct LocalExecutor { + repo: PathBuf, + running: StdMutex<HashMap<String, JoinHandle<Status>>>, +} + +impl LocalExecutor { + /// An executor materializing effects from (and snapshotting caches back + /// to) `repo`. + #[must_use] + pub fn new(repo: impl Into<PathBuf>) -> Self { + Self { + repo: repo.into(), + running: StdMutex::new(HashMap::new()), + } + } +} + +/// Lock `mutex`, recovering the guard from a poisoned lock rather than +/// panicking — same rationale as `engine::lock`: a lost worker entry is +/// worth an error result, never a torn-down process. +fn lock<T>(mutex: &StdMutex<T>) -> std::sync::MutexGuard<'_, T> { + mutex.lock().unwrap_or_else(PoisonError::into_inner) +} + +impl EffectExecutor for LocalExecutor { + fn spawn( + &self, + effect: &EffectDef, + inputs: MaterializedInputs, + ) -> git_backend::Result<EffectHandle> { + let Some(command) = effect.command.clone() else { + return Err(Error::Effect(format!( + "effect {} is composite (no command); its outcome derives from its \ + dependencies instead of a spawn", + effect.name + ))); + }; + docker::ensure_docker().map_err(Error::Effect)?; + let sandbox = local::Sandbox::new().map_err(Error::Effect)?; + local::sync_tree(&self.repo, &sandbox, inputs.tree).map_err(Error::Effect)?; + + // The map's keys name the toolchains to materialize; the PATH + // entries activated in-container are derived from the Docker + // backend's own bind-mount layout, exactly as + // `engine::Backend::resolve_toolchains` derives them per backend — + // a caller-resolved entry describes some other context's + // filesystem, which this container never sees. + let names: Vec<String> = inputs.toolchain_paths.keys().cloned().collect(); + local::export_toolchains(&self.repo, &sandbox, &names).map_err(Error::Effect)?; + let dirs: HashMap<String, String> = names + .iter() + .map(|name| { + ( + name.clone(), + format!("{}/{name}/bin", docker::TOOLCHAINS_DIR), + ) + }) + .collect(); + let mut command = engine::activate(&command, &names, &dirs); + + if let Some(name) = &inputs.cache { + cache::restore_local(&self.repo, &sandbox.cache_dir(name), name) + .map_err(Error::Effect)?; + command = + engine::with_cache_env(&command, Some(&format!("{}/{name}", docker::CACHE_DIR))); + } + + let id = uuid::Uuid::new_v4().to_string(); + let repo = self.repo.clone(); + let name = effect.name.clone(); + let cache_name = inputs.cache.clone(); + let worker = std::thread::spawn(move || { + let live = Arc::new(StdMutex::new(String::new())); + let result = engine::run_one_docker(&sandbox, &name, &command, &live); + if let Some(cache_name) = cache_name + && let Err(e) = + cache::snapshot_local(&repo, &sandbox.cache_dir(&cache_name), &cache_name) + { + eprintln!("effects: could not snapshot cache {cache_name}: {e}"); + } + result.status + }); + lock(&self.running).insert(id.clone(), worker); + Ok(EffectHandle { id }) + } + + fn wait(&self, handle: &EffectHandle) -> git_backend::Result<EffectStatus> { + let Some(worker) = lock(&self.running).remove(&handle.id) else { + return Err(Error::Effect(format!( + "unknown effect handle {}", + handle.id + ))); + }; + let status = match worker.join() { + Ok(status) => status, + Err(_panic) => { + return Err(Error::Effect( + "the effect's worker thread panicked".to_owned(), + )); + } + }; + Ok(match status { + Status::Pass => EffectStatus::Pass, + Status::Fail => EffectStatus::Fail, + // `run_one_docker` only settles Pass/Fail/Error; the queued / + // running / skipped states are engine bookkeeping it never + // returns. + _ => EffectStatus::Error, + }) + } +} + +#[cfg(test)] +mod tests { + #![allow( + clippy::unwrap_used, + clippy::assertions_on_result_states, + reason = "unit test" + )] + + use std::collections::BTreeMap; + use std::process::Command; + + use gix_hash::ObjectId; + + use super::*; + + fn inputs(tree: ObjectId) -> MaterializedInputs { + MaterializedInputs { + tree, + toolchain_paths: BTreeMap::new(), + cache: None, + } + } + + fn zero_tree() -> ObjectId { + ObjectId::from_hex(b"0000000000000000000000000000000000000000").unwrap() + } + + // @relation(checks.sandbox, role=Verifies) + #[test] + fn a_composite_effect_is_never_spawned() { + let executor = LocalExecutor::new("/nonexistent"); + let effect = EffectDef { + name: "all".to_owned(), + command: None, + image: None, + }; + assert!(executor.spawn(&effect, inputs(zero_tree())).is_err()); + } + + // @relation(checks.sandbox, role=Verifies) + #[test] + fn waiting_on_an_unknown_handle_is_an_error() { + let executor = LocalExecutor::new("/nonexistent"); + let handle = EffectHandle { + id: "no-such-run".to_owned(), + }; + assert!(executor.wait(&handle).is_err()); + } + + // @relation(checks.sandbox, role=Verifies) + #[test] + #[cfg_attr( + windows, + ignore = "windows runners use Windows containers; no Linux image support" + )] + fn local_executor_runs_a_trivial_effect() { + if docker::ensure_docker().is_err() { + eprintln!("skipping local_executor_runs_a_trivial_effect: docker is not available"); + return; + } + + let repo = crate::testutil::unique_repo("exec-local"); + let status = Command::new("git") + .arg("-C") + .arg(&repo) + .args(["commit", "--allow-empty", "-q", "-m", "seed"]) + .status() + .unwrap(); + assert!(status.success()); + let tree = Command::new("git") + .arg("-C") + .arg(&repo) + .args(["rev-parse", "HEAD^{tree}"]) + .output() + .unwrap(); + assert!(tree.status.success()); + let tree = + ObjectId::from_hex(String::from_utf8(tree.stdout).unwrap().trim().as_bytes()).unwrap(); + + let executor = LocalExecutor::new(&repo); + let effect = EffectDef { + name: "hello".to_owned(), + command: Some("echo hi-from-exec-local".to_owned()), + image: None, + }; + let handle = executor.spawn(&effect, inputs(tree)).unwrap(); + assert_eq!(executor.wait(&handle).unwrap(), EffectStatus::Pass); + // The handle's backend-side state is consumed by the first wait. + assert!(executor.wait(&handle).is_err()); + } +}