From a414c3060b2b74ec57b78c3d41e4a9d52fa71d26 Mon Sep 17 00:00:00 2001 From: cat_in_136 Date: Sat, 1 Jun 2024 19:43:31 +0900 Subject: [PATCH 1/4] add tests module to cli --- src/cli.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/cli.rs b/src/cli.rs index 4c0a442..1482777 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -152,3 +152,13 @@ impl ValueEnum for AutoReqMode { }) } } + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn verify_cli() { + use clap::CommandFactory; + Cli::command().debug_assert() + } +} From 668918b237beedfdd0b9f80fb39bbdb340530bb5 Mon Sep 17 00:00:00 2001 From: cat_in_136 Date: Sat, 1 Jun 2024 20:32:34 +0900 Subject: [PATCH 2/4] add test codes --- src/cli.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/cli.rs b/src/cli.rs index 1482777..a4531ed 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -161,4 +161,50 @@ mod tests { use clap::CommandFactory; Cli::command().debug_assert() } + + #[test] + fn test_metadata_overwrite() { + let args = Cli::try_parse_from([ + "", + "--metadata-overwrite", + "TOML_FILE.toml", + "--metadata-overwrite", + "TOML_FILE.toml#TOML.PATH", + ]) + .unwrap(); + assert_eq!( + args.metadata_overwrite, + vec!["TOML_FILE.toml", "TOML_FILE.toml#TOML.PATH"] + ); + } + + #[test] + fn test_set_metadata() { + let args = Cli::try_parse_from([ + "", + "-s", + "toml \"text1\"", + "--set-metadata", + "toml \"text2\"", + ]) + .unwrap(); + assert_eq!( + args.set_metadata, + vec!["TOML_FILE.toml", "TOML_FILE.toml#TOML.PATH"] + ); + } + + #[test] + fn test_auto_req() { + //let args = Cli::try_parse_from(["", "--auto-req", "auto"]).unwrap(); + //assert!(matches!(args.auto_req, Some(AutoReqMode::Auto))); + let args = Cli::try_parse_from(["", "--auto-req", "builtin"]).unwrap(); + assert!(matches!(args.auto_req, Some(AutoReqMode::Builtin))); + let args = Cli::try_parse_from(["", "--auto-req", "find-requires"]).unwrap(); + assert!(matches!(args.auto_req, Some(AutoReqMode::FindRequires))); + //let args = Cli::try_parse_from(["", "--auto-req", "/usr/lib/rpm/find-requires"]).unwrap(); + //assert!(matches!(args.auto_req, Some(AutoReqMode::Script(v)) if v.eq("/usr/lib/rpm/find-requires"))); + let args = Cli::try_parse_from(["", "--auto-req", "no"]).unwrap(); + assert!(matches!(args.auto_req, Some(AutoReqMode::Disabled))); + } } From eb7a34b7e9308c7f86f5020c04820b81c1112202 Mon Sep 17 00:00:00 2001 From: cat_in_136 Date: Sun, 2 Jun 2024 01:45:30 +0900 Subject: [PATCH 3/4] fix bug where `--auto-req` option does not accept `auto` and the script path --- Cargo.toml | 1 + src/auto_req/mod.rs | 17 +++--- src/cli.rs | 123 ++++++++++++++++++++++++++------------------ src/config/mod.rs | 3 +- 4 files changed, 82 insertions(+), 62 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0b14518..29bd1f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ rpm = { version = "0.13.1", default-features = false } toml = "0.7" cargo_toml = "0.15" clap = { version = "4.3", features = ["derive"] } +color-print = "0.3" thiserror = "1" elf = "0.7" diff --git a/src/auto_req/mod.rs b/src/auto_req/mod.rs index ca7bfb4..7c76971 100644 --- a/src/auto_req/mod.rs +++ b/src/auto_req/mod.rs @@ -19,17 +19,14 @@ pub enum AutoReqMode { BuiltIn, } -impl From<&Option> for AutoReqMode { - fn from(value: &Option) -> Self { - use cli::AutoReqMode as M; - use AutoReqMode::*; - +impl From for AutoReqMode { + fn from(value: cli::AutoReqMode) -> Self { match value { - None => Auto, - Some(M::Disabled) => Disabled, - Some(M::Builtin) => BuiltIn, - Some(M::Script(path)) => Script(path.into()), - Some(M::FindRequires) => Script(PathBuf::from(RPM_FIND_REQUIRES)), + cli::AutoReqMode::Auto => AutoReqMode::Auto, + cli::AutoReqMode::Disabled => AutoReqMode::Disabled, + cli::AutoReqMode::Builtin => AutoReqMode::BuiltIn, + cli::AutoReqMode::FindRequires => AutoReqMode::Script(PathBuf::from(RPM_FIND_REQUIRES)), + cli::AutoReqMode::Script(path) => AutoReqMode::Script(path), } } } diff --git a/src/cli.rs b/src/cli.rs index a4531ed..f06cc9e 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,4 +1,8 @@ -use clap::{builder::PossibleValue, Parser, ValueEnum}; +use clap::{ + builder::{PathBufValueParser, PossibleValuesParser, TypedValueParser, ValueParserFactory}, + Arg, Command, Parser, ValueEnum, +}; +use std::ffi::OsStr; use std::path::PathBuf; /// Wrapper used when the application is executed as Cargo plugin @@ -29,7 +33,18 @@ pub struct Cli { pub package: Option, /// Automatic dependency processing mode. - #[arg(long)] + #[arg(long, + help = "Automatic dependency processing mode. \ + [default: auto] \ + [possible values: auto, disabled, builtin, find-requires, /path/to/find-requires]", + long_help = color_print::cstr!("Automatic dependency processing mode.\n\n\ + [default: auto]\n\ + Possible values:\n\ + - auto: Automatic discovery of dependencies.\n\ + - disabled: Disable automatic discovery of dependencies. [alias: no]\n\ + - builtin: Use the builtin procedure based on ldd.\n\ + - find-requires: Use the external program specified in RPM_FIND_REQUIRES.\n\ + - /path/to/find-requires: Use the specified external program."))] pub auto_req: Option, /// Sub-directory name for all generated artifacts. May be @@ -102,54 +117,59 @@ impl From for rpm::CompressionWithLevel { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum AutoReqMode { + Auto, Disabled, Builtin, FindRequires, - Script(String), + Script(PathBuf), } -static AUTO_REQ_VARIANTS: &[AutoReqMode] = &[ - AutoReqMode::Disabled, - AutoReqMode::Builtin, - AutoReqMode::FindRequires, - AutoReqMode::Script(String::new()), -]; +impl ValueParserFactory for AutoReqMode { + type Parser = AutoReqModeParser; -impl ValueEnum for AutoReqMode { - fn value_variants<'a>() -> &'a [Self] { - AUTO_REQ_VARIANTS + fn value_parser() -> Self::Parser { + AutoReqModeParser } +} - fn to_possible_value(&self) -> Option { - use AutoReqMode::*; - - let val = match self { - Disabled => PossibleValue::new("disabled") - .help("Disable automatic discovery of dependencies") - .alias("no"), - Builtin => { - PossibleValue::new("builtin").help("Use the builtin procedure based on ldd.") +#[derive(Clone, Debug)] +pub struct AutoReqModeParser; + +impl TypedValueParser for AutoReqModeParser { + type Value = AutoReqMode; + fn parse_ref( + &self, + cmd: &Command, + arg: Option<&Arg>, + value: &OsStr, + ) -> Result { + const VALUES: [(&str, AutoReqMode); 5] = [ + ("auto", AutoReqMode::Auto), + ("disabled", AutoReqMode::Disabled), + ("no", AutoReqMode::Disabled), + ("builtin", AutoReqMode::Builtin), + ("find-requires", AutoReqMode::FindRequires), + ]; + + let inner = PossibleValuesParser::new(VALUES.iter().map(|(k, _v)| k)); + match inner.parse_ref(cmd, arg, value) { + Ok(name) => Ok(VALUES + .iter() + .find(|(k, _v)| name.as_str() == (k.as_ref() as &str)) + .unwrap() + .1 + .clone()), + Err(e) if e.kind() == clap::error::ErrorKind::InvalidValue => { + let inner = PathBufValueParser::new(); + match inner.parse_ref(cmd, arg, value) { + Ok(v) => Ok(AutoReqMode::Script(v)), + Err(e) => Err(e), + } } - FindRequires => PossibleValue::new("find-requires") - .help("Use the external program specified in RPM_FIND_REQUIRES."), - _ => PossibleValue::new("/path/to/find-requires") - .help("Use the specified external program."), - }; - Some(val) - } - - // Provided method - fn from_str(input: &str, ignore_case: bool) -> Result { - let lowercase = String::from(input).to_lowercase(); - let val = if ignore_case { &lowercase } else { input }; - Ok(match val { - "disabled" => Self::Disabled, - "builtin" => Self::Builtin, - "find-requires" => Self::FindRequires, - _ => Self::Script(input.into()), - }) + Err(e) => Err(e), + } } } @@ -188,23 +208,24 @@ mod tests { "toml \"text2\"", ]) .unwrap(); - assert_eq!( - args.set_metadata, - vec!["TOML_FILE.toml", "TOML_FILE.toml#TOML.PATH"] - ); + assert_eq!(args.set_metadata, vec!["toml \"text1\"", "toml \"text2\""]); } #[test] fn test_auto_req() { - //let args = Cli::try_parse_from(["", "--auto-req", "auto"]).unwrap(); - //assert!(matches!(args.auto_req, Some(AutoReqMode::Auto))); + let args = Cli::try_parse_from([""]).unwrap(); + assert_eq!(args.auto_req, None); + let args = Cli::try_parse_from(["", "--auto-req", "auto"]).unwrap(); + assert_eq!(args.auto_req, Some(AutoReqMode::Auto)); let args = Cli::try_parse_from(["", "--auto-req", "builtin"]).unwrap(); - assert!(matches!(args.auto_req, Some(AutoReqMode::Builtin))); + assert_eq!(args.auto_req, Some(AutoReqMode::Builtin)); let args = Cli::try_parse_from(["", "--auto-req", "find-requires"]).unwrap(); - assert!(matches!(args.auto_req, Some(AutoReqMode::FindRequires))); - //let args = Cli::try_parse_from(["", "--auto-req", "/usr/lib/rpm/find-requires"]).unwrap(); - //assert!(matches!(args.auto_req, Some(AutoReqMode::Script(v)) if v.eq("/usr/lib/rpm/find-requires"))); + assert_eq!(args.auto_req, Some(AutoReqMode::FindRequires)); + let args = Cli::try_parse_from(["", "--auto-req", "/usr/lib/rpm/find-requires"]).unwrap(); + assert!( + matches!(args.auto_req, Some(AutoReqMode::Script(v)) if v == PathBuf::from("/usr/lib/rpm/find-requires")) + ); let args = Cli::try_parse_from(["", "--auto-req", "no"]).unwrap(); - assert!(matches!(args.auto_req, Some(AutoReqMode::Disabled))); + assert_eq!(args.auto_req, Some(AutoReqMode::Disabled)); } } diff --git a/src/config/mod.rs b/src/config/mod.rs index 1ad8d15..69e74a6 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -303,7 +303,8 @@ impl Config { let meta_aut_req = metadata.get_str("auto-req")?; let auto_req = match (&cfg.args.auto_req, meta_aut_req) { (None, Some("no" | "disabled")) => AutoReqMode::Disabled, - (r, _) => r.into(), + (None, _) => AutoReqMode::Auto, + (Some(v), _) => AutoReqMode::from(v.clone()), }; for requires in find_requires(expanded_file_paths, auto_req)? { From 56007e27220f4a0222faf72fc4cb5266585dc954 Mon Sep 17 00:00:00 2001 From: cat_in_136 Date: Sun, 2 Jun 2024 08:48:10 +0900 Subject: [PATCH 4/4] refactor and rewrite comments and doc of auto-req --- README.md | 16 ++++++++-------- src/auto_req/mod.rs | 1 + src/cli.rs | 24 +++++++++++------------- src/config/mod.rs | 5 ++--- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index f4e54cb..7478fa5 100644 --- a/README.md +++ b/README.md @@ -131,17 +131,17 @@ It is necessary to place a space between version and symbols such as `<`, `<=`, This command automatically determines what shared libraries a package requires. There may be times when the automatic dependency processing is not desired. -In this case, the package author may set `package.metadata.generate-rpm.auto-req` to `"no"` or -the user who executes this command may specify command line option `--auto-req no`. +The packege author and users can configure the processing. -* `--auto-req auto`: The following rules are used to determine the preferred automatic dependency process: +* `--auto-req auto` or `--auto-req` not specified: Use the preferred automatic dependency process. + The following rules are used: * If `package.metadata.generate-rpm.auto-req` set to `"no"` or `"disabled"`, the process is disabled. - * If `/usr/lib/rpm/find-requires` exists, it is used (same behaviour as `--auto-req /usr/lib/rpm/find-requires`). + * If `/usr/lib/rpm/find-requires` exists, it is used (same behaviour as `--auto-req find-requires`). * Otherwise, builtin procedure is used (same behaviour as `--auto-req builtin`). -* `--auto-req builtin`: the builtin procedure using `ldd` is used. -* `--auto-req /path/to/find-requires`: the specified external program is used. This behavior is the same as the - original `rpmbuild`. -* `--auto-req no`: the process is disabled. +* `--auto-req disabled`, `--auto-req no`: Disable the discovery of dependencies. +* `--auto-req builtin`: Use the builtin procedure based on `ldd`. +* `--auto-req find-requires`: Use `/usr/lib/rpm/find-requires`. This behavior is the same as the original `rpmbuild`. +* `--auto-req /path/to/find-requires`: Use the specified external program is used. `/bin/sh` is always added to the package requirements. To disable it, set `package.metadata.generate-rpm.require-sh` to `false`. You should not do this if you use scripts such as `pre_install_script` or if your assets contain shell diff --git a/src/auto_req/mod.rs b/src/auto_req/mod.rs index 7c76971..581222a 100644 --- a/src/auto_req/mod.rs +++ b/src/auto_req/mod.rs @@ -4,6 +4,7 @@ use std::path::{Path, PathBuf}; mod builtin; mod script; +/// The path to the system default find-requires program const RPM_FIND_REQUIRES: &str = "/usr/lib/rpm/find-requires"; /// The method to auto-req diff --git a/src/cli.rs b/src/cli.rs index f06cc9e..5b1f2c1 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -33,19 +33,17 @@ pub struct Cli { pub package: Option, /// Automatic dependency processing mode. - #[arg(long, + #[arg(long, default_value = "auto", help = "Automatic dependency processing mode. \ - [default: auto] \ [possible values: auto, disabled, builtin, find-requires, /path/to/find-requires]", long_help = color_print::cstr!("Automatic dependency processing mode.\n\n\ - [default: auto]\n\ Possible values:\n\ - - auto: Automatic discovery of dependencies.\n\ - - disabled: Disable automatic discovery of dependencies. [alias: no]\n\ + - auto: Use the preferred automatic dependency process.\n\ + - disabled: Disable the discovery of dependencies. [alias: no]\n\ - builtin: Use the builtin procedure based on ldd.\n\ - - find-requires: Use the external program specified in RPM_FIND_REQUIRES.\n\ + - find-requires: Use /usr/lib/rpm/find-requires.\n\ - /path/to/find-requires: Use the specified external program."))] - pub auto_req: Option, + pub auto_req: AutoReqMode, /// Sub-directory name for all generated artifacts. May be /// specified with CARGO_BUILD_TARGET environment @@ -214,18 +212,18 @@ mod tests { #[test] fn test_auto_req() { let args = Cli::try_parse_from([""]).unwrap(); - assert_eq!(args.auto_req, None); + assert_eq!(args.auto_req, AutoReqMode::Auto); let args = Cli::try_parse_from(["", "--auto-req", "auto"]).unwrap(); - assert_eq!(args.auto_req, Some(AutoReqMode::Auto)); + assert_eq!(args.auto_req, AutoReqMode::Auto); let args = Cli::try_parse_from(["", "--auto-req", "builtin"]).unwrap(); - assert_eq!(args.auto_req, Some(AutoReqMode::Builtin)); + assert_eq!(args.auto_req, AutoReqMode::Builtin); let args = Cli::try_parse_from(["", "--auto-req", "find-requires"]).unwrap(); - assert_eq!(args.auto_req, Some(AutoReqMode::FindRequires)); + assert_eq!(args.auto_req, AutoReqMode::FindRequires); let args = Cli::try_parse_from(["", "--auto-req", "/usr/lib/rpm/find-requires"]).unwrap(); assert!( - matches!(args.auto_req, Some(AutoReqMode::Script(v)) if v == PathBuf::from("/usr/lib/rpm/find-requires")) + matches!(args.auto_req, AutoReqMode::Script(v) if v == PathBuf::from("/usr/lib/rpm/find-requires")) ); let args = Cli::try_parse_from(["", "--auto-req", "no"]).unwrap(); - assert_eq!(args.auto_req, Some(AutoReqMode::Disabled)); + assert_eq!(args.auto_req, AutoReqMode::Disabled); } } diff --git a/src/config/mod.rs b/src/config/mod.rs index 69e74a6..a903381 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -302,9 +302,8 @@ impl Config { let meta_aut_req = metadata.get_str("auto-req")?; let auto_req = match (&cfg.args.auto_req, meta_aut_req) { - (None, Some("no" | "disabled")) => AutoReqMode::Disabled, - (None, _) => AutoReqMode::Auto, - (Some(v), _) => AutoReqMode::from(v.clone()), + (crate::cli::AutoReqMode::Auto, Some("no" | "disabled")) => AutoReqMode::Disabled, + (v, _) => AutoReqMode::from(v.clone()), }; for requires in find_requires(expanded_file_paths, auto_req)? {