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/bin')
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<String>,
@@ -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<deploy::data::Data, GetDeploymentDataError> {
- 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/bin')
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<String>,
@@ -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<deploy::data::Data, GetDeploymentDataError> {
@@ -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