feat(snix-glue): Correctly propagate output placeholders into the build.
Nix's `builtin.placeholder` function produces output paths that are not known ahead of time, so before propagating these values into the build we need to replace them in all env variables and arguments to their corresponding output store paths. fix #101 Change-Id: I2670c749f2c578e276d698e511598a76a99ebb96 Reviewed-on: https://cl.snix.dev/c/snix/+/30310 Tested-by: besadii Reviewed-by: Florian Klink <flokli@flokli.de> Autosubmit: Vova Kryachko <v.kryachko@gmail.com>
This commit is contained in:
parent
8bb8400304
commit
2bbd06753b
2 changed files with 126 additions and 7 deletions
|
|
@ -6,13 +6,17 @@ use std::future::Future;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
|
|
||||||
use async_stream::try_stream;
|
use async_stream::try_stream;
|
||||||
|
use bstr::BString;
|
||||||
use bytes::Bytes;
|
use bytes::Bytes;
|
||||||
use futures::Stream;
|
use futures::Stream;
|
||||||
|
use nix_compat::derivation::Output;
|
||||||
|
use nix_compat::store_path::hash_placeholder;
|
||||||
use nix_compat::{derivation::Derivation, nixbase32, store_path::StorePath};
|
use nix_compat::{derivation::Derivation, nixbase32, store_path::StorePath};
|
||||||
use sha2::{Digest, Sha256};
|
use sha2::{Digest, Sha256};
|
||||||
use snix_build::buildservice::{AdditionalFile, BuildConstraints, BuildRequest, EnvVar};
|
use snix_build::buildservice::{AdditionalFile, BuildConstraints, BuildRequest, EnvVar};
|
||||||
use snix_castore::Node;
|
use snix_castore::Node;
|
||||||
use snix_store::path_info::PathInfo;
|
use snix_store::path_info::PathInfo;
|
||||||
|
use tracing::warn;
|
||||||
|
|
||||||
use crate::known_paths::KnownPaths;
|
use crate::known_paths::KnownPaths;
|
||||||
|
|
||||||
|
|
@ -95,7 +99,14 @@ pub(crate) fn derivation_to_build_request(
|
||||||
// produce command_args, which is builder and arguments in a Vec.
|
// produce command_args, which is builder and arguments in a Vec.
|
||||||
let mut command_args: Vec<String> = Vec::with_capacity(derivation.arguments.len() + 1);
|
let mut command_args: Vec<String> = Vec::with_capacity(derivation.arguments.len() + 1);
|
||||||
command_args.push(derivation.builder.clone());
|
command_args.push(derivation.builder.clone());
|
||||||
command_args.extend_from_slice(&derivation.arguments);
|
command_args.extend_from_slice(
|
||||||
|
derivation
|
||||||
|
.arguments
|
||||||
|
.iter()
|
||||||
|
.map(|arg| replace_placeholders(arg, &derivation.outputs))
|
||||||
|
.collect::<Vec<String>>()
|
||||||
|
.as_slice(),
|
||||||
|
);
|
||||||
|
|
||||||
// Produce environment_vars and additional files.
|
// Produce environment_vars and additional files.
|
||||||
// We use a BTreeMap while producing, and only realize the resulting Vec
|
// We use a BTreeMap while producing, and only realize the resulting Vec
|
||||||
|
|
@ -113,12 +124,12 @@ pub(crate) fn derivation_to_build_request(
|
||||||
// extend / overwrite with the keys set in the derivation environment itself.
|
// extend / overwrite with the keys set in the derivation environment itself.
|
||||||
// TODO: check if this order is correct, and environment vars set in the
|
// TODO: check if this order is correct, and environment vars set in the
|
||||||
// *Derivation actually* have priority.
|
// *Derivation actually* have priority.
|
||||||
environment_vars.extend(
|
environment_vars.extend(derivation.environment.iter().map(|(k, v)| {
|
||||||
derivation
|
(
|
||||||
.environment
|
k.clone(),
|
||||||
.iter()
|
Bytes::from(replace_placeholders_b(v, &derivation.outputs).to_vec()),
|
||||||
.map(|(k, v)| (k.clone(), Bytes::from(v.to_vec()))),
|
)
|
||||||
);
|
}));
|
||||||
|
|
||||||
handle_pass_as_file(&mut environment_vars, &mut additional_files)?;
|
handle_pass_as_file(&mut environment_vars, &mut additional_files)?;
|
||||||
|
|
||||||
|
|
@ -247,9 +258,47 @@ fn calculate_pass_as_file_env(k: &str) -> (String, String) {
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Replace all references to `placeholder outputName` inside the derivation
|
||||||
|
fn replace_placeholders(s: &str, outputs: &BTreeMap<String, Output>) -> String {
|
||||||
|
let mut s = s.to_owned();
|
||||||
|
for (out_name, output) in outputs {
|
||||||
|
let placeholder = hash_placeholder(out_name.as_str());
|
||||||
|
if let Some(path) = output.path.as_ref() {
|
||||||
|
s = s.replace(&placeholder, &path.to_absolute_path());
|
||||||
|
} else {
|
||||||
|
warn!(
|
||||||
|
output.name = out_name,
|
||||||
|
"output should have a path during placeholder replacement"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
s
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Replace all references to `placeholder outputName` inside the derivation
|
||||||
|
fn replace_placeholders_b(s: &BString, outputs: &BTreeMap<String, Output>) -> BString {
|
||||||
|
use bstr::ByteSlice;
|
||||||
|
let mut s = s.clone();
|
||||||
|
for (out_name, output) in outputs {
|
||||||
|
let placeholder = hash_placeholder(out_name.as_str());
|
||||||
|
if let Some(path) = output.path.as_ref() {
|
||||||
|
s = s
|
||||||
|
.replace(placeholder.as_bytes(), path.to_absolute_path().as_bytes())
|
||||||
|
.into();
|
||||||
|
} else {
|
||||||
|
warn!(
|
||||||
|
output.name = out_name,
|
||||||
|
"output should have a path during placeholder replacement"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
s
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod test {
|
mod test {
|
||||||
use bytes::Bytes;
|
use bytes::Bytes;
|
||||||
|
use nix_compat::store_path::hash_placeholder;
|
||||||
use nix_compat::{derivation::Derivation, store_path::StorePath};
|
use nix_compat::{derivation::Derivation, store_path::StorePath};
|
||||||
use snix_castore::fixtures::DUMMY_DIGEST;
|
use snix_castore::fixtures::DUMMY_DIGEST;
|
||||||
use snix_castore::{Node, PathComponent};
|
use snix_castore::{Node, PathComponent};
|
||||||
|
|
@ -356,6 +405,75 @@ mod test {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_drv_with_placeholders_to_build_request() {
|
||||||
|
let aterm_bytes =
|
||||||
|
include_bytes!("tests/18m7y1d025lqgrzx8ypnhjbvq23z2kda-with-placeholders.drv");
|
||||||
|
let derivation = Derivation::from_aterm_bytes(aterm_bytes).expect("must parse");
|
||||||
|
|
||||||
|
let build_request =
|
||||||
|
derivation_to_build_request(&derivation, &BTreeMap::from([])).expect("must succeed");
|
||||||
|
let mut expected_environment_vars = vec![
|
||||||
|
EnvVar {
|
||||||
|
key: "FOO".into(),
|
||||||
|
value: "/nix/store/dgapb8kh5gis4w7hzfl5725sx5gam0nz-with-placeholders".into(),
|
||||||
|
},
|
||||||
|
EnvVar {
|
||||||
|
key: "BAR".into(),
|
||||||
|
// Non existent output placeholders should not get replaced.
|
||||||
|
value: hash_placeholder("non-existent").into(),
|
||||||
|
},
|
||||||
|
EnvVar {
|
||||||
|
key: "builder".into(),
|
||||||
|
value: "/bin/sh".into(),
|
||||||
|
},
|
||||||
|
EnvVar {
|
||||||
|
key: "name".into(),
|
||||||
|
value: "with-placeholders".into(),
|
||||||
|
},
|
||||||
|
EnvVar {
|
||||||
|
key: "out".into(),
|
||||||
|
value: "/nix/store/dgapb8kh5gis4w7hzfl5725sx5gam0nz-with-placeholders".into(),
|
||||||
|
},
|
||||||
|
EnvVar {
|
||||||
|
key: "system".into(),
|
||||||
|
value: "x86_64-linux".into(),
|
||||||
|
},
|
||||||
|
];
|
||||||
|
expected_environment_vars.extend(NIX_ENVIRONMENT_VARS.iter().map(|(k, v)| EnvVar {
|
||||||
|
key: k.to_string(),
|
||||||
|
value: Bytes::from_static(v.as_bytes()),
|
||||||
|
}));
|
||||||
|
|
||||||
|
expected_environment_vars.sort_unstable_by_key(|e| e.key.to_owned());
|
||||||
|
assert_eq!(
|
||||||
|
BuildRequest {
|
||||||
|
command_args: vec![
|
||||||
|
"/bin/sh".into(),
|
||||||
|
"-c".into(),
|
||||||
|
"/nix/store/dgapb8kh5gis4w7hzfl5725sx5gam0nz-with-placeholders".into(),
|
||||||
|
// Non existent output placeholders should not get replaced.
|
||||||
|
hash_placeholder("non-existent"),
|
||||||
|
],
|
||||||
|
outputs: vec![
|
||||||
|
"nix/store/dgapb8kh5gis4w7hzfl5725sx5gam0nz-with-placeholders".into()
|
||||||
|
],
|
||||||
|
environment_vars: expected_environment_vars,
|
||||||
|
inputs: BTreeMap::new(),
|
||||||
|
inputs_dir: "nix/store".into(),
|
||||||
|
constraints: HashSet::from([
|
||||||
|
BuildConstraints::System(derivation.system.clone()),
|
||||||
|
BuildConstraints::ProvideBinSh
|
||||||
|
]),
|
||||||
|
additional_files: vec![],
|
||||||
|
working_dir: "build".into(),
|
||||||
|
scratch_paths: vec!["build".into(), "nix/store".into()],
|
||||||
|
refscan_needles: vec!["dgapb8kh5gis4w7hzfl5725sx5gam0nz".into()],
|
||||||
|
},
|
||||||
|
build_request
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_fod_to_build_request() {
|
fn test_fod_to_build_request() {
|
||||||
let aterm_bytes = include_bytes!("tests/0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv");
|
let aterm_bytes = include_bytes!("tests/0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv");
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1 @@
|
||||||
|
Derive([("out","/nix/store/dgapb8kh5gis4w7hzfl5725sx5gam0nz-with-placeholders","","")],[],[],"x86_64-linux","/bin/sh",["-c","/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9","/1zwrijlc0ixcpdjslshwq3nd28rbbmp6812lh5c49ifrcc3gk3mz"],[("BAR","/1zwrijlc0ixcpdjslshwq3nd28rbbmp6812lh5c49ifrcc3gk3mz"),("FOO","/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9"),("builder","/bin/sh"),("name","with-placeholders"),("out","/nix/store/dgapb8kh5gis4w7hzfl5725sx5gam0nz-with-placeholders"),("system","x86_64-linux")])
|
||||||
Loading…
Add table
Add a link
Reference in a new issue