diff --git a/src/cmd/argv.rs b/src/cmd/argv.rs index 06c1573..02fe891 100644 --- a/src/cmd/argv.rs +++ b/src/cmd/argv.rs @@ -1,4 +1,5 @@ use std::{ + ffi::{OsStr, OsString}, fmt::Display, fs::File, os::unix::process::CommandExt, @@ -13,7 +14,7 @@ use crate::output::write_err; // a program and its arguments: the only thing in ahab that knows what argv looks like #[derive(Default, Clone)] -pub struct Argv(Vec); +pub struct Argv(Vec); impl Display for Argv { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -22,42 +23,57 @@ impl Display for Argv { } impl Argv { - pub fn new(program: &str) -> Self { - Self(vec![program.to_string()]) + pub fn new(program: impl AsRef) -> Self { + Self(vec![program.as_ref().to_owned()]) } - pub fn arg(mut self, arg: impl AsRef) -> Self { - self.0.push(arg.as_ref().to_string()); + pub fn arg(mut self, arg: impl AsRef) -> Self { + self.0.push(arg.as_ref().to_owned()); self } pub fn args(mut self, args: I) -> Self where I: IntoIterator, - S: AsRef, + S: AsRef, { self.0 - .extend(args.into_iter().map(|arg| arg.as_ref().to_string())); + .extend(args.into_iter().map(|arg| arg.as_ref().to_owned())); self } // a long option and the value it takes - pub fn flag(self, name: &str, value: impl AsRef) -> Self { + pub fn flag(self, name: &str, value: impl AsRef) -> Self { self.arg(name).arg(value) } - pub fn program(&self) -> &str { - self.0.first().map(String::as_str).unwrap_or_default() + // for a message that names the tool being run, so lossy is what is wanted + pub fn program(&self) -> String { + self.0 + .first() + .map(|word| word.to_string_lossy().into_owned()) + .unwrap_or_default() } - pub fn words(&self) -> &[String] { + pub fn words(&self) -> &[OsString] { &self.0 } - // one word list for `sh -c`, quoted so a value with a space survives the shell + // one word list for `sh -c`, quoted so a value with a space survives the + // shell. lossy: this is for reading, and for scripts built only out of the + // ascii and compose-derived words that reach a pipeline pub fn quoted(&self) -> String { - // a nul byte is the only thing shlex refuses, and no argv can hold one - shlex::try_join(self.0.iter().map(String::as_str)).unwrap_or_default() + let words: Vec = self + .0 + .iter() + .map(|word| word.to_string_lossy().into_owned()) + .collect(); + + // a nul byte is the only thing shlex refuses, and no argv can hold one, + // so this only happens for a word that could never have been run. it + // still has to be readable: an empty string would name no command at + // all in the very message explaining what went wrong + shlex::try_join(words.iter().map(String::as_str)).unwrap_or_else(|_| words.join(" ")) } pub fn run(&self, ctx: &Ctx) -> Result<()> { @@ -72,6 +88,14 @@ impl Argv { // reading changes nothing, so a dry run answers the question for real pub fn capture(&self, ctx: &Ctx) -> Result { + let stdout = self.capture_bytes(ctx)?; + + String::from_utf8(stdout).with_context(|| format!("reading the output of `{self}`")) + } + + // git's -z listings are raw bytes: a path on unix is bytes, not text, and + // one that is not utf-8 would otherwise fail the whole listing it appears in + pub fn capture_bytes(&self, ctx: &Ctx) -> Result> { let out = self.command(ctx)?.output()?; if !out.status.success() { @@ -85,7 +109,7 @@ impl Argv { }); } - Ok(String::from_utf8(out.stdout)?) + Ok(out.stdout) } // replaces this process, so the command's exit code and signals become ours diff --git a/src/cmd/docker.rs b/src/cmd/docker.rs index b57d9b1..5962de9 100644 --- a/src/cmd/docker.rs +++ b/src/cmd/docker.rs @@ -1,3 +1,6 @@ +use std::ffi::OsString; +use std::path::Path; + use super::{Argv, Cmd}; // docker exec, around a command that runs inside the container @@ -47,24 +50,25 @@ impl Cmd for Exec { } } -// docker cp, in either direction +// docker cp, in either direction. the local side stays an OsString: it is a +// path the user named, and a lossy one would copy something else pub struct Cp { - from: String, - to: String, + from: OsString, + to: OsString, } impl Cp { - pub fn into_container(local: &str, container: &str, remote: &str) -> Self { + pub fn into_container(local: &Path, container: &str, remote: &str) -> Self { Self { - from: local.to_string(), - to: format!("{container}:{remote}"), + from: local.as_os_str().to_owned(), + to: OsString::from(format!("{container}:{remote}")), } } - pub fn out_of_container(container: &str, remote: &str, local: &str) -> Self { + pub fn out_of_container(container: &str, remote: &str, local: &Path) -> Self { Self { - from: format!("{container}:{remote}"), - to: local.to_string(), + from: OsString::from(format!("{container}:{remote}")), + to: local.as_os_str().to_owned(), } } } diff --git a/src/cmd/git.rs b/src/cmd/git.rs index cf7d211..a3f0d07 100644 --- a/src/cmd/git.rs +++ b/src/cmd/git.rs @@ -1,10 +1,22 @@ +use std::ffi::OsString; use std::path::Path; use super::{Argv, Cmd}; // -C has no long form; it runs git as though from that directory fn git(root: &Path) -> Argv { - Argv::new("git").arg("-C").arg(root.to_string_lossy()) + Argv::new("git").arg("-C").arg(root) +} + +// `--` stops git reading a leading dash as an option, but pathspec magic is +// read after it too, so a file actually named `:(exclude)x` or `:!x` would ask +// about a different set of paths than the one in hand. built as an OsString: a +// path is bytes, and a lossy one asks git about a name nothing on disk has +fn literal(path: &Path) -> OsString { + let mut spec = OsString::from(":(literal)"); + spec.push(path); + + spec } // the root of the repository the working directory is in @@ -31,7 +43,7 @@ pub struct LsFiles<'a> { root: &'a Path, tracked: bool, ignored: bool, - pathspecs: Vec, + pathspecs: Vec, } impl<'a> LsFiles<'a> { @@ -64,7 +76,7 @@ impl<'a> LsFiles<'a> { pub fn limited_to>(mut self, pathspecs: &[P]) -> Self { self.pathspecs = pathspecs .iter() - .map(|path| path.as_ref().to_string_lossy().to_string()) + .map(|path| literal(path.as_ref())) .collect(); self } @@ -72,15 +84,14 @@ impl<'a> LsFiles<'a> { impl Cmd for LsFiles<'_> { fn argv(&self) -> Argv { - let mut argv = git(self.root).arg("ls-files"); + // -z has no long form; it separates the paths with NUL, which is the + // only separator a filename cannot contain + let mut argv = git(self.root).arg("ls-files").arg("-z"); if self.tracked { argv = argv.arg("--cached"); } else { - // -z has no long form; it separates the paths with NUL, which is the - // only separator a filename cannot contain argv = argv - .arg("-z") .arg("--others") .arg("--exclude-standard") .arg("--directory") @@ -113,7 +124,7 @@ impl Cmd for CheckIgnore<'_> { .arg("check-ignore") .arg("--quiet") .arg("--") - .arg(self.path.to_string_lossy()) + .arg(self.path) } } @@ -134,7 +145,7 @@ mod tests { assert_eq!( argv.quoted(), "git -C /repo ls-files -z --others --exclude-standard --directory \ - --no-empty-directory --ignored -- 'a b'" + --no-empty-directory --ignored -- ':(literal)a b'", ); } @@ -144,7 +155,25 @@ mod tests { .limited_to(&[PathBuf::from(".env")]) .argv(); - assert_eq!(argv.quoted(), "git -C /repo ls-files --cached -- .env"); + assert_eq!( + argv.quoted(), + "git -C /repo ls-files -z --cached -- ':(literal).env'" + ); + } + + #[test] + fn a_pathspec_is_asked_for_literally_however_it_is_spelled() { + // git reads magic after `--` too, so these names must not become one + for name in [":(exclude)secret", ":!secret", ":(glob)**"] { + let argv = LsFiles::tracked(Path::new("/repo")) + .limited_to(&[PathBuf::from(name)]) + .argv(); + + assert_eq!( + argv.words().last().unwrap(), + format!(":(literal){name}").as_str() + ); + } } #[test] diff --git a/src/cmd/mod.rs b/src/cmd/mod.rs index 1c13afa..638a012 100644 --- a/src/cmd/mod.rs +++ b/src/cmd/mod.rs @@ -55,6 +55,10 @@ pub trait Cmd { self.argv().capture(ctx) } + fn capture_bytes(&self, ctx: &Ctx) -> Result> { + self.argv().capture_bytes(ctx) + } + fn replace(&self, ctx: &Ctx) -> Result<()> { self.argv().replace(ctx) } diff --git a/src/commands/link/check.rs b/src/commands/link/check.rs index d3c3791..03bf210 100644 --- a/src/commands/link/check.rs +++ b/src/commands/link/check.rs @@ -1,4 +1,6 @@ use fs_err::{read_dir, read_link}; +use std::ffi::OsString; +use std::os::unix::ffi::{OsStrExt, OsStringExt}; use std::path::{Path, PathBuf}; use anyhow::{Result, anyhow}; @@ -6,7 +8,7 @@ use anyhow::{Result, anyhow}; use super::store::{Repo, resolve, symlink_metadata_opt}; use crate::cmd::{Cmd, LsFiles}; use crate::ctx::Ctx; -use crate::output::{line, text}; +use crate::output::{line, text, write_bytes}; // whether anything is outside the store, which main turns into an exit code pub fn check( @@ -23,13 +25,10 @@ pub fn check( // git lists untracked and ignored separately for (mark, ignored) in [(UNTRACKED, false), (IGNORED, true)] { for entry in list_others(ctx, &repo, ignored, &pathspecs)? { - let rel = PathBuf::from(entry.trim_end_matches('/')); - // --directory collapses a wholly untracked dir into `dir/` - if entry.ends_with('/') { - exposed.extend(walk(&repo, &rel, mark)?.1); - } else { - exposed.extend(classify(&repo, &rel, mark)?); + match entry.strip_suffix(b"/") { + Some(dir) => exposed.extend(walk(&repo, &path_from(dir), mark)?.1), + None => exposed.extend(classify(&repo, &path_from(&entry), mark)?), } } } @@ -46,19 +45,36 @@ pub fn check( } fn print_porcelain(exposed: &[Exposed], null: bool) { - let end = if null { '\0' } else { '\n' }; - // a filename can hold an arrow but not a NUL, as `git status -z` also assumes - let between = if null { "\0" } else { " -> " }; + // -z is the format for scripts that must survive any filename, so its + // records are written as the bytes a path actually is. a filename can hold + // an arrow but not a NUL, as `git status -z` also assumes + if null { + for item in exposed { + let mut record = item.code().into_bytes(); + record.push(b' '); + record.extend_from_slice(item.name.as_os_str().as_bytes()); + + if let Some(dest) = &item.dest { + record.push(0); + record.extend_from_slice(dest.as_os_str().as_bytes()); + } + + record.push(0); + write_bytes(&record); + } + + return; + } for item in exposed { match &item.dest { Some(dest) => text!( - "{} {}{between}{}{end}", + "{} {} -> {}\n", item.code(), - item.name, + item.name.display(), dest.display() ), - None => text!("{} {}{end}", item.code(), item.name), + None => text!("{} {}\n", item.code(), item.name.display()), } } } @@ -98,8 +114,8 @@ fn print_listing(repo: &Repo, exposed: &[Exposed]) { line!("\n{heading}\n{hint}"); for item in items { match &item.dest { - Some(dest) => line!("\t{} -> {}", item.name, dest.display()), - None => line!("\t{}", item.name), + Some(dest) => line!("\t{} -> {}", item.name.display(), dest.display()), + None => line!("\t{}", item.name.display()), } } } @@ -126,12 +142,12 @@ const ELSEWHERE: char = '>'; struct Exposed { mark: char, - name: String, + name: PathBuf, dest: Option, } impl Exposed { - fn content(mark: char, name: String) -> Self { + fn content(mark: char, name: PathBuf) -> Self { Self { mark, name, @@ -151,7 +167,7 @@ impl Exposed { fn classify(repo: &Repo, rel: &Path, mark: char) -> Result> { let src = repo.root.join(rel); - let name = rel.display().to_string(); + let name = rel.to_path_buf(); let Some(meta) = symlink_metadata_opt(&src)? else { return Ok(None); @@ -196,8 +212,10 @@ fn walk(repo: &Repo, rel: &Path, mark: char) -> Result<(usize, Vec)> { // nothing below is in the store, so collapse to one line if handled == 0 && !exposed.is_empty() { - let name = format!("{}/", rel.display()); - return Ok((0, vec![Exposed::content(mark, name)])); + let mut name = rel.as_os_str().to_owned(); + name.push("/"); + + return Ok((0, vec![Exposed::content(mark, PathBuf::from(name))])); } Ok((handled, exposed)) @@ -230,16 +248,25 @@ fn list_others( repo: &Repo, ignored: bool, pathspecs: &[PathBuf], -) -> Result> { +) -> Result>> { let mut listing = LsFiles::untracked(&repo.root).limited_to(pathspecs); if ignored { listing = listing.ignored(); } - Ok(listing - .capture(ctx)? - .split('\0') - .filter(|p| !p.is_empty()) - .map(String::from) - .collect()) + Ok(split_nul(&listing.capture_bytes(ctx)?)) +} + +// every listing check reads is asked for with -z, and kept as the bytes git +// wrote: a path is not obliged to be utf-8, and one that is not used to fail +// the whole listing rather than the one entry +fn split_nul(out: &[u8]) -> Vec> { + out.split(|byte| *byte == 0) + .filter(|record| !record.is_empty()) + .map(<[u8]>::to_vec) + .collect() +} + +fn path_from(bytes: &[u8]) -> PathBuf { + PathBuf::from(OsString::from_vec(bytes.to_vec())) } diff --git a/src/commands/link/store.rs b/src/commands/link/store.rs index ef7c89b..aa33807 100644 --- a/src/commands/link/store.rs +++ b/src/commands/link/store.rs @@ -1,6 +1,7 @@ use fs_err as fs; use std::env; use std::ffi::OsString; +use std::os::unix::ffi::OsStringExt; use std::path::{Path, PathBuf}; use anyhow::{Context, Result, anyhow}; @@ -69,16 +70,21 @@ pub(super) fn resolve(path: &Path) -> Result { } fn git_root(ctx: &Ctx) -> Result { + // bytes, since the checkout can live at a path that is not utf-8, and with + // the cause kept: git not being installed and the directory not being a + // repository are different problems with the same one-line answer otherwise let root = RevParse - .capture(ctx) - .map_err(|_| anyhow!("not inside a git repository"))?; + .capture_bytes(ctx) + .context("asking git for the repository root")?; - let root = root.trim().to_string(); + let root = root.strip_suffix(b"\n").unwrap_or(&root); if root.is_empty() { return Err(anyhow!("git reported an empty repository root")); } - Ok(fs::canonicalize(&root)?) + Ok(fs::canonicalize(PathBuf::from(OsString::from_vec( + root.to_vec(), + )))?) } fn repo_components(ctx: &Ctx, root: &Path) -> Result { @@ -170,9 +176,12 @@ fn non_empty_var(name: &str) -> Option { } pub(super) fn tracked(ctx: &Ctx, repo: &Repo, rel: &Path) -> Result { + // bytes: the listing echoes back the path asked about, which is not obliged + // to be utf-8, and failing to read it would refuse the path for the wrong + // reason rather than answering whether git tracks it let listed = LsFiles::tracked(&repo.root) .limited_to(&[rel]) - .capture(ctx)?; + .capture_bytes(ctx)?; Ok(!listed.is_empty()) } diff --git a/src/commands/postgres.rs b/src/commands/postgres.rs index 17c11a5..a36b9b2 100644 --- a/src/commands/postgres.rs +++ b/src/commands/postgres.rs @@ -81,11 +81,7 @@ pub fn import(ctx: &Ctx, file: &Path) -> Result<()> { // a directory cannot be streamed, so it is the one shape that gets copied in if matches!(dump, Dump::Directory) { - when_ready( - ctx, - &db, - &Cp::into_container(&file.to_string_lossy(), &db.container, &remote), - )?; + when_ready(ctx, &db, &Cp::into_container(file, &db.container, &remote))?; } let kind = match &dump { @@ -284,7 +280,7 @@ fn dump_directory(ctx: &Ctx, db: &Database, target: &Path) -> Result<()> { .in_container(&db.container) .run(ctx)?; - let copied = Cp::out_of_container(&db.container, &remote, &target.to_string_lossy()).run(ctx); + let copied = Cp::out_of_container(&db.container, &remote, target).run(ctx); let _ = Rm::recursive(&remote).in_container(&db.container).run(ctx); diff --git a/src/output.rs b/src/output.rs index 57710d0..14086f7 100644 --- a/src/output.rs +++ b/src/output.rs @@ -53,6 +53,13 @@ pub(crate) fn write_text(args: Arguments) { } } +// a path on unix is bytes, not text, so a record naming one is written as bytes +pub(crate) fn write_bytes(bytes: &[u8]) { + if writable() { + finish(io::stdout().write_all(bytes)); + } +} + // stderr is commentary about the work, not the work itself. there is nowhere to // report that it could not be written, so the error goes nowhere -- eprintln! // panics instead, which turns a closed pipe into a crash