diff --git a/hive-jobq/src/builder.rs b/hive-jobq/src/builder.rs index fa1e9807..ed894cae 100644 --- a/hive-jobq/src/builder.rs +++ b/hive-jobq/src/builder.rs @@ -29,7 +29,7 @@ use std::cell::RefCell; use std::collections::HashMap; -use crate::{Dep, DepWhen, NodeId, TerminalState}; +use crate::{Dep, DepWhen, GraphError, NodeId, TerminalState}; /// An opaque identity for a node **within the job being built**. /// @@ -108,6 +108,10 @@ pub enum BuildError { /// The target of the unsatisfiable edge. dep: NodeGuid, }, + /// The graph rejected an otherwise well-formed node — an out-of-group edge, + /// an unsatisfiable [`DepWhen`], and so on. + #[error(transparent)] + Graph(#[from] GraphError), } /// Reject a job whose own declarations don't hold up — **before anything is @@ -306,15 +310,14 @@ impl JobBuilder { /// /// # Errors /// - /// A [`BuildError`] if the job's own declarations don't hold up — see - /// [`check_job_shape`], which decides every one of them **before** the - /// first insert. The sink itself is infallible: by the time it runs, the - /// job is known-good, so no node can be rejected half-way through. + /// [`BuildError::ForwardEdge`] / [`BuildError::ForwardParent`] if a node + /// references one declared after it, or [`BuildError::Graph`] if the graph + /// rejects a node (see [`crate::Graph::insert`]). pub(crate) fn insert_with( self, root_parent: Option, wanted: &[NodeGuid], - mut insert: impl FnMut(N, Vec>, Option) -> NodeId, + mut insert: impl FnMut(N, Vec>, Option) -> Result, ) -> Result, BuildError> { let pending = self.nodes.into_inner(); check_job_shape(&pending, wanted, root_parent)?; @@ -334,10 +337,7 @@ impl JobBuilder { .into_iter() .map(|(name, count)| Dep::Resource { name, count }), ); - // Infallible by construction: `check_job_shape` above decided every - // rejection the graph could raise, so there is nothing left here to - // abandon a half-inserted job on. - let id = insert(node.payload, deps, parent); + let id = insert(node.payload, deps, parent)?; ids.insert(node.guid, id); } Ok(wanted.iter().map(|g| ids[g]).collect()) diff --git a/hive-jobq/src/lib.rs b/hive-jobq/src/lib.rs index 838c0c29..e6f61e54 100644 --- a/hive-jobq/src/lib.rs +++ b/hive-jobq/src/lib.rs @@ -419,18 +419,11 @@ impl Graph { /// graph that minted it, so a dangling reference is rejected here rather than /// surfacing as a broken edge later. /// - /// **Crate-private on purpose.** Nodes enter a graph only through - /// [`crate::builder::JobBuilder`] (via - /// [`crate::scheduler::Scheduler::insert_job`]) or - /// [`crate::scheduler::Scheduler::append`]. A second public way in would be - /// a second place validation has to agree with the builder's, and the two - /// would drift. - /// /// # Errors /// Returns [`GraphError::UnknownDep`] / [`GraphError::UnknownParent`] for a /// dangling dependency or parent id, or [`GraphError::DepOutsideParent`] if a /// `Dep::Node` edge points outside the node's own parent group. - pub(crate) fn insert( + pub fn insert( &mut self, payload: N, deps: Vec>, @@ -451,25 +444,6 @@ impl Graph { } } } - Ok(self.insert_unchecked(payload, deps, parent)) - } - - /// Insert without re-validating — **only** for a node the builder has - /// already proved well-formed. - /// - /// [`crate::builder::check_job_shape`] decides every rejection - /// [`Graph::insert`] could raise, before the first node lands. Re-checking - /// here would not add safety: the insert loop mutates as it goes, so a - /// rejection at node `i` would leave `0..i` in the graph — a loud error - /// *after* the corruption rather than instead of it. Making the sink - /// infallible is what turns the builder's atomicity from an accident of the - /// pre-pass being exhaustive into a property of the types. - pub(crate) fn insert_unchecked( - &mut self, - payload: N, - deps: Vec>, - parent: Option, - ) -> NodeId { let id = self.mint_id(); self.nodes.push(Node { id, @@ -481,7 +455,7 @@ impl Graph { finished_at: None, error: None, }); - id + Ok(id) } /// Borrow a node by id. diff --git a/hive-jobq/src/scheduler.rs b/hive-jobq/src/scheduler.rs index 337c6b2d..2727f0fa 100644 --- a/hive-jobq/src/scheduler.rs +++ b/hive-jobq/src/scheduler.rs @@ -110,12 +110,10 @@ impl Scheduler { /// insert one itself, so there is no way to end up with a job-shaped value /// being passed around as a spec. /// - /// The one insertion entry point for a job. Nodes go straight into - /// [`Graph::insert_unchecked`]: [`crate::builder::check_job_shape`] has - /// already decided every rejection the graph could raise, so re-validating - /// per node could only report a problem *after* the earlier nodes were - /// inserted. Call [`Scheduler::settle`] afterwards to start whatever became - /// runnable. + /// The one insertion entry point: every node goes through + /// [`Scheduler::append`], so a caller never has to reach past the scheduler + /// at the graph underneath. Call [`Scheduler::settle`] afterwards to start + /// whatever became runnable. /// /// **Atomic in the job's own shape.** A forward edge, a forward parent, or /// a request for a handle this job never declared is rejected *before* the @@ -132,9 +130,8 @@ impl Scheduler { ) -> Result, BuildError> { let job = JobBuilder::new(); let wanted = declare(&job); - let graph = &mut self.graph; job.insert_with(root_parent, &wanted, |payload, deps, parent| { - graph.insert_unchecked(payload, deps, parent) + self.append(payload, deps, parent) }) }