refactor(tvix/store): use bytes for node names and symlink targets

Some paths might use names that are not valid UTF-8. We should be able
to represent them.

We don't actually need to touch the PathInfo structures, as they need to
represent StorePaths, which come with their own harder restrictions,
which can't encode non-UTF8 data.

While this doesn't change any of the wire format of the gRPC messages,
it does however change the interface of tvix_eval::EvalIO - its
read_dir() method does now return a list of Vec<u8>, rather than
SmolStr. Maybe this should be OsString instead?

Change-Id: I821016d9a58ec441ee081b0b9f01c9240723af0b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8974
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
This commit is contained in:
Florian Klink 2023-07-18 19:37:25 +03:00 committed by clbot
parent 638f3e874d
commit 72e82ffcb1
27 changed files with 245 additions and 253 deletions

View file

@ -20,7 +20,7 @@ fn size() {
{
let d = Directory {
directories: vec![DirectoryNode {
name: String::from("foo"),
name: "foo".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 0,
}],
@ -31,7 +31,7 @@ fn size() {
{
let d = Directory {
directories: vec![DirectoryNode {
name: String::from("foo"),
name: "foo".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 4,
}],
@ -42,7 +42,7 @@ fn size() {
{
let d = Directory {
files: vec![FileNode {
name: String::from("foo"),
name: "foo".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
executable: false,
@ -54,8 +54,8 @@ fn size() {
{
let d = Directory {
symlinks: vec![SymlinkNode {
name: String::from("foo"),
target: String::from("bar"),
name: "foo".into(),
target: "bar".into(),
}],
..Default::default()
};
@ -89,7 +89,7 @@ fn validate_invalid_names() {
{
let d = Directory {
directories: vec![DirectoryNode {
name: "".to_string(),
name: "".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
}],
@ -97,7 +97,7 @@ fn validate_invalid_names() {
};
match d.validate().expect_err("must fail") {
ValidateDirectoryError::InvalidName(n) => {
assert_eq!(n, "")
assert_eq!(n, b"")
}
_ => panic!("unexpected error"),
};
@ -106,7 +106,7 @@ fn validate_invalid_names() {
{
let d = Directory {
directories: vec![DirectoryNode {
name: ".".to_string(),
name: ".".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
}],
@ -114,7 +114,7 @@ fn validate_invalid_names() {
};
match d.validate().expect_err("must fail") {
ValidateDirectoryError::InvalidName(n) => {
assert_eq!(n, ".")
assert_eq!(n, b".")
}
_ => panic!("unexpected error"),
};
@ -123,7 +123,7 @@ fn validate_invalid_names() {
{
let d = Directory {
files: vec![FileNode {
name: "..".to_string(),
name: "..".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
executable: false,
@ -132,7 +132,7 @@ fn validate_invalid_names() {
};
match d.validate().expect_err("must fail") {
ValidateDirectoryError::InvalidName(n) => {
assert_eq!(n, "..")
assert_eq!(n, b"..")
}
_ => panic!("unexpected error"),
};
@ -141,14 +141,14 @@ fn validate_invalid_names() {
{
let d = Directory {
symlinks: vec![SymlinkNode {
name: "\x00".to_string(),
target: "foo".to_string(),
name: "\x00".into(),
target: "foo".into(),
}],
..Default::default()
};
match d.validate().expect_err("must fail") {
ValidateDirectoryError::InvalidName(n) => {
assert_eq!(n, "\x00")
assert_eq!(n, b"\x00")
}
_ => panic!("unexpected error"),
};
@ -157,14 +157,14 @@ fn validate_invalid_names() {
{
let d = Directory {
symlinks: vec![SymlinkNode {
name: "foo/bar".to_string(),
target: "foo".to_string(),
name: "foo/bar".into(),
target: "foo".into(),
}],
..Default::default()
};
match d.validate().expect_err("must fail") {
ValidateDirectoryError::InvalidName(n) => {
assert_eq!(n, "foo/bar")
assert_eq!(n, b"foo/bar")
}
_ => panic!("unexpected error"),
};
@ -175,7 +175,7 @@ fn validate_invalid_names() {
fn validate_invalid_digest() {
let d = Directory {
directories: vec![DirectoryNode {
name: "foo".to_string(),
name: "foo".into(),
digest: vec![0x00, 0x42], // invalid length
size: 42,
}],
@ -196,12 +196,12 @@ fn validate_sorting() {
let d = Directory {
directories: vec![
DirectoryNode {
name: "b".to_string(),
name: "b".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
},
DirectoryNode {
name: "a".to_string(),
name: "a".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
},
@ -210,7 +210,7 @@ fn validate_sorting() {
};
match d.validate().expect_err("must fail") {
ValidateDirectoryError::WrongSorting(s) => {
assert_eq!(s, "a".to_string());
assert_eq!(s, b"a");
}
_ => panic!("unexpected error"),
}
@ -221,12 +221,12 @@ fn validate_sorting() {
let d = Directory {
directories: vec![
DirectoryNode {
name: "a".to_string(),
name: "a".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
},
DirectoryNode {
name: "a".to_string(),
name: "a".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
},
@ -235,7 +235,7 @@ fn validate_sorting() {
};
match d.validate().expect_err("must fail") {
ValidateDirectoryError::DuplicateName(s) => {
assert_eq!(s, "a".to_string());
assert_eq!(s, b"a");
}
_ => panic!("unexpected error"),
}
@ -246,12 +246,12 @@ fn validate_sorting() {
let d = Directory {
directories: vec![
DirectoryNode {
name: "a".to_string(),
name: "a".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
},
DirectoryNode {
name: "b".to_string(),
name: "b".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
},
@ -267,19 +267,19 @@ fn validate_sorting() {
let d = Directory {
directories: vec![
DirectoryNode {
name: "b".to_string(),
name: "b".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
},
DirectoryNode {
name: "c".to_string(),
name: "c".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 42,
},
],
symlinks: vec![SymlinkNode {
name: "a".to_string(),
target: "foo".to_string(),
name: "a".into(),
target: "foo".into(),
}],
..Default::default()
};

View file

@ -1,7 +1,7 @@
use crate::proto::node::Node;
use crate::proto::Directory;
use crate::proto::DirectoryNode;
use crate::proto::FileNode;
use crate::proto::NamedNode;
use crate::proto::SymlinkNode;
#[test]
@ -9,68 +9,66 @@ fn iterator() {
let d = Directory {
directories: vec![
DirectoryNode {
name: "c".to_string(),
name: "c".into(),
..DirectoryNode::default()
},
DirectoryNode {
name: "d".to_string(),
name: "d".into(),
..DirectoryNode::default()
},
DirectoryNode {
name: "h".to_string(),
name: "h".into(),
..DirectoryNode::default()
},
DirectoryNode {
name: "l".to_string(),
name: "l".into(),
..DirectoryNode::default()
},
],
files: vec![
FileNode {
name: "b".to_string(),
name: "b".into(),
..FileNode::default()
},
FileNode {
name: "e".to_string(),
name: "e".into(),
..FileNode::default()
},
FileNode {
name: "g".to_string(),
name: "g".into(),
..FileNode::default()
},
FileNode {
name: "j".to_string(),
name: "j".into(),
..FileNode::default()
},
],
symlinks: vec![
SymlinkNode {
name: "a".to_string(),
name: "a".into(),
..SymlinkNode::default()
},
SymlinkNode {
name: "f".to_string(),
name: "f".into(),
..SymlinkNode::default()
},
SymlinkNode {
name: "i".to_string(),
name: "i".into(),
..SymlinkNode::default()
},
SymlinkNode {
name: "k".to_string(),
name: "k".into(),
..SymlinkNode::default()
},
],
};
// We keep this strings here and convert to string to make the comparison
// less messy.
let mut node_names: Vec<String> = vec![];
for node in d.nodes() {
match node {
Node::Directory(n) => node_names.push(n.name),
Node::File(n) => node_names.push(n.name),
Node::Symlink(n) => node_names.push(n.name),
};
node_names.push(String::from_utf8(node.get_name().to_vec()).unwrap());
}
assert_eq!(

View file

@ -190,8 +190,8 @@ async fn put_reject_failed_validation() {
// construct a broken Directory message that fails validation
let broken_directory = Directory {
symlinks: vec![SymlinkNode {
name: "".to_string(),
target: "doesntmatter".to_string(),
name: "".into(),
target: "doesntmatter".into(),
}],
..Default::default()
};
@ -214,7 +214,7 @@ async fn put_reject_wrong_size() {
// Construct a directory referring to DIRECTORY_A, but with wrong size.
let broken_parent_directory = Directory {
directories: vec![DirectoryNode {
name: "foo".to_string(),
name: "foo".into(),
digest: DIRECTORY_A.digest().to_vec(),
size: 42,
}],

View file

@ -48,8 +48,8 @@ async fn put_get() {
let path_info = PathInfo {
node: Some(Node {
node: Some(Symlink(SymlinkNode {
name: "00000000000000000000000000000000-foo".to_string(),
target: "doesntmatter".to_string(),
name: "00000000000000000000000000000000-foo".into(),
target: "doesntmatter".into(),
})),
}),
..Default::default()

View file

@ -43,7 +43,7 @@ fn validate_no_node(
#[test_case(
proto::DirectoryNode {
name: DUMMY_NAME.to_string(),
name: DUMMY_NAME.into(),
digest: DUMMY_DIGEST.to_vec(),
size: 0,
},
@ -52,7 +52,7 @@ fn validate_no_node(
)]
#[test_case(
proto::DirectoryNode {
name: DUMMY_NAME.to_string(),
name: DUMMY_NAME.into(),
digest: vec![],
size: 0,
},
@ -61,12 +61,12 @@ fn validate_no_node(
)]
#[test_case(
proto::DirectoryNode {
name: "invalid".to_string(),
name: "invalid".into(),
digest: DUMMY_DIGEST.to_vec(),
size: 0,
},
Err(ValidatePathInfoError::InvalidNodeName(
"invalid".to_string(),
"invalid".into(),
store_path::Error::InvalidLength()
));
"invalid node name"
@ -87,7 +87,7 @@ fn validate_directory(
#[test_case(
proto::FileNode {
name: DUMMY_NAME.to_string(),
name: DUMMY_NAME.into(),
digest: DUMMY_DIGEST.to_vec(),
size: 0,
executable: false,
@ -97,7 +97,7 @@ fn validate_directory(
)]
#[test_case(
proto::FileNode {
name: DUMMY_NAME.to_string(),
name: DUMMY_NAME.into(),
digest: vec![],
..Default::default()
},
@ -106,12 +106,12 @@ fn validate_directory(
)]
#[test_case(
proto::FileNode {
name: "invalid".to_string(),
name: "invalid".into(),
digest: DUMMY_DIGEST.to_vec(),
..Default::default()
},
Err(ValidatePathInfoError::InvalidNodeName(
"invalid".to_string(),
"invalid".into(),
store_path::Error::InvalidLength()
));
"invalid node name"
@ -129,7 +129,7 @@ fn validate_file(t_file_node: proto::FileNode, t_result: Result<StorePath, Valid
#[test_case(
proto::SymlinkNode {
name: DUMMY_NAME.to_string(),
name: DUMMY_NAME.into(),
..Default::default()
},
Ok(StorePath::from_str(DUMMY_NAME).expect("must succeed"));
@ -137,11 +137,11 @@ fn validate_file(t_file_node: proto::FileNode, t_result: Result<StorePath, Valid
)]
#[test_case(
proto::SymlinkNode {
name: "invalid".to_string(),
name: "invalid".into(),
..Default::default()
},
Err(ValidatePathInfoError::InvalidNodeName(
"invalid".to_string(),
"invalid".into(),
store_path::Error::InvalidLength()
));
"invalid node name"
@ -166,7 +166,7 @@ fn validate_references() {
let path_info = PathInfo {
node: Some(Node {
node: Some(proto::node::Node::Directory(proto::DirectoryNode {
name: DUMMY_NAME.to_string(),
name: DUMMY_NAME.into(),
digest: DUMMY_DIGEST.to_vec(),
size: 0,
})),