merge: bug fixes

This commit is contained in:
2026-09-08 13:18:24 +02:00
5 changed files with 89 additions and 13 deletions

View File

@@ -1,6 +1,7 @@
use super::{Django, Link, Postgres}; use super::{Django, Link, Postgres};
use clap::builder::styling::{AnsiColor, Effects, Styles}; use clap::builder::styling::{AnsiColor, Effects, Styles};
use clap::{Parser, Subcommand}; use clap::error::ErrorKind;
use clap::{CommandFactory, Parser, Subcommand};
use clap_complete::Shell; use clap_complete::Shell;
/// A program for interacting with various dockerized applications /// A program for interacting with various dockerized applications
@@ -55,13 +56,63 @@ fn help_styles() -> Styles {
.placeholder(AnsiColor::Cyan.on_default()) .placeholder(AnsiColor::Cyan.on_default())
} }
/// Exit with a usage error when `--dry-run` would be a lie
pub fn reject_unsupported_dry_run(command: &Commands) {
if let Some(name) = writes_locally(command) {
Ahab::command()
.error(
ErrorKind::ArgumentConflict,
format!("--dry-run is not supported by `{name}`, which writes to the working tree"),
)
.exit()
}
}
// TODO:(@janezicmatej) honour --dry-run in these commands instead of refusing it
fn writes_locally(command: &Commands) -> Option<&'static str> {
match command {
Commands::Link { command } => match command {
Link::Add { .. } => Some("link add"),
Link::Restore { .. } => Some("link restore"),
Link::Check { .. } => None,
},
Commands::Django { command } => match command {
Django::MakeCommand { .. } => Some("django make-command"),
_ => None,
},
Commands::Postgres { .. } | Commands::Completions { .. } => None,
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::Ahab; use super::Ahab;
use clap::CommandFactory; use clap::{CommandFactory, Parser};
#[test] #[test]
fn the_command_is_well_formed() { fn the_command_is_well_formed() {
Ahab::command().debug_assert(); Ahab::command().debug_assert();
} }
#[test]
fn dry_run_is_refused_only_where_it_cannot_be_honoured() {
let command = |args: &[&str]| Ahab::try_parse_from(args).unwrap().command;
assert_eq!(
super::writes_locally(&command(&["ahab", "link", "add", ".env"])),
Some("link add")
);
assert_eq!(
super::writes_locally(&command(&["ahab", "django", "make-command", "app", "name"])),
Some("django make-command")
);
assert_eq!(
super::writes_locally(&command(&["ahab", "link", "check"])),
None
);
assert_eq!(
super::writes_locally(&command(&["ahab", "postgres", "dump", "out.sql"])),
None
);
}
} }

View File

@@ -3,7 +3,7 @@ mod django;
mod link; mod link;
mod postgres; mod postgres;
pub use ahab::{Ahab, Commands}; pub use ahab::{Ahab, Commands, reject_unsupported_dry_run};
pub use django::Django; pub use django::Django;
pub use link::{Link, Store}; pub use link::{Link, Store};
pub use postgres::{Format, Postgres}; pub use postgres::{Format, Postgres};

View File

@@ -13,6 +13,10 @@ fn main() -> ExitCode {
dry_run: args.dry_run, dry_run: args.dry_run,
}); });
if args.dry_run {
cli::reject_unsupported_dry_run(&args.command);
}
match run(args.command) { match run(args.command) {
Ok(code) => code, Ok(code) => code,
Err(e) => { Err(e) => {

View File

@@ -99,7 +99,7 @@ fn restore_one(repo: &Repo, path: &Path, report: &Report) -> Result<()> {
fs::remove_file(&src).with_context(|| format!("removing {}", src.display()))?; fs::remove_file(&src).with_context(|| format!("removing {}", src.display()))?;
move_path(&stored, &src)?; move_path(&stored, &src)?;
prune_empty(stored.parent(), &store_root()?); prune_empty(stored.parent(), &repo.base);
report.line("restored", &rel); report.line("restored", &rel);
Ok(()) Ok(())
@@ -425,22 +425,24 @@ fn list_others(repo: &Repo, ignored: bool, pathspecs: &[PathBuf]) -> Result<Vec<
struct Repo { struct Repo {
root: PathBuf, root: PathBuf,
store: PathBuf, store: PathBuf,
// the configured store root, above the per-repository directories
base: PathBuf,
} }
impl Repo { impl Repo {
fn discover(store: Option<&Path>) -> Result<Self> { fn discover(store: Option<&Path>) -> Result<Self> {
let root = git_root()?; let root = git_root()?;
let store = match store { let base = match store {
Some(store) => store.to_path_buf(), Some(store) => store.to_path_buf(),
None => store_root()?, None => store_root()?,
}; };
Ok(Self { Ok(Self {
root: root.clone(), store: base.join(repo_components(&root)?),
store: store.join(repo_components(&root)?), root,
base,
}) })
} }
fn relative(&self, src: &Path) -> Result<PathBuf> { fn relative(&self, src: &Path) -> Result<PathBuf> {
let rel = src.strip_prefix(&self.root).map_err(|_| { let rel = src.strip_prefix(&self.root).map_err(|_| {
anyhow!( anyhow!(

View File

@@ -180,6 +180,13 @@ fn read_header(path: &Path) -> Result<Vec<u8>> {
Ok(header) Ok(header)
} }
// a pipeline reports the last stage's status by default, so a first stage that
// dies mid-stream looks like success; not for pipelines that close the pipe
// early on purpose, where SIGPIPE would then read as failure
fn pipefail(script: &str) -> String {
format!("set -o pipefail; {script}")
}
fn piped(db: &Database, script: &str, input: &Path) -> Result<std::process::Command> { fn piped(db: &Database, script: &str, input: &Path) -> Result<std::process::Command> {
let file = File::open(input).with_context(|| format!("opening {}", input.display()))?; let file = File::open(input).with_context(|| format!("opening {}", input.display()))?;
let mut command = CommandBuilder::docker() let mut command = CommandBuilder::docker()
@@ -294,7 +301,10 @@ pub fn import(file: &Path) -> Result<()> {
let kind = Kind::of(&out.stdout); let kind = Kind::of(&out.stdout);
(kind, Some(format!("gunzip -c | {}", db.restore_with(kind)))) (
kind,
Some(pipefail(&format!("gunzip -c | {}", db.restore_with(kind)))),
)
} }
}; };
@@ -377,9 +387,14 @@ pub fn dump(file: &PathBuf, format: Format, gzip: bool) -> Result<()> {
// written beside the target and renamed once the dump succeeds, so a failure // written beside the target and renamed once the dump succeeds, so a failure
// cannot destroy the dump that is already there // cannot destroy the dump that is already there
let partial = suffixed(file, ".partial"); let partial = suffixed(file, ".partial");
let stdout = Stdio::from( // a dry run produces no dump, so it must not lay a hand on the target either
let stdout = if command_builder::is_dry_run() {
Stdio::null()
} else {
Stdio::from(
File::create(&partial).with_context(|| format!("creating {}", partial.display()))?, File::create(&partial).with_context(|| format!("creating {}", partial.display()))?,
); )
};
let dumped = if gzip { let dumped = if gzip {
// the whole pipeline has to arrive as one shell argument // the whole pipeline has to arrive as one shell argument
@@ -387,7 +402,7 @@ pub fn dump(file: &PathBuf, format: Format, gzip: bool) -> Result<()> {
.args("exec") .args("exec")
.arg(&db.container) .arg(&db.container)
.args("sh -c") .args("sh -c")
.arg(format!("{} | gzip", dump_command(&db, format))) .arg(pipefail(&format!("{} | gzip", dump_command(&db, format))))
.exec_redirect_stdout(stdout) .exec_redirect_stdout(stdout)
} else { } else {
let dumping = match format.flag() { let dumping = match format.flag() {
@@ -407,6 +422,10 @@ pub fn dump(file: &PathBuf, format: Format, gzip: bool) -> Result<()> {
return Err(e); return Err(e);
} }
if command_builder::is_dry_run() {
return Ok(());
}
fs::rename(&partial, file) fs::rename(&partial, file)
.with_context(|| format!("renaming {} to {}", partial.display(), file.display()))?; .with_context(|| format!("renaming {} to {}", partial.display(), file.display()))?;