mdebray/tvl-depot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(tvix/nix-compat): Use StorePath in Output

Browse source 
https: //b.tvl.fyi/issues/264
Change-Id: Icb09be9643245cc68d09f01d7723af2d44d6bd1a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11001
Autosubmit: Peter Kolloch <info@eigenvalue.net>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This commit is contained in:
Peter Kolloch 2024-02-21 18:31:35 +07:00 committed by clbot
parent 035f617b7f
commit fde488ec6d
11 changed files with 119 additions and 82 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
tvix/glue/src/builtins/derivation.rs
View file

@ -116,7 +116,7 @@ fn handle_fixed_output(
drv.outputs.insert(
"out".to_string(),
Output {
path: "".to_string(),
path: None,
ca_hash: match hash_mode_str.as_deref() {
None | Some("flat") => Some(nixhash::CAHash::Flat(nixhash)),
Some("recursive") => Some(nixhash::CAHash::Nar(nixhash)),
@ -486,7 +486,7 @@ pub(crate) mod derivation_builtins {
(
name.clone(),
(
output.path,
output.path.unwrap().to_absolute_path(),
Some(
NixContextElement::Single {
name,

8
tvix/glue/src/known_paths.rs
View file

@ -74,14 +74,8 @@ impl KnownPaths {
// For all output paths, update our lookup table.
// We only write into the lookup table once.
for output in drv.outputs.values() {
// We assume derivations to be passed validated, so ignoring rest
// and expecting parsing is ok.
// TODO: b/264
let (output_path, _rest) =
StorePath::from_absolute_path_full(&output.path).expect("parse output path");
self.outputs_to_drvpath
.entry(output_path)
.entry(output.path.as_ref().expect("missing store path").clone())
.or_insert(drv_path.to_owned());
}

2
tvix/glue/src/tvix_build.rs
View file

@ -52,7 +52,7 @@ pub(crate) fn derivation_to_build_request(
let mut output_paths: Vec<String> = derivation
.outputs
.values()
.map(|e| e.path[1..].to_owned())
.map(|e| e.path_str()[1..].to_owned())
.collect();
// Sort the outputs. We can use sort_unstable, as these are unique strings.

17
tvix/glue/src/tvix_store_io.rs
View file

@ -4,10 +4,7 @@ use async_recursion::async_recursion;
use bytes::Bytes;
use futures::Stream;
use futures::{StreamExt, TryStreamExt};
use nix_compat::{
nixhash::CAHash,
store_path::{StorePath, StorePathRef},
};
use nix_compat::{nixhash::CAHash, store_path::StorePath};
use std::{
cell::RefCell,
collections::BTreeSet,
@ -153,16 +150,14 @@ impl TvixStoreIO {
let output_paths: Vec<StorePath> = output_names
.iter()
.map(|output_name| {
let output_path = &input_drv
input_drv
.outputs
.get(output_name)
.expect("missing output_name")
.path;
// since Derivation is validated, we this can be parsed.
StorePathRef::from_absolute_path(output_path.as_bytes())
.expect("invalid output path")
.to_owned()
.path
.as_ref()
.expect("missing output path")
.clone()
})
.collect();
// For each output, ask for the castore node.

2
tvix/nix-compat/src/derivation/errors.rs
View file

@ -53,6 +53,8 @@ pub enum DerivationError {
pub enum OutputError {
#[error("Invalid output path {0}: {1}")]
InvalidOutputPath(String, store_path::Error),
#[error("Missing output path")]
MissingOutputPath,
#[error("Invalid CAHash: {:?}", .0)]
InvalidCAHash(CAHash),
}

12
tvix/nix-compat/src/derivation/mod.rs
View file

@ -161,7 +161,13 @@ impl Derivation {
"fixed:out:{}{}:{}",
ca_kind_prefix(ca_hash),
ca_hash.digest().to_nix_hex_string(),
out_output.path
out_output
.path
.as_ref()
.map(StorePath::to_absolute_path)
.as_ref()
.map(|s| s as &str)
.unwrap_or(""),
))
.finalize()
.into(),
@ -239,7 +245,7 @@ impl Derivation {
// Assert that outputs are not yet populated, to avoid using this function wrongly.
// We don't also go over self.environment, but it's a sufficient
// footgun prevention mechanism.
assert!(output.path.is_empty());
assert!(output.path.is_none());
let path_name = output_path_name(name, output_name);
@ -258,7 +264,7 @@ impl Derivation {
})?
};
output.path = abs_store_path.to_absolute_path();
output.path = Some(abs_store_path.to_owned());
self.environment.insert(
output_name.to_string(),
abs_store_path.to_absolute_path().into(),

57
tvix/nix-compat/src/derivation/output.rs
View file

@ -1,14 +1,16 @@
use crate::derivation::OutputError;
use crate::nixhash::CAHash;
use crate::store_path::StorePathRef;
use crate::{derivation::OutputError, store_path::StorePath};
use serde::de::Unexpected;
use serde::{Deserialize, Serialize};
use serde_json::Map;
use std::borrow::Cow;
/// References the derivation output.
#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize)]
pub struct Output {
/// Store path of build result.
pub path: String,
pub path: Option<StorePath>,
#[serde(flatten)]
pub ca_hash: Option<CAHash>, // we can only represent a subset here.
@ -20,18 +22,21 @@ impl<'de> Deserialize<'de> for Output {
D: serde::Deserializer<'de>,
{
let fields = Map::deserialize(deserializer)?;
let path: &str = fields
.get("path")
.ok_or(serde::de::Error::missing_field(
"`path` is missing but required for outputs",
))?
.as_str()
.ok_or(serde::de::Error::invalid_type(
serde::de::Unexpected::Other("certainly not a string"),
&"a string",
))?;
let path = StorePathRef::from_absolute_path(path.as_bytes())
.map_err(|_| serde::de::Error::invalid_value(Unexpected::Str(path), &"StorePath"))?;
Ok(Self {
path: fields
.get("path")
.ok_or(serde::de::Error::missing_field(
"`path` is missing but required for outputs",
))?
.as_str()
.ok_or(serde::de::Error::invalid_type(
serde::de::Unexpected::Other("certainly not a string"),
&"a string",
))?
.to_owned(),
path: Some(path.to_owned()),
ca_hash: CAHash::from_map::<D>(&fields)?,
})
}
@ -42,6 +47,14 @@ impl Output {
self.ca_hash.is_some()
}
/// The output path as a string -- use `""` to indicate an unset output path.
pub fn path_str(&self) -> Cow<str> {
match &self.path {
None => Cow::Borrowed(""),
Some(path) => Cow::Owned(path.to_absolute_path()),
}
}
pub fn validate(&self, validate_output_paths: bool) -> Result<(), OutputError> {
if let Some(fixed_output_hash) = &self.ca_hash {
match fixed_output_hash {
@ -52,10 +65,8 @@ impl Output {
}
}
if validate_output_paths {
if let Err(e) = StorePathRef::from_absolute_path(self.path.as_bytes()) {
return Err(OutputError::InvalidOutputPath(self.path.to_string(), e));
}
if validate_output_paths && self.path.is_none() {
return Err(OutputError::MissingOutputPath);
}
Ok(())
}
@ -67,7 +78,7 @@ impl Output {
fn deserialize_valid_input_addressed_output() {
let json_bytes = r#"
{
"path": "/nix/store/blablabla"
"path": "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432"
}"#;
let output: Output = serde_json::from_str(json_bytes).expect("must parse");
@ -80,7 +91,7 @@ fn deserialize_valid_input_addressed_output() {
fn deserialize_valid_fixed_output() {
let json_bytes = r#"
{
"path": "/nix/store/blablablabla",
"path": "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432",
"hash": "08813cbee9903c62be4c5027726a418a300da4500b2d369d3af9286f4815ceba",
"hashAlgo": "r:sha256"
}"#;
@ -95,7 +106,7 @@ fn deserialize_valid_fixed_output() {
fn deserialize_with_error_invalid_hash_encoding_fixed_output() {
let json_bytes = r#"
{
"path": "/nix/store/blablablabla",
"path": "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432",
"hash": "IAMNOTVALIDNIXBASE32",
"hashAlgo": "r:sha256"
}"#;
@ -110,7 +121,7 @@ fn deserialize_with_error_invalid_hash_encoding_fixed_output() {
fn deserialize_with_error_invalid_hash_algo_fixed_output() {
let json_bytes = r#"
{
"path": "/nix/store/blablablabla",
"path": "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432",
"hash": "08813cbee9903c62be4c5027726a418a300da4500b2d369d3af9286f4815ceba",
"hashAlgo": "r:sha1024"
}"#;
@ -125,7 +136,7 @@ fn deserialize_with_error_invalid_hash_algo_fixed_output() {
fn deserialize_with_error_missing_hash_algo_fixed_output() {
let json_bytes = r#"
{
"path": "/nix/store/blablablabla",
"path": "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432",
"hash": "08813cbee9903c62be4c5027726a418a300da4500b2d369d3af9286f4815ceba",
}"#;
let output: Result<Output, _> = serde_json::from_str(json_bytes);
@ -139,7 +150,7 @@ fn deserialize_with_error_missing_hash_algo_fixed_output() {
fn deserialize_with_error_missing_hash_fixed_output() {
let json_bytes = r#"
{
"path": "/nix/store/blablablabla",
"path": "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432",
"hashAlgo": "r:sha1024"
}"#;
let output: Result<Output, _> = serde_json::from_str(json_bytes);

77
tvix/nix-compat/src/derivation/parser.rs
View file

@ -97,7 +97,13 @@ fn parse_output(i: &[u8]) -> NomResult<&[u8], (String, Output)> {
Ok(hash_with_mode) => Ok((
output_name,
Output {
path: output_path,
// TODO: Check if allowing empty paths here actually makes sense
// or we should make this code stricter.
path: if output_path.is_empty() {
None
} else {
Some(string_to_store_path(i, output_path)?)
},
ca_hash: hash_with_mode,
},
)),
@ -164,21 +170,7 @@ fn parse_input_derivations(i: &[u8]) -> NomResult<&[u8], BTreeMap<StorePath, BTr
new_output_names.insert(output_name);
}
#[cfg(debug_assertions)]
let input_derivation_str = input_derivation.clone();
let input_derivation: StorePath =
StorePathRef::from_absolute_path(input_derivation.as_bytes())
.map_err(|e: store_path::Error| {
nom::Err::Failure(NomError {
input: i,
code: e.into(),
})
})?
.to_owned();
#[cfg(debug_assertions)]
assert_eq!(input_derivation_str, input_derivation.to_absolute_path());
let input_derivation: StorePath = string_to_store_path(i, input_derivation)?;
input_derivations.insert(input_derivation, new_output_names);
}
@ -191,14 +183,7 @@ fn parse_input_sources(i: &[u8]) -> NomResult<&[u8], BTreeSet<StorePath>> {
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();
let input_source: StorePath = string_to_store_path(i, input_source)?;
if input_sources.contains(&input_source) {
return Err(nom::Err::Failure(NomError {
input: i,
@ -212,6 +197,28 @@ fn parse_input_sources(i: &[u8]) -> NomResult<&[u8], BTreeSet<StorePath>> {
Ok((i, input_sources))
}
fn string_to_store_path(
i: &[u8],
path_str: String,
) -> Result<StorePath, nom::Err<NomError<&[u8]>>> {
#[cfg(debug_assertions)]
let path_str2 = path_str.clone();
let path: StorePath = StorePathRef::from_absolute_path(path_str.as_bytes())
.map_err(|e: store_path::Error| {
nom::Err::Failure(NomError {
input: i,
code: e.into(),
})
})?
.to_owned();
#[cfg(debug_assertions)]
assert_eq!(path_str2, path.to_absolute_path());
Ok(path)
}
pub fn parse_derivation(i: &[u8]) -> NomResult<&[u8], Derivation> {
use nom::Parser;
preceded(
@ -343,15 +350,24 @@ mod tests {
b.insert(
"lib".to_string(),
Output {
path: "/nix/store/2vixb94v0hy2xc6p7mbnxxcyc095yyia-has-multi-out-lib"
.to_string(),
path: Some(
StorePath::from_bytes(
b"2vixb94v0hy2xc6p7mbnxxcyc095yyia-has-multi-out-lib",
)
.unwrap(),
),
ca_hash: None,
},
);
b.insert(
"out".to_string(),
Output {
path: "/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-has-multi-out".to_string(),
path: Some(
StorePath::from_bytes(
b"55lwldka5nyxa08wnvlizyqw02ihy8ic-has-multi-out".as_bytes(),
)
.unwrap(),
),
ca_hash: None,
},
);
@ -506,14 +522,17 @@ mod tests {
#[test_case(
br#"("out","/nix/store/5vyvcwah9l9kf07d52rcgdk70g2f4y13-foo","","")"#,
("out".to_string(), Output {
path: "/nix/store/5vyvcwah9l9kf07d52rcgdk70g2f4y13-foo".to_string(),
path: Some(
StorePathRef::from_absolute_path("/nix/store/5vyvcwah9l9kf07d52rcgdk70g2f4y13-foo".as_bytes()).unwrap().to_owned()),
ca_hash: None
}); "simple"
)]
#[test_case(
br#"("out","/nix/store/4q0pg5zpfmznxscq3avycvf9xdvx50n3-bar","r:sha256","08813cbee9903c62be4c5027726a418a300da4500b2d369d3af9286f4815ceba")"#,
("out".to_string(), Output {
path: "/nix/store/4q0pg5zpfmznxscq3avycvf9xdvx50n3-bar".to_string(),
path: Some(
StorePathRef::from_absolute_path(
"/nix/store/4q0pg5zpfmznxscq3avycvf9xdvx50n3-bar".as_bytes()).unwrap().to_owned()),
ca_hash: Some(from_algo_and_mode_and_digest("r:sha256",
data_encoding::HEXLOWER.decode(b"08813cbee9903c62be4c5027726a418a300da4500b2d369d3af9286f4815ceba").unwrap() ).unwrap()),
}); "fod"

17
tvix/nix-compat/src/derivation/tests/mod.rs
View file

@ -170,7 +170,7 @@ fn derivation_with_trimmed_output_paths(derivation: &Derivation) -> Derivation {
trimmed_outputs.insert(
output_name.to_string(),
Output {
path: "".to_string(),
path: None,
..output.clone()
},
);
@ -314,7 +314,7 @@ fn output_path_construction() {
bar_drv.outputs.insert(
"out".to_string(),
Output {
path: "".to_string(), // will be calculated
path: None, // will be calculated
ca_hash: Some(crate::nixhash::CAHash::Nar(
crate::nixhash::from_algo_and_digest(
crate::nixhash::HashAlgo::Sha256,
@ -365,7 +365,16 @@ fn output_path_construction() {
// assemble foo env
let foo_env = &mut foo_drv.environment;
foo_env.insert("bar".to_string(), bar_output_path.to_owned().into());
// foo_env.insert("bar".to_string(), StorePathRef:: bar_output_path.to_owned().try_into().unwrap());
foo_env.insert(
"bar".to_string(),
bar_output_path
.as_ref()
.unwrap()
.to_absolute_path()
.as_bytes()
.into(),
);
foo_env.insert("builder".to_string(), ":".into());
foo_env.insert("name".to_string(), "foo".into());
foo_env.insert("out".to_string(), "".into()); // will be calculated
@ -375,7 +384,7 @@ fn output_path_construction() {
foo_drv.outputs.insert(
"out".to_string(),
Output {
path: "".to_string(), // will be calculated
path: None, // will be calculated
ca_hash: None,
},
);

2
tvix/nix-compat/src/derivation/validate.rs
View file

@ -123,7 +123,7 @@ mod test {
outputs.insert(
"out".to_string(),
Output {
path: "".to_string(),
path: None,
ca_hash: Some(CAHash::Text([0; 32])), // This is disallowed
},
);

3
tvix/nix-compat/src/derivation/write.rs
View file

@ -132,7 +132,8 @@ pub(crate) fn write_outputs(
write_char(writer, PAREN_OPEN)?;
let mut elements: Vec<&str> = vec![output_name, &output.path];
let path_str = output.path_str();
let mut elements: Vec<&str> = vec![output_name, &path_str];
let (mode_and_algo, digest) = match &output.ca_hash {
Some(ca_hash) => (