fix(tvix/glue/import): builtins.storeDir fixes

This didn't support store paths with a subpath joined to them, while
Nix does.

Use state.path_exists, which does. This also means we can drop the
`store_path_exists` helper, which was only used here.

Change-Id: I918ccb270f64acbdc41cb4d2a9c3c5871ce15002
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12618
Tested-by: BuildkiteCI
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
Reviewed-by: Jörg Thalheim <joerg@thalheim.io>
Autosubmit: flokli <flokli@flokli.de>
This commit is contained in:
Florian Klink 2024-10-14 15:57:34 +03:00 committed by clbot
parent 330145fa1f
commit e6b39135bc
3 changed files with 23 additions and 39 deletions

View file

@ -69,8 +69,8 @@ pub enum ImportError {
#[error("hash mismatch at ingestion of '{0}', expected: '{1}', got: '{2}'")] #[error("hash mismatch at ingestion of '{0}', expected: '{1}', got: '{2}'")]
HashMismatch(String, NixHash, NixHash), HashMismatch(String, NixHash, NixHash),
#[error("path '{}' is not in the Nix store", .0.display())] #[error("path '{}' is not absolute or invalid", .0.display())]
PathNotInStore(PathBuf), PathNotAbsoluteOrInvalid(PathBuf),
} }
impl From<ImportError> for tvix_eval::ErrorKind { impl From<ImportError> for tvix_eval::ErrorKind {

View file

@ -1,6 +1,6 @@
//! Implements builtins used to import paths in the store. //! Implements builtins used to import paths in the store.
use crate::builtins::errors::ImportError; use crate::tvix_store_io::TvixStoreIO;
use std::path::Path; use std::path::Path;
use tvix_castore::import::ingest_entries; use tvix_castore::import::ingest_entries;
use tvix_castore::Node; use tvix_castore::Node;
@ -106,15 +106,15 @@ async fn filtered_ingest(
#[builtins(state = "Rc<TvixStoreIO>")] #[builtins(state = "Rc<TvixStoreIO>")]
mod import_builtins { mod import_builtins {
use std::os::unix::ffi::OsStrExt;
use std::rc::Rc;
use super::*; use super::*;
use crate::builtins::ImportError;
use crate::tvix_store_io::TvixStoreIO; use crate::tvix_store_io::TvixStoreIO;
use bstr::ByteSlice;
use nix_compat::nixhash::{CAHash, NixHash}; use nix_compat::nixhash::{CAHash, NixHash};
use nix_compat::store_path::StorePathRef; use nix_compat::store_path::StorePathRef;
use sha2::Digest; use sha2::Digest;
use std::rc::Rc;
use tokio::io::AsyncWriteExt; use tokio::io::AsyncWriteExt;
use tvix_eval::builtins::coerce_value_to_path; use tvix_eval::builtins::coerce_value_to_path;
use tvix_eval::generators::Gen; use tvix_eval::generators::Gen;
@ -367,39 +367,33 @@ mod import_builtins {
co: GenCo, co: GenCo,
path: Value, path: Value,
) -> Result<Value, ErrorKind> { ) -> Result<Value, ErrorKind> {
let p = std::str::from_utf8(match &path { let p = match &path {
Value::String(s) => s.as_bytes(), Value::String(s) => Path::new(s.as_bytes().to_os_str()?),
Value::Path(p) => p.as_os_str().as_bytes(), Value::Path(p) => p.as_path(),
_ => { _ => {
return Err(ErrorKind::TypeError { return Err(ErrorKind::TypeError {
expected: "string or path", expected: "string or path",
actual: path.type_of(), actual: path.type_of(),
}) })
} }
})?; };
let path_exists = // For this builtin, the path needs to start with an absolute store path.
if let Ok((store_path, sub_path)) = StorePathRef::from_absolute_path_full(p) { let (store_path, _sub_path) = StorePathRef::from_absolute_path_full(p)
if !sub_path.as_os_str().is_empty() { .map_err(|_e| ImportError::PathNotAbsoluteOrInvalid(p.to_path_buf()))?;
false
} else {
state.store_path_exists(store_path.as_ref()).await?
}
} else {
false
};
if !path_exists { if state.path_exists(p)? {
return Err(ImportError::PathNotInStore(p.into()).into()); Ok(Value::String(NixString::new_context_from(
[NixContextElement::Plain(store_path.to_absolute_path())].into(),
p.as_os_str().as_encoded_bytes(),
)))
} else {
Err(ErrorKind::IO {
path: Some(p.to_path_buf()),
error: Rc::new(std::io::ErrorKind::NotFound.into()),
})
} }
Ok(Value::String(NixString::new_context_from(
[NixContextElement::Plain(p.into())].into(),
p,
)))
} }
} }
pub use import_builtins::builtins as import_builtins; pub use import_builtins::builtins as import_builtins;
use crate::tvix_store_io::TvixStoreIO;

View file

@ -1,7 +1,6 @@
//! This module provides an implementation of EvalIO talking to tvix-store. //! This module provides an implementation of EvalIO talking to tvix-store.
use bytes::Bytes; use bytes::Bytes;
use futures::{StreamExt, TryStreamExt}; use futures::{StreamExt, TryStreamExt};
use nix_compat::store_path::StorePathRef;
use nix_compat::{nixhash::CAHash, store_path::StorePath}; use nix_compat::{nixhash::CAHash, store_path::StorePath};
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::{ use std::{
@ -435,15 +434,6 @@ impl TvixStoreIO {
let path_info = self.node_to_path_info(name, path, ca, root_node).await?; let path_info = self.node_to_path_info(name, path, ca, root_node).await?;
Ok(self.path_info_service.as_ref().put(path_info).await?) Ok(self.path_info_service.as_ref().put(path_info).await?)
} }
pub async fn store_path_exists<'a>(&'a self, store_path: StorePathRef<'a>) -> io::Result<bool> {
Ok(self
.path_info_service
.as_ref()
.get(*store_path.digest())
.await?
.is_some())
}
} }
impl EvalIO for TvixStoreIO { impl EvalIO for TvixStoreIO {