From 2bbd06753baac4ce0e762d38b14cf43e5c55e459 Mon Sep 17 00:00:00 2001 From: Vova Kryachko Date: Fri, 11 Apr 2025 16:47:36 +0000 Subject: [PATCH] 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 Autosubmit: Vova Kryachko --- snix/glue/src/snix_build.rs | 132 +++++++++++++++++- ...grzx8ypnhjbvq23z2kda-with-placeholders.drv | 1 + 2 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 snix/glue/src/tests/18m7y1d025lqgrzx8ypnhjbvq23z2kda-with-placeholders.drv diff --git a/snix/glue/src/snix_build.rs b/snix/glue/src/snix_build.rs index 27aadd920..bb74c6e9c 100644 --- a/snix/glue/src/snix_build.rs +++ b/snix/glue/src/snix_build.rs @@ -6,13 +6,17 @@ use std::future::Future; use std::path::PathBuf; use async_stream::try_stream; +use bstr::BString; use bytes::Bytes; 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 sha2::{Digest, Sha256}; use snix_build::buildservice::{AdditionalFile, BuildConstraints, BuildRequest, EnvVar}; use snix_castore::Node; use snix_store::path_info::PathInfo; +use tracing::warn; 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. let mut command_args: Vec = Vec::with_capacity(derivation.arguments.len() + 1); 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::>() + .as_slice(), + ); // Produce environment_vars and additional files. // 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. // TODO: check if this order is correct, and environment vars set in the // *Derivation actually* have priority. - environment_vars.extend( - derivation - .environment - .iter() - .map(|(k, v)| (k.clone(), Bytes::from(v.to_vec()))), - ); + environment_vars.extend(derivation.environment.iter().map(|(k, v)| { + ( + k.clone(), + Bytes::from(replace_placeholders_b(v, &derivation.outputs).to_vec()), + ) + })); 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 { + 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) -> 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)] mod test { use bytes::Bytes; + use nix_compat::store_path::hash_placeholder; use nix_compat::{derivation::Derivation, store_path::StorePath}; use snix_castore::fixtures::DUMMY_DIGEST; 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] fn test_fod_to_build_request() { let aterm_bytes = include_bytes!("tests/0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv"); diff --git a/snix/glue/src/tests/18m7y1d025lqgrzx8ypnhjbvq23z2kda-with-placeholders.drv b/snix/glue/src/tests/18m7y1d025lqgrzx8ypnhjbvq23z2kda-with-placeholders.drv new file mode 100644 index 000000000..2f706a377 --- /dev/null +++ b/snix/glue/src/tests/18m7y1d025lqgrzx8ypnhjbvq23z2kda-with-placeholders.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")]) \ No newline at end of file