From 035f617b7f11f2ec4a9e08e3a31a175e71a6544b Mon Sep 17 00:00:00 2001 From: Peter Kolloch Date: Wed, 21 Feb 2024 14:07:37 +0700 Subject: [PATCH] feat(tvix/nix-compat): input_sources as StorePath https: //b.tvl.fyi/issues/264 Change-Id: I7a235734dc1f8e93e387a04ba369f3b702c6d5b6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10992 Autosubmit: Peter Kolloch Reviewed-by: flokli Reviewed-by: Peter Kolloch Tested-by: BuildkiteCI --- tvix/glue/src/builtins/derivation.rs | 7 +++-- tvix/nix-compat/src/derivation/mod.rs | 18 +++++-------- tvix/nix-compat/src/derivation/parse_error.rs | 7 +++-- tvix/nix-compat/src/derivation/parser.rs | 26 ++++++++++++++++--- tvix/nix-compat/src/derivation/validate.rs | 12 +-------- tvix/nix-compat/src/derivation/write.rs | 14 ++++++++-- 6 files changed, 53 insertions(+), 31 deletions(-) diff --git a/tvix/glue/src/builtins/derivation.rs b/tvix/glue/src/builtins/derivation.rs index 1208ca94d..87ae8bb94 100644 --- a/tvix/glue/src/builtins/derivation.rs +++ b/tvix/glue/src/builtins/derivation.rs @@ -4,7 +4,7 @@ use crate::tvix_store_io::TvixStoreIO; use bstr::BString; use nix_compat::derivation::{Derivation, Output}; use nix_compat::nixhash; -use nix_compat::store_path::StorePath; +use nix_compat::store_path::{StorePath, StorePathRef}; use std::collections::{btree_map, BTreeSet}; use std::rc::Rc; use tvix_eval::builtin_macros::builtins; @@ -24,7 +24,10 @@ fn populate_inputs(drv: &mut Derivation, full_context: NixContext) { for element in full_context.iter() { match element { NixContextElement::Plain(source) => { - drv.input_sources.insert(source.clone()); + let sp = StorePathRef::from_absolute_path(source.as_bytes()) + .expect("invalid store path") + .to_owned(); + drv.input_sources.insert(sp); } NixContextElement::Single { diff --git a/tvix/nix-compat/src/derivation/mod.rs b/tvix/nix-compat/src/derivation/mod.rs index 3cf9a5ba7..2e4178b87 100644 --- a/tvix/nix-compat/src/derivation/mod.rs +++ b/tvix/nix-compat/src/derivation/mod.rs @@ -40,7 +40,7 @@ pub struct Derivation { /// Plain store paths of additional inputs. #[serde(rename = "inputSrcs")] - pub input_sources: BTreeSet, + pub input_sources: BTreeSet, /// Maps output names to Output. pub outputs: BTreeMap, @@ -131,16 +131,12 @@ impl Derivation { // collect the list of paths from input_sources and input_derivations // into a (sorted, guaranteed by BTreeSet) list of references - let references: BTreeSet = { - let mut inputs = self.input_sources.clone(); - let input_derivation_keys: Vec = self - .input_derivations - .keys() - .map(|k| k.to_absolute_path()) - .collect(); - inputs.extend(input_derivation_keys); - inputs - }; + let references: BTreeSet = self + .input_sources + .iter() + .chain(self.input_derivations.keys()) + .map(StorePath::to_absolute_path) + .collect(); build_text_path(name, self.to_aterm_bytes(), references) .map(|s| s.to_owned()) diff --git a/tvix/nix-compat/src/derivation/parse_error.rs b/tvix/nix-compat/src/derivation/parse_error.rs index 68228b07c..fc97f1a98 100644 --- a/tvix/nix-compat/src/derivation/parse_error.rs +++ b/tvix/nix-compat/src/derivation/parse_error.rs @@ -2,7 +2,10 @@ //! Derivations from ATerm. use nom::IResult; -use crate::{nixhash, store_path}; +use crate::{ + nixhash, + store_path::{self, StorePath}, +}; pub type NomResult = IResult>; @@ -17,7 +20,7 @@ pub enum ErrorKind { DuplicateInputDerivationOutputName(String, String), #[error("duplicate input source: {0}")] - DuplicateInputSource(String), + DuplicateInputSource(StorePath), #[error("nix hash error: {0}")] NixHashError(nixhash::Error), diff --git a/tvix/nix-compat/src/derivation/parser.rs b/tvix/nix-compat/src/derivation/parser.rs index 2cfcf2eae..9f5a5f090 100644 --- a/tvix/nix-compat/src/derivation/parser.rs +++ b/tvix/nix-compat/src/derivation/parser.rs @@ -186,11 +186,19 @@ fn parse_input_derivations(i: &[u8]) -> NomResult<&[u8], BTreeMap NomResult<&[u8], BTreeSet> { +fn parse_input_sources(i: &[u8]) -> NomResult<&[u8], BTreeSet> { let (i, input_sources_lst) = aterm::parse_str_list(i).map_err(into_nomerror)?; let mut input_sources: BTreeSet<_> = BTreeSet::new(); for input_source in input_sources_lst.into_iter() { + let input_source: StorePath = StorePathRef::from_absolute_path(input_source.as_bytes()) + .map_err(|e: store_path::Error| { + nom::Err::Failure(NomError { + input: i, + code: e.into(), + }) + })? + .to_owned(); if input_sources.contains(&input_source) { return Err(nom::Err::Failure(NomError { input: i, @@ -312,6 +320,7 @@ where #[cfg(test)] mod tests { + use crate::store_path::StorePathRef; use std::collections::{BTreeMap, BTreeSet}; use crate::{ @@ -460,7 +469,14 @@ mod tests { fn parse_input_sources(input: &'static [u8], expected: &BTreeSet) { let (rest, parsed) = super::parse_input_sources(input).expect("must parse"); - assert_eq!(expected, &parsed, "parsed mismatch"); + assert_eq!( + expected, + &parsed + .iter() + .map(StorePath::to_absolute_path) + .collect::>(), + "parsed mismatch" + ); assert!(rest.is_empty(), "rest must be empty"); } @@ -474,7 +490,11 @@ mod tests { nom::Err::Failure(e) => { assert_eq!( ErrorKind::DuplicateInputSource( - "/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-foo".to_string() + StorePathRef::from_absolute_path( + "/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-foo".as_bytes() + ) + .unwrap() + .to_owned() ), e.code ); diff --git a/tvix/nix-compat/src/derivation/validate.rs b/tvix/nix-compat/src/derivation/validate.rs index 757326816..af7dff2fd 100644 --- a/tvix/nix-compat/src/derivation/validate.rs +++ b/tvix/nix-compat/src/derivation/validate.rs @@ -1,5 +1,5 @@ use crate::derivation::{Derivation, DerivationError}; -use crate::store_path::{self, StorePathRef}; +use crate::store_path; impl Derivation { /// validate ensures a Derivation struct is properly populated, @@ -87,16 +87,6 @@ impl Derivation { } } - // Validate all input_sources - for input_source in self.input_sources.iter() { - if let Err(e) = StorePathRef::from_absolute_path(input_source.as_bytes()) { - return Err(DerivationError::InvalidInputSourcesPath( - input_source.to_string(), - e, - )); - } - } - // validate platform if self.system.is_empty() { return Err(DerivationError::InvalidPlatform(self.system.to_string())); diff --git a/tvix/nix-compat/src/derivation/write.rs b/tvix/nix-compat/src/derivation/write.rs index 83106cd9e..04810e736 100644 --- a/tvix/nix-compat/src/derivation/write.rs +++ b/tvix/nix-compat/src/derivation/write.rs @@ -33,6 +33,13 @@ pub const QUOTE: char = '"'; /// the context a lot. pub(crate) trait AtermWriteable: Display { fn aterm_write(&self, writer: &mut impl Write) -> std::io::Result<()>; + + fn aterm_bytes(&self) -> Vec { + let mut bytes = Vec::new(); + self.aterm_write(&mut bytes) + .expect("unexpected write errors to Vec"); + bytes + } } impl AtermWriteable for StorePathRef<'_> { @@ -182,12 +189,15 @@ pub(crate) fn write_input_derivations( pub(crate) fn write_input_sources( writer: &mut impl Write, - input_sources: &BTreeSet, + input_sources: &BTreeSet, ) -> Result<(), io::Error> { write_char(writer, BRACKET_OPEN)?; write_array_elements( writer, - &input_sources.iter().map(String::from).collect::>(), + &input_sources + .iter() + .map(StorePath::to_absolute_path) + .collect::>(), )?; write_char(writer, BRACKET_CLOSE)?;