switch from std path to camino

CHANGELOG-foldiff.md

@@ -6,6 +6,10 @@
 - replace `anyhow` with custom error types
 - write custom threading utilities
 
+## pending
+- use `camino` for better path handling internally
+- remove unused dependency on serde-bytes
+
 ## 1.3.1
 - reflinks now apply for duplicated files too
 

Cargo.toml

@@ -3,4 +3,7 @@ resolver = "2"
 members = [
 	"foldiff",
 	"libfoldiff",
-]
+]
+
+#[profile.test]
+#opt-level = 3

foldiff/Cargo.toml

@@ -14,3 +14,4 @@ dialoguer = { version = "0.11.0", features = [] }
 indicatif = "0.17.8"
 console = "0.15.8"
 num_cpus = "1.16.0"
+camino = "1.1.9"

foldiff/src/main.rs

@@ -1,7 +1,7 @@
 use std::fs::File;
 use anyhow::{bail, ensure, Context, Result};
+use camino::{Utf8Path, Utf8PathBuf};
 use clap::{Parser, Subcommand};
-use std::path::{Path, PathBuf};
 use libfoldiff::FoldiffCfg;
 use libfoldiff::manifest::DiffManifest;
 
@@ -105,8 +105,8 @@ fn main() -> Result<()> {
 				level_diff: *level_diff
 			};
 
-			let old_root: PathBuf = old.into();
-			let new_root: PathBuf = new.into();
+			let old_root: Utf8PathBuf = old.into();
+			let new_root: Utf8PathBuf = new.into();
 			// check both exist
 			ensure!(std::fs::metadata(&old_root).context("old path must exist")?.is_dir(), "old path must be a directory");
 			ensure!(std::fs::metadata(&new_root).context("new path must exist")?.is_dir(), "new path must be a directory");
@@ -136,12 +136,12 @@ fn main() -> Result<()> {
 			//println!("{diff_state:?}");
 
 			// emit the diff to disk
-			diff_state.write_to_file::<cliutils::Bar, cliutils::Spinner<false>>(Path::new(diff), &cfg)?;
+			diff_state.write_to_file::<cliutils::Bar, cliutils::Spinner<false>>(Utf8Path::new(diff), &cfg)?;
 
 		}
 		Commands::Apply { old, diff, new } => {
-			let old_root: PathBuf = old.into();
-			let new_root: PathBuf = new.into();
+			let old_root: Utf8PathBuf = old.into();
+			let new_root: Utf8PathBuf = new.into();
 			// check existence
 			ensure!(std::fs::metadata(&old_root).context("old path must exist")?.is_dir(), "old path must be a directory");
 			ensure!(std::fs::metadata(diff).context("diff must exist")?.is_file(), "diff must be a file");
@@ -158,7 +158,7 @@ fn main() -> Result<()> {
 				std::fs::remove_dir_all(new).context("Failed to remove folder")?;
 			}
 
-			let mut diff_state = libfoldiff::applying::read_diff_from_file(&PathBuf::from(diff))?;
+			let mut diff_state = libfoldiff::applying::read_diff_from_file(&Utf8PathBuf::from(diff))?;
 			diff_state.apply::<
 				cliutils::MultiWrapper,
 				cliutils::Spinner<false>,
@@ -169,10 +169,10 @@ fn main() -> Result<()> {
 			if let Some(diff) = diff {
 				let f = File::open(diff).context("Failed to open diff file to verify with")?;
 				let manifest = DiffManifest::read_from(f).context("Failed to read diff file to verify with")?;
-				libfoldiff::verify::verify_against_diff::<cliutils::Spinner<true>>(Path::new(old), Path::new(new), &manifest)?;
+				libfoldiff::verify::verify_against_diff::<cliutils::Spinner<true>>(old.as_str().into(), new.as_str().into(), &manifest)?;
 			}
 			else {
-				libfoldiff::verify::test_dir_equality::<cliutils::Spinner<true>>(Path::new(old), Path::new(new))?;
+				libfoldiff::verify::test_dir_equality::<cliutils::Spinner<true>>(old.as_str().into(), new.as_str().into())?;
 			}
 		},
 		Commands::Upgrade { new, old } => {

libfoldiff/Cargo.toml

@@ -13,12 +13,12 @@ derivative = "2.2.0"
 infer = "0.16.0"
 rmp-serde = "1.3.0"
 serde = { version = "1.0.209", features = ["derive"] }
-serde_bytes = "0.11.15"
 twox-hash = "1.6.3"
 zstd = { version = "0.13.2", features = ["zstdmt"] }
 rayon = "1.10.0"
 memmap2 = "0.9.4"
 reflink = "0.1.3"
+camino = "1.1.9"
 
 [dev-dependencies]
 tempfile = "3.12.0"

libfoldiff/src/applying.rs

@@ -7,7 +7,7 @@ use memmap2::Mmap;
 use rayon::prelude::*;
 use std::fs::File;
 use std::io::{Cursor, Read, Seek};
-use std::path::{Path, PathBuf};
+use camino::{Utf8Path, Utf8PathBuf};
 use std::sync::Mutex;
 
 /// An in-memory representation of a diff, used for the applying process
@@ -17,16 +17,16 @@ pub struct ApplyingDiff {
 	blobs_new: Vec<u64>,   // offset into diff file
 	blobs_patch: Vec<u64>, // offset into diff file
 	read: Option<Mmap>, // the diff file map
-	old_root: PathBuf,
-	new_root: PathBuf,
+	old_root: Utf8PathBuf,
+	new_root: Utf8PathBuf,
 }
 
 impl ApplyingDiff {
 	pub fn apply<
 		TWrap: ReportingMultiWrapper,
 		TSpin: Reporter + CanBeWrappedBy<TWrap> + Sync,
 		TBar: ReporterSized + CanBeWrappedBy<TWrap> + Sync
-	>(&mut self, old_root: PathBuf, new_root: PathBuf) -> anyhow::Result<()> {
+	>(&mut self, old_root: Utf8PathBuf, new_root: Utf8PathBuf) -> anyhow::Result<()> {
 		self.old_root = old_root;
 		self.new_root = new_root;
 
@@ -297,7 +297,7 @@ impl ApplyingDiff {
 }
 
 /// handles initialising an in-memory applying state from disk
-pub fn read_diff_from_file(path: &Path) -> anyhow::Result<ApplyingDiff> {
+pub fn read_diff_from_file(path: &Utf8Path) -> anyhow::Result<ApplyingDiff> {
 	let f = File::open(path).context("Failed to open file to read diff")?;
 
 	// safety: UB if the underlying diff is modified by someone else
@@ -317,7 +317,7 @@ pub fn read_diff_from(reader: &mut (impl Read + Seek)) -> anyhow::Result<Applyin
 	let mut new_self = ApplyingDiff::default();
 	new_self.manifest = manifest;
 
-	let mut new_blob_count = [0u8, 0, 0, 0, 0, 0, 0, 0];
+	let mut new_blob_count = [0u8; 8];
 	reader
 		.read_exact(&mut new_blob_count)
 		.context("Failed to read new file count")?;
@@ -328,7 +328,7 @@ pub fn read_diff_from(reader: &mut (impl Read + Seek)) -> anyhow::Result<Applyin
 		new_self.blobs_new.push(reader.stream_position()?);
 
 		// read blob length
-		let mut len = [0u8, 0, 0, 0, 0, 0, 0, 0];
+		let mut len = [0u8; 8];
 		reader
 			.read_exact(&mut len)
 			.context("Failed to read new file length")?;
@@ -340,7 +340,7 @@ pub fn read_diff_from(reader: &mut (impl Read + Seek)) -> anyhow::Result<Applyin
 			.context("Failed to seek while skipping new file")?;
 	}
 
-	let mut patched_blob_count = [0u8, 0, 0, 0, 0, 0, 0, 0];
+	let mut patched_blob_count = [0u8; 8];
 	reader
 		.read_exact(&mut patched_blob_count)
 		.context("Failed to read patched file count")?;

libfoldiff/src/common.rs

@@ -1,6 +1,6 @@
 use std::fs::File;
-use std::path::Path;
 use anyhow::Context;
+use camino::Utf8Path;
 use crate::hash;
 
 pub const MAGIC_BYTES: [u8; 4] = *b"FLDF";
@@ -17,15 +17,15 @@ pub struct FoldiffCfg {
 }
 
 /// creates a file and all necessary parent directories
-pub fn create_file(p: &Path) -> std::io::Result<File> {
+pub fn create_file(p: &Utf8Path) -> std::io::Result<File> {
 	if let Some(p) = p.parent() {
 		std::fs::create_dir_all(p)?;
 	}
 	File::create(p)
 }
 
 // Reflinks or copies a file and hashes it
-pub fn copy_rl_hash(src_p: impl AsRef<Path>, dst_p: impl AsRef<Path>) -> anyhow::Result<u64> {
+pub fn copy_rl_hash(src_p: impl AsRef<Utf8Path>, dst_p: impl AsRef<Utf8Path>) -> anyhow::Result<u64> {
 	let src_p = src_p.as_ref();
 	let dst_p = dst_p.as_ref();
 
@@ -47,7 +47,7 @@ pub fn copy_rl_hash(src_p: impl AsRef<Path>, dst_p: impl AsRef<Path>) -> anyhow:
 	}
 }
 
-pub fn copy_rl(src_p: impl AsRef<Path>, dst_p: impl AsRef<Path>) -> std::io::Result<()> {
+pub fn copy_rl(src_p: impl AsRef<Utf8Path>, dst_p: impl AsRef<Utf8Path>) -> std::io::Result<()> {
 	let src_p = src_p.as_ref();
 	let dst_p = dst_p.as_ref();
 

libfoldiff/src/diffing.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 use std::fs::File;
 use std::io::{copy, Seek, Write};
-use std::path::{Path, PathBuf};
+use camino::{Utf8Path, Utf8PathBuf};
 use anyhow::{anyhow, bail, Context};
 use rmp_serde::Serializer;
 use serde::Serialize;
@@ -14,27 +14,27 @@ use crate::reporting::{AutoSpin, Reporter, ReporterSized};
 /// An in-memory representation of a diff, used for the diff creation process
 #[derive(Clone, Debug, Default)]
 pub struct DiffingDiff {
-	blobs_new: Vec<PathBuf>,
-	blobs_patch: Vec<PathBuf>,
-	old_root: PathBuf,
-	new_root: PathBuf,
+	blobs_new: Vec<Utf8PathBuf>,
+	blobs_patch: Vec<Utf8PathBuf>,
+	old_root: Utf8PathBuf,
+	new_root: Utf8PathBuf,
 	files: BTreeMap<u64, DiffingFileData>,
 	// for efficient lookups, must be kept in sync
-	file_paths_old: BTreeMap<PathBuf, u64>,
-	file_paths_new: BTreeMap<PathBuf, u64>,
+	file_paths_old: BTreeMap<Utf8PathBuf, u64>,
+	file_paths_new: BTreeMap<Utf8PathBuf, u64>,
 }
 
 /// the looked up value of DiffingDiff::files entries
 #[derive(Clone, Debug)]
 struct DiffingFileData {
-	paths_old: Vec<PathBuf>,
-	paths_new: Vec<PathBuf>,
+	paths_old: Vec<Utf8PathBuf>,
+	paths_new: Vec<Utf8PathBuf>,
 	inferred_mime: Option<&'static str>,
 }
 
 
 impl DiffingDiff {
-	pub fn new(old_root: PathBuf, new_root: PathBuf) -> Self {
+	pub fn new(old_root: Utf8PathBuf, new_root: Utf8PathBuf) -> Self {
 		Self {
 			old_root,
 			new_root,
@@ -123,7 +123,7 @@ impl DiffingDiff {
 		Ok(())
 	}
 
-	pub fn write_to_file<TBar: ReporterSized, TSpin: Reporter+Sync>(&mut self, path: &Path, cfg: &FoldiffCfg) -> anyhow::Result<()> {
+	pub fn write_to_file<TBar: ReporterSized, TSpin: Reporter+Sync>(&mut self, path: &Utf8Path, cfg: &FoldiffCfg) -> anyhow::Result<()> {
 		// create file
 		let mut f = File::create_new(path).context("Failed to create file to save diff")?;
 
@@ -139,14 +139,13 @@ impl DiffingDiff {
 		// and figure out what blobs must be generated by write_to, and generate the manifest.
 
 		// convenience func
-		let path_to_string = |p: &PathBuf| -> anyhow::Result<String> {
-			let str = p.to_str().ok_or(anyhow!("Found a non-UTF-8 path name. Just no. Why."))?;
+		let path_to_string = |p: &Utf8PathBuf| -> anyhow::Result<String> {
 			Ok(if cfg!(windows) {
 				// path replacement
 				assert!(p.is_relative(), "Cannot fix separators in a non-relative path, as this is not accepted by the windows apis for verbatim paths. This should never occur as the diff manifest only contains relative paths.");
-				str.replace('\\', "/")
+				p.as_str().replace('\\', "/")
 			} else {
-				str.to_string()
+				p.to_string()
 			})
 		};
 
@@ -252,7 +251,7 @@ impl DiffingDiff {
 
 	/// adds a new file to the diff
 	/// you should not pass a file that is already in the diff - this will return an Err
-	fn add_file(&mut self, in_new: bool, path: &Path) -> anyhow::Result<()> {
+	fn add_file(&mut self, in_new: bool, path: &Utf8Path) -> anyhow::Result<()> {
 		// check if the path is already there
 		let paths = if in_new { &mut self.file_paths_new } else { &mut self.file_paths_old };
 		if paths.contains_key(path) {
@@ -290,7 +289,7 @@ impl DiffingDiff {
 		Ok(())
 	}
 
-	fn scan_internal(&mut self, dir: &Path, new: bool, spn: &impl Reporter) -> anyhow::Result<()> {
+	fn scan_internal(&mut self, dir: &Utf8Path, new: bool, spn: &impl Reporter) -> anyhow::Result<()> {
 		let root = if new { &self.new_root } else { &self.old_root };
 		// we need to clone this, aw
 		let root = root.clone();
@@ -309,7 +308,11 @@ impl DiffingDiff {
 				bail!("Entry at '{:?}' is a symlink, bailing", entry.path());
 			}
 			// strip the root off the front of the path else we get errors
-			let path = entry.path();
+			let path: Utf8PathBuf = match entry.path().try_into()
+			{
+				Ok(p) => p,
+				Err(_) => continue, // just ignore non-UTF-8 paths!
+			};
 			let path = path.strip_prefix(&root)?;
 			if ftype.is_dir() {
 				// recurse
@@ -325,17 +328,17 @@ impl DiffingDiff {
 	}
 }
 
-pub fn scan_to_diff<TSpin: Reporter+Sync>(old_root: PathBuf, new_root: PathBuf) -> anyhow::Result<DiffingDiff> {
+pub fn scan_to_diff<TSpin: Reporter+Sync>(old_root: Utf8PathBuf, new_root: Utf8PathBuf) -> anyhow::Result<DiffingDiff> {
 	let mut new_self = DiffingDiff::new(old_root, new_root);
 
 	let spn = TSpin::new("Scanning old files");
 	let aspn = AutoSpin::spin(&spn);
-	new_self.scan_internal(Path::new(""), false, &spn)?;
+	new_self.scan_internal(Utf8Path::new(""), false, &spn)?;
 	aspn.all_good();
 
 	let spn = TSpin::new("Scanning new files");
 	let aspn = AutoSpin::spin(&spn);
-	new_self.scan_internal(Path::new(""), true, &spn)?;
+	new_self.scan_internal(Utf8Path::new(""), true, &spn)?;
 	aspn.all_good();
 
 	Ok(new_self)

libfoldiff/src/hash.rs

@@ -1,7 +1,7 @@
 use std::fs::File;
 use std::hash::Hasher;
 use std::io::{Read, Write};
-use std::path::Path;
+use camino::Utf8Path;
 use twox_hash::XxHash64;
 
 #[derive(Clone, Default)]
@@ -36,7 +36,7 @@ pub fn hash_stream(s: &mut impl Read) -> std::io::Result<u64> {
 	Ok(h.finish())
 }
 
-pub fn hash_file(p: &Path) -> anyhow::Result<u64> {
+pub fn hash_file(p: &Utf8Path) -> anyhow::Result<u64> {
 	Ok(hash_stream(&mut File::open(p)?)?)
 }