From a5619f5660a00f58c2b7c16d89058e92327ac9b8 Mon Sep 17 00:00:00 2001
From: 2xsaiko <me@dblsaiko.net>
Date: Thu, 29 Dec 2022 16:19:59 +0100
Subject: [PATCH] Build every profile first, then push (#158)

Try to build everything first before pushing to remotes. Since the build
is more likely to fail than the upload, if there is an error the deployment
will fail sooner and before uploading any potentially unusable configuration.
---
 src/cli.rs  | 33 +++++++++++++++++++++------------
 src/push.rs | 33 +++++++++++++++++++--------------
 2 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/src/cli.rs b/src/cli.rs
index ab9499ab..f2595637 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -545,18 +545,27 @@ async fn run_deploy(
         print_deployment(&parts[..])?;
     }
 
-    for (deploy_flake, deploy_data, deploy_defs) in &parts {
-        deploy::push::push_profile(deploy::push::PushProfileData {
-            supports_flakes,
-            check_sigs,
-            repo: deploy_flake.repo,
-            deploy_data,
-            deploy_defs,
-            keep_result,
-            result_path,
-            extra_build_args,
-        })
-        .await?;
+    let data_iter = || {
+        parts.iter().map(
+            |(deploy_flake, deploy_data, deploy_defs)| deploy::push::PushProfileData {
+                supports_flakes,
+                check_sigs,
+                repo: deploy_flake.repo,
+                deploy_data,
+                deploy_defs,
+                keep_result,
+                result_path,
+                extra_build_args,
+            },
+        )
+    };
+
+    for data in data_iter() {
+        deploy::push::build_profile(data).await?;
+    }
+
+    for data in data_iter() {
+        deploy::push::push_profile(data).await?;
     }
 
     let mut succeeded: Vec<(&deploy::DeployData, &deploy::DeployDefs)> = vec![];
diff --git a/src/push.rs b/src/push.rs
index 0801bd99..6d9f10d2 100644
--- a/src/push.rs
+++ b/src/push.rs
@@ -209,7 +209,7 @@ pub async fn build_profile_remotely(data: &PushProfileData<'_>, derivation_name:
     Ok(())
 }
 
-pub async fn push_profile(data: PushProfileData<'_>) -> Result<(), PushProfileError> {
+pub async fn build_profile(data: PushProfileData<'_>) -> Result<(), PushProfileError> {
     debug!(
         "Finding the deriver of store path for {}",
         &data.deploy_data.profile.profile_settings.path
@@ -218,17 +218,6 @@ pub async fn push_profile(data: PushProfileData<'_>) -> Result<(), PushProfileEr
     // `nix-store --query --deriver` doesn't work on invalid paths, so we parse output of show-derivation :(
     let mut show_derivation_command = Command::new("nix");
 
-    let ssh_opts_str = data
-        .deploy_data
-        .merged_settings
-        .ssh_opts
-        // This should provide some extra safety, but it also breaks for some reason, oh well
-        // .iter()
-        // .map(|x| format!("'{}'", x))
-        // .collect::<Vec<String>>()
-        .join(" ");
-
-
     show_derivation_command
         .arg("show-derivation")
         .arg(&data.deploy_data.profile.profile_settings.path);
@@ -259,12 +248,28 @@ pub async fn push_profile(data: PushProfileData<'_>) -> Result<(), PushProfileEr
             return Err(PushProfileError::RemoteBuildWithLegacyNix)
         }
 
-        // remote building guarantees that the resulting derivation is stored on the target system
-        // no need to copy after building
         build_profile_remotely(&data, derivation_name).await?;
     } else {
         build_profile_locally(&data, derivation_name).await?;
+    }
+
+    Ok(())
+}
+
+pub async fn push_profile(data: PushProfileData<'_>) -> Result<(), PushProfileError> {
+    let ssh_opts_str = data
+        .deploy_data
+        .merged_settings
+        .ssh_opts
+        // This should provide some extra safety, but it also breaks for some reason, oh well
+        // .iter()
+        // .map(|x| format!("'{}'", x))
+        // .collect::<Vec<String>>()
+        .join(" ");
 
+    // remote building guarantees that the resulting derivation is stored on the target system
+    // no need to copy after building
+    if !data.deploy_data.merged_settings.remote_build.unwrap_or(false) {
         info!(
             "Copying profile `{}` to node `{}`",
             data.deploy_data.profile_name, data.deploy_data.node_name