* [pbs-devel] [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive`
@ 2022-12-09 11:14 Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 1/4] debug cli: show more file attributes for `diff archive` command Lukas Wagner
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Lukas Wagner @ 2022-12-09 11:14 UTC (permalink / raw)
To: pbs-devel
This patch series contains a few improvements for the `diff archive` tool,
mainly based on Wolfgang's and Thomas' suggestions.
First, the output of is now much more detailed and shows
some relevant file attributes, including what has changed between
snapshots. Changed attributes are highlighted by a "*" and are
highlighted by colored text.
For instance:
$ proxmox-backup-debug diff archive ... --compare-content
A f 644 10045 10000 0 B 2022-11-28 13:44:51 add.txt
M f 644 10045 10000 6 B *2022-11-28 13:45:05 *content.txt
D f 644 10045 10000 0 B 2022-11-28 13:17:09 deleted.txt
M f 644 10045 *29 0 B 2022-11-28 13:16:20 gid.txt
M f *777 10045 10000 0 B 2022-11-28 13:42:47 mode.txt
M f 644 10045 10000 0 B *2022-11-28 13:44:33 mtime.txt
M f 644 10045 10000 *7 B *2022-11-28 13:44:59 size.txt
M f 644 *64045 10000 0 B 2022-11-28 13:16:18 uid.txt
M *f 644 10045 10000 10 B 2022-11-28 13:44:59 type.txt
Furthermore, there now exists the possiblity to pass
the --compare-content flag to the tool. If the flag is passed,
the tool will compare the file content instead of relying on mtime
alone to detect modifications.
This patch series adds new dependencies to the `termcolor` crate.
That being said, the crate was already pulled in by other crates as
transitive dependencies.
Changes from v3:
- Detect if device node changes from blockdev -> chardev and vice versa.
- Use the default mechanism provided by the #[api] macro for `compare_content`
Changes from v2:
- Increase buffersize to 4K
- Added workaround for weird issue with tokio::try_join! and #[api]
- Drop dependency on `atty` crate
- Remove modificiations to debian/control
- `diff_archive_cmd`: Moved parameters into the function signature,
instead of manually extracting it from `Value`.
Changes from v1:
- Made `changed indicator for file content` a bit less confusing.
For regular files, it is now only displayed if
- the --comapare-content flag is set and
- the file contents *actually* differ
- Removed unnecessesary namespace prefix for std::task::Pin in unit tests
- Added color output, controllable via --color {always,auto,never} flag.
Lukas Wagner (4):
debug cli: show more file attributes for `diff archive` command
debug cli: add 'compare-content' flag to `diff archive` command
debug cli: add colored output for `diff archive`
debug cli: move parameters into the function signature
Cargo.toml | 1 +
src/bin/proxmox_backup_debug/diff.rs | 613 ++++++++++++++++++++++---
src/bin/proxmox_backup_manager/ldap.rs | 102 ++++
3 files changed, 646 insertions(+), 70 deletions(-)
create mode 100644 src/bin/proxmox_backup_manager/ldap.rs
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH v4 proxmox-backup 1/4] debug cli: show more file attributes for `diff archive` command
2022-12-09 11:14 [pbs-devel] [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive` Lukas Wagner
@ 2022-12-09 11:14 ` Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 2/4] debug cli: add 'compare-content' flag to " Lukas Wagner
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Lukas Wagner @ 2022-12-09 11:14 UTC (permalink / raw)
To: pbs-devel
This commit enriches the output of the `diff archive` command,
showing pxar entry type, mode, uid, gid, size, mtime and filename.
Attributes that changed between both snapshots are prefixed
with a "*".
For instance:
$ proxmox-backup-debug diff archive ...
A f 644 10045 10000 0 B 2022-11-28 13:44:51 add.txt
M f 644 10045 10000 6 B *2022-11-28 13:45:05 content.txt
D f 644 10045 10000 0 B 2022-11-28 13:17:09 deleted.txt
M f 644 10045 *29 0 B 2022-11-28 13:16:20 gid.txt
M f *777 10045 10000 0 B 2022-11-28 13:42:47 mode.txt
M f 644 10045 10000 0 B *2022-11-28 13:44:33 mtime.txt
M f 644 10045 10000 *7 B *2022-11-28 13:44:59 *size.txt
M f 644 *64045 10000 0 B 2022-11-28 13:16:18 uid.txt
M *f 644 10045 10000 10 B 2022-11-28 13:44:59 type_changed.txt
Also, this commit ensures that we always show the *new* type.
Previously, the command showed the old type if it was changed.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Changes from v3:
- Detect if device node changes from blockdev -> chardev and vice versa.
src/bin/proxmox_backup_debug/diff.rs | 223 ++++++++++++++++++++-------
1 file changed, 169 insertions(+), 54 deletions(-)
diff --git a/src/bin/proxmox_backup_debug/diff.rs b/src/bin/proxmox_backup_debug/diff.rs
index cb157ec2..db9a8dbe 100644
--- a/src/bin/proxmox_backup_debug/diff.rs
+++ b/src/bin/proxmox_backup_debug/diff.rs
@@ -11,7 +11,7 @@ use futures::FutureExt;
use proxmox_router::cli::{CliCommand, CliCommandMap, CommandLineInterface};
use proxmox_schema::api;
-use pbs_api_types::{BackupNamespace, BackupPart};
+use pbs_api_types::{BackupNamespace, BackupPart, HumanByte};
use pbs_client::tools::key_source::{
crypto_parameters, format_key_source, get_encryption_key_password, KEYFD_SCHEMA,
};
@@ -173,15 +173,18 @@ async fn diff_archive(
// If file is present in both snapshots, it *might* be modified, but does not have to be.
// If another, unmodified file resides in the same chunk as an actually modified one,
// it will also show up as modified here...
- let potentially_modified: HashMap<&OsStr, &FileEntry> = files_in_a
+ let potentially_modified: HashMap<&OsStr, (&FileEntry, &FileEntry)> = files_in_a
.iter()
- .filter(|(path, _)| files_in_b.contains_key(*path))
- .map(|(path, entry)| (path.as_os_str(), entry))
+ .filter_map(|(path, entry_a)| {
+ files_in_b
+ .get(path)
+ .map(|entry_b| (path.as_os_str(), (entry_a, entry_b)))
+ })
.collect();
// ... so we compare the file metadata/contents to narrow the selection down to files
// which where *really* modified.
- let modified_files = compare_files(&files_in_a, &files_in_b, potentially_modified).await?;
+ let modified_files = compare_files(potentially_modified).await?;
show_file_list(&added_files, &deleted_files, &modified_files);
@@ -335,7 +338,6 @@ fn visit_directory<'f, 'c>(
let digest = chunk_list.get(chunk_index).context("Invalid chunk index")?;
if chunk_diff.get(digest).is_some() {
- // files.insert(file_path.clone().into_os_string());
entries.insert(file_path.into_os_string(), entry);
break;
}
@@ -349,62 +351,179 @@ fn visit_directory<'f, 'c>(
/// Check if files were actually modified
async fn compare_files<'a>(
- entries_a: &HashMap<OsString, FileEntry>,
- entries_b: &HashMap<OsString, FileEntry>,
- files: HashMap<&'a OsStr, &'a FileEntry>,
-) -> Result<HashMap<&'a OsStr, &'a FileEntry>, Error> {
+ files: HashMap<&'a OsStr, (&'a FileEntry, &'a FileEntry)>,
+) -> Result<HashMap<&'a OsStr, (&'a FileEntry, ChangedProperties)>, Error> {
let mut modified_files = HashMap::new();
- for (path, entry) in files {
- let p = path.to_os_string();
- let file_a = entries_a.get(&p).context("File entry not in map")?;
- let file_b = entries_b.get(&p).context("File entry not in map")?;
-
- if !compare_file(file_a, file_b).await {
- modified_files.insert(path, entry);
+ for (path, (entry_a, entry_b)) in files {
+ if let Some(changed) = compare_file(entry_a, entry_b).await? {
+ modified_files.insert(path, (entry_b, changed));
}
}
Ok(modified_files)
}
-async fn compare_file(file_a: &FileEntry, file_b: &FileEntry) -> bool {
- if file_a.metadata() != file_b.metadata() {
- // Check if mtime, permissions, ACLs, etc. have changed - if they have changed, we consider
- // the file as modified.
- return false;
- }
+async fn compare_file(
+ file_a: &FileEntry,
+ file_b: &FileEntry,
+) -> Result<Option<ChangedProperties>, Error> {
+ let mut changed = ChangedProperties::default();
+
+ changed.set_from_metadata(file_a, file_b);
match (file_a.kind(), file_b.kind()) {
(EntryKind::Symlink(a), EntryKind::Symlink(b)) => {
// Check whether the link target has changed.
- a.as_os_str() == b.as_os_str()
+ changed.content = a.as_os_str() != b.as_os_str();
}
(EntryKind::Hardlink(a), EntryKind::Hardlink(b)) => {
// Check whether the link target has changed.
- a.as_os_str() == b.as_os_str()
+ changed.content = a.as_os_str() != b.as_os_str();
+ }
+ (EntryKind::Device(a), EntryKind::Device(b)) => {
+ changed.content = a.major != b.major
+ || a.minor != b.minor
+ || file_a.metadata().stat.is_blockdev() != file_b.metadata().stat.is_blockdev();
}
- (EntryKind::Device(a), EntryKind::Device(b)) => a.major == b.major && a.minor == b.minor,
- (EntryKind::Socket, EntryKind::Socket) => true,
- (EntryKind::Fifo, EntryKind::Fifo) => true,
(EntryKind::File { size: size_a, .. }, EntryKind::File { size: size_b, .. }) => {
- // At this point we know that all metadata including mtime is
- // the same. To speed things up, we consider the files as equal if they also have
- // the same size.
- // If one were completely paranoid, one could compare the actual file contents,
- // but this decreases performance drastically.
- size_a == size_b
+ if size_a != size_b {
+ changed.size = true;
+ changed.content = true;
+ };
+ }
+ (EntryKind::Directory, EntryKind::Directory) => {}
+ (EntryKind::Socket, EntryKind::Socket) => {}
+ (EntryKind::Fifo, EntryKind::Fifo) => {}
+ (_, _) => {
+ changed.entry_type = true;
}
- (EntryKind::Directory, EntryKind::Directory) => true,
- (_, _) => false, // Kind has changed, so we of course consider it modified.
+ }
+
+ if changed.any() {
+ Ok(Some(changed))
+ } else {
+ Ok(None)
+ }
+}
+
+#[derive(Copy, Clone, Default)]
+struct ChangedProperties {
+ entry_type: bool,
+ mtime: bool,
+ acl: bool,
+ xattrs: bool,
+ fcaps: bool,
+ quota_project_id: bool,
+ mode: bool,
+ flags: bool,
+ uid: bool,
+ gid: bool,
+ size: bool,
+ content: bool,
+}
+
+impl ChangedProperties {
+ fn set_from_metadata(&mut self, file_a: &FileEntry, file_b: &FileEntry) {
+ let a = file_a.metadata();
+ let b = file_b.metadata();
+
+ self.acl = a.acl != b.acl;
+ self.xattrs = a.xattrs != b.xattrs;
+ self.fcaps = a.fcaps != b.fcaps;
+ self.quota_project_id = a.quota_project_id != b.quota_project_id;
+ self.mode = a.stat.mode != b.stat.mode;
+ self.flags = a.stat.flags != b.stat.flags;
+ self.uid = a.stat.uid != b.stat.uid;
+ self.gid = a.stat.gid != b.stat.gid;
+ self.mtime = a.stat.mtime != b.stat.mtime;
+ }
+
+ fn any(&self) -> bool {
+ self.entry_type
+ || self.mtime
+ || self.acl
+ || self.xattrs
+ || self.fcaps
+ || self.quota_project_id
+ || self.mode
+ || self.flags
+ || self.uid
+ || self.gid
+ || self.content
+ }
+}
+
+fn change_indicator(changed: bool) -> &'static str {
+ if changed {
+ "*"
+ } else {
+ " "
+ }
+}
+
+fn format_filesize(entry: &FileEntry, changed: bool) -> String {
+ if let Some(size) = entry.file_size() {
+ format!(
+ "{}{:.1}",
+ change_indicator(changed),
+ HumanByte::new_decimal(size as f64)
+ )
+ } else {
+ String::new()
}
}
+fn format_mtime(entry: &FileEntry, changed: bool) -> String {
+ let mtime = &entry.metadata().stat.mtime;
+
+ let format = if changed { "*%F %T" } else { " %F %T" };
+
+ proxmox_time::strftime_local(format, mtime.secs).unwrap_or_default()
+}
+
+fn format_mode(entry: &FileEntry, changed: bool) -> String {
+ let mode = entry.metadata().stat.mode & 0o7777;
+ format!("{}{:o}", change_indicator(changed), mode)
+}
+
+fn format_entry_type(entry: &FileEntry, changed: bool) -> String {
+ let kind = match entry.kind() {
+ EntryKind::Symlink(_) => "l",
+ EntryKind::Hardlink(_) => "h",
+ EntryKind::Device(_) if entry.metadata().stat.is_blockdev() => "b",
+ EntryKind::Device(_) => "c",
+ EntryKind::Socket => "s",
+ EntryKind::Fifo => "p",
+ EntryKind::File { .. } => "f",
+ EntryKind::Directory => "d",
+ _ => " ",
+ };
+
+ format!("{}{}", change_indicator(changed), kind)
+}
+
+fn format_uid(entry: &FileEntry, changed: bool) -> String {
+ format!("{}{}", change_indicator(changed), entry.metadata().stat.uid)
+}
+
+fn format_gid(entry: &FileEntry, changed: bool) -> String {
+ format!("{}{}", change_indicator(changed), entry.metadata().stat.gid)
+}
+
+fn format_file_name(entry: &FileEntry, changed: bool) -> String {
+ format!(
+ "{}{}",
+ change_indicator(changed),
+ entry.file_name().to_string_lossy()
+ )
+}
+
/// Display a sorted list of added, modified, deleted files.
fn show_file_list(
added: &HashMap<&OsStr, &FileEntry>,
deleted: &HashMap<&OsStr, &FileEntry>,
- modified: &HashMap<&OsStr, &FileEntry>,
+ modified: &HashMap<&OsStr, (&FileEntry, ChangedProperties)>,
) {
let mut all: Vec<&OsStr> = Vec::new();
@@ -415,28 +534,24 @@ fn show_file_list(
all.sort();
for file in all {
- let (op, entry) = if let Some(entry) = added.get(file) {
- ("A", *entry)
+ let (op, entry, changed) = if let Some(entry) = added.get(file) {
+ ("A", entry, ChangedProperties::default())
} else if let Some(entry) = deleted.get(file) {
- ("D", *entry)
- } else if let Some(entry) = modified.get(file) {
- ("M", *entry)
+ ("D", entry, ChangedProperties::default())
+ } else if let Some((entry, changed)) = modified.get(file) {
+ ("M", entry, *changed)
} else {
unreachable!();
};
- let entry_kind = match entry.kind() {
- EntryKind::Symlink(_) => "l",
- EntryKind::Hardlink(_) => "h",
- EntryKind::Device(_) if entry.metadata().stat.is_blockdev() => "b",
- EntryKind::Device(_) => "c",
- EntryKind::Socket => "s",
- EntryKind::Fifo => "p",
- EntryKind::File { .. } => "f",
- EntryKind::Directory => "d",
- _ => " ",
- };
+ let entry_type = format_entry_type(entry, changed.entry_type);
+ let uid = format_uid(entry, changed.uid);
+ let gid = format_gid(entry, changed.gid);
+ let mode = format_mode(entry, changed.mode);
+ let size = format_filesize(entry, changed.size);
+ let mtime = format_mtime(entry, changed.mtime);
+ let name = format_file_name(entry, changed.content);
- println!("{} {} {}", op, entry_kind, file.to_string_lossy());
+ println!("{op} {entry_type:>2} {mode:>5} {uid:>6} {gid:>6} {size:>10} {mtime:11} {name}");
}
}
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH v4 proxmox-backup 2/4] debug cli: add 'compare-content' flag to `diff archive` command
2022-12-09 11:14 [pbs-devel] [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive` Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 1/4] debug cli: show more file attributes for `diff archive` command Lukas Wagner
@ 2022-12-09 11:14 ` Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 3/4] debug cli: add colored output for `diff archive` Lukas Wagner
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Lukas Wagner @ 2022-12-09 11:14 UTC (permalink / raw)
To: pbs-devel
When --compare-content is set, the command will compare the
file content instead on relying on mtime to detect modified files.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/bin/proxmox_backup_debug/diff.rs | 158 +++++++++++++++++++++++--
src/bin/proxmox_backup_manager/ldap.rs | 102 ++++++++++++++++
2 files changed, 248 insertions(+), 12 deletions(-)
create mode 100644 src/bin/proxmox_backup_manager/ldap.rs
diff --git a/src/bin/proxmox_backup_debug/diff.rs b/src/bin/proxmox_backup_debug/diff.rs
index db9a8dbe..06667fe1 100644
--- a/src/bin/proxmox_backup_debug/diff.rs
+++ b/src/bin/proxmox_backup_debug/diff.rs
@@ -28,12 +28,15 @@ use pbs_tools::json::required_string_param;
use pxar::accessor::ReadAt;
use pxar::EntryKind;
use serde_json::Value;
+use tokio::io::AsyncReadExt;
type ChunkDigest = [u8; 32];
type FileEntry = pxar::accessor::aio::FileEntry<Arc<dyn ReadAt + Send + Sync>>;
type Accessor = pxar::accessor::aio::Accessor<Arc<dyn ReadAt + Send + Sync>>;
type Directory = pxar::accessor::aio::Directory<Arc<dyn ReadAt + Send + Sync>>;
+const BUFFERSIZE: usize = 4096;
+
pub fn diff_commands() -> CommandLineInterface {
let cmd_def = CliCommandMap::new().insert(
"archive",
@@ -79,18 +82,30 @@ pub fn diff_commands() -> CommandLineInterface {
schema: KEYFD_SCHEMA,
optional: true,
},
+ "compare-content": {
+ optional: true,
+ type: bool,
+ description: "Compare file content rather than solely relying on mtime for detecting modified files.",
+ },
}
}
)]
/// Diff an archive in two snapshots. The command will output a list of added, modified and deleted files.
-/// For modified files, only the file metadata (e.g. mtime, size, etc.) will be considered. The actual
-/// file contents will not be compared.
+/// For modified files, the file metadata (e.g. mode, uid, gid, size, etc.) will be considered. For detecting
+/// modification of file content, only mtime will be used by default. If the --compare-content flag is provided,
+/// mtime is ignored and file content will be compared.
async fn diff_archive_cmd(param: Value) -> Result<(), Error> {
let repo = extract_repository_from_value(¶m)?;
let snapshot_a = required_string_param(¶m, "prev-snapshot")?;
let snapshot_b = required_string_param(¶m, "snapshot")?;
let archive_name = required_string_param(¶m, "archive-name")?;
+ let compare_contents = match param.get("compare-content") {
+ Some(Value::Bool(value)) => *value,
+ Some(_) => bail!("invalid flag for compare-content"),
+ None => false,
+ };
+
let namespace = match param.get("ns") {
Some(Value::String(ns)) => ns.parse()?,
Some(_) => bail!("invalid namespace parameter"),
@@ -120,7 +135,14 @@ async fn diff_archive_cmd(param: Value) -> Result<(), Error> {
if archive_name.ends_with(".pxar") {
let file_name = format!("{}.didx", archive_name);
- diff_archive(snapshot_a, snapshot_b, &file_name, &repo_params).await?;
+ diff_archive(
+ snapshot_a,
+ snapshot_b,
+ &file_name,
+ &repo_params,
+ compare_contents,
+ )
+ .await?;
} else {
bail!("Only .pxar files are supported");
}
@@ -133,6 +155,7 @@ async fn diff_archive(
snapshot_b: &str,
file_name: &str,
repo_params: &RepoParams,
+ compare_contents: bool,
) -> Result<(), Error> {
let (index_a, accessor_a) = open_dynamic_index(snapshot_a, file_name, repo_params).await?;
let (index_b, accessor_b) = open_dynamic_index(snapshot_b, file_name, repo_params).await?;
@@ -184,7 +207,7 @@ async fn diff_archive(
// ... so we compare the file metadata/contents to narrow the selection down to files
// which where *really* modified.
- let modified_files = compare_files(potentially_modified).await?;
+ let modified_files = compare_files(potentially_modified, compare_contents).await?;
show_file_list(&added_files, &deleted_files, &modified_files);
@@ -352,11 +375,12 @@ fn visit_directory<'f, 'c>(
/// Check if files were actually modified
async fn compare_files<'a>(
files: HashMap<&'a OsStr, (&'a FileEntry, &'a FileEntry)>,
+ compare_contents: bool,
) -> Result<HashMap<&'a OsStr, (&'a FileEntry, ChangedProperties)>, Error> {
let mut modified_files = HashMap::new();
for (path, (entry_a, entry_b)) in files {
- if let Some(changed) = compare_file(entry_a, entry_b).await? {
+ if let Some(changed) = compare_file(entry_a, entry_b, compare_contents).await? {
modified_files.insert(path, (entry_b, changed));
}
}
@@ -367,6 +391,7 @@ async fn compare_files<'a>(
async fn compare_file(
file_a: &FileEntry,
file_b: &FileEntry,
+ compare_contents: bool,
) -> Result<Option<ChangedProperties>, Error> {
let mut changed = ChangedProperties::default();
@@ -387,10 +412,22 @@ async fn compare_file(
|| file_a.metadata().stat.is_blockdev() != file_b.metadata().stat.is_blockdev();
}
(EntryKind::File { size: size_a, .. }, EntryKind::File { size: size_b, .. }) => {
- if size_a != size_b {
- changed.size = true;
- changed.content = true;
- };
+ changed.size = size_a != size_b;
+
+ if compare_contents {
+ if changed.size {
+ changed.content = true;
+ } else {
+ let content_identical = compare_file_contents(file_a, file_b).await?;
+ if content_identical && !changed.any_without_mtime() {
+ // If the content is identical and nothing, exluding mtime,
+ // has changed, we don't consider the entry as modified.
+ changed.mtime = false;
+ }
+
+ changed.content = !content_identical;
+ }
+ }
}
(EntryKind::Directory, EntryKind::Directory) => {}
(EntryKind::Socket, EntryKind::Socket) => {}
@@ -407,6 +444,45 @@ async fn compare_file(
}
}
+async fn compare_file_contents(file_a: &FileEntry, file_b: &FileEntry) -> Result<bool, Error> {
+ let mut contents_a = file_a.contents().await?;
+ let mut contents_b = file_b.contents().await?;
+
+ compare_readers(&mut contents_a, &mut contents_b).await
+}
+
+async fn compare_readers<T>(reader_a: &mut T, reader_b: &mut T) -> Result<bool, Error>
+where
+ T: AsyncReadExt + Unpin,
+{
+ let mut buf_a = Box::new([0u8; BUFFERSIZE]);
+ let mut buf_b = Box::new([0u8; BUFFERSIZE]);
+
+ loop {
+ // Put the both read calls into their own async blocks, otherwise
+ // tokio::try_join! in combination with our #[api] macro leads to some
+ // weird `higher-order lifetime error`
+ let read_fut_a = async { reader_a.read(buf_a.as_mut_slice()).await };
+ let read_fut_b = async { reader_b.read(buf_b.as_mut_slice()).await };
+
+ let (bytes_read_a, bytes_read_b) = tokio::try_join!(read_fut_a, read_fut_b)?;
+
+ if bytes_read_a != bytes_read_b {
+ return Ok(false);
+ }
+
+ if bytes_read_a == 0 {
+ break;
+ }
+
+ if buf_a[..bytes_read_a] != buf_b[..bytes_read_b] {
+ return Ok(false);
+ }
+ }
+
+ Ok(true)
+}
+
#[derive(Copy, Clone, Default)]
struct ChangedProperties {
entry_type: bool,
@@ -440,8 +516,11 @@ impl ChangedProperties {
}
fn any(&self) -> bool {
+ self.any_without_mtime() || self.mtime
+ }
+
+ fn any_without_mtime(&self) -> bool {
self.entry_type
- || self.mtime
|| self.acl
|| self.xattrs
|| self.fcaps
@@ -477,9 +556,10 @@ fn format_filesize(entry: &FileEntry, changed: bool) -> String {
fn format_mtime(entry: &FileEntry, changed: bool) -> String {
let mtime = &entry.metadata().stat.mtime;
- let format = if changed { "*%F %T" } else { " %F %T" };
+ let mut format = change_indicator(changed).to_owned();
+ format.push_str("%F %T");
- proxmox_time::strftime_local(format, mtime.secs).unwrap_or_default()
+ proxmox_time::strftime_local(&format, mtime.secs).unwrap_or_default()
}
fn format_mode(entry: &FileEntry, changed: bool) -> String {
@@ -555,3 +635,57 @@ fn show_file_list(
println!("{op} {entry_type:>2} {mode:>5} {uid:>6} {gid:>6} {size:>10} {mtime:11} {name}");
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ use std::{
+ io::Cursor,
+ pin::Pin,
+ task::{Context, Poll},
+ };
+ use tokio::io::{AsyncRead, ReadBuf};
+
+ struct MockedAsyncReader(Cursor<Vec<u8>>);
+
+ impl AsyncRead for MockedAsyncReader {
+ fn poll_read(
+ self: Pin<&mut Self>,
+ _cx: &mut Context<'_>,
+ read_buf: &mut ReadBuf<'_>,
+ ) -> Poll<std::io::Result<()>> {
+ let mut buf = vec![0u8; 100];
+
+ let res = std::io::Read::read(&mut self.get_mut().0, &mut buf);
+
+ if let Ok(bytes) = res {
+ read_buf.put_slice(&buf[..bytes]);
+ }
+
+ Poll::Ready(res.map(|_| ()))
+ }
+ }
+
+ #[test]
+ fn test_do_compare_file_contents() {
+ fn compare(a: Vec<u8>, b: Vec<u8>) -> Result<bool, Error> {
+ let mut mock_a = MockedAsyncReader(Cursor::new(a));
+ let mut mock_b = MockedAsyncReader(Cursor::new(b));
+
+ proxmox_async::runtime::block_on(compare_readers(&mut mock_a, &mut mock_b))
+ }
+
+ assert!(matches!(compare(vec![0; 15], vec![0; 15]), Ok(true)));
+ assert!(matches!(compare(vec![0; 15], vec![0; 14]), Ok(false)));
+ assert!(matches!(compare(vec![0; 15], vec![1; 15]), Ok(false)));
+
+ let mut buf = vec![1u8; 2 * BUFFERSIZE];
+ buf[BUFFERSIZE] = 0;
+ assert!(matches!(compare(vec![1u8; 2 * BUFFERSIZE], buf), Ok(false)));
+
+ let mut buf = vec![1u8; 2 * BUFFERSIZE];
+ buf[2 * BUFFERSIZE - 1] = 0;
+ assert!(matches!(compare(vec![1u8; 2 * BUFFERSIZE], buf), Ok(false)));
+ }
+}
diff --git a/src/bin/proxmox_backup_manager/ldap.rs b/src/bin/proxmox_backup_manager/ldap.rs
new file mode 100644
index 00000000..a2f10f66
--- /dev/null
+++ b/src/bin/proxmox_backup_manager/ldap.rs
@@ -0,0 +1,102 @@
+use anyhow::Error;
+use serde_json::Value;
+
+use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::REALM_ID_SCHEMA;
+
+use proxmox_backup::api2;
+
+#[api(
+ input: {
+ properties: {
+ "output-format": {
+ schema: OUTPUT_FORMAT,
+ optional: true,
+ },
+ }
+ }
+)]
+/// List configured LDAP realms
+fn list_ldap_realms(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+ let output_format = get_output_format(¶m);
+
+ let info = &api2::config::access::ldap::API_METHOD_LIST_LDAP_REALMS;
+ let mut data = match info.handler {
+ ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+ _ => unreachable!(),
+ };
+
+ let options = default_table_format_options()
+ .column(ColumnConfig::new("realm"))
+ // .column(ColumnConfig::new("issuer-url"))
+ .column(ColumnConfig::new("comment"));
+
+ format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+ Ok(Value::Null)
+}
+#[api(
+ input: {
+ properties: {
+ realm: {
+ schema: REALM_ID_SCHEMA,
+ },
+ "output-format": {
+ schema: OUTPUT_FORMAT,
+ optional: true,
+ },
+ }
+ }
+)]
+
+/// Show LDAP realm configuration
+fn show_ldap_realm(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+ let output_format = get_output_format(¶m);
+
+ let info = &api2::config::access::ldap::API_METHOD_READ_LDAP_REALM;
+ let mut data = match info.handler {
+ ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+ _ => unreachable!(),
+ };
+
+ let options = default_table_format_options();
+ format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+ Ok(Value::Null)
+}
+
+pub fn ldap_commands() -> CommandLineInterface {
+ let cmd_def = CliCommandMap::new()
+ .insert("list", CliCommand::new(&API_METHOD_LIST_LDAP_REALMS))
+ .insert(
+ "show",
+ CliCommand::new(&API_METHOD_SHOW_LDAP_REALM)
+ .arg_param(&["realm"])
+ .completion_cb("realm", pbs_config::domains::complete_ldap_realm_name),
+ )
+ .insert(
+ "create",
+ CliCommand::new(&api2::config::access::ldap::API_METHOD_CREATE_LDAP_REALM)
+ .arg_param(&["realm"])
+ .arg_param(&["realm"])
+ .completion_cb("realm", pbs_config::domains::complete_ldap_realm_name),
+ )
+ .insert(
+ "update",
+ CliCommand::new(&api2::config::access::ldap::API_METHOD_UPDATE_LDAP_REALM)
+ .arg_param(&["realm"])
+ .arg_param(&["realm"])
+ .completion_cb("realm", pbs_config::domains::complete_ldap_realm_name),
+ )
+ .insert(
+ "delete",
+ CliCommand::new(&api2::config::access::ldap::API_METHOD_DELETE_LDAP_REALM)
+ .arg_param(&["realm"])
+ .arg_param(&["realm"])
+ .completion_cb("realm", pbs_config::domains::complete_ldap_realm_name),
+ );
+
+ cmd_def.into()
+}
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH v4 proxmox-backup 3/4] debug cli: add colored output for `diff archive`
2022-12-09 11:14 [pbs-devel] [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive` Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 1/4] debug cli: show more file attributes for `diff archive` command Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 2/4] debug cli: add 'compare-content' flag to " Lukas Wagner
@ 2022-12-09 11:14 ` Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 4/4] debug cli: move parameters into the function signature Lukas Wagner
2022-12-09 12:42 ` [pbs-devel] applied-series: [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive` Wolfgang Bumiller
4 siblings, 0 replies; 6+ messages in thread
From: Lukas Wagner @ 2022-12-09 11:14 UTC (permalink / raw)
To: pbs-devel
This commit adds the `--color` flag to the `diff archive` tool.
Valid values are `always`, `auto` and `never`. `always` and
`never` should be self-explanatory, whereas `auto` will enable
colors unless one of the following is true:
- STDOUT is not a tty
- TERM=dumb is set
- NO_COLOR is set
The tool will highlight changed file attributes in yellow.
Furthermore, (A)dded files are highlighted in green,
(M)odified in yellow and (D)eleted in red.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Cargo.toml | 1 +
src/bin/proxmox_backup_debug/diff.rs | 371 ++++++++++++++++++++++-----
2 files changed, 306 insertions(+), 66 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index 8ee48127..7fac1bfa 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -75,6 +75,7 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
siphasher = "0.3"
syslog = "4.0"
+termcolor = "1.1.2"
tokio = { version = "1.6", features = [ "fs", "io-util", "io-std", "macros", "net", "parking_lot", "process", "rt", "rt-multi-thread", "signal", "time" ] }
tokio-openssl = "0.6.1"
tokio-stream = "0.1.0"
diff --git a/src/bin/proxmox_backup_debug/diff.rs b/src/bin/proxmox_backup_debug/diff.rs
index 06667fe1..2f621f8d 100644
--- a/src/bin/proxmox_backup_debug/diff.rs
+++ b/src/bin/proxmox_backup_debug/diff.rs
@@ -1,5 +1,6 @@
use std::collections::{HashMap, HashSet};
use std::ffi::{OsStr, OsString};
+use std::io::Write;
use std::iter::FromIterator;
use std::path::{Path, PathBuf};
use std::sync::Arc;
@@ -28,6 +29,8 @@ use pbs_tools::json::required_string_param;
use pxar::accessor::ReadAt;
use pxar::EntryKind;
use serde_json::Value;
+
+use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
use tokio::io::AsyncReadExt;
type ChunkDigest = [u8; 32];
@@ -87,6 +90,11 @@ pub fn diff_commands() -> CommandLineInterface {
type: bool,
description: "Compare file content rather than solely relying on mtime for detecting modified files.",
},
+ "color": {
+ optional: true,
+ type: String,
+ description: "Set mode for colored output. Can be `always`, `auto` or `never`. `auto` will display colors only if stdout is a tty. Defaults to `auto`."
+ }
}
}
)]
@@ -106,6 +114,12 @@ async fn diff_archive_cmd(param: Value) -> Result<(), Error> {
None => false,
};
+ let color = match param.get("color") {
+ Some(Value::String(color)) => color.as_str().try_into()?,
+ Some(_) => bail!("invalid color parameter. Valid choices are `always`, `auto` and `never`"),
+ None => ColorMode::Auto,
+ };
+
let namespace = match param.get("ns") {
Some(Value::String(ns)) => ns.parse()?,
Some(_) => bail!("invalid namespace parameter"),
@@ -133,6 +147,8 @@ async fn diff_archive_cmd(param: Value) -> Result<(), Error> {
namespace,
};
+ let output_params = OutputParams { color };
+
if archive_name.ends_with(".pxar") {
let file_name = format!("{}.didx", archive_name);
diff_archive(
@@ -141,6 +157,7 @@ async fn diff_archive_cmd(param: Value) -> Result<(), Error> {
&file_name,
&repo_params,
compare_contents,
+ &output_params,
)
.await?;
} else {
@@ -156,6 +173,7 @@ async fn diff_archive(
file_name: &str,
repo_params: &RepoParams,
compare_contents: bool,
+ output_params: &OutputParams,
) -> Result<(), Error> {
let (index_a, accessor_a) = open_dynamic_index(snapshot_a, file_name, repo_params).await?;
let (index_b, accessor_b) = open_dynamic_index(snapshot_b, file_name, repo_params).await?;
@@ -209,17 +227,40 @@ async fn diff_archive(
// which where *really* modified.
let modified_files = compare_files(potentially_modified, compare_contents).await?;
- show_file_list(&added_files, &deleted_files, &modified_files);
+ show_file_list(&added_files, &deleted_files, &modified_files, output_params)?;
Ok(())
}
+enum ColorMode {
+ Always,
+ Auto,
+ Never,
+}
+
+impl TryFrom<&str> for ColorMode {
+ type Error = Error;
+
+ fn try_from(value: &str) -> Result<Self, Self::Error> {
+ match value {
+ "auto" => Ok(Self::Auto),
+ "always" => Ok(Self::Always),
+ "never" => Ok(Self::Never),
+ _ => bail!("invalid color parameter. Valid choices are `always`, `auto` and `never`"),
+ }
+ }
+}
+
struct RepoParams {
repo: BackupRepository,
crypt_config: Option<Arc<CryptConfig>>,
namespace: BackupNamespace,
}
+struct OutputParams {
+ color: ColorMode,
+}
+
async fn open_dynamic_index(
snapshot: &str,
archive_name: &str,
@@ -533,70 +574,271 @@ impl ChangedProperties {
}
}
-fn change_indicator(changed: bool) -> &'static str {
- if changed {
- "*"
- } else {
- " "
- }
+enum FileOperation {
+ Added,
+ Modified,
+ Deleted,
}
-fn format_filesize(entry: &FileEntry, changed: bool) -> String {
- if let Some(size) = entry.file_size() {
- format!(
- "{}{:.1}",
- change_indicator(changed),
- HumanByte::new_decimal(size as f64)
- )
- } else {
- String::new()
+struct ColumnWidths {
+ operation: usize,
+ entry_type: usize,
+ uid: usize,
+ gid: usize,
+ mode: usize,
+ filesize: usize,
+ mtime: usize,
+}
+
+impl Default for ColumnWidths {
+ fn default() -> Self {
+ Self {
+ operation: 1,
+ entry_type: 2,
+ uid: 6,
+ gid: 6,
+ mode: 6,
+ filesize: 10,
+ mtime: 11,
+ }
}
}
-fn format_mtime(entry: &FileEntry, changed: bool) -> String {
- let mtime = &entry.metadata().stat.mtime;
+struct FileEntryPrinter {
+ stream: StandardStream,
+ column_widths: ColumnWidths,
+ changed_color: Color,
+}
- let mut format = change_indicator(changed).to_owned();
- format.push_str("%F %T");
+impl FileEntryPrinter {
+ pub fn new(output_params: &OutputParams) -> Self {
+ let color_choice = match output_params.color {
+ ColorMode::Always => ColorChoice::Always,
+ ColorMode::Auto => {
+ if unsafe { libc::isatty(1) == 1 } {
+ // Show colors unless `TERM=dumb` or `NO_COLOR` is set.
+ ColorChoice::Auto
+ } else {
+ ColorChoice::Never
+ }
+ }
+ ColorMode::Never => ColorChoice::Never,
+ };
- proxmox_time::strftime_local(&format, mtime.secs).unwrap_or_default()
-}
+ let stdout = StandardStream::stdout(color_choice);
-fn format_mode(entry: &FileEntry, changed: bool) -> String {
- let mode = entry.metadata().stat.mode & 0o7777;
- format!("{}{:o}", change_indicator(changed), mode)
-}
+ Self {
+ stream: stdout,
+ column_widths: ColumnWidths::default(),
+ changed_color: Color::Yellow,
+ }
+ }
-fn format_entry_type(entry: &FileEntry, changed: bool) -> String {
- let kind = match entry.kind() {
- EntryKind::Symlink(_) => "l",
- EntryKind::Hardlink(_) => "h",
- EntryKind::Device(_) if entry.metadata().stat.is_blockdev() => "b",
- EntryKind::Device(_) => "c",
- EntryKind::Socket => "s",
- EntryKind::Fifo => "p",
- EntryKind::File { .. } => "f",
- EntryKind::Directory => "d",
- _ => " ",
- };
+ fn change_indicator(&self, changed: bool) -> &'static str {
+ if changed {
+ "*"
+ } else {
+ " "
+ }
+ }
- format!("{}{}", change_indicator(changed), kind)
-}
+ fn set_color_if_changed(&mut self, changed: bool) -> Result<(), Error> {
+ if changed {
+ self.stream
+ .set_color(ColorSpec::new().set_fg(Some(self.changed_color)))?;
+ }
-fn format_uid(entry: &FileEntry, changed: bool) -> String {
- format!("{}{}", change_indicator(changed), entry.metadata().stat.uid)
-}
+ Ok(())
+ }
-fn format_gid(entry: &FileEntry, changed: bool) -> String {
- format!("{}{}", change_indicator(changed), entry.metadata().stat.gid)
-}
+ fn write_operation(&mut self, op: FileOperation) -> Result<(), Error> {
+ let (text, color) = match op {
+ FileOperation::Added => ("A", Color::Green),
+ FileOperation::Modified => ("M", Color::Yellow),
+ FileOperation::Deleted => ("D", Color::Red),
+ };
-fn format_file_name(entry: &FileEntry, changed: bool) -> String {
- format!(
- "{}{}",
- change_indicator(changed),
- entry.file_name().to_string_lossy()
- )
+ self.stream
+ .set_color(ColorSpec::new().set_fg(Some(color)))?;
+
+ write!(
+ self.stream,
+ "{text:>width$}",
+ width = self.column_widths.operation,
+ )?;
+
+ self.stream.reset()?;
+
+ Ok(())
+ }
+
+ fn write_filesize(&mut self, entry: &FileEntry, changed: bool) -> Result<(), Error> {
+ let output = if let Some(size) = entry.file_size() {
+ format!(
+ "{}{:.1}",
+ self.change_indicator(changed),
+ HumanByte::new_decimal(size as f64)
+ )
+ } else {
+ String::new()
+ };
+
+ self.set_color_if_changed(changed)?;
+ write!(
+ self.stream,
+ "{output:>width$}",
+ width = self.column_widths.filesize,
+ )?;
+ self.stream.reset()?;
+
+ Ok(())
+ }
+
+ fn write_mtime(&mut self, entry: &FileEntry, changed: bool) -> Result<(), Error> {
+ let mtime = &entry.metadata().stat.mtime;
+
+ let mut format = self.change_indicator(changed).to_owned();
+ format.push_str("%F %T");
+
+ let output = proxmox_time::strftime_local(&format, mtime.secs).unwrap_or_default();
+
+ self.set_color_if_changed(changed)?;
+ write!(
+ self.stream,
+ "{output:>width$}",
+ width = self.column_widths.mtime,
+ )?;
+ self.stream.reset()?;
+
+ Ok(())
+ }
+
+ fn write_mode(&mut self, entry: &FileEntry, changed: bool) -> Result<(), Error> {
+ let mode = entry.metadata().stat.mode & 0o7777;
+ let output = format!("{}{:o}", self.change_indicator(changed), mode);
+
+ self.set_color_if_changed(changed)?;
+ write!(
+ self.stream,
+ "{output:>width$}",
+ width = self.column_widths.mode,
+ )?;
+ self.stream.reset()?;
+
+ Ok(())
+ }
+
+ fn write_entry_type(&mut self, entry: &FileEntry, changed: bool) -> Result<(), Error> {
+ let kind = match entry.kind() {
+ EntryKind::Symlink(_) => "l",
+ EntryKind::Hardlink(_) => "h",
+ EntryKind::Device(_) if entry.metadata().stat.is_blockdev() => "b",
+ EntryKind::Device(_) => "c",
+ EntryKind::Socket => "s",
+ EntryKind::Fifo => "p",
+ EntryKind::File { .. } => "f",
+ EntryKind::Directory => "d",
+ _ => " ",
+ };
+
+ let output = format!("{}{}", self.change_indicator(changed), kind);
+
+ self.set_color_if_changed(changed)?;
+ write!(
+ self.stream,
+ "{output:>width$}",
+ width = self.column_widths.entry_type,
+ )?;
+ self.stream.reset()?;
+
+ Ok(())
+ }
+
+ fn write_uid(&mut self, entry: &FileEntry, changed: bool) -> Result<(), Error> {
+ let output = format!(
+ "{}{}",
+ self.change_indicator(changed),
+ entry.metadata().stat.uid
+ );
+
+ self.set_color_if_changed(changed)?;
+ write!(
+ self.stream,
+ "{output:>width$}",
+ width = self.column_widths.uid,
+ )?;
+ self.stream.reset()?;
+ Ok(())
+ }
+
+ fn write_gid(&mut self, entry: &FileEntry, changed: bool) -> Result<(), Error> {
+ let output = format!(
+ "{}{}",
+ self.change_indicator(changed),
+ entry.metadata().stat.gid
+ );
+
+ self.set_color_if_changed(changed)?;
+ write!(
+ self.stream,
+ "{output:>width$}",
+ width = self.column_widths.gid,
+ )?;
+ self.stream.reset()?;
+ Ok(())
+ }
+
+ fn write_file_name(&mut self, entry: &FileEntry, changed: bool) -> Result<(), Error> {
+ self.set_color_if_changed(changed)?;
+ write!(
+ self.stream,
+ "{}{}",
+ self.change_indicator(changed),
+ entry.file_name().to_string_lossy()
+ )?;
+ self.stream.reset()?;
+
+ Ok(())
+ }
+
+ fn write_column_seperator(&mut self) -> Result<(), Error> {
+ write!(self.stream, " ")?;
+ Ok(())
+ }
+
+ /// Print a file entry, including `changed` indicators and column seperators
+ pub fn print_file_entry(
+ &mut self,
+ entry: &FileEntry,
+ changed: &ChangedProperties,
+ operation: FileOperation,
+ ) -> Result<(), Error> {
+ self.write_operation(operation)?;
+ self.write_column_seperator()?;
+
+ self.write_entry_type(entry, changed.entry_type)?;
+ self.write_column_seperator()?;
+
+ self.write_uid(entry, changed.uid)?;
+ self.write_column_seperator()?;
+
+ self.write_gid(entry, changed.gid)?;
+ self.write_column_seperator()?;
+
+ self.write_mode(entry, changed.mode)?;
+ self.write_column_seperator()?;
+
+ self.write_filesize(entry, changed.size)?;
+ self.write_column_seperator()?;
+
+ self.write_mtime(entry, changed.mtime)?;
+ self.write_column_seperator()?;
+
+ self.write_file_name(entry, changed.content)?;
+ writeln!(self.stream)?;
+
+ Ok(())
+ }
}
/// Display a sorted list of added, modified, deleted files.
@@ -604,7 +846,8 @@ fn show_file_list(
added: &HashMap<&OsStr, &FileEntry>,
deleted: &HashMap<&OsStr, &FileEntry>,
modified: &HashMap<&OsStr, (&FileEntry, ChangedProperties)>,
-) {
+ output_params: &OutputParams,
+) -> Result<(), Error> {
let mut all: Vec<&OsStr> = Vec::new();
all.extend(added.keys());
@@ -613,27 +856,23 @@ fn show_file_list(
all.sort();
+ let mut printer = FileEntryPrinter::new(output_params);
+
for file in all {
- let (op, entry, changed) = if let Some(entry) = added.get(file) {
- ("A", entry, ChangedProperties::default())
+ let (operation, entry, changed) = if let Some(entry) = added.get(file) {
+ (FileOperation::Added, entry, ChangedProperties::default())
} else if let Some(entry) = deleted.get(file) {
- ("D", entry, ChangedProperties::default())
+ (FileOperation::Deleted, entry, ChangedProperties::default())
} else if let Some((entry, changed)) = modified.get(file) {
- ("M", entry, *changed)
+ (FileOperation::Modified, entry, *changed)
} else {
unreachable!();
};
- let entry_type = format_entry_type(entry, changed.entry_type);
- let uid = format_uid(entry, changed.uid);
- let gid = format_gid(entry, changed.gid);
- let mode = format_mode(entry, changed.mode);
- let size = format_filesize(entry, changed.size);
- let mtime = format_mtime(entry, changed.mtime);
- let name = format_file_name(entry, changed.content);
-
- println!("{op} {entry_type:>2} {mode:>5} {uid:>6} {gid:>6} {size:>10} {mtime:11} {name}");
+ printer.print_file_entry(entry, &changed, operation)?;
}
+
+ Ok(())
}
#[cfg(test)]
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH v4 proxmox-backup 4/4] debug cli: move parameters into the function signature
2022-12-09 11:14 [pbs-devel] [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive` Lukas Wagner
` (2 preceding siblings ...)
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 3/4] debug cli: add colored output for `diff archive` Lukas Wagner
@ 2022-12-09 11:14 ` Lukas Wagner
2022-12-09 12:42 ` [pbs-devel] applied-series: [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive` Wolfgang Bumiller
4 siblings, 0 replies; 6+ messages in thread
From: Lukas Wagner @ 2022-12-09 11:14 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Changes from v3:
- Use the default mechanism provided by the #[api] macro for `compare_content`
src/bin/proxmox_backup_debug/diff.rs | 65 +++++++++++-----------------
1 file changed, 25 insertions(+), 40 deletions(-)
diff --git a/src/bin/proxmox_backup_debug/diff.rs b/src/bin/proxmox_backup_debug/diff.rs
index 2f621f8d..36c856e9 100644
--- a/src/bin/proxmox_backup_debug/diff.rs
+++ b/src/bin/proxmox_backup_debug/diff.rs
@@ -25,9 +25,9 @@ use pbs_config::key_config::decrypt_key;
use pbs_datastore::dynamic_index::{BufferedDynamicReader, DynamicIndexReader, LocalDynamicReadAt};
use pbs_datastore::index::IndexFile;
use pbs_tools::crypt_config::CryptConfig;
-use pbs_tools::json::required_string_param;
use pxar::accessor::ReadAt;
use pxar::EntryKind;
+use serde::Deserialize;
use serde_json::Value;
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
@@ -88,12 +88,12 @@ pub fn diff_commands() -> CommandLineInterface {
"compare-content": {
optional: true,
type: bool,
+ default: false,
description: "Compare file content rather than solely relying on mtime for detecting modified files.",
},
"color": {
optional: true,
- type: String,
- description: "Set mode for colored output. Can be `always`, `auto` or `never`. `auto` will display colors only if stdout is a tty. Defaults to `auto`."
+ type: ColorMode,
}
}
}
@@ -102,29 +102,19 @@ pub fn diff_commands() -> CommandLineInterface {
/// For modified files, the file metadata (e.g. mode, uid, gid, size, etc.) will be considered. For detecting
/// modification of file content, only mtime will be used by default. If the --compare-content flag is provided,
/// mtime is ignored and file content will be compared.
-async fn diff_archive_cmd(param: Value) -> Result<(), Error> {
+async fn diff_archive_cmd(
+ prev_snapshot: String,
+ snapshot: String,
+ archive_name: String,
+ compare_content: bool,
+ color: Option<ColorMode>,
+ ns: Option<BackupNamespace>,
+ param: Value,
+) -> Result<(), Error> {
let repo = extract_repository_from_value(¶m)?;
- let snapshot_a = required_string_param(¶m, "prev-snapshot")?;
- let snapshot_b = required_string_param(¶m, "snapshot")?;
- let archive_name = required_string_param(¶m, "archive-name")?;
-
- let compare_contents = match param.get("compare-content") {
- Some(Value::Bool(value)) => *value,
- Some(_) => bail!("invalid flag for compare-content"),
- None => false,
- };
- let color = match param.get("color") {
- Some(Value::String(color)) => color.as_str().try_into()?,
- Some(_) => bail!("invalid color parameter. Valid choices are `always`, `auto` and `never`"),
- None => ColorMode::Auto,
- };
-
- let namespace = match param.get("ns") {
- Some(Value::String(ns)) => ns.parse()?,
- Some(_) => bail!("invalid namespace parameter"),
- None => BackupNamespace::root(),
- };
+ let color = color.unwrap_or_default();
+ let namespace = ns.unwrap_or_else(BackupNamespace::root);
let crypto = crypto_parameters(¶m)?;
@@ -152,11 +142,11 @@ async fn diff_archive_cmd(param: Value) -> Result<(), Error> {
if archive_name.ends_with(".pxar") {
let file_name = format!("{}.didx", archive_name);
diff_archive(
- snapshot_a,
- snapshot_b,
+ &prev_snapshot,
+ &snapshot,
&file_name,
&repo_params,
- compare_contents,
+ compare_content,
&output_params,
)
.await?;
@@ -232,25 +222,20 @@ async fn diff_archive(
Ok(())
}
+#[api(default: "auto")]
+#[derive(Default, Deserialize)]
+#[serde(rename_all = "lowercase")]
+/// Color output options
enum ColorMode {
+ /// Always output colors
Always,
+ /// Output colors if STDOUT is a tty and neither of TERM=dumb or NO_COLOR is set
+ #[default]
Auto,
+ /// Never output colors
Never,
}
-impl TryFrom<&str> for ColorMode {
- type Error = Error;
-
- fn try_from(value: &str) -> Result<Self, Self::Error> {
- match value {
- "auto" => Ok(Self::Auto),
- "always" => Ok(Self::Always),
- "never" => Ok(Self::Never),
- _ => bail!("invalid color parameter. Valid choices are `always`, `auto` and `never`"),
- }
- }
-}
-
struct RepoParams {
repo: BackupRepository,
crypt_config: Option<Arc<CryptConfig>>,
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] applied-series: [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive`
2022-12-09 11:14 [pbs-devel] [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive` Lukas Wagner
` (3 preceding siblings ...)
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 4/4] debug cli: move parameters into the function signature Lukas Wagner
@ 2022-12-09 12:42 ` Wolfgang Bumiller
4 siblings, 0 replies; 6+ messages in thread
From: Wolfgang Bumiller @ 2022-12-09 12:42 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pbs-devel
applied series, thanks
On Fri, Dec 09, 2022 at 12:14:22PM +0100, Lukas Wagner wrote:
> This patch series contains a few improvements for the `diff archive` tool,
> mainly based on Wolfgang's and Thomas' suggestions.
>
> First, the output of is now much more detailed and shows
> some relevant file attributes, including what has changed between
> snapshots. Changed attributes are highlighted by a "*" and are
> highlighted by colored text.
>
> For instance:
>
> $ proxmox-backup-debug diff archive ... --compare-content
> A f 644 10045 10000 0 B 2022-11-28 13:44:51 add.txt
> M f 644 10045 10000 6 B *2022-11-28 13:45:05 *content.txt
> D f 644 10045 10000 0 B 2022-11-28 13:17:09 deleted.txt
> M f 644 10045 *29 0 B 2022-11-28 13:16:20 gid.txt
> M f *777 10045 10000 0 B 2022-11-28 13:42:47 mode.txt
> M f 644 10045 10000 0 B *2022-11-28 13:44:33 mtime.txt
> M f 644 10045 10000 *7 B *2022-11-28 13:44:59 size.txt
> M f 644 *64045 10000 0 B 2022-11-28 13:16:18 uid.txt
> M *f 644 10045 10000 10 B 2022-11-28 13:44:59 type.txt
>
> Furthermore, there now exists the possiblity to pass
> the --compare-content flag to the tool. If the flag is passed,
> the tool will compare the file content instead of relying on mtime
> alone to detect modifications.
>
> This patch series adds new dependencies to the `termcolor` crate.
> That being said, the crate was already pulled in by other crates as
> transitive dependencies.
>
> Changes from v3:
> - Detect if device node changes from blockdev -> chardev and vice versa.
> - Use the default mechanism provided by the #[api] macro for `compare_content`
>
> Changes from v2:
> - Increase buffersize to 4K
> - Added workaround for weird issue with tokio::try_join! and #[api]
> - Drop dependency on `atty` crate
> - Remove modificiations to debian/control
> - `diff_archive_cmd`: Moved parameters into the function signature,
> instead of manually extracting it from `Value`.
>
>
> Changes from v1:
> - Made `changed indicator for file content` a bit less confusing.
> For regular files, it is now only displayed if
> - the --comapare-content flag is set and
> - the file contents *actually* differ
>
> - Removed unnecessesary namespace prefix for std::task::Pin in unit tests
> - Added color output, controllable via --color {always,auto,never} flag.
>
> Lukas Wagner (4):
> debug cli: show more file attributes for `diff archive` command
> debug cli: add 'compare-content' flag to `diff archive` command
> debug cli: add colored output for `diff archive`
> debug cli: move parameters into the function signature
>
> Cargo.toml | 1 +
> src/bin/proxmox_backup_debug/diff.rs | 613 ++++++++++++++++++++++---
> src/bin/proxmox_backup_manager/ldap.rs | 102 ++++
> 3 files changed, 646 insertions(+), 70 deletions(-)
> create mode 100644 src/bin/proxmox_backup_manager/ldap.rs
>
> --
> 2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-09 12:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-09 11:14 [pbs-devel] [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive` Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 1/4] debug cli: show more file attributes for `diff archive` command Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 2/4] debug cli: add 'compare-content' flag to " Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 3/4] debug cli: add colored output for `diff archive` Lukas Wagner
2022-12-09 11:14 ` [pbs-devel] [PATCH v4 proxmox-backup 4/4] debug cli: move parameters into the function signature Lukas Wagner
2022-12-09 12:42 ` [pbs-devel] applied-series: [PATCH v4 proxmox-backup 0/4] debug cli: improve output, optionally compare file content for `diff archive` Wolfgang Bumiller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox