From e66fa4096b5747f270d33f1352b5025233e8ad95 Mon Sep 17 00:00:00 2001 From: marshmallow Date: Wed, 8 Jul 2026 09:44:43 +1000 Subject: [PATCH] optimize Eval scheduling for local deploys (#538) --- CHANGELOG.md | 7 +- crates/cli/src/apply.rs | 3 +- crates/core/src/commands/common.rs | 8 +- crates/core/src/errors.rs | 3 + crates/core/src/hive/executor.rs | 66 +++- crates/core/src/hive/node.rs | 3 - crates/core/src/hive/plan.rs | 412 +++++++++++++++++++++---- crates/core/src/hive/steps/activate.rs | 11 +- crates/core/src/hive/steps/build.rs | 287 ++++++++++------- crates/core/src/hive/steps/evaluate.rs | 30 +- crates/core/src/hive/steps/keys.rs | 7 +- crates/core/src/hive/steps/push.rs | 19 +- 12 files changed, 644 insertions(+), 212 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a05276..fd35eea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,7 +32,10 @@ for the up to date details! ### Changed - Changed public cache to `https://cache.forall.systems`, as noted above. -- The pre-activation key stage is now scheduled after the evaluation stage. The +- The Evaluate step is now only scheduled when the `.drv` output is actually + required. This means that when building locally the Build step can construct + the nix attribute directly skipping the Evaluate step. +- The pre-activation key step is now scheduled after the evaluation step. The rationale for this is that while iterating on evaluation errors it was often quite annoying to repeatedly enter your password for key deployment. - Ipv6 addresses are now displayed in a nicer format in "Authenticate for ..." @@ -52,6 +55,8 @@ for the up to date details! - Cache database now uses `SqliteJournalMode::Wal` & `SqliteSynchronous::Normal`. - Attempting to reconnect to a rebooting node will now wait far longer before giving up. +- Internally, Step store path outputs are now passed between steps via handles + instead of mutable state. ### Fixed diff --git a/crates/cli/src/apply.rs b/crates/cli/src/apply.rs index b3c11ae..a389d91 100644 --- a/crates/cli/src/apply.rs +++ b/crates/cli/src/apply.rs @@ -170,7 +170,8 @@ where should_quit.clone(), cached_evaluations .as_mut() - .and_then(|cache| cache.remove(name)), + .and_then(|cache| cache.remove(name)) + .as_ref(), ); let (sender, receiver) = oneshot::channel(); diff --git a/crates/core/src/commands/common.rs b/crates/core/src/commands/common.rs index 204feec..a6e50ec 100644 --- a/crates/core/src/commands/common.rs +++ b/crates/core/src/commands/common.rs @@ -112,11 +112,11 @@ pub async fn evaluate_hive_attribute( let attribute = match location { HiveLocation::Flake { uri, .. } => { format!( - "{uri}#wire --apply \"hive: {}\"", + "{uri}#wire.{}", match goal { - EvalGoal::Inspect => "hive.inspect".to_string(), - EvalGoal::Names => "hive.names".to_string(), - EvalGoal::GetTopLevel(node) => format!("hive.topLevels.{node}"), + EvalGoal::Inspect => "inspect".to_string(), + EvalGoal::Names => "names".to_string(), + EvalGoal::GetTopLevel(node) => format!("topLevels.{node}"), } ) } diff --git a/crates/core/src/errors.rs b/crates/core/src/errors.rs index 7753308..f87c714 100644 --- a/crates/core/src/errors.rs +++ b/crates/core/src/errors.rs @@ -276,6 +276,9 @@ pub enum HiveLibError { #[diagnostic(transparent)] StorePath(StorePathError), + #[error("a step required output from a previous step that was not in the execution plan")] + MissingStepOutput, + #[error("Failed to apply key {}", .0)] KeyError( String, diff --git a/crates/core/src/hive/executor.rs b/crates/core/src/hive/executor.rs index 833a9e5..192d842 100644 --- a/crates/core/src/hive/executor.rs +++ b/crates/core/src/hive/executor.rs @@ -6,7 +6,7 @@ use crate::{ use std::debug_assert_matches; use std::sync::Arc; -use tokio::sync::oneshot; +use tokio::sync::{RwLock, oneshot}; use tracing::{Instrument, Span, debug, error, event, instrument}; use crate::{ @@ -20,6 +20,70 @@ use crate::{ }, }; +/// A shared struct that "Store Path Producing" steps place outputs into. +/// +/// Steps such as Build, Evaluate, `PushKeyAgent` write their store path into +/// this struct, and "Store Path Consuming" steps can +/// read from (Build reads from Evaluate, Keys reads from `PushKeyAgent`, etc) +/// +/// Cloning this value will keep pointing to the same handle. +#[derive(Debug, Clone)] +pub struct OutputHandle { + inner: Arc>>>, +} + +pub type BuildOutputHandle = OutputHandle; +pub type EvaluationOutputHandle = OutputHandle; +pub type KeyAgentPathHandle = OutputHandle; + +#[cfg(test)] +impl PartialEq for OutputHandle { + fn eq(&self, other: &Self) -> bool { + if Arc::ptr_eq(&self.inner, &other.inner) { + return true; + } + + // fall back to comparing the inner store path + match (self.inner.try_read(), other.inner.try_read()) { + (Ok(a), Ok(b)) => *a == *b, + _ => false, + } + } +} + +#[cfg(test)] +impl Eq for OutputHandle {} + +impl OutputHandle { + /// Create a handle for which the store path is not already known + pub(crate) fn new() -> Self { + Self { + inner: Arc::new(RwLock::new(None)), + } + } + + /// Create a handle for a store path which is already known + pub(crate) fn new_known(store_path: SafeStorePath) -> Self { + Self { + inner: Arc::new(RwLock::new(Some(store_path))), + } + } + + pub(crate) async fn set(&self, store_path: SafeStorePath) { + *self.inner.write().await = Some(store_path); + } + + pub(crate) async fn get(&self) -> Option> { + self.inner.read().await.clone() + } + + /// Attempt to read the previously written store path. If the store path + /// producing step was never planned, this will fail. + pub(crate) async fn require(&self) -> Result, HiveLibError> { + self.get().await.ok_or(HiveLibError::MissingStepOutput) + } +} + /// returns Err if the application should shut down. fn app_shutdown_guard(context: &Context) -> Result<(), HiveLibError> { if context diff --git a/crates/core/src/hive/node.rs b/crates/core/src/hive/node.rs index 00f7b3a..a573e92 100644 --- a/crates/core/src/hive/node.rs +++ b/crates/core/src/hive/node.rs @@ -282,10 +282,7 @@ pub enum HandleUnreachable { #[derive(Default)] pub struct StepState { - pub evaluation: Option>, pub evaluation_rx: Option, HiveLibError>>>, - pub build: Option>, - pub key_agent_directory: Option>, } pub type BuildNameMap = Arc>>>; diff --git a/crates/core/src/hive/plan.rs b/crates/core/src/hive/plan.rs index e2f7283..86f641a 100644 --- a/crates/core/src/hive/plan.rs +++ b/crates/core/src/hive/plan.rs @@ -9,13 +9,14 @@ use crate::{ SafeStorePath, SubCommandModifiers, hive::{ HiveLocation, + executor::{BuildOutputHandle, EvaluationOutputHandle, OutputHandle}, node::{ ApplyGoal, Context, HandleUnreachable, Name, Node, SharedTarget, Step, StepState, SwitchToConfigurationGoal, }, steps::{ activate::SwitchToConfiguration, - build::Build, + build::{Build, BuildMetadata, NixCommandBuildMetadata}, evaluate::Evaluate, keys::{Keys, PushKeyAgent, UploadKeyAt}, ping::Ping, @@ -61,6 +62,7 @@ fn apply_plan_keys( } = args; let mut front_steps = Vec::new(); let mut end_steps = Vec::new(); + let key_agent_directory = OutputHandle::new(); let (pre_keys, post_keys) = match goal { ApplyGoal::SwitchToConfiguration(SwitchToConfigurationGoal::Switch) => node @@ -84,6 +86,7 @@ fn apply_plan_keys( } else { Some(target.clone()) }, + key_agent_directory: key_agent_directory.clone(), })); } @@ -96,6 +99,7 @@ fn apply_plan_keys( Some(target.clone()) }, privilege_escalation_command: node.privilege_escalation_command.clone(), + key_agent_directory: key_agent_directory.clone(), })); } @@ -108,12 +112,35 @@ fn apply_plan_keys( Some(target.clone()) }, privilege_escalation_command: node.privilege_escalation_command.clone(), + key_agent_directory, })); } (front_steps, end_steps) } +/// Push an `Evaluate` step onto `steps` when a real (non-cached) evaluation +/// is required, writing its result into `output`. +/// +/// Returns `true` when the step was pushed. This is also exactly the +/// condition when callers should enable greedy evaluation. +fn push_evaluate_step( + steps: &mut Vec, + output: &EvaluationOutputHandle, + needs_evaluate: bool, + has_cached_evaluation: bool, +) -> bool { + if needs_evaluate && !has_cached_evaluation { + steps.push(Step::Evaluate(Evaluate { + output: output.clone(), + })); + true + } else { + false + } +} + +#[allow(clippy::too_many_lines)] fn apply_plan( args: &ApplyGoalArgs, node: &Node, @@ -121,7 +148,7 @@ fn apply_plan( modifiers: SubCommandModifiers, hive_location: Arc, should_quit: Arc, - cached_evaluation: Option>, + cached_evaluation: Option<&SafeStorePath>, ) -> NodePlan { let ApplyGoalArgs { goal, @@ -136,7 +163,14 @@ fn apply_plan( let mut steps: Vec = Vec::new(); let mut end: Vec = Vec::new(); let target = SharedTarget(Arc::new(RwLock::new(node.target.clone()))); - let has_cached_evaluation = cached_evaluation.is_some(); + let build_will_have_target = node.build_remotely && !*should_apply_locally; + + let evaluation_output_handle = cached_evaluation + .map_or_else(EvaluationOutputHandle::new, |cached_evaluation| { + EvaluationOutputHandle::new_known(cached_evaluation.clone()) + }); + + let build_output_handle = BuildOutputHandle::new(); if !*should_apply_locally { steps.push(Step::Ping(Ping { @@ -144,9 +178,24 @@ fn apply_plan( })); } - if !matches!(goal, ApplyGoal::Keys) { - steps.push(Step::Evaluate(Evaluate { cached_evaluation })); - } + let needs_separate_eval = match goal { + ApplyGoal::Keys => false, + // a `.drv` must be known before so it can be pushed + ApplyGoal::Push => true, + // the experimental nix client still requires the path to build + _ if modifiers.experimental_nix_client => true, + // only evaluate if the build step will require a real `.drv` on the remote system + // or if it can use an attribute that exists on the local host directly + _ => build_will_have_target, + }; + let has_cached_evaluation = cached_evaluation.is_some(); + + let greedy_evaluate = push_evaluate_step( + &mut steps, + &evaluation_output_handle, + needs_separate_eval, + has_cached_evaluation, + ); if !matches!(goal, ApplyGoal::Keys) && !should_apply_locally @@ -155,15 +204,37 @@ fn apply_plan( steps.push(Step::PushEvaluatedOutput(PushEvaluatedOutput { substitute_on_destination: *substitute_on_destination, target: target.clone(), + path: evaluation_output_handle.clone(), })); } if !matches!(goal, ApplyGoal::Keys | ApplyGoal::Push) { steps.push(Step::Build(Build { - target: if node.build_remotely && !*should_apply_locally { - Some(target.clone()) + output: build_output_handle.clone(), + metadata: if modifiers.experimental_nix_client { + BuildMetadata::BuildWithNixDaemon { + target: if build_will_have_target { + Some(target.clone()) + } else { + None + }, + derivation: evaluation_output_handle, + } } else { - None + BuildMetadata::NixCommand(if build_will_have_target { + NixCommandBuildMetadata::Remotely { + target: target.clone(), + derivation: evaluation_output_handle, + } + } else { + NixCommandBuildMetadata::Locally { + cached_derivation: if cached_evaluation.is_some() { + Some(evaluation_output_handle) + } else { + None + }, + } + }) }, })); } @@ -175,6 +246,7 @@ fn apply_plan( steps.push(Step::PushBuildOutput(PushBuildOutput { substitute_on_destination: *substitute_on_destination, target: target.clone(), + path: build_output_handle.clone(), })); } @@ -199,6 +271,7 @@ fn apply_plan( Some(target) }, privilege_escalation_command: node.privilege_escalation_command.clone(), + top_level: build_output_handle, })); } @@ -214,7 +287,7 @@ fn apply_plan( build_id_names: Arc::new(Mutex::new(HashMap::new())), }, steps, - greedy_evaluate: !matches!(&goal, ApplyGoal::Keys) && !has_cached_evaluation, + greedy_evaluate, ignore_failed_ping: matches!(handle_unreachable, HandleUnreachable::Ignore), } } @@ -227,27 +300,56 @@ pub fn plan_for_node( hive_location: Arc, modifiers: &SubCommandModifiers, should_quit: Arc, - cached_evaluation: Option>, + cached_evaluation: Option<&SafeStorePath>, ) -> NodePlan { - let greedy_evaluate = cached_evaluation.is_none(); - match goal { - Goal::Build => NodePlan { - context: Context { - state: StepState::default(), - modifiers: *modifiers, - hive_location, - should_quit, - name, - build_id_names: Arc::new(Mutex::new(HashMap::new())), - }, - steps: vec![ - Step::Evaluate(Evaluate { cached_evaluation }), - Step::Build(Build { target: None }), - ], - greedy_evaluate, - ignore_failed_ping: false, - }, + Goal::Build => { + let evaluation_output_handle = cached_evaluation + .map_or_else(EvaluationOutputHandle::new, |cached_evaluation| { + EvaluationOutputHandle::new_known(cached_evaluation.clone()) + }); + + let mut steps = Vec::new(); + + let greedy_evaluate = push_evaluate_step( + &mut steps, + &evaluation_output_handle, + modifiers.experimental_nix_client, + cached_evaluation.is_some(), + ); + + steps.push(Step::Build(Build { + output: BuildOutputHandle::new(), + metadata: if modifiers.experimental_nix_client { + BuildMetadata::BuildWithNixDaemon { + target: None, + derivation: evaluation_output_handle, + } + } else { + BuildMetadata::NixCommand(NixCommandBuildMetadata::Locally { + cached_derivation: if cached_evaluation.is_some() { + Some(evaluation_output_handle) + } else { + None + }, + }) + }, + })); + + NodePlan { + context: Context { + state: StepState::default(), + modifiers: *modifiers, + hive_location, + should_quit, + name, + build_id_names: Arc::new(Mutex::new(HashMap::new())), + }, + steps, + greedy_evaluate, + ignore_failed_ping: false, + } + } Goal::Apply(args) => apply_plan( args, node, @@ -265,8 +367,9 @@ mod tests { use tokio::sync::RwLock; use crate::{ - SubCommandModifiers, function_name, get_test_path, + SafeStorePath, SubCommandModifiers, function_name, get_test_path, hive::{ + executor::{BuildOutputHandle, EvaluationOutputHandle, OutputHandle}, node::{ ApplyGoal, HandleUnreachable, Name, Node, SharedTarget, Step, SwitchToConfigurationGoal, @@ -274,11 +377,11 @@ mod tests { plan::{ApplyGoalArgs, Goal, plan_for_node}, steps::{ activate::SwitchToConfiguration, - build::Build, + build::{Build, BuildMetadata, NixCommandBuildMetadata}, evaluate::Evaluate, keys::{Key, Keys, PushKeyAgent, Source, UploadKeyAt}, ping::Ping, - push::PushEvaluatedOutput, + push::{PushBuildOutput, PushEvaluatedOutput}, }, }, location, @@ -323,13 +426,16 @@ mod tests { assert_eq!( plan.steps, vec![ - Evaluate { - cached_evaluation: None + Build { + output: BuildOutputHandle::new(), + metadata: BuildMetadata::NixCommand(NixCommandBuildMetadata::Locally { + cached_derivation: None, + }), } - .into(), - Build { target: None }.into() + .into() ] ); + assert!(!plan.greedy_evaluate); } #[tokio::test] @@ -367,21 +473,27 @@ mod tests { target: target.clone() } .into(), - crate::hive::steps::evaluate::Evaluate { - cached_evaluation: None + Evaluate { + output: EvaluationOutputHandle::new(), } .into(), - crate::hive::steps::push::PushEvaluatedOutput { + PushEvaluatedOutput { substitute_on_destination: true, - target: target.clone() + target: target.clone(), + path: EvaluationOutputHandle::new(), } .into(), - crate::hive::steps::build::Build { - target: Some(target.clone()) + Build { + output: BuildOutputHandle::new(), + metadata: BuildMetadata::NixCommand(NixCommandBuildMetadata::Remotely { + target: target.clone(), + derivation: EvaluationOutputHandle::new(), + }), } .into(), ] ); + assert!(plan.greedy_evaluate); let node = Node { build_remotely: false, @@ -412,18 +524,22 @@ mod tests { target: target.clone() } .into(), - crate::hive::steps::evaluate::Evaluate { - cached_evaluation: None + Build { + output: BuildOutputHandle::new(), + metadata: BuildMetadata::NixCommand(NixCommandBuildMetadata::Locally { + cached_derivation: None, + }), } .into(), - crate::hive::steps::build::Build { target: None }.into(), - crate::hive::steps::push::PushBuildOutput { + PushBuildOutput { substitute_on_destination: true, - target + target, + path: BuildOutputHandle::new(), } .into(), ] ); + assert!(!plan.greedy_evaluate); } #[tokio::test] @@ -469,18 +585,21 @@ mod tests { PushKeyAgent { substitute_on_destination: true, target: Some(target.clone()), - host_platform: node.host_platform.clone() + host_platform: node.host_platform.clone(), + key_agent_directory: OutputHandle::new(), } .into(), Keys { target: Some(target), // test that all keys are included keys: node.keys.clone(), - privilege_escalation_command: node.privilege_escalation_command + privilege_escalation_command: node.privilege_escalation_command, + key_agent_directory: OutputHandle::new(), } .into(), ] ); + assert!(!plan_apply_keys.greedy_evaluate); } #[tokio::test] @@ -533,7 +652,8 @@ mod tests { PushKeyAgent { substitute_on_destination: true, target: None, - host_platform: node.host_platform.clone() + host_platform: node.host_platform.clone(), + key_agent_directory: OutputHandle::new(), } .into(), Keys { @@ -544,7 +664,8 @@ mod tests { .filter(|key| matches!(key.upload_at, UploadKeyAt::PreActivation)) .cloned() .collect::>(), - privilege_escalation_command: node.privilege_escalation_command.clone() + privilege_escalation_command: node.privilege_escalation_command.clone(), + key_agent_directory: OutputHandle::new(), } .into(), Keys { @@ -555,7 +676,8 @@ mod tests { .filter(|key| matches!(key.upload_at, UploadKeyAt::PostActivation)) .cloned() .collect::>(), - privilege_escalation_command: node.privilege_escalation_command.clone() + privilege_escalation_command: node.privilege_escalation_command.clone(), + key_agent_directory: OutputHandle::new(), } .into(), ] @@ -595,16 +717,18 @@ mod tests { } .into(), Evaluate { - cached_evaluation: None + output: EvaluationOutputHandle::new(), } .into(), PushEvaluatedOutput { substitute_on_destination: true, - target + target, + path: EvaluationOutputHandle::new(), } .into() ] ); + assert!(plan.greedy_evaluate); } #[tokio::test] @@ -643,16 +767,21 @@ mod tests { } .into(), Evaluate { - cached_evaluation: None + output: EvaluationOutputHandle::new(), } .into(), PushEvaluatedOutput { substitute_on_destination: true, - target: target.clone() + target: target.clone(), + path: EvaluationOutputHandle::new(), } .into(), Build { - target: Some(target.clone()) + output: BuildOutputHandle::new(), + metadata: BuildMetadata::NixCommand(NixCommandBuildMetadata::Remotely { + target: target.clone(), + derivation: EvaluationOutputHandle::new(), + }), } .into(), SwitchToConfiguration { @@ -660,10 +789,12 @@ mod tests { reboot: false, target: Some(target), privilege_escalation_command: node.privilege_escalation_command, + top_level: BuildOutputHandle::new(), } .into(), ] ); + assert!(plan.greedy_evaluate); } #[tokio::test] @@ -703,16 +834,21 @@ mod tests { } .into(), Evaluate { - cached_evaluation: None + output: EvaluationOutputHandle::new(), } .into(), PushEvaluatedOutput { substitute_on_destination: true, - target: target.clone() + target: target.clone(), + path: EvaluationOutputHandle::new(), } .into(), Build { - target: Some(target.clone()) + output: BuildOutputHandle::new(), + metadata: BuildMetadata::NixCommand(NixCommandBuildMetadata::Remotely { + target: target.clone(), + derivation: EvaluationOutputHandle::new(), + }), } .into(), SwitchToConfiguration { @@ -720,10 +856,12 @@ mod tests { reboot: false, target: Some(target), privilege_escalation_command: node.privilege_escalation_command, + top_level: BuildOutputHandle::new(), } .into(), ] ); + assert!(plan.greedy_evaluate); } #[tokio::test] @@ -753,19 +891,167 @@ mod tests { assert_eq!( plan.steps, vec![ - Evaluate { - cached_evaluation: None + Build { + output: BuildOutputHandle::new(), + metadata: BuildMetadata::NixCommand(NixCommandBuildMetadata::Locally { + cached_derivation: None, + }), } .into(), - Build { target: None }.into(), SwitchToConfiguration { goal: SwitchToConfigurationGoal::Switch, reboot: false, target: None, privilege_escalation_command: node.privilege_escalation_command, + top_level: BuildOutputHandle::new(), + } + .into(), + ] + ); + assert!(!plan.greedy_evaluate); + } + + #[tokio::test] + async fn order_build_cached_evaluation() { + let location = location!(get_test_path!()); + let node = Node::default(); + let name = &Name(function_name!().into()); + let should_quit = Arc::new(AtomicBool::new(false)); + let cached = SafeStorePath::::from_absolute_path(b"/nix/store/name").unwrap(); + let plan = plan_for_node( + &node, + name.clone(), + &Goal::Build, + location.into(), + &SubCommandModifiers::default(), + should_quit, + Some(&cached), + ); + + // a cached evaluation means no Evaluate step is scheduled, and the + // local build reuses the known derivation path. + assert_eq!( + plan.steps, + vec![ + Build { + output: BuildOutputHandle::new(), + metadata: BuildMetadata::NixCommand(NixCommandBuildMetadata::Locally { + cached_derivation: Some(EvaluationOutputHandle::new_known(cached)), + }), + } + .into() + ] + ); + assert!(!plan.greedy_evaluate); + } + + #[tokio::test] + async fn order_build_experimental_nix_client() { + let location = location!(get_test_path!()); + let node = Node::default(); + let name = &Name(function_name!().into()); + let should_quit = Arc::new(AtomicBool::new(false)); + let plan = plan_for_node( + &node, + name.clone(), + &Goal::Build, + location.into(), + &SubCommandModifiers { + experimental_nix_client: true, + ..Default::default() + }, + should_quit, + None, + ); + + // the experimental nix client needs a real `.drv`, so an Evaluate + // step is scheduled even for a local Build. + assert_eq!( + plan.steps, + vec![ + Evaluate { + output: EvaluationOutputHandle::new(), + } + .into(), + Build { + output: BuildOutputHandle::new(), + metadata: BuildMetadata::BuildWithNixDaemon { + target: None, + derivation: EvaluationOutputHandle::new(), + }, + } + .into(), + ] + ); + assert!(plan.greedy_evaluate); + } + + #[tokio::test] + async fn order_remote_build_experimental_nix_client() { + let location = location!(get_test_path!()); + let node = Node { + build_remotely: true, + ..Default::default() + }; + let name = &Name(function_name!().into()); + let should_quit = Arc::new(AtomicBool::new(false)); + let target = SharedTarget(Arc::new(RwLock::new(node.target.clone()))); + let plan = plan_for_node( + &node, + name.clone(), + &Goal::Apply(ApplyGoalArgs { + goal: ApplyGoal::SwitchToConfiguration(SwitchToConfigurationGoal::Switch), + should_apply_locally: false, + no_keys: true, + substitute_on_destination: true, + reboot: false, + host_platform: "x86_64-linux".into(), + handle_unreachable: HandleUnreachable::default(), + }), + location.into(), + &SubCommandModifiers { + experimental_nix_client: true, + ..Default::default() + }, + should_quit, + None, + ); + + assert_eq!( + plan.steps, + vec![ + Ping { + target: target.clone() + } + .into(), + Evaluate { + output: EvaluationOutputHandle::new(), + } + .into(), + PushEvaluatedOutput { + substitute_on_destination: true, + target: target.clone(), + path: EvaluationOutputHandle::new(), + } + .into(), + Build { + output: BuildOutputHandle::new(), + metadata: BuildMetadata::BuildWithNixDaemon { + target: Some(target.clone()), + derivation: EvaluationOutputHandle::new(), + }, + } + .into(), + SwitchToConfiguration { + goal: SwitchToConfigurationGoal::Switch, + reboot: false, + target: Some(target), + privilege_escalation_command: node.privilege_escalation_command, + top_level: BuildOutputHandle::new(), } .into(), ] ); + assert!(plan.greedy_evaluate); } } diff --git a/crates/core/src/hive/steps/activate.rs b/crates/core/src/hive/steps/activate.rs index d33cce7..4df41dc 100644 --- a/crates/core/src/hive/steps/activate.rs +++ b/crates/core/src/hive/steps/activate.rs @@ -9,7 +9,10 @@ use crate::{ HiveLibError, SafeStorePath, commands::{CommandArguments, WireCommandChip, builder::CommandStringBuilder, run_command}, errors::{ActivationError, NetworkError}, - hive::node::{Context, ExecuteStep, SharedTarget, SwitchToConfigurationGoal}, + hive::{ + executor::BuildOutputHandle, + node::{Context, ExecuteStep, SharedTarget, SwitchToConfigurationGoal}, + }, }; #[derive(Debug)] @@ -19,6 +22,8 @@ pub struct SwitchToConfiguration { pub reboot: bool, pub target: Option, pub privilege_escalation_command: Arc>>, + + pub top_level: BuildOutputHandle, } impl Display for SwitchToConfiguration { @@ -94,7 +99,7 @@ impl ExecuteStep for SwitchToConfiguration { #[allow(clippy::too_many_lines)] #[instrument(skip_all, name = "activate")] async fn execute(&self, ctx: &mut Context) -> Result<(), HiveLibError> { - let built_path = ctx.state.build.as_ref().unwrap(); + let built_path = self.top_level.require().await?; if matches!( self.goal, @@ -102,7 +107,7 @@ impl ExecuteStep for SwitchToConfiguration { // https://github.com/NixOS/nixpkgs/blob/a2c92aa34735a04010671e3378e2aa2d109b2a72/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/services.py#L224 SwitchToConfigurationGoal::Switch | SwitchToConfigurationGoal::Boot ) { - self.set_profile(built_path, ctx).await?; + self.set_profile(&built_path, ctx).await?; } info!("Running switch-to-configuration {}", self.goal); diff --git a/crates/core/src/hive/steps/build.rs b/crates/core/src/hive/steps/build.rs index 4d4a99f..0bc510a 100644 --- a/crates/core/src/hive/steps/build.rs +++ b/crates/core/src/hive/steps/build.rs @@ -12,16 +12,44 @@ use crate::{ CommandArguments, Either, WireCommandChip, builder::CommandStringBuilder, run_command_with_env, trace_nix_log_message, }, - hive::node::{Context, ExecuteStep, SharedTarget}, + hive::{ + HiveLocation, + executor::{BuildOutputHandle, EvaluationOutputHandle}, + node::{Context, ExecuteStep, SharedTarget}, + }, open_remote_client, }; const SYSTEM_OUTPUT: &str = "out"; +#[derive(Debug)] +#[cfg_attr(test, derive(PartialEq))] +pub(crate) enum NixCommandBuildMetadata { + Locally { + cached_derivation: Option, + }, + Remotely { + target: SharedTarget, + derivation: EvaluationOutputHandle, + }, +} + +#[derive(Debug)] +#[cfg_attr(test, derive(PartialEq))] +pub(crate) enum BuildMetadata { + NixCommand(NixCommandBuildMetadata), + BuildWithNixDaemon { + target: Option, + derivation: EvaluationOutputHandle, + }, +} + #[derive(Debug)] #[cfg_attr(test, derive(PartialEq))] pub struct Build { - pub(crate) target: Option, + pub(crate) metadata: BuildMetadata, + /// the handle this step places its produced path to + pub(crate) output: BuildOutputHandle, } impl Display for Build { @@ -31,123 +59,162 @@ impl Display for Build { } impl ExecuteStep for Build { + #[allow(clippy::too_many_lines)] #[instrument(skip_all, name = "build")] async fn execute(&self, ctx: &mut Context) -> Result<(), HiveLibError> { - let top_level = ctx.state.evaluation.as_ref().unwrap(); - - if ctx.modifiers.experimental_nix_client { - // use experimental nix daemon client - let mut connection = if let Some(ref target) = self.target { - let target = target.0.read().await; - - Either::Left( - open_remote_client( - &target, - ctx.modifiers, - trace_nix_log_message, - ctx.should_quit.clone(), + match &self.metadata { + BuildMetadata::BuildWithNixDaemon { target, derivation } => { + let top_level = derivation.require().await?; + + // use experimental nix daemon client + let mut connection = if let Some(target) = target { + let target = target.0.read().await; + + Either::Left( + open_remote_client( + &target, + ctx.modifiers, + trace_nix_log_message, + ctx.should_quit.clone(), + ) + .await? + .0, ) - .await? - .0, - ) - } else { - Either::Right( - NixClient::open_local( - trace_nix_log_message, - ctx.should_quit.clone(), - ctx.modifiers.print_build_logs, + } else { + Either::Right( + NixClient::open_local( + trace_nix_log_message, + ctx.should_quit.clone(), + ctx.modifiers.print_build_logs, + ) + .await + .map_err(HiveLibError::NixDaemonClientError)?, ) - .await - .map_err(HiveLibError::NixDaemonClientError)?, - ) - }; - - let mut output_map = match connection { - Either::Left(ref mut conn) => conn.query_derivation_output_map(top_level).await, - Either::Right(ref mut conn) => conn.query_derivation_output_map(top_level).await, - } - .map_err(|err| HiveLibError::NixBuildError { - name: ctx.name.clone(), - source: err, - })?; - - debug!(output_map = ?output_map, "got output map"); - - let output_path = - output_map - .remove(SYSTEM_OUTPUT) - .flatten() - .ok_or(HiveLibError::NixBuildError { + }; + + let mut output_map = match connection { + Either::Left(ref mut conn) => { + conn.query_derivation_output_map(&top_level).await + } + Either::Right(ref mut conn) => { + conn.query_derivation_output_map(&top_level).await + } + } + .map_err(|err| HiveLibError::NixBuildError { + name: ctx.name.clone(), + source: err, + })?; + + debug!(output_map = ?output_map, "got output map"); + + let output_path = output_map.remove(SYSTEM_OUTPUT).flatten().ok_or( + HiveLibError::NixBuildError { name: ctx.name.clone(), source: NixDaemonClientError::NixDaemonInvalidResponse(format!( "Derivation {top_level:?} did not have output {SYSTEM_OUTPUT:?}" )), - })?; - - let derived_path = DerivedPath { - store_path: top_level, - outputs: DerivedPathOutput::OutputNames(&[SYSTEM_OUTPUT]), - }; - - match connection { - Either::Left(mut conn) => conn.build(&vec![derived_path]).await, - Either::Right(mut conn) => conn.build(&vec![derived_path]).await, + }, + )?; + + let derived_path = DerivedPath { + store_path: &top_level, + outputs: DerivedPathOutput::OutputNames(&[SYSTEM_OUTPUT]), + }; + + match connection { + Either::Left(mut conn) => conn.build(&vec![derived_path]).await, + Either::Right(mut conn) => conn.build(&vec![derived_path]).await, + } + .map_err(|source| HiveLibError::NixBuildError { + name: ctx.name.clone(), + source, + })?; + + info!("Built output: {output_path:?}"); + + // print built path to stdout + let clobber_guard = acquire_stdin_lock().await; + println!("{}", output_path.to_absolute_path()); + drop(clobber_guard); + + self.output.set(output_path).await; + } + BuildMetadata::NixCommand(metadata) => { + let attribute = match metadata { + NixCommandBuildMetadata::Remotely { derivation, .. } + | NixCommandBuildMetadata::Locally { + cached_derivation: Some(derivation), + } => { + format!( + "{}^{SYSTEM_OUTPUT}", + derivation.require().await?.to_absolute_path() + ) + } + NixCommandBuildMetadata::Locally { + cached_derivation: None, + } => match &*ctx.hive_location { + HiveLocation::Flake { uri, .. } => { + format!("{uri}#wire.nodes.{}.config.system.build.toplevel", ctx.name) + } + HiveLocation::HiveNix(path) => { + format!( + "--file {} nodes.{}.config.system.build.toplevel", + path.to_string_lossy(), + ctx.name + ) + } + }, + }; + + // use regular nix build command + let mut command_string = CommandStringBuilder::nix(); + command_string.args(&[ + "--extra-experimental-features", + "nix-command", + "build", + "--no-link", + "--print-out-paths", + ]); + command_string.opt_arg(ctx.modifiers.print_build_logs, "--print-build-logs"); + command_string.arg(&attribute); + + let status = run_command_with_env( + &CommandArguments::new(command_string, ctx.modifiers) + // build remotely if asked for AND we isnt applying locally + .execute_on_remote(match metadata { + NixCommandBuildMetadata::Remotely { target, .. } => { + Some(target.clone()) + } + NixCommandBuildMetadata::Locally { .. } => None, + }) + .mode(crate::commands::ChildOutputMode::Nix) + .log_stdout(), + std::collections::HashMap::new(), + ) + .await? + .wait_till_success() + .await + .map_err(|source| HiveLibError::NixBuildCliError { + name: ctx.name.clone(), + source, + })?; + + let stdout = match status { + Either::Left((_, stdout)) | Either::Right((_, stdout)) => stdout, + }; + + info!("Built output: {stdout:?}"); + + let clobber_guard = acquire_stdin_lock().await; + println!("{stdout}"); + drop(clobber_guard); + + self.output + .set(SafeStorePath::::from_absolute_path( + stdout.as_bytes(), + )?) + .await; } - .map_err(|source| HiveLibError::NixBuildError { - name: ctx.name.clone(), - source, - })?; - - info!("Built output: {output_path:?}"); - - // print built path to stdout - let clobber_guard = acquire_stdin_lock().await; - println!("{}", output_path.to_absolute_path()); - drop(clobber_guard); - - ctx.state.build = Some(output_path); - } else { - // use regular nix build command - let mut command_string = CommandStringBuilder::nix(); - command_string.args(&[ - "--extra-experimental-features", - "nix-command", - "build", - "--no-link", - "--print-out-paths", - ]); - command_string.opt_arg(ctx.modifiers.print_build_logs, "--print-build-logs"); - command_string.arg(format!("{}^out", top_level.to_absolute_path())); - - let status = run_command_with_env( - &CommandArguments::new(command_string, ctx.modifiers) - // build remotely if asked for AND we isnt applying locally - .execute_on_remote(self.target.clone()) - .mode(crate::commands::ChildOutputMode::Nix) - .log_stdout(), - std::collections::HashMap::new(), - ) - .await? - .wait_till_success() - .await - .map_err(|source| HiveLibError::NixBuildCliError { - name: ctx.name.clone(), - source, - })?; - - let stdout = match status { - Either::Left((_, stdout)) | Either::Right((_, stdout)) => stdout, - }; - - info!("Built output: {stdout:?}"); - - let clobber_guard = acquire_stdin_lock().await; - println!("{stdout}"); - drop(clobber_guard); - - ctx.state.build = Some(SafeStorePath::::from_absolute_path( - stdout.as_bytes(), - )?); } Ok(()) diff --git a/crates/core/src/hive/steps/evaluate.rs b/crates/core/src/hive/steps/evaluate.rs index a263fd9..3da5d4c 100644 --- a/crates/core/src/hive/steps/evaluate.rs +++ b/crates/core/src/hive/steps/evaluate.rs @@ -3,17 +3,21 @@ use std::fmt::Display; -use tracing::{info, instrument}; +use tracing::instrument; use crate::{ - HiveLibError, SafeStorePath, - hive::node::{Context, ExecuteStep}, + HiveLibError, + hive::{ + executor::EvaluationOutputHandle, + node::{Context, ExecuteStep}, + }, }; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] +#[cfg_attr(test, derive(PartialEq, Eq))] pub struct Evaluate { - /// evaluation that was previously built & cached - pub cached_evaluation: Option>, + /// output handle to write to once the greedy eval is complete + pub output: EvaluationOutputHandle, } impl Display for Evaluate { @@ -25,17 +29,9 @@ impl Display for Evaluate { impl ExecuteStep for Evaluate { #[instrument(skip_all, name = "eval")] async fn execute(&self, ctx: &mut Context) -> Result<(), HiveLibError> { - if let Some(ref cached_evaluation) = self.cached_evaluation { - info!( - "Skipping evaluation, cached as {}", - cached_evaluation.to_absolute_path() - ); - ctx.state.evaluation = Some(cached_evaluation.clone()); - } else { - let rx = ctx.state.evaluation_rx.take().unwrap(); - - ctx.state.evaluation = Some(rx.await.unwrap()?); - } + let rx = ctx.state.evaluation_rx.take().unwrap(); + + self.output.set(rx.await.unwrap()?).await; Ok(()) } diff --git a/crates/core/src/hive/steps/keys.rs b/crates/core/src/hive/steps/keys.rs index ce69c78..b1420b8 100644 --- a/crates/core/src/hive/steps/keys.rs +++ b/crates/core/src/hive/steps/keys.rs @@ -29,6 +29,7 @@ use tracing::{debug, instrument}; use crate::commands::builder::CommandStringBuilder; use crate::commands::{CommandArguments, WireCommandChip, run_command}; use crate::errors::KeyError; +use crate::hive::executor::KeyAgentPathHandle; use crate::hive::node::{Context, ExecuteStep, Push, SharedTarget}; use crate::{HiveLibError, SafeStorePath, push_with_daemon}; @@ -184,6 +185,7 @@ pub struct Keys { pub keys: Vec>, pub target: Option, pub privilege_escalation_command: Arc>>, + pub key_agent_directory: KeyAgentPathHandle, } #[derive(Debug)] @@ -192,6 +194,7 @@ pub struct PushKeyAgent { pub substitute_on_destination: bool, pub host_platform: Arc, pub target: Option, + pub key_agent_directory: KeyAgentPathHandle, } impl Display for Keys { @@ -235,7 +238,7 @@ where impl ExecuteStep for Keys { #[instrument(skip_all, name = "keys")] async fn execute(&self, ctx: &mut Context) -> Result<(), HiveLibError> { - let agent_directory = ctx.state.key_agent_directory.as_ref().unwrap(); + let agent_directory = self.key_agent_directory.require().await?; let mut keys = self.select_keys(&self.keys).await?; @@ -332,7 +335,7 @@ impl ExecuteStep for PushKeyAgent { .await?; } - ctx.state.key_agent_directory = Some(agent_store_path); + self.key_agent_directory.set(agent_store_path).await; Ok(()) } diff --git a/crates/core/src/hive/steps/push.rs b/crates/core/src/hive/steps/push.rs index 78b2fba..ca50192 100644 --- a/crates/core/src/hive/steps/push.rs +++ b/crates/core/src/hive/steps/push.rs @@ -7,7 +7,10 @@ use tracing::instrument; use crate::{ HiveLibError, - hive::node::{Context, ExecuteStep, SharedTarget}, + hive::{ + executor::{BuildOutputHandle, EvaluationOutputHandle}, + node::{Context, ExecuteStep, SharedTarget}, + }, }; #[derive(Debug)] @@ -15,6 +18,7 @@ use crate::{ pub struct PushEvaluatedOutput { pub substitute_on_destination: bool, pub target: SharedTarget, + pub path: EvaluationOutputHandle, } #[derive(Debug)] @@ -22,6 +26,7 @@ pub struct PushEvaluatedOutput { pub struct PushBuildOutput { pub substitute_on_destination: bool, pub target: SharedTarget, + pub path: BuildOutputHandle, } impl Display for PushEvaluatedOutput { @@ -39,13 +44,13 @@ impl Display for PushBuildOutput { impl ExecuteStep for PushEvaluatedOutput { #[instrument(skip_all, name = "push_eval")] async fn execute(&self, ctx: &mut Context) -> Result<(), HiveLibError> { - let top_level = ctx.state.evaluation.as_ref().unwrap(); + let top_level = self.path.require().await?; if ctx.modifiers.experimental_nix_client { crate::push_with_daemon( ctx, &self.target, - crate::hive::node::Push::Derivation(top_level), + crate::hive::node::Push::Derivation(&top_level), self.substitute_on_destination, ) .await?; @@ -53,7 +58,7 @@ impl ExecuteStep for PushEvaluatedOutput { crate::commands::common::push( ctx, &self.target, - crate::hive::node::Push::Derivation(top_level), + crate::hive::node::Push::Derivation(&top_level), self.substitute_on_destination, ) .await?; @@ -66,13 +71,13 @@ impl ExecuteStep for PushEvaluatedOutput { impl ExecuteStep for PushBuildOutput { #[instrument(skip_all, name = "push_build")] async fn execute(&self, ctx: &mut Context) -> Result<(), HiveLibError> { - let built_path = ctx.state.build.as_ref().unwrap(); + let built_path = self.path.require().await?; if ctx.modifiers.experimental_nix_client { crate::push_with_daemon( ctx, &self.target, - crate::hive::node::Push::Path(built_path), + crate::hive::node::Push::Path(&built_path), self.substitute_on_destination, ) .await?; @@ -80,7 +85,7 @@ impl ExecuteStep for PushBuildOutput { crate::commands::common::push( ctx, &self.target, - crate::hive::node::Push::Path(built_path), + crate::hive::node::Push::Path(&built_path), self.substitute_on_destination, ) .await?; -- 2.51.2