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

fix(nix-compat/derivation): handle dups

Browse source 
This now properly checks for duplicate output names in input derivation
output names, and duplicate input sources.

Change-Id: I0053854bfbf504f4f511fb3fe1a77a82b3aa61dd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9738
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
This commit is contained in:
Florian Klink 2023-10-15 09:29:48 +01:00 committed by clbot
parent 2410f2292f
commit 8934b34489
2 changed files with 173 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
tvix/nix-compat/src/derivation/parse_error.rs
View file

@ -6,15 +6,23 @@ use crate::nixhash;
pub type NomResult<I, O> = IResult<I, O, NomError<I>>;
#[derive(Debug, PartialEq)]
#[derive(Debug, thiserror::Error, PartialEq)]
pub enum ErrorKind {
// duplicate key in map
/// duplicate key in map
#[error("duplicate map key: {0}")]
DuplicateMapKey(String),
// Digest parsing error
/// Input derivation has two outputs with the same name
#[error("duplicate output name {1} for input derivation {0}")]
DuplicateInputDerivationOutputName(String, String),
#[error("duplicate input source: {0}")]
DuplicateInputSource(String),
#[error("nix hash error: {0}")]
NixHashError(nixhash::Error),
// error kind wrapped from native nom errors
#[error("nom error: {0:?}")]
Nom(nom::error::ErrorKind),
}

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

@ -125,8 +125,45 @@ fn parse_outputs(i: &[u8]) -> NomResult<&[u8], BTreeMap<String, Output>> {
}
}
fn parse_input_derivations(i: &[u8]) -> NomResult<&[u8], BTreeMap<String, Vec<String>>> {
parse_kv::<Vec<String>, _>(aterm::parse_str_list)(i)
fn parse_input_derivations(i: &[u8]) -> NomResult<&[u8], BTreeMap<String, BTreeSet<String>>> {
let (i, input_derivations_list) = parse_kv::<Vec<String>, _>(aterm::parse_str_list)(i)?;
// This is a HashMap of drv paths to a list of output names.
let mut input_derivations: BTreeMap<String, BTreeSet<String>> = BTreeMap::new();
for (input_derivation, output_names) in input_derivations_list {
let mut new_output_names = BTreeSet::new();
for output_name in output_names.into_iter() {
if !new_output_names.insert(output_name.clone()) {
return Err(nom::Err::Failure(NomError {
input: i,
code: ErrorKind::DuplicateInputDerivationOutputName(
input_derivation.to_string(),
output_name.to_string(),
),
}));
}
}
input_derivations.insert(input_derivation, new_output_names);
}
Ok((i, input_derivations))
}
fn parse_input_sources(i: &[u8]) -> NomResult<&[u8], BTreeSet<String>> {
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() {
if !input_sources.insert(input_source.clone()) {
return Err(nom::Err::Failure(NomError {
input: i,
code: ErrorKind::DuplicateInputSource(input_source),
}));
}
}
Ok((i, input_sources))
}
pub fn parse_derivation(i: &[u8]) -> NomResult<&[u8], Derivation> {
@ -144,7 +181,7 @@ pub fn parse_derivation(i: &[u8]) -> NomResult<&[u8], Derivation> {
// // parse input derivations
terminated(parse_input_derivations, nomchar(',')),
// // parse input sources
|i| terminated(aterm::parse_str_list, nomchar(','))(i).map_err(into_nomerror),
terminated(parse_input_sources, nomchar(',')),
// // parse system
|i| terminated(aterm::parse_string_field, nomchar(','))(i).map_err(into_nomerror),
// // parse builder
@ -166,24 +203,12 @@ pub fn parse_derivation(i: &[u8]) -> NomResult<&[u8], Derivation> {
arguments,
environment,
)| {
// All values in input_derivations need to be converted from
// Vec<String> to BTreeSet<String>
let mut input_derivations_new: BTreeMap<_, BTreeSet<_>> = BTreeMap::new();
for (k, v) in input_derivations.into_iter() {
let values_new: BTreeSet<_> = BTreeSet::from_iter(v.into_iter());
input_derivations_new.insert(k, values_new);
// TODO: actually check they're not duplicate in the parser side!
}
// Input sources need to be converted from Vec<_> to BTreeSet<_>
let input_sources_new: BTreeSet<_> = BTreeSet::from_iter(input_sources);
Derivation {
arguments,
builder,
environment,
input_derivations: input_derivations_new,
input_sources: input_sources_new,
input_derivations,
input_sources,
outputs,
system,
}
@ -246,9 +271,9 @@ where
#[cfg(test)]
mod tests {
use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};
use crate::derivation::Output;
use crate::derivation::{parse_error::ErrorKind, Output};
use bstr::{BString, ByteSlice};
use lazy_static::lazy_static;
use test_case::test_case;
@ -279,8 +304,44 @@ mod tests {
b.insert("b".to_string(), b"2".as_bstr().to_owned());
b
};
static ref EXP_INPUT_DERIVATIONS_SIMPLE: BTreeMap<String, BTreeSet<String>> = {
let mut b = BTreeMap::new();
b.insert(
"/nix/store/8bjm87p310sb7r2r0sg4xrynlvg86j8k-hello-2.12.1.tar.gz.drv".to_string(),
{
let mut output_names = BTreeSet::new();
output_names.insert("out".to_string());
output_names
},
);
b.insert(
"/nix/store/p3jc8aw45dza6h52v81j7lk69khckmcj-bash-5.2-p15.drv".to_string(),
{
let mut output_names = BTreeSet::new();
output_names.insert("out".to_string());
output_names.insert("lib".to_string());
output_names
},
);
b
};
static ref EXP_INPUT_DERIVATIONS_SIMPLE_ATERM: String = {
format!(
"[(\"{0}\",[\"out\"]),(\"{1}\",[\"out\",\"lib\"])]",
"/nix/store/8bjm87p310sb7r2r0sg4xrynlvg86j8k-hello-2.12.1.tar.gz.drv",
"/nix/store/p3jc8aw45dza6h52v81j7lk69khckmcj-bash-5.2-p15.drv"
)
};
static ref EXP_INPUT_SOURCES_SIMPLE: BTreeSet<String> = {
let mut b = BTreeSet::new();
b.insert("/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-has-multi-out".to_string());
b.insert("/nix/store/2vixb94v0hy2xc6p7mbnxxcyc095yyia-has-multi-out-lib".to_string());
b
};
}
/// Ensure parsing KVs works
#[test_case(b"[]", &BTreeMap::new(), b""; "empty")]
#[test_case(b"[(\"a\",\"1\"),(\"b\",\"2\")]", &EXP_AB_MAP, b""; "simple")]
fn parse_kv(input: &'static [u8], expected: &BTreeMap<String, BString>, exp_rest: &[u8]) {
let (rest, parsed) = super::parse_kv::<BString, _>(crate::aterm::parse_bstr_field)(input)
@ -289,6 +350,87 @@ mod tests {
assert_eq!(*expected, parsed);
}
/// Ensures the kv parser complains about duplicate map keys
#[test]
fn parse_kv_fail_dup_keys() {
let input: &'static [u8] = b"[(\"a\",\"1\"),(\"a\",\"2\")]";
let e = super::parse_kv::<BString, _>(crate::aterm::parse_bstr_field)(input)
.expect_err("must fail");
match e {
nom::Err::Failure(e) => {
assert_eq!(ErrorKind::DuplicateMapKey("a".to_string()), e.code);
}
_ => panic!("unexpected error"),
}
}
/// Ensure parsing input derivations works.
#[test_case(b"[]", &BTreeMap::new(); "empty")]
#[test_case(EXP_INPUT_DERIVATIONS_SIMPLE_ATERM.as_bytes(), &EXP_INPUT_DERIVATIONS_SIMPLE; "simple")]
fn parse_input_derivations(
input: &'static [u8],
expected: &BTreeMap<String, BTreeSet<String>>,
) {
let (rest, parsed) = super::parse_input_derivations(input).expect("must parse");
assert_eq!(expected, &parsed, "parsed mismatch");
assert!(rest.is_empty(), "rest must be empty");
}
/// Ensures the input derivation parser complains about duplicate output names
#[test]
fn parse_input_derivations_fail_dup_output_names() {
let input_str = format!(
"[(\"{0}\",[\"out\"]),(\"{1}\",[\"out\",\"out\"])]",
"/nix/store/8bjm87p310sb7r2r0sg4xrynlvg86j8k-hello-2.12.1.tar.gz.drv",
"/nix/store/p3jc8aw45dza6h52v81j7lk69khckmcj-bash-5.2-p15.drv"
);
let e = super::parse_input_derivations(input_str.as_bytes()).expect_err("must fail");
match e {
nom::Err::Failure(e) => {
assert_eq!(
ErrorKind::DuplicateInputDerivationOutputName(
"/nix/store/p3jc8aw45dza6h52v81j7lk69khckmcj-bash-5.2-p15.drv".to_string(),
"out".to_string()
),
e.code
);
}
_ => panic!("unexpected error"),
}
}
/// Ensure parsing input sources works
#[test_case(b"[]", &BTreeSet::new(); "empty")]
#[test_case(b"[\"/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-has-multi-out\",\"/nix/store/2vixb94v0hy2xc6p7mbnxxcyc095yyia-has-multi-out-lib\"]", &EXP_INPUT_SOURCES_SIMPLE; "simple")]
fn parse_input_sources(input: &'static [u8], expected: &BTreeSet<String>) {
let (rest, parsed) = super::parse_input_sources(input).expect("must parse");
assert_eq!(expected, &parsed, "parsed mismatch");
assert!(rest.is_empty(), "rest must be empty");
}
/// Ensures the input sources parser complains about duplicate input sources
#[test]
fn parse_input_sources_fail_dup_keys() {
let input: &'static [u8] = b"[\"/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-foo\",\"/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-foo\"]";
let e = super::parse_input_sources(input).expect_err("must fail");
match e {
nom::Err::Failure(e) => {
assert_eq!(
ErrorKind::DuplicateInputSource(
"/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-foo".to_string()
),
e.code
);
}
_ => panic!("unexpected error"),
}
}
#[test_case(
br#"("out","/nix/store/5vyvcwah9l9kf07d52rcgdk70g2f4y13-foo","","")"#,
("out".to_string(), Output {