diff --git a/Cargo.lock b/Cargo.lock index bf8ff8f..1215e19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -330,7 +330,7 @@ checksum = "460fbee9c2c2f33933d720630a6a0bac33ba7053db5344fac858d4b8952d77d5" [[package]] name = "diskghost-cli" -version = "0.10.2" +version = "0.11.0" dependencies = [ "clap", "diskghost-core", @@ -340,7 +340,7 @@ dependencies = [ [[package]] name = "diskghost-core" -version = "0.10.2" +version = "0.11.0" dependencies = [ "blake3", "criterion", diff --git a/Cargo.toml b/Cargo.toml index 80de6f2..f50363d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ exclude = ["gui"] resolver = "2" [workspace.package] -version = "0.10.2" +version = "0.11.0" edition = "2021" license = "MIT" authors = ["kingchenc"] diff --git a/README.md b/README.md index 4886c31..591ab14 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ cargo install --path crates/diskghost-cli # Biggest sub-folders and files diskghost scan "C:\Users\me\Desktop" --top 20 -# Scan options +# Scan options (an invalid exclude glob is an error, never silently skipped) diskghost scan . --exclude node_modules --exclude "*.tmp" --max-depth 3 --follow-symlinks # Duplicates (ignore anything under 5 MB) @@ -109,11 +109,22 @@ Add `--json` to any command for machine-readable output: ```bash diskghost scan . --json # total_size, children, top_files, root_files, skipped, cancelled … -diskghost dupes . --min-mb 10 --json # duplicate groups with reclaimable bytes +diskghost scan . --save last.dgs --json # ...plus "snapshot_saved": the file written +diskghost dupes . --min-mb 10 --json # {"groups": [...]}: duplicate groups with reclaimable bytes +diskghost dupes . --reclaim trash --json # ...plus "reclaim": removed, reclaimed, errors, dry_run diskghost scan . --since old.json --json # the report plus a "diff" object (grown/shrunk/added/removed) diskghost diff old.json new.json --json # just the diff ``` +With `--json`, a successful run writes exactly one JSON document to stdout +and nothing to stderr (apart from the live progress line when stderr is a +terminal): notices and warnings are fields of the JSON +(`snapshot_saved`, `mft_fallback`, `warnings`), so the output stays valid +even when stdout and stderr end up in the same file (`2>&1`). Errors go to +stderr with a non-zero exit code and leave stdout empty; only `rm` and +`dupes --reclaim` still print their report when some files could not be +removed, with the failures in its `errors`. + Perfect for wiring into an automation/agent that decides what to clean. ## Desktop app (GUI) diff --git a/crates/diskghost-cli/src/main.rs b/crates/diskghost-cli/src/main.rs index c7d099a..c517c2e 100644 --- a/crates/diskghost-cli/src/main.rs +++ b/crates/diskghost-cli/src/main.rs @@ -4,7 +4,7 @@ //! scripts and agents (headless use). use std::io::{IsTerminal, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::ExitCode; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -13,7 +13,7 @@ use clap::{Args, Parser, Subcommand, ValueEnum}; use diskghost_core::{ diff, find_duplicates_cached, format_timestamp, human_delta, human_size, phase, reclaim, remove_path, validate_globs, DiffReport, DupGroup, HashCache, Options, Progress, ReclaimAction, - RemoveMode, RemoveReport, ScanReport, ScanTree, Snapshot, + ReclaimReport, RemoveMode, RemoveReport, ScanReport, ScanTree, Snapshot, }; use serde::Serialize; @@ -59,14 +59,14 @@ fn with_progress(job: impl FnOnce(&Progress) -> T) -> T { out } -fn warn_invalid_globs(patterns: &[String]) { +/// Refuse invalid exclude globs up front: skipping one would scan, or reclaim, +/// exactly what the caller meant to leave out. +fn check_globs(patterns: &[String]) -> bool { let bad = validate_globs(patterns); if !bad.is_empty() { - eprintln!( - "warning: ignoring invalid exclude glob(s): {}", - bad.join(", ") - ); + eprintln!("error: invalid exclude glob(s): {}", bad.join(", ")); } + bad.is_empty() } #[derive(Parser)] @@ -222,7 +222,9 @@ fn main() -> ExitCode { eprintln!("error: not a directory: {}", path.display()); return ExitCode::FAILURE; } - warn_invalid_globs(&walk.exclude); + if !check_globs(&walk.exclude) { + return ExitCode::FAILURE; + } let opts = walk.to_options(); // Load the old snapshot before scanning, so a bad file fails fast. let old = match since.as_deref().map(Snapshot::load) { @@ -251,12 +253,24 @@ fn main() -> ExitCode { }, _ => None, }; + // Save before printing, so a failure leaves stdout empty. + let saved = match (&save, &snapshot) { + (Some(file), Some(snap)) => { + if let Err(e) = snap.save(file) { + eprintln!("error: cannot save snapshot: {e}"); + return ExitCode::FAILURE; + } + Some(file.as_path()) + } + _ => None, + }; if cli.json { let out = ScanOutput { report: &report, diff: changes.as_ref(), engine: tree.engine(), mft_fallback: tree.mft_fallback(), + snapshot_saved: saved, }; println!("{}", serde_json::to_string_pretty(&out).unwrap()); } else { @@ -274,14 +288,9 @@ fn main() -> ExitCode { if let Some(why) = tree.mft_fallback() { eprintln!("note: --mft not used: {why}"); } - } - if let (Some(file), Some(snap)) = (&save, &snapshot) { - if let Err(e) = snap.save(file) { - eprintln!("error: cannot save snapshot: {e}"); - return ExitCode::FAILURE; + if let Some(file) = saved { + eprintln!("snapshot saved to {}", file.display()); } - // Keep stdout clean for --json consumers. - eprintln!("snapshot saved to {}", file.display()); } } Command::Diff { old, new, at, top } => { @@ -319,24 +328,42 @@ fn main() -> ExitCode { eprintln!("error: not a directory: {}", path.display()); return ExitCode::FAILURE; } - warn_invalid_globs(&walk.exclude); + if !check_globs(&walk.exclude) { + return ExitCode::FAILURE; + } let opts = walk.to_options(); let hashes = cache.as_deref().map(HashCache::load).unwrap_or_default(); let groups = with_progress(|p| { find_duplicates_cached(&path, min_mb * 1024 * 1024, &opts, p, &hashes) }); + // With --json, warnings go into the output instead of stderr. + let mut warnings = Vec::new(); if let Some(file) = &cache { if let Err(e) = hashes.save(file) { - eprintln!("warning: cannot save hash cache: {e}"); + let w = format!("cannot save hash cache: {e}"); + if cli.json { + warnings.push(w); + } else { + eprintln!("warning: {w}"); + } } } - if cli.json { - println!("{}", serde_json::to_string_pretty(&groups).unwrap()); - } else { + if !cli.json { print_dupes(&groups); } - if let Some(arg) = reclaim_arg { - run_reclaim(&groups, arg.into(), !apply); + let reclaimed = reclaim_arg.map(|arg| run_reclaim(&groups, arg.into(), !apply)); + if cli.json { + let out = DupesOutput { + groups: &groups, + reclaim: reclaimed.as_ref(), + warnings, + }; + println!("{}", serde_json::to_string_pretty(&out).unwrap()); + } else if let Some(r) = &reclaimed { + print_reclaim(r); + } + if reclaimed.is_some_and(|r| !r.errors.is_empty()) { + return ExitCode::FAILURE; } } Command::Rm { path, trash, apply } => { @@ -391,6 +418,20 @@ struct ScanOutput<'a> { /// Why --mft was asked for but not used. #[serde(skip_serializing_if = "Option::is_none")] mft_fallback: Option<&'a str>, + /// Where `--save` wrote the snapshot. + #[serde(skip_serializing_if = "Option::is_none")] + snapshot_saved: Option<&'a Path>, +} + +/// `dupes --json` output: the groups, plus `reclaim` when `--reclaim` was +/// given and `warnings` for problems that did not stop the search. +#[derive(Serialize)] +struct DupesOutput<'a> { + groups: &'a [DupGroup], + #[serde(skip_serializing_if = "Option::is_none")] + reclaim: Option<&'a ReclaimReport>, + #[serde(skip_serializing_if = "Vec::is_empty")] + warnings: Vec, } fn print_diff(d: &DiffReport) { @@ -557,34 +598,42 @@ fn print_dupes(groups: &[DupGroup]) { } /// Reclaim space across all groups, keeping the first file of each group. -fn run_reclaim(groups: &[DupGroup], action: ReclaimAction, dry_run: bool) { - let mut removed = 0usize; - let mut reclaimed = 0u64; - let mut errors = 0usize; +fn run_reclaim(groups: &[DupGroup], action: ReclaimAction, dry_run: bool) -> ReclaimReport { + let mut total = ReclaimReport { + removed: 0, + reclaimed: 0, + errors: Vec::new(), + dry_run, + }; for g in groups { if g.files.len() < 2 { continue; } let keep = &g.files[0]; let report = reclaim(keep, &g.files[1..], g.size, action, dry_run); - removed += report.removed; - reclaimed += report.reclaimed; - errors += report.errors.len(); - for e in &report.errors { - eprintln!(" ! {e}"); - } + total.removed += report.removed; + total.reclaimed += report.reclaimed; + total.errors.extend(report.errors); + } + total +} + +fn print_reclaim(r: &ReclaimReport) { + for e in &r.errors { + eprintln!(" ! {e}"); } - let mode = if dry_run { + let mode = if r.dry_run { "DRY-RUN — nothing changed; pass --apply to act" } else { "applied" }; print!( - "\nReclaim [{mode}]: {removed} file(s), {} reclaimable", - human_size(reclaimed) + "\nReclaim [{mode}]: {} file(s), {} reclaimable", + r.removed, + human_size(r.reclaimed) ); - if errors > 0 { - print!(", {errors} error(s)"); + if !r.errors.is_empty() { + print!(", {} error(s)", r.errors.len()); } println!(); } diff --git a/crates/diskghost-cli/tests/cli.rs b/crates/diskghost-cli/tests/cli.rs index 7f2261a..fc86212 100644 --- a/crates/diskghost-cli/tests/cli.rs +++ b/crates/diskghost-cli/tests/cli.rs @@ -39,7 +39,7 @@ fn scan_json_is_valid() { } #[test] -fn dupes_json_is_array() { +fn dupes_json_has_groups() { let out = Command::new(bin()) .args(["dupes", sample_dir(), "--min-mb", "999999", "--json"]) .output() @@ -47,7 +47,8 @@ fn dupes_json_is_array() { assert!(out.status.success()); let s = String::from_utf8_lossy(&out.stdout); let v: serde_json::Value = serde_json::from_str(&s).expect("valid JSON"); - assert!(v.is_array()); + assert!(v["groups"].as_array().unwrap().is_empty()); + assert!(v.get("reclaim").is_none() && v.get("warnings").is_none()); } #[test] @@ -198,6 +199,177 @@ fn scan_save_then_since_and_diff() { std::fs::remove_dir_all(&d).ok(); } +/// A successful `--json` run writes one JSON document to stdout and nothing to +/// stderr, so the output stays valid even when both streams are merged. +fn json_only(out: &std::process::Output) -> serde_json::Value { + assert!(out.status.success(), "{out:?}"); + let err = String::from_utf8_lossy(&out.stderr); + assert!(err.is_empty(), "stderr should be empty:\n{err}"); + serde_json::from_slice(&out.stdout).expect("stdout is pure JSON") +} + +#[test] +fn scan_json_save_reports_in_json() { + let d = scratch("jsonsave"); + let data = d.join("data"); + std::fs::create_dir_all(&data).unwrap(); + std::fs::write(data.join("a.bin"), [0u8; 100]).unwrap(); + let snap = d.join("snap.json"); + + let out = Command::new(bin()) + .args([ + "scan", + data.to_str().unwrap(), + "--json", + "--save", + snap.to_str().unwrap(), + ]) + .output() + .expect("run diskghost"); + let v = json_only(&out); + assert!(snap.is_file(), "snapshot file should exist"); + assert!(v.get("total_size").is_some()); + assert_eq!(v["snapshot_saved"].as_str(), snap.to_str()); + + // Without --save there is no such key. + let out = Command::new(bin()) + .args(["scan", data.to_str().unwrap(), "--json"]) + .output() + .expect("run diskghost"); + assert!(json_only(&out).get("snapshot_saved").is_none()); + + std::fs::remove_dir_all(&d).ok(); +} + +#[test] +fn scan_json_save_failure_leaves_stdout_empty() { + let d = scratch("jsonsavefail"); + // A directory where the snapshot file should go cannot be overwritten. + let taken = d.join("taken"); + std::fs::create_dir_all(&taken).unwrap(); + + let out = Command::new(bin()) + .args([ + "scan", + sample_dir(), + "--json", + "--save", + taken.to_str().unwrap(), + ]) + .output() + .expect("run diskghost"); + assert!(!out.status.success()); + assert!(out.stdout.is_empty(), "{out:?}"); + assert!(String::from_utf8_lossy(&out.stderr).contains("cannot save snapshot")); + + std::fs::remove_dir_all(&d).ok(); +} + +/// Three copies of one file plus a unique one, under a fresh scratch dir. +fn dupes_fixture(label: &str) -> (std::path::PathBuf, std::path::PathBuf) { + let d = scratch(label); + let data = d.join("data"); + std::fs::create_dir_all(&data).unwrap(); + for name in ["a.bin", "b.bin", "c.bin"] { + std::fs::write(data.join(name), [7u8; 50_000]).unwrap(); + } + std::fs::write(data.join("unique.bin"), [9u8; 50_000]).unwrap(); + (d, data) +} + +#[test] +fn dupes_json_reclaim_reports_in_json() { + let (d, data) = dupes_fixture("jsonreclaim"); + + let out = Command::new(bin()) + .args([ + "dupes", + data.to_str().unwrap(), + "--min-mb", + "0", + "--reclaim", + "delete", + "--json", + ]) + .output() + .expect("run diskghost"); + let v = json_only(&out); + let groups = v["groups"].as_array().unwrap(); + assert_eq!(groups.len(), 1); + assert_eq!(groups[0]["files"].as_array().unwrap().len(), 3); + let r = &v["reclaim"]; + assert_eq!(r["dry_run"], true); + assert_eq!(r["removed"], 2); + assert_eq!(r["reclaimed"], 100_000); + assert!(r["errors"].as_array().unwrap().is_empty()); + // A dry run changes nothing. + for name in ["a.bin", "b.bin", "c.bin", "unique.bin"] { + assert!(data.join(name).is_file(), "{name} should still exist"); + } + + std::fs::remove_dir_all(&d).ok(); +} + +#[test] +fn dupes_json_cache_warning_goes_into_json() { + let (d, data) = dupes_fixture("jsonwarn"); + // A directory where the cache file should go cannot be overwritten. + let taken = d.join("taken"); + std::fs::create_dir_all(&taken).unwrap(); + + let out = Command::new(bin()) + .args([ + "dupes", + data.to_str().unwrap(), + "--min-mb", + "0", + "--cache", + taken.to_str().unwrap(), + "--json", + ]) + .output() + .expect("run diskghost"); + let v = json_only(&out); + assert_eq!(v["groups"].as_array().unwrap().len(), 1); + let warnings = v["warnings"].as_array().unwrap(); + assert_eq!(warnings.len(), 1); + assert!(warnings[0] + .as_str() + .unwrap() + .starts_with("cannot save hash cache")); + + // Without --json the warning stays on stderr. + let out = Command::new(bin()) + .args([ + "dupes", + data.to_str().unwrap(), + "--min-mb", + "0", + "--cache", + taken.to_str().unwrap(), + ]) + .output() + .expect("run diskghost"); + assert!(out.status.success()); + assert!(String::from_utf8_lossy(&out.stderr).contains("warning: cannot save hash cache")); + + std::fs::remove_dir_all(&d).ok(); +} + +#[test] +fn invalid_exclude_glob_fails() { + for cmd in ["scan", "dupes"] { + let out = Command::new(bin()) + .args([cmd, sample_dir(), "--exclude", "a[", "--json"]) + .output() + .expect("run diskghost"); + assert!(!out.status.success(), "{cmd} should fail"); + assert!(out.stdout.is_empty(), "{cmd}: {out:?}"); + let err = String::from_utf8_lossy(&out.stderr); + assert!(err.contains("invalid exclude glob(s): a["), "{cmd}: {err}"); + } +} + #[test] fn snapshot_errors_are_reported() { let d = scratch("snaperr"); diff --git a/gui/src-tauri/Cargo.lock b/gui/src-tauri/Cargo.lock index c658863..56fd74b 100644 --- a/gui/src-tauri/Cargo.lock +++ b/gui/src-tauri/Cargo.lock @@ -645,7 +645,7 @@ dependencies = [ [[package]] name = "diskghost-core" -version = "0.10.2" +version = "0.11.0" dependencies = [ "blake3", "fs4", @@ -659,7 +659,7 @@ dependencies = [ [[package]] name = "diskghost-gui" -version = "0.10.2" +version = "0.11.0" dependencies = [ "diskghost-core", "serde", diff --git a/gui/src-tauri/Cargo.toml b/gui/src-tauri/Cargo.toml index c84eaa2..ef3fe0c 100644 --- a/gui/src-tauri/Cargo.toml +++ b/gui/src-tauri/Cargo.toml @@ -4,7 +4,7 @@ [package] name = "diskghost-gui" -version = "0.10.2" +version = "0.11.0" edition = "2021" license = "MIT" description = "Diskghost desktop GUI (Tauri) — modern disk usage & duplicate finder." diff --git a/gui/src-tauri/tauri.conf.json b/gui/src-tauri/tauri.conf.json index dff8cbd..3d0be86 100644 --- a/gui/src-tauri/tauri.conf.json +++ b/gui/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "Diskghost", - "version": "0.10.2", + "version": "0.11.0", "identifier": "org.diskghost.app", "build": { "frontendDist": "../ui"