[pmg-devel] applied: [PATCH log-tracker v2] use pico-args instead of clap
Stoiko Ivanov
s.ivanov at proxmox.com
Tue Feb 13 15:56:49 CET 2024
Nice one! - Thanks for the patch
Applied with Mira's R-b, T-b tags
(and gave it a spin earlier as well)
On Wed, 25 Oct 2023 13:20:26 +0200
Dominik Csapak <d.csapak at proxmox.com> wrote:
> Instead of upgrading from clap3 to clap4 (which seems to change their
> interface every year or so), switch to the much smaller pico-args. (Same
> as we did for termproxy recently, see [0])
>
> It has almost all features we need (except producing help output) and
> supports OsString, but wihout any dependencies. This decreases compile
> time and reduces the size of the resulting binary. It also reduces the
> lines of code.
>
> The only difference is now the different output for errors, e.g. for
> missing values of options.
>
> Help output is copied from the old clap output.
>
> 0: https://git.proxmox.com/?p=pve-xtermjs.git;a=commitdiff;h=24d707d0506b120a085b06b5f2b6000696879a1e
>
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> changes from v1:
> * extracted the pkg_version into a variable for interpolated use
> * improved commit message, e.g. by referencing the termproxy commit
>
> Cargo.toml | 2 +-
> src/main.rs | 223 +++++++++++++++++++---------------------------------
> 2 files changed, 83 insertions(+), 142 deletions(-)
>
> diff --git a/Cargo.toml b/Cargo.toml
> index 725976f..82a9a4f 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -11,7 +11,7 @@ exclude = [ "build", "debian" ]
>
> [dependencies]
> anyhow = "1"
> -clap = { version = "3.2.23", features = ["cargo"] }
> flate2 = "1.0"
> libc = "0.2"
> proxmox-time = "1.1"
> +pico-args = { version = "0.4", features = ["combined-flags"] }
> diff --git a/src/main.rs b/src/main.rs
> index e55f17b..e7aeeb4 100644
> --- a/src/main.rs
> +++ b/src/main.rs
> @@ -13,111 +13,55 @@ use anyhow::{bail, Error};
> use flate2::read;
> use libc::time_t;
>
> -use clap::{App, Arg};
> -
> mod time;
> use time::{Tm, CAL_MTOD};
>
> +fn print_usage() {
> + let pkg_version = env!("CARGO_PKG_VERSION");
> + println!(
> + "\
> +pmg-log-tracker {pkg_version}
> +Proxmox Mailgateway Log Tracker. Tool to scan mail logs.
> +
> +USAGE:
> + pmg-log-tracker [OPTIONS]
> +
> +OPTIONS:
> + -e, --endtime <TIME> End time (YYYY-MM-DD HH:MM:SS) or seconds since epoch
> + -f, --from <SENDER> Mails from SENDER
> + -g, --exclude-greylist Exclude greylist entries
> + -h, --host <HOST> Hostname or Server IP
> + --help Print help information
> + -i, --inputfile <INPUTFILE> Input file to use instead of /var/log/syslog, or '-' for stdin
> + -l, --limit <MAX> Print MAX entries [default: 0]
> + -m, --message-id <MSGID> Message ID (exact match)
> + -n, --exclude-ndr Exclude NDR entries
> + -q, --queue-id <QID> Queue ID (exact match), can be specified multiple times
> + -s, --starttime <TIME> Start time (YYYY-MM-DD HH:MM:SS) or seconds since epoch
> + -t, --to <RECIPIENT> Mails to RECIPIENT
> + -v, --verbose Verbose output, can be specified multiple times
> + -V, --version Print version information
> + -x, --search-string <STRING> Search for string",
> + );
> +}
> +
> fn main() -> Result<(), Error> {
> - let matches = App::new(clap::crate_name!())
> - .version(clap::crate_version!())
> - .about(clap::crate_description!())
> - .arg(
> - Arg::with_name("verbose")
> - .short('v')
> - .long("verbose")
> - .help("Verbose output, can be specified multiple times")
> - .action(clap::ArgAction::Count),
> - )
> - .arg(
> - Arg::with_name("inputfile")
> - .short('i')
> - .long("inputfile")
> - .help("Input file to use instead of /var/log/syslog, or '-' for stdin")
> - .value_name("INPUTFILE"),
> - )
> - .arg(
> - Arg::with_name("host")
> - .short('h')
> - .long("host")
> - .help("Hostname or Server IP")
> - .value_name("HOST"),
> - )
> - .arg(
> - Arg::with_name("from")
> - .short('f')
> - .long("from")
> - .help("Mails from SENDER")
> - .value_name("SENDER"),
> - )
> - .arg(
> - Arg::with_name("to")
> - .short('t')
> - .long("to")
> - .help("Mails to RECIPIENT")
> - .value_name("RECIPIENT"),
> - )
> - .arg(
> - Arg::with_name("start")
> - .short('s')
> - .long("starttime")
> - .help("Start time (YYYY-MM-DD HH:MM:SS) or seconds since epoch")
> - .value_name("TIME"),
> - )
> - .arg(
> - Arg::with_name("end")
> - .short('e')
> - .long("endtime")
> - .help("End time (YYYY-MM-DD HH:MM:SS) or seconds since epoch")
> - .value_name("TIME"),
> - )
> - .arg(
> - Arg::with_name("msgid")
> - .short('m')
> - .long("message-id")
> - .help("Message ID (exact match)")
> - .value_name("MSGID"),
> - )
> - .arg(
> - Arg::with_name("qids")
> - .short('q')
> - .long("queue-id")
> - .help("Queue ID (exact match), can be specified multiple times")
> - .value_name("QID")
> - .multiple(true)
> - .number_of_values(1),
> - )
> - .arg(
> - Arg::with_name("search")
> - .short('x')
> - .long("search-string")
> - .help("Search for string")
> - .value_name("STRING"),
> - )
> - .arg(
> - Arg::with_name("limit")
> - .short('l')
> - .long("limit")
> - .help("Print MAX entries")
> - .value_name("MAX")
> - .default_value("0"),
> - )
> - .arg(
> - Arg::with_name("exclude_greylist")
> - .short('g')
> - .long("exclude-greylist")
> - .help("Exclude greylist entries"),
> - )
> - .arg(
> - Arg::with_name("exclude_ndr")
> - .short('n')
> - .long("exclude-ndr")
> - .help("Exclude NDR entries"),
> - )
> - .get_matches();
> + let mut args = pico_args::Arguments::from_env();
> + if args.contains("--help") {
> + print_usage();
> + return Ok(());
> + }
>
> let mut parser = Parser::new()?;
> - parser.handle_args(matches)?;
> + parser.handle_args(&mut args)?;
> +
> + let remaining_options = args.finish();
> + if !remaining_options.is_empty() {
> + bail!(
> + "Found invalid arguments: {:?}",
> + remaining_options.join(", ".as_ref())
> + )
> + }
>
> println!("# LogReader: {}", std::process::id());
> println!("# Query options");
> @@ -1970,16 +1914,16 @@ impl Parser {
> count + 1
> }
>
> - fn handle_args(&mut self, args: clap::ArgMatches) -> Result<(), Error> {
> - if let Some(inputfile) = args.value_of("inputfile") {
> - self.options.inputfile = inputfile.to_string();
> + fn handle_args(&mut self, args: &mut pico_args::Arguments) -> Result<(), Error> {
> + if let Some(inputfile) = args.opt_value_from_str(["-i", "--inputfile"])? {
> + self.options.inputfile = inputfile;
> }
>
> - if let Some(start) = args.value_of("start") {
> - if let Ok(res) = time::strptime(start, c_str!("%F %T")) {
> + if let Some(start) = args.opt_value_from_str::<_, String>(["-s", "--starttime"])? {
> + if let Ok(res) = time::strptime(&start, c_str!("%F %T")) {
> self.options.start = res.as_utc_to_epoch();
> self.start_tm = res;
> - } else if let Ok(res) = time::strptime(start, c_str!("%s")) {
> + } else if let Ok(res) = time::strptime(&start, c_str!("%s")) {
> self.options.start = res.as_utc_to_epoch();
> self.start_tm = res;
> } else {
> @@ -1994,11 +1938,11 @@ impl Parser {
> self.start_tm = ltime;
> }
>
> - if let Some(end) = args.value_of("end") {
> - if let Ok(res) = time::strptime(end, c_str!("%F %T")) {
> + if let Some(end) = args.opt_value_from_str::<_, String>(["-e", "--endtime"])? {
> + if let Ok(res) = time::strptime(&end, c_str!("%F %T")) {
> self.options.end = res.as_utc_to_epoch();
> self.end_tm = res;
> - } else if let Ok(res) = time::strptime(end, c_str!("%s")) {
> + } else if let Ok(res) = time::strptime(&end, c_str!("%s")) {
> self.options.end = res.as_utc_to_epoch();
> self.end_tm = res;
> } else {
> @@ -2013,53 +1957,50 @@ impl Parser {
> bail!("end time before start time");
> }
>
> - self.options.limit = match args.value_of("limit") {
> + self.options.limit = match args.opt_value_from_str::<_, String>(["-l", "--limit"])? {
> Some(l) => l.parse().unwrap(),
> None => 0,
> };
>
> - if let Some(qids) = args.values_of("qids") {
> - for q in qids {
> - let ltime: time_t = 0;
> - let rel_line_nr: libc::c_ulong = 0;
> - let input = CString::new(q)?;
> - let bytes = concat!("T%08lXL%08lX", "\0");
> - let format =
> - unsafe { std::ffi::CStr::from_bytes_with_nul_unchecked(bytes.as_bytes()) };
> - if unsafe {
> - libc::sscanf(input.as_ptr(), format.as_ptr(), <ime, &rel_line_nr) == 2
> - } {
> - self.options
> - .match_list
> - .push(Match::RelLineNr(ltime, rel_line_nr));
> - } else {
> - self.options
> - .match_list
> - .push(Match::Qid(q.as_bytes().into()));
> - }
> + while let Some(q) = args.opt_value_from_str::<_, String>(["-q", "--queue-id"])? {
> + let ltime: time_t = 0;
> + let rel_line_nr: libc::c_ulong = 0;
> + let input = CString::new(q.as_str())?;
> + let bytes = concat!("T%08lXL%08lX", "\0");
> + let format = unsafe { std::ffi::CStr::from_bytes_with_nul_unchecked(bytes.as_bytes()) };
> + if unsafe { libc::sscanf(input.as_ptr(), format.as_ptr(), <ime, &rel_line_nr) == 2 } {
> + self.options
> + .match_list
> + .push(Match::RelLineNr(ltime, rel_line_nr));
> + } else {
> + self.options
> + .match_list
> + .push(Match::Qid(q.as_bytes().into()));
> }
> }
>
> - if let Some(from) = args.value_of("from") {
> - self.options.from = from.to_string();
> + if let Some(from) = args.opt_value_from_str(["-f", "--from"])? {
> + self.options.from = from;
> }
> - if let Some(to) = args.value_of("to") {
> - self.options.to = to.to_string();
> + if let Some(to) = args.opt_value_from_str(["-t", "--to"])? {
> + self.options.to = to;
> }
> - if let Some(host) = args.value_of("host") {
> - self.options.host = host.to_string();
> + if let Some(host) = args.opt_value_from_str(["-h", "--host"])? {
> + self.options.host = host;
> }
> - if let Some(msgid) = args.value_of("msgid") {
> - self.options.msgid = msgid.to_string();
> + if let Some(msgid) = args.opt_value_from_str(["-m", "--msgid"])? {
> + self.options.msgid = msgid;
> }
>
> - self.options.exclude_greylist = args.is_present("exclude_greylist");
> - self.options.exclude_ndr = args.is_present("exclude_ndr");
> + self.options.exclude_greylist = args.contains(["-g", "--exclude-greylist"]);
> + self.options.exclude_ndr = args.contains(["-n", "--exclude-ndr"]);
>
> - self.options.verbose = args.get_count("verbose") as _;
> + while args.contains(["-v", "--verbose"]) {
> + self.options.verbose += 1;
> + }
>
> - if let Some(string_match) = args.value_of("search") {
> - self.options.string_match = string_match.to_string();
> + if let Some(string_match) = args.opt_value_from_str(["-x", "--search-string"])? {
> + self.options.string_match = string_match;
> }
>
> Ok(())
More information about the pmg-devel
mailing list