From 80ab1d753818752405acb8d4d3f3dc11ef83f5a9 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Sat, 6 Feb 2021 01:34:01 +0300 Subject: Evaluate deploy output lazily Currently, we evaluate the `#deploy` output strictly. This means - Longer eval times - Extraneous evaluation errors with `--skip-checks` - `-- --impure` even when the path we're currently deploying is pure - etc. With this change, evaluation happens lazily -- we only evaluate the nodes and profiles we really need. It is only implemented for flaky Nix, and it is on by default. To get the old behavior, one can specify `--strict-eval`. I have tested that this indeed dramatically increases evaluation speed in all of our repos, and removes the need to deploy Agora with `--impure`. Hooray! --- src/bin/deploy.rs | 92 ++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 70 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/bin/deploy.rs b/src/bin/deploy.rs index caf3d4e..6da2110 100644 --- a/src/bin/deploy.rs +++ b/src/bin/deploy.rs @@ -52,6 +52,10 @@ struct Opts { #[clap(short, long)] skip_checks: bool, + /// Always evaluate deploy attribute strictly + #[clap(long)] + strict_eval: bool, + /// Override the SSH user with the given value #[clap(long)] ssh_user: Option, @@ -161,38 +165,83 @@ enum GetDeploymentDataError { /// Evaluates the Nix in the given `repo` and return the processed Data from it async fn get_deployment_data( supports_flakes: bool, - repo: &str, + strict_eval: bool, + flake: &deploy::DeployFlake<'_>, extra_build_args: &[String], ) -> Result { - info!("Evaluating flake in {}", repo); + info!("Evaluating flake in {}", flake.repo); - let mut c = match supports_flakes { - true => Command::new("nix"), - false => Command::new("nix-instantiate"), + let mut c = if supports_flakes { + Command::new("nix") + } else { + Command::new("nix-instantiate") }; - let mut build_command = match supports_flakes { - true => { - c.arg("eval") + if supports_flakes { + c + .arg("eval") .arg("--json") - .arg(format!("{}#deploy", repo)) - } - false => { - c - .arg("--strict") - .arg("--read-write-mode") - .arg("--json") - .arg("--eval") - .arg("-E") - .arg(format!("let r = import {}/.; in if builtins.isFunction r then (r {{}}).deploy else r.deploy", repo)) + .arg(format!("{}#deploy", flake.repo)); + match (&flake.node, strict_eval) { + (Some(node), false) => { + // We use --apply instead of --expr so that we don't have to deal with builtins.getFlake + c.arg("--apply"); + info!("Evaluating the flake lazily; Use --strict-eval for old behavior"); + match &flake.profile { + None => { + // Ignore all nodes but the one we're evaluating + c.arg(format!( + r#" + deploy: + (deploy // {{ + nodes = {{ + inherit (deploy.nodes) "{}"; + }}; + }}) + "#, + node + )) + } + Some(profile) => { + // Ignore all nodes and all profiles but the one we're evaluating + c.arg(format!( + r#" + deploy: + (deploy // {{ + nodes = {{ + "{0}" = deploy.nodes."{0}" // {{ + profiles = {{ + inherit (deploy.nodes."{0}".profiles) "{1}"; + }}; + }}; + }}; + }}) + "#, + node, profile + )) + } + } + } + (_, _) => { + // We need to evaluate all profiles of all nodes anyway, so just do it strictly + &c + } } + } else { + c + .arg("--strict") + .arg("--read-write-mode") + .arg("--json") + .arg("--eval") + .arg("-E") + .arg(format!("let r = import {}/.; in if builtins.isFunction r then (r {{}}).deploy else r.deploy", flake.repo)) }; for extra_arg in extra_build_args { - build_command = build_command.arg(extra_arg); + c.arg(extra_arg); } - let build_child = build_command + let build_child = c .stdout(Stdio::piped()) .spawn() .map_err(GetDeploymentDataError::NixEval)?; @@ -524,8 +573,7 @@ async fn run() -> Result<(), RunError> { check_deployment(supports_flakes, deploy_flake.repo, &opts.extra_build_args).await?; } - let data = - get_deployment_data(supports_flakes, deploy_flake.repo, &opts.extra_build_args).await?; + let data = get_deployment_data(supports_flakes, opts.strict_eval, &deploy_flake, &opts.extra_build_args).await?; let result_path = opts.result_path.as_deref(); -- cgit v1.2.3 From 99f2127ccee5e612b1c26a7300f7380f5d1575b0 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Sun, 7 Feb 2021 13:37:41 +0300 Subject: fixup! Evaluate deploy output lazily --- src/bin/deploy.rs | 92 +++++++++++++++++++++++++------------------------------ 1 file changed, 42 insertions(+), 50 deletions(-) (limited to 'src') diff --git a/src/bin/deploy.rs b/src/bin/deploy.rs index 6da2110..30ebd25 100644 --- a/src/bin/deploy.rs +++ b/src/bin/deploy.rs @@ -52,10 +52,6 @@ struct Opts { #[clap(short, long)] skip_checks: bool, - /// Always evaluate deploy attribute strictly - #[clap(long)] - strict_eval: bool, - /// Override the SSH user with the given value #[clap(long)] ssh_user: Option, @@ -160,12 +156,13 @@ enum GetDeploymentDataError { DecodeUtf8(#[from] std::string::FromUtf8Error), #[error("Error decoding the JSON from evaluation: {0}")] DecodeJson(#[from] serde_json::error::Error), + #[error("Impossible happened: profile is set but node is not")] + ProfileNoNode, } /// Evaluates the Nix in the given `repo` and return the processed Data from it async fn get_deployment_data( supports_flakes: bool, - strict_eval: bool, flake: &deploy::DeployFlake<'_>, extra_build_args: &[String], ) -> Result { @@ -178,54 +175,49 @@ async fn get_deployment_data( }; if supports_flakes { - c - .arg("eval") + c.arg("eval") .arg("--json") - .arg(format!("{}#deploy", flake.repo)); - match (&flake.node, strict_eval) { - (Some(node), false) => { - // We use --apply instead of --expr so that we don't have to deal with builtins.getFlake - c.arg("--apply"); - info!("Evaluating the flake lazily; Use --strict-eval for old behavior"); - match &flake.profile { - None => { - // Ignore all nodes but the one we're evaluating - c.arg(format!( - r#" - deploy: - (deploy // {{ - nodes = {{ - inherit (deploy.nodes) "{}"; - }}; - }}) - "#, - node - )) - } - Some(profile) => { - // Ignore all nodes and all profiles but the one we're evaluating - c.arg(format!( - r#" - deploy: - (deploy // {{ - nodes = {{ - "{0}" = deploy.nodes."{0}" // {{ - profiles = {{ - inherit (deploy.nodes."{0}".profiles) "{1}"; - }}; - }}; - }}; - }}) - "#, - node, profile - )) - } - } + .arg(format!("{}#deploy", flake.repo)) + // We use --apply instead of --expr so that we don't have to deal with builtins.getFlake + .arg("--apply"); + match (&flake.node, &flake.profile) { + (Some(node), Some(profile)) => { + // Ignore all nodes and all profiles but the one we're evaluating + c.arg(format!( + r#" + deploy: + (deploy // {{ + nodes = {{ + "{0}" = deploy.nodes."{0}" // {{ + profiles = {{ + inherit (deploy.nodes."{0}".profiles) "{1}"; + }}; + }}; + }}; + }}) + "#, + node, profile + )) + } + (Some(node), None) => { + // Ignore all nodes but the one we're evaluating + c.arg(format!( + r#" + deploy: + (deploy // {{ + nodes = {{ + inherit (deploy.nodes) "{}"; + }}; + }}) + "#, + node + )) } - (_, _) => { + (None, None) => { // We need to evaluate all profiles of all nodes anyway, so just do it strictly - &c + c.arg(format!("deploy: deploy")) } + (None, Some(_)) => return Err(GetDeploymentDataError::ProfileNoNode), } } else { c @@ -573,7 +565,7 @@ async fn run() -> Result<(), RunError> { check_deployment(supports_flakes, deploy_flake.repo, &opts.extra_build_args).await?; } - let data = get_deployment_data(supports_flakes, opts.strict_eval, &deploy_flake, &opts.extra_build_args).await?; + let data = get_deployment_data(supports_flakes, &deploy_flake, &opts.extra_build_args).await?; let result_path = opts.result_path.as_deref(); -- cgit v1.2.3