maurice/snix
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(nix-compat/store_path): make StorePath generic on S

Browse source 
Similar to how cl/12253 already did this for `Signature`, we apply the
same logic to `StorePath`.

`StorePathRef<'a>'` is now a `StorePath<&'a str>`, and there's less
redundant code for the two different implementation.

`.as_ref()` returns a `StorePathRef<'_>`, `.to_owned()` gives a
`StorePath<String>` (for now).

I briefly thought about only publicly exporting `StorePath<String>`
as `StorePath`, but the diff is not too large and this will make it
easier to gradually introduce more flexibility in which store paths to
accept.

Also, remove some silliness in `StorePath::from_absolute_path_full`,
which now doesn't allocate anymore.

Change-Id: Ife8843857a1a0a3a99177ca997649fd45b8198e6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12258
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This commit is contained in:
Florian Klink 2024-08-20 16:52:07 +03:00 • committed by clbot
parent 413135b925
commit 2beabe968c
15 changed files with 301 additions and 315 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -36,11 +36,11 @@ pub struct Derivation {
/// Map from drv path to output names used from this derivation.
#[serde(rename = "inputDrvs")]
pub input_derivations: BTreeMap<StorePath, BTreeSet<String>>,
pub input_derivations: BTreeMap<StorePath<String>, BTreeSet<String>>,
/// Plain store paths of additional inputs.
#[serde(rename = "inputSrcs")]
pub input_sources: BTreeSet<StorePath>,
pub input_sources: BTreeSet<StorePath<String>>,
/// Maps output names to Output.
pub outputs: BTreeMap<String, Output>,
@ -127,7 +127,10 @@ impl Derivation {
/// the `name` with a `.drv` suffix as name, all [Derivation::input_sources] and
/// keys of [Derivation::input_derivations] as references, and the ATerm string of
/// the [Derivation] as content.
pub fn calculate_derivation_path(&self, name: &str) -> Result<StorePath, DerivationError> {
pub fn calculate_derivation_path(
&self,
name: &str,
) -> Result<StorePath<String>, DerivationError> {
// append .drv to the name
let name = &format!("{}.drv", name);
@ -141,7 +144,6 @@ impl Derivation {
.collect();
build_text_path(name, self.to_aterm_bytes(), references)
.map(|s| s.to_owned())
.map_err(|_e| DerivationError::InvalidOutputName(name.to_string()))
}
@ -210,7 +212,7 @@ impl Derivation {
self.input_derivations
.iter()
.map(|(drv_path, output_names)| {
let hash = fn_lookup_hash_derivation_modulo(&drv_path.into());
let hash = fn_lookup_hash_derivation_modulo(&drv_path.as_ref());
(hash, output_names.to_owned())
}),

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

@ -1,5 +1,4 @@
use crate::nixhash::CAHash;
use crate::store_path::StorePathRef;
use crate::{derivation::OutputError, store_path::StorePath};
use serde::de::Unexpected;
use serde::{Deserialize, Serialize};
@ -10,7 +9,7 @@ use std::borrow::Cow;
#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize)]
pub struct Output {
/// Store path of build result.
pub path: Option<StorePath>,
pub path: Option<StorePath<String>>,
#[serde(flatten)]
pub ca_hash: Option<CAHash>, // we can only represent a subset here.
@ -33,10 +32,10 @@ impl<'de> Deserialize<'de> for Output {
&"a string",
))?;
let path = StorePathRef::from_absolute_path(path.as_bytes())
let path = StorePath::from_absolute_path(path.as_bytes())
.map_err(|_| serde::de::Error::invalid_value(Unexpected::Str(path), &"StorePath"))?;
Ok(Self {
path: Some(path.to_owned()),
path: Some(path),
ca_hash: CAHash::from_map::<D>(&fields)?,
})
}

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

@ -20,7 +20,7 @@ pub enum ErrorKind {
DuplicateInputDerivationOutputName(String, String),
#[error("duplicate input source: {0}")]
DuplicateInputSource(StorePath),
DuplicateInputSource(StorePath<String>),
#[error("nix hash error: {0}")]
NixHashError(nixhash::Error),

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

@ -13,7 +13,7 @@ use thiserror;
use crate::derivation::parse_error::{into_nomerror, ErrorKind, NomError, NomResult};
use crate::derivation::{write, CAHash, Derivation, Output};
use crate::store_path::{self, StorePath, StorePathRef};
use crate::store_path::{self, StorePath};
use crate::{aterm, nixhash};
#[derive(Debug, thiserror::Error)]
@ -101,7 +101,7 @@ fn parse_output(i: &[u8]) -> NomResult<&[u8], (String, Output)> {
path: if output_path.is_empty() {
None
} else {
Some(string_to_store_path(i, output_path)?)
Some(string_to_store_path(i, &output_path)?)
},
ca_hash: hash_with_mode,
},
@ -131,12 +131,12 @@ fn parse_outputs(i: &[u8]) -> NomResult<&[u8], BTreeMap<String, Output>> {
match res {
Ok((rst, outputs_lst)) => {
let mut outputs: BTreeMap<String, Output> = BTreeMap::default();
let mut outputs = BTreeMap::default();
for (output_name, output) in outputs_lst.into_iter() {
if outputs.contains_key(&output_name) {
return Err(nom::Err::Failure(NomError {
input: i,
code: ErrorKind::DuplicateMapKey(output_name),
code: ErrorKind::DuplicateMapKey(output_name.to_string()),
}));
}
outputs.insert(output_name, output);
@ -148,11 +148,13 @@ fn parse_outputs(i: &[u8]) -> NomResult<&[u8], BTreeMap<String, Output>> {
}
}
fn parse_input_derivations(i: &[u8]) -> NomResult<&[u8], BTreeMap<StorePath, BTreeSet<String>>> {
fn parse_input_derivations(
i: &[u8],
) -> NomResult<&[u8], BTreeMap<StorePath<String>, BTreeSet<String>>> {
let (i, input_derivations_list) = parse_kv(aterm::parse_string_list)(i)?;
// This is a HashMap of drv paths to a list of output names.
let mut input_derivations: BTreeMap<StorePath, BTreeSet<_>> = BTreeMap::new();
let mut input_derivations: BTreeMap<StorePath<String>, BTreeSet<_>> = BTreeMap::new();
for (input_derivation, output_names) in input_derivations_list {
let mut new_output_names = BTreeSet::new();
@ -169,7 +171,7 @@ fn parse_input_derivations(i: &[u8]) -> NomResult<&[u8], BTreeMap<StorePath, BTr
new_output_names.insert(output_name);
}
let input_derivation: StorePath = string_to_store_path(i, input_derivation)?;
let input_derivation = string_to_store_path(i, input_derivation.as_str())?;
input_derivations.insert(input_derivation, new_output_names);
}
@ -177,16 +179,16 @@ fn parse_input_derivations(i: &[u8]) -> NomResult<&[u8], BTreeMap<StorePath, BTr
Ok((i, input_derivations))
}
fn parse_input_sources(i: &[u8]) -> NomResult<&[u8], BTreeSet<StorePath>> {
fn parse_input_sources(i: &[u8]) -> NomResult<&[u8], BTreeSet<StorePath<String>>> {
let (i, input_sources_lst) = aterm::parse_string_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 = string_to_store_path(i, input_source)?;
let input_source = string_to_store_path(i, input_source.as_str())?;
if input_sources.contains(&input_source) {
return Err(nom::Err::Failure(NomError {
input: i,
code: ErrorKind::DuplicateInputSource(input_source),
code: ErrorKind::DuplicateInputSource(input_source.to_owned()),
}));
} else {
input_sources.insert(input_source);
@ -196,24 +198,27 @@ 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| {
fn string_to_store_path<'a, 'i, S>(
i: &'i [u8],
path_str: &'a str,
) -> Result<StorePath<S>, nom::Err<NomError<&'i [u8]>>>
where
S: std::cmp::Eq
+ std::fmt::Display
+ std::clone::Clone
+ std::ops::Deref<Target = str>
+ std::convert::From<&'a str>,
{
let path =
StorePath::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());
assert_eq!(path_str, path.to_absolute_path());
Ok(path)
}
@ -375,11 +380,11 @@ mod tests {
};
static ref EXP_AB_MAP: BTreeMap<String, BString> = {
let mut b = BTreeMap::new();
b.insert("a".to_string(), b"1".as_bstr().to_owned());
b.insert("b".to_string(), b"2".as_bstr().to_owned());
b.insert("a".to_string(), b"1".into());
b.insert("b".to_string(), b"2".into());
b
};
static ref EXP_INPUT_DERIVATIONS_SIMPLE: BTreeMap<StorePath, BTreeSet<String>> = {
static ref EXP_INPUT_DERIVATIONS_SIMPLE: BTreeMap<StorePath<String>, BTreeSet<String>> = {
let mut b = BTreeMap::new();
b.insert(
StorePath::from_bytes(b"8bjm87p310sb7r2r0sg4xrynlvg86j8k-hello-2.12.1.tar.gz.drv")
@ -452,7 +457,7 @@ mod tests {
#[case::simple(EXP_INPUT_DERIVATIONS_SIMPLE_ATERM.as_bytes(), &EXP_INPUT_DERIVATIONS_SIMPLE)]
fn parse_input_derivations(
#[case] input: &'static [u8],
#[case] expected: &BTreeMap<StorePath, BTreeSet<String>>,
#[case] expected: &BTreeMap<StorePath<String>, BTreeSet<String>>,
) {
let (rest, parsed) = super::parse_input_derivations(input).expect("must parse");

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

@ -6,7 +6,7 @@
use crate::aterm::escape_bytes;
use crate::derivation::{ca_kind_prefix, output::Output};
use crate::nixbase32;
use crate::store_path::{StorePath, StorePathRef, STORE_DIR_WITH_SLASH};
use crate::store_path::{StorePath, STORE_DIR_WITH_SLASH};
use bstr::BString;
use data_encoding::HEXLOWER;
@ -34,7 +34,10 @@ pub(crate) trait AtermWriteable {
fn aterm_write(&self, writer: &mut impl Write) -> std::io::Result<()>;
}
impl AtermWriteable for StorePathRef<'_> {
impl<S> AtermWriteable for StorePath<S>
where
S: std::cmp::Eq + std::ops::Deref<Target = str>,
{
fn aterm_write(&self, writer: &mut impl Write) -> std::io::Result<()> {
write_char(writer, QUOTE)?;
writer.write_all(STORE_DIR_WITH_SLASH.as_bytes())?;
@ -46,13 +49,6 @@ impl AtermWriteable for StorePathRef<'_> {
}
}
impl AtermWriteable for StorePath {
fn aterm_write(&self, writer: &mut impl Write) -> std::io::Result<()> {
let r: StorePathRef = self.into();
r.aterm_write(writer)
}
}
impl AtermWriteable for String {
fn aterm_write(&self, writer: &mut impl Write) -> std::io::Result<()> {
write_field(writer, self, true)
@ -179,7 +175,7 @@ pub(crate) fn write_input_derivations(
pub(crate) fn write_input_sources(
writer: &mut impl Write,
input_sources: &BTreeSet<StorePath>,
input_sources: &BTreeSet<StorePath<String>>,
) -> Result<(), io::Error> {
write_char(writer, BRACKET_OPEN)?;
write_array_elements(