sirenia: Add cli module to libsirenia to avoid duplication.
This refactors code from sirenia::cli into a reusable form in
libsirenia::cli so it can be used by cronista.
BUG=None
TEST=cargo test --workspace
Cq-Depend: chromium:2602628
Change-Id: Ibf736493a6ab68f5636e18ac14602daf4c7d0a37
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2601643
Tested-by: Allen Webb <allenwebb@google.com>
Reviewed-by: Nicole Anderson-Au <nvaa@google.com>
Commit-Queue: Allen Webb <allenwebb@google.com>
diff --git a/sirenia/Cargo.toml b/sirenia/Cargo.toml
index 600ed96..b12b952 100644
--- a/sirenia/Cargo.toml
+++ b/sirenia/Cargo.toml
@@ -39,3 +39,4 @@
libsirenia = { path = "libsirenia" } # provided by ebuild
serde = { version = "1.0.114", features = ["derive"] }
sys_util = { path = "../../platform/crosvm/sys_util" } # provided by ebuild
+thiserror = "1.0.20"
diff --git a/sirenia/libsirenia/Cargo.toml b/sirenia/libsirenia/Cargo.toml
index 5b36cbc..d2f2dd6 100644
--- a/sirenia/libsirenia/Cargo.toml
+++ b/sirenia/libsirenia/Cargo.toml
@@ -7,6 +7,7 @@
[dependencies]
flexbuffers = "0.1.1"
+getopts = "0.2"
libc = "0.2.44"
libchromeos = { path = "../../libchromeos-rs" } # provided by ebuild
minijail = { path = "../../../aosp/external/minijail/rust/minijail" } # provided by ebuild
diff --git a/sirenia/libsirenia/src/cli/mod.rs b/sirenia/libsirenia/src/cli/mod.rs
new file mode 100644
index 0000000..31e6801
--- /dev/null
+++ b/sirenia/libsirenia/src/cli/mod.rs
@@ -0,0 +1,228 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+use std::marker::PhantomData;
+use std::process::exit;
+
+use getopts::{self, Matches, Options};
+use thiserror::Error as ThisError;
+
+use crate::transport::{self, TransportType, LOOPBACK_DEFAULT};
+
+#[derive(ThisError, Debug)]
+pub enum Error {
+ #[error("failed to parse transport type: {0}")]
+ TransportParse(transport::Error),
+}
+
+/// The result of an operation in this crate.
+pub type Result<T> = std::result::Result<T, Error>;
+
+const DEFAULT_TRANSPORT_TYPE_SHORT_NAME: &str = "U";
+const DEFAULT_TRANSPORT_TYPE_LONG_NAME: &str = "server-url";
+const DEFAULT_TRANSPORT_TYPE_DESC: &str = "URL to the server";
+
+const HELP_OPTION_SHORT_NAME: &str = "h";
+
+/// A TransportType command-line option that allows for multiple transport options to be defined for
+/// the same set of arguments.
+pub struct TransportTypeOption {
+ short_name: String,
+}
+
+impl TransportTypeOption {
+ /// Add the default TransportTypeOption to the specified Options.
+ pub fn default(mut opts: &mut Options) -> Self {
+ Self::new(
+ DEFAULT_TRANSPORT_TYPE_SHORT_NAME,
+ DEFAULT_TRANSPORT_TYPE_LONG_NAME,
+ DEFAULT_TRANSPORT_TYPE_DESC,
+ LOOPBACK_DEFAULT,
+ &mut opts,
+ )
+ }
+
+ /// Add a customized TransportTypeOption to the specified Options.
+ /// Prefer Self::default unless it has already been used.
+ pub fn new(
+ short_name: &str,
+ long_name: &str,
+ desc: &str,
+ hint: &str,
+ opts: &mut Options,
+ ) -> Self {
+ opts.optflagopt(short_name, long_name, desc, hint);
+ TransportTypeOption {
+ short_name: short_name.to_string(),
+ }
+ }
+
+ /// Checks the command line argument matches and returns:
+ /// * Ok(None) - if the option wasn't set.
+ /// * Ok(Some(_: TransportType)) - if parsing succeeded.
+ /// * Err(Error::TransportParse(_)) - if parsing failed.
+ pub fn from_matches(&self, matches: &Matches) -> Result<Option<TransportType>> {
+ match matches.opt_str(&self.short_name) {
+ Some(value) => value
+ .parse::<TransportType>()
+ .map_err(Error::TransportParse)
+ .map(Some),
+ None => Ok(None),
+ }
+ }
+}
+
+trait PrivateTrait {}
+
+/// A help option that wraps commonly used logic. Specifically, it defines the "-h", "--help"
+/// options and provides a helper for showing the usage strings when the option is set or when
+/// the command line options fail to parse.
+pub struct HelpOption {
+ // Force use of the constructor through a private field.
+ phantom: PhantomData<dyn PrivateTrait>,
+}
+
+impl HelpOption {
+ /// Adds a newly created HelpOption to the specified Options.
+ pub fn new(opts: &mut Options) -> Self {
+ let help = HelpOption {
+ phantom: PhantomData,
+ };
+ opts.optflag(help.get_short_name(), "help", "Show this help string.");
+ help
+ }
+
+ /// Return the short_name used by the HelpOption for direct use with Options.
+ pub fn get_short_name(&self) -> &str {
+ HELP_OPTION_SHORT_NAME
+ }
+
+ /// Mocks process::exit for testability. See: parse_and_check_self
+ fn parse_and_check_self_impl<F: Fn(i32) -> ()>(
+ &self,
+ opts: &Options,
+ args: &[String],
+ get_usage: fn() -> String,
+ exit_fn: F,
+ ) -> Option<Matches> {
+ let matches = opts.parse(args).map(Some).unwrap_or_else(|e| {
+ eprintln!("{}", e);
+ println!("{}", opts.usage(&get_usage()));
+ exit_fn(1);
+ None
+ })?;
+
+ if matches.opt_present(self.get_short_name()) {
+ println!("{}", opts.usage(&get_usage()));
+ exit_fn(0);
+ }
+
+ Some(matches)
+ }
+
+ /// A wrapper around Options::parse that handles printing the help string on a parsing error or
+ /// when the help option is set.
+ pub fn parse_and_check_self(
+ &self,
+ opts: &Options,
+ args: &[String],
+ get_usage: fn() -> String,
+ ) -> Matches {
+ self.parse_and_check_self_impl(opts, args, get_usage, |x| exit(x))
+ .unwrap()
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ use std::cell::RefCell;
+ use std::rc::Rc;
+
+ fn test_usage() -> String {
+ "test usage string".to_string()
+ }
+
+ fn get_counted_mock_exit(expected: i32) -> (Rc<RefCell<usize>>, impl Fn(i32)) {
+ let counter = Rc::new(RefCell::new(0 as usize));
+ (counter.clone(), move |code: i32| {
+ assert_eq!(code, expected);
+ *counter.borrow_mut() += 1;
+ })
+ }
+
+ #[test]
+ fn transporttypeoption_frommatches_notset() {
+ let test_args: Vec<String> = Vec::new();
+
+ let mut opts = Options::new();
+ let url_option = TransportTypeOption::default(&mut opts);
+
+ let matches = opts.parse(&test_args).unwrap();
+
+ assert!(matches!(url_option.from_matches(&matches), Ok(None)));
+ }
+
+ #[test]
+ fn transporttypeoption_frommatches_valid() {
+ let test_args: Vec<String> = vec!["-U".to_string(), LOOPBACK_DEFAULT.to_string()];
+
+ let mut opts = Options::new();
+ let url_option = TransportTypeOption::default(&mut opts);
+
+ let matches = opts.parse(&test_args).unwrap();
+ assert!(matches!(url_option.from_matches(&matches), Ok(Some(_))));
+ }
+
+ #[test]
+ fn transporttypeoption_frommatches_notvalid() {
+ let test_args: Vec<String> =
+ vec!["-U".to_string(), "not a valid transport type".to_string()];
+
+ let mut opts = Options::new();
+ let url_option = TransportTypeOption::default(&mut opts);
+
+ let matches = opts.parse(&test_args).unwrap();
+ assert!(matches!(
+ url_option.from_matches(&matches),
+ Err(Error::TransportParse(_))
+ ));
+ }
+
+ #[test]
+ fn helpoption_parseandcheckself_notset() {
+ let test_args: Vec<String> = Vec::new();
+
+ let mut opts = Options::new();
+ let help_option = HelpOption::new(&mut opts);
+ help_option.parse_and_check_self_impl(&opts, &test_args, test_usage, |_| panic!());
+ }
+
+ #[test]
+ fn helpoption_parseandcheckself_set() {
+ let test_args: Vec<String> = vec!["-h".to_string()];
+ let (counter, exit_fn) = get_counted_mock_exit(0);
+
+ let mut opts = Options::new();
+ let help_option = HelpOption::new(&mut opts);
+ let matches =
+ help_option.parse_and_check_self_impl(&opts, &test_args, test_usage, |x| exit_fn(x));
+ assert!(matches.is_some());
+ assert_eq!(*counter.borrow(), 1);
+ }
+
+ #[test]
+ fn helpoption_parseandcheckself_invalid() {
+ let test_args: Vec<String> = vec!["--not-an-option".to_string()];
+ let (counter, exit_fn) = get_counted_mock_exit(1);
+
+ let mut opts = Options::new();
+ let help_option = HelpOption::new(&mut opts);
+ let matches =
+ help_option.parse_and_check_self_impl(&opts, &test_args, test_usage, |x| exit_fn(x));
+ assert!(matches.is_none());
+ assert_eq!(*counter.borrow(), 1);
+ }
+}
diff --git a/sirenia/libsirenia/src/lib.rs b/sirenia/libsirenia/src/lib.rs
index 09f824e..eea97f1 100644
--- a/sirenia/libsirenia/src/lib.rs
+++ b/sirenia/libsirenia/src/lib.rs
@@ -5,6 +5,7 @@
//! Ties together the various modules that make up the Sirenia library used by
//! both Trichechus and Dugong.
+pub mod cli;
pub mod communication;
pub mod linux;
pub mod sandbox;
diff --git a/sirenia/src/cli/mod.rs b/sirenia/src/cli/mod.rs
index 2315d45..60c132c 100644
--- a/sirenia/src/cli/mod.rs
+++ b/sirenia/src/cli/mod.rs
@@ -6,31 +6,19 @@
//! Trichechus and Dugong daemons.
use std::env::current_exe;
-use std::fmt::{self, Display};
-use std::process::exit;
use getopts::{self, Options};
use libchromeos::vsock::{SocketAddr as VSocketAddr, VsockCid};
-use libsirenia::transport::{self, TransportType, DEFAULT_SERVER_PORT, LOOPBACK_DEFAULT};
+use libsirenia::cli::{self, HelpOption, TransportTypeOption};
+use libsirenia::transport::{TransportType, DEFAULT_SERVER_PORT};
+use thiserror::Error as ThisError;
use super::build_info::BUILD_TIMESTAMP;
-#[derive(Debug)]
+#[derive(ThisError, Debug)]
pub enum Error {
- /// Error parsing command line options.
- CLIParse(getopts::Fail),
- TransportParse(transport::Error),
-}
-
-impl Display for Error {
- fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- use self::Error::*;
-
- match self {
- CLIParse(e) => write!(f, "failed to parse the command line options: {}", e),
- TransportParse(e) => write!(f, "failed to parse transport type: {}", e),
- }
- }
+ #[error("failed to get transport type option: {0}")]
+ FromMatches(cli::Error),
}
/// The result of an operation in this crate.
@@ -74,30 +62,17 @@
connection_type: default_connection,
};
- let url_name = "U";
-
let mut opts = Options::new();
- opts.optflagopt(
- url_name,
- "server-url",
- "URL to the server",
- LOOPBACK_DEFAULT,
- );
- opts.optflag("h", "help", "Show this help string.");
- let matches = opts.parse(&args[..]).map_err(|e| {
- println!("{}", opts.usage(&get_name_and_version_string()));
- Error::CLIParse(e)
- })?;
+ let help_option = HelpOption::new(&mut opts);
+ let url_option = TransportTypeOption::default(&mut opts);
- if matches.opt_present("h") {
- println!("{}", opts.usage(&get_name_and_version_string()));
- exit(0);
- }
+ let matches = help_option.parse_and_check_self(&opts, &args[..], get_name_and_version_string);
- if let Some(value) = matches.opt_str(url_name) {
- config.connection_type = value
- .parse::<TransportType>()
- .map_err(Error::TransportParse)?
+ if let Some(value) = url_option
+ .from_matches(&matches)
+ .map_err(Error::FromMatches)?
+ {
+ config.connection_type = value;
};
Ok(config)
}
@@ -109,16 +84,6 @@
use std::net::{IpAddr, Ipv4Addr, SocketAddr};
#[test]
- fn initialize_common_arguments_invalid_args() {
- let value: [String; 1] = ["-foo".to_string()];
- let act_result = initialize_common_arguments(&value);
- match &act_result {
- Err(Error::CLIParse(_)) => (),
- _ => panic!("Got unexpected result: {:?}", &act_result),
- }
- }
-
- #[test]
fn initialize_common_arguments_ip_valid() {
let exp_socket = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(1, 1, 1, 1)), 1234);
let exp_result = CommonConfig {
diff --git a/sirenia/src/dugong.rs b/sirenia/src/dugong.rs
index 4fe318b..3026940 100644
--- a/sirenia/src/dugong.rs
+++ b/sirenia/src/dugong.rs
@@ -215,9 +215,6 @@
error!("{}", s);
println!("{}", s);
}
- _ => {
- panic!("unexpected message received: {:?}", message);
- }
}
}
}
