From a223b4df3bb16412981b5b3f16d6202d9fcaee28 Mon Sep 17 00:00:00 2001 From: James Graham Date: Mon, 18 Dec 2017 15:44:54 +0000 Subject: [PATCH 1/3] Update the runner creation API to be a builder. This mirrors the API of std::process::Command, and provides more flexible access to the handles for stdout and stderr than the previous iteration of the API. --- src/runner.rs | 205 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 138 insertions(+), 67 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 08fbb4d..980465c 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -5,20 +5,45 @@ use std::collections::HashMap; use std::convert::From; use std::env; use std::error::Error; +use std::ffi::{OsStr, OsString}; use std::fmt; use std::io::{Result as IoResult, Error as IoError, ErrorKind}; use std::path::{Path, PathBuf}; +use std::process::{Command, Child, Stdio}; use std::process; -use std::process::{Command, Stdio}; pub trait Runner { - fn args(&mut self) -> &mut Vec; - fn build_command(&self, &mut Command); - fn envs(&mut self) -> &mut HashMap; - fn is_running(&mut self) -> bool; - fn start(&mut self) -> Result<(), RunnerError>; + type Process; + + fn arg<'a, S> (&'a mut self, arg: S) -> &'a mut Self where + S: AsRef; + + fn args<'a, I, S>(&'a mut self, args: I) -> &'a mut Self where + I: IntoIterator, + S: AsRef; + + fn env<'a, K, V>(&'a mut self, key: K, value: V) -> &'a mut Self where + K: AsRef, + V: AsRef; + + fn envs<'a, I, K, V>(&'a mut self, envs: I) -> &'a mut Self where + I: IntoIterator, + K: AsRef, + V: AsRef; + + fn stdout<'a, T>(&'a mut self, stdout: T) -> &'a mut Self where + T: Into; + + fn stderr<'a, T>(&'a mut self, stderr: T) -> &'a mut Self where + T: Into; + + fn start(self) -> Result; +} + +pub trait RunnerProcess { fn status(&mut self) -> IoResult>; - fn stop(&mut self) -> IoResult>; + fn stop(&mut self) -> IoResult; + fn is_running(&mut self) -> bool; } #[derive(Debug)] @@ -66,94 +91,140 @@ impl From for RunnerError { } } +#[derive(Debug)] +pub struct FirefoxProcess { + process: Child +} + + +impl RunnerProcess for FirefoxProcess { + fn status(&mut self) -> IoResult> { + self.process.try_wait() + } + + fn is_running(&mut self) -> bool { + self.status().unwrap().is_none() + } + + fn stop(&mut self) -> IoResult { + self.process.kill()?; + self.process.wait() + } +} + #[derive(Debug)] pub struct FirefoxRunner { - pub binary: PathBuf, - args: Vec, - envs: HashMap, - process: Option, - pub profile: Profile + binary: PathBuf, + profile: Profile, + args: Vec, + envs: HashMap, + stdout: Option, + stderr: Option, } impl FirefoxRunner { - pub fn new(binary: &Path, profile: Option) -> IoResult { - let prof = match profile { - Some(p) => p, - None => try!(Profile::new(None)) - }; + pub fn new(binary: &Path, profile: Profile) -> FirefoxRunner { + let mut envs: HashMap = HashMap::new(); + envs.insert("MOZ_NO_REMOTE".into(), "1".into()); + envs.insert("NO_EM_RESTART".into(), "1".into()); - let mut envs = HashMap::new(); - envs.insert("MOZ_NO_REMOTE".to_string(), "1".to_string()); - envs.insert("NO_EM_RESTART".to_string(), "1".to_string()); - - Ok(FirefoxRunner { + FirefoxRunner { binary: binary.to_path_buf(), - process: None, - args: Vec::new(), envs: envs, - profile: prof - }) + profile: profile, + args: vec![], + stdout: None, + stderr: None, + } } } impl Runner for FirefoxRunner { - fn args(&mut self) -> &mut Vec { - &mut self.args - } + type Process = FirefoxProcess; - fn build_command(&self, command: &mut Command) { - command - .args(&self.args[..]) - .envs(&self.envs); + fn arg<'a, S> (&'a mut self, arg: S) -> &'a mut FirefoxRunner where + S: AsRef + { + self.args.push((&arg).into()); + self + } - if !self.args.iter().any(|x| is_profile_arg(x)) { - command.arg("-profile").arg(&self.profile.path); + fn args<'a, I, S>(&'a mut self, args: I) -> &'a mut FirefoxRunner where + I: IntoIterator, + S: AsRef + { + for arg in args { + self.args.push((&arg).into()); } - command.stdout(Stdio::inherit()) - .stderr(Stdio::inherit()); + self } - fn envs(&mut self) -> &mut HashMap { - &mut self.envs + fn env<'a, K, V>(&'a mut self, key: K, value: V) -> &'a mut FirefoxRunner where + K: AsRef, + V: AsRef + { + self.envs.insert((&key).into(), (&value).into()); + self } - fn is_running(&mut self) -> bool { - self.process.is_some() && self.status().unwrap().is_none() + fn envs<'a, I, K, V>(&'a mut self, envs: I) -> &'a mut FirefoxRunner where + I: IntoIterator, + K: AsRef, + V: AsRef + { + for (key, value) in envs { + self.envs.insert((&key).into(), (&value).into()); + } + self } - fn start(&mut self) -> Result<(), RunnerError> { - let mut cmd = Command::new(&self.binary); - self.build_command(&mut cmd); - - let prefs = try!(self.profile.user_prefs()); - try!(prefs.write()); - - info!("Running command: {:?}", cmd); - let process = try!(cmd.spawn()); - self.process = Some(process); - Ok(()) + fn stdout<'a, T>(&'a mut self, stdout: T) -> &'a mut Self where + T: Into + { + self.stdout = Some(stdout.into()); + self } - fn status(&mut self) -> IoResult> { - self.process.as_mut().map(|p| p.try_wait()).unwrap_or(Ok(None)) + fn stderr<'a, T>(&'a mut self, stderr: T) -> &'a mut Self where + T: Into { + self.stderr = Some(stderr.into()); + self } - fn stop(&mut self) -> IoResult> { - let mut retval = None; + fn start(mut self) -> Result { + let stdout = self.stdout.unwrap_or_else(|| Stdio::inherit()); + let stderr = self.stderr.unwrap_or_else(|| Stdio::inherit()); - if let Some(ref mut p) = self.process { - try!(p.kill()); - retval = Some(try!(p.wait())); - }; - Ok(retval) + let mut cmd = Command::new(&self.binary); + cmd.args(&self.args[..]) + .envs(&self.envs) + .stdout(stdout) + .stderr(stderr); + + if !self.args.iter().any(|x| is_profile_arg(x)) { + cmd.arg("-profile").arg(&self.profile.path); + } + cmd.stdout(Stdio::inherit()) + .stderr(Stdio::inherit()); + + self.profile.user_prefs()?.write()?; + + info!("Running command: {:?}", cmd); + let process = cmd.spawn()?; + Ok(FirefoxProcess { + process: process + }) } } -fn parse_arg_name(arg: &str) -> Option<&str> { +fn parse_arg_name(arg: T) -> Option where T: AsRef { + let arg_os_str: &OsStr = arg.as_ref(); + let arg_str = arg_os_str.to_string_lossy(); + let mut start = 0; let mut end = 0; - for (i, c) in arg.chars().enumerate() { + for (i, c) in arg_str.chars().enumerate() { if i == 0 { if !platform::arg_prefix_char(c) { break; @@ -178,7 +249,7 @@ fn parse_arg_name(arg: &str) -> Option<&str> { } if start > 0 && end > start { - Some(&arg[start..end]) + Some(arg_str[start..end].into()) } else { None } @@ -193,7 +264,7 @@ fn name_end_char(c: char) -> bool { /// Returns a boolean indicating whether a given string /// contains one of the `-P`, `-Profile` or `-ProfileManager` /// arguments, respecting the various platform-specific conventions. -pub fn is_profile_arg(arg: &str) -> bool { +pub fn is_profile_arg(arg: T) -> bool where T: AsRef { if let Some(name) = parse_arg_name(arg) { name.eq_ignore_ascii_case("profile") || name.eq_ignore_ascii_case("p") || @@ -335,7 +406,7 @@ mod tests { fn parse(arg: &str, name: Option<&str>) { let result = parse_arg_name(arg); - assert_eq!(result, name); + assert_eq!(result, name.map(|x| x.to_string())); } #[test] @@ -374,7 +445,7 @@ mod tests { #[test] fn test_is_profile_arg() { - assert!(is_profile_arg("--profile")); + assert!(is_profile_arg(&"--profile")); assert!(is_profile_arg("-p")); assert!(is_profile_arg("-PROFILEMANAGER")); assert!(is_profile_arg("-ProfileMANAGER")); From 9cb0f239b0291c919bf269a7e43ead1ca54d6429 Mon Sep 17 00:00:00 2001 From: James Graham Date: Tue, 19 Dec 2017 12:34:45 +0000 Subject: [PATCH 2/3] Format code using the default nightly rustfmt --- src/runner.rs | 132 ++++++++++++++++++++++++++++---------------------- 1 file changed, 74 insertions(+), 58 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 980465c..27462a1 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -7,34 +7,40 @@ use std::env; use std::error::Error; use std::ffi::{OsStr, OsString}; use std::fmt; -use std::io::{Result as IoResult, Error as IoError, ErrorKind}; +use std::io::{Error as IoError, ErrorKind, Result as IoResult}; use std::path::{Path, PathBuf}; -use std::process::{Command, Child, Stdio}; +use std::process::{Child, Command, Stdio}; use std::process; pub trait Runner { type Process; - fn arg<'a, S> (&'a mut self, arg: S) -> &'a mut Self where + fn arg<'a, S>(&'a mut self, arg: S) -> &'a mut Self + where S: AsRef; - fn args<'a, I, S>(&'a mut self, args: I) -> &'a mut Self where + fn args<'a, I, S>(&'a mut self, args: I) -> &'a mut Self + where I: IntoIterator, S: AsRef; - fn env<'a, K, V>(&'a mut self, key: K, value: V) -> &'a mut Self where + fn env<'a, K, V>(&'a mut self, key: K, value: V) -> &'a mut Self + where K: AsRef, V: AsRef; - fn envs<'a, I, K, V>(&'a mut self, envs: I) -> &'a mut Self where + fn envs<'a, I, K, V>(&'a mut self, envs: I) -> &'a mut Self + where I: IntoIterator, K: AsRef, V: AsRef; - fn stdout<'a, T>(&'a mut self, stdout: T) -> &'a mut Self where + fn stdout<'a, T>(&'a mut self, stdout: T) -> &'a mut Self + where T: Into; - fn stderr<'a, T>(&'a mut self, stderr: T) -> &'a mut Self where + fn stderr<'a, T>(&'a mut self, stderr: T) -> &'a mut Self + where T: Into; fn start(self) -> Result; @@ -61,12 +67,10 @@ impl fmt::Display for RunnerError { impl Error for RunnerError { fn description(&self) -> &str { match *self { - RunnerError::Io(ref err) => { - match err.kind() { - ErrorKind::NotFound => "no such file or directory", - _ => err.description(), - } - } + RunnerError::Io(ref err) => match err.kind() { + ErrorKind::NotFound => "no such file or directory", + _ => err.description(), + }, RunnerError::PrefReader(ref err) => err.description(), } } @@ -93,10 +97,9 @@ impl From for RunnerError { #[derive(Debug)] pub struct FirefoxProcess { - process: Child + process: Child, } - impl RunnerProcess for FirefoxProcess { fn status(&mut self) -> IoResult> { self.process.try_wait() @@ -142,16 +145,18 @@ impl FirefoxRunner { impl Runner for FirefoxRunner { type Process = FirefoxProcess; - fn arg<'a, S> (&'a mut self, arg: S) -> &'a mut FirefoxRunner where - S: AsRef + fn arg<'a, S>(&'a mut self, arg: S) -> &'a mut FirefoxRunner + where + S: AsRef, { self.args.push((&arg).into()); self } - fn args<'a, I, S>(&'a mut self, args: I) -> &'a mut FirefoxRunner where + fn args<'a, I, S>(&'a mut self, args: I) -> &'a mut FirefoxRunner + where I: IntoIterator, - S: AsRef + S: AsRef, { for arg in args { self.args.push((&arg).into()); @@ -159,18 +164,20 @@ impl Runner for FirefoxRunner { self } - fn env<'a, K, V>(&'a mut self, key: K, value: V) -> &'a mut FirefoxRunner where + fn env<'a, K, V>(&'a mut self, key: K, value: V) -> &'a mut FirefoxRunner + where K: AsRef, - V: AsRef + V: AsRef, { self.envs.insert((&key).into(), (&value).into()); self } - fn envs<'a, I, K, V>(&'a mut self, envs: I) -> &'a mut FirefoxRunner where + fn envs<'a, I, K, V>(&'a mut self, envs: I) -> &'a mut FirefoxRunner + where I: IntoIterator, K: AsRef, - V: AsRef + V: AsRef, { for (key, value) in envs { self.envs.insert((&key).into(), (&value).into()); @@ -178,15 +185,18 @@ impl Runner for FirefoxRunner { self } - fn stdout<'a, T>(&'a mut self, stdout: T) -> &'a mut Self where - T: Into + fn stdout<'a, T>(&'a mut self, stdout: T) -> &'a mut Self + where + T: Into, { self.stdout = Some(stdout.into()); self } - fn stderr<'a, T>(&'a mut self, stderr: T) -> &'a mut Self where - T: Into { + fn stderr<'a, T>(&'a mut self, stderr: T) -> &'a mut Self + where + T: Into, + { self.stderr = Some(stderr.into()); self } @@ -204,20 +214,20 @@ impl Runner for FirefoxRunner { if !self.args.iter().any(|x| is_profile_arg(x)) { cmd.arg("-profile").arg(&self.profile.path); } - cmd.stdout(Stdio::inherit()) - .stderr(Stdio::inherit()); + cmd.stdout(Stdio::inherit()).stderr(Stdio::inherit()); self.profile.user_prefs()?.write()?; info!("Running command: {:?}", cmd); let process = cmd.spawn()?; - Ok(FirefoxProcess { - process: process - }) + Ok(FirefoxProcess { process: process }) } } -fn parse_arg_name(arg: T) -> Option where T: AsRef { +fn parse_arg_name(arg: T) -> Option +where + T: AsRef, +{ let arg_os_str: &OsStr = arg.as_ref(); let arg_str = arg_os_str.to_string_lossy(); @@ -264,28 +274,28 @@ fn name_end_char(c: char) -> bool { /// Returns a boolean indicating whether a given string /// contains one of the `-P`, `-Profile` or `-ProfileManager` /// arguments, respecting the various platform-specific conventions. -pub fn is_profile_arg(arg: T) -> bool where T: AsRef { +pub fn is_profile_arg(arg: T) -> bool +where + T: AsRef, +{ if let Some(name) = parse_arg_name(arg) { - name.eq_ignore_ascii_case("profile") || - name.eq_ignore_ascii_case("p") || - name.eq_ignore_ascii_case("profilemanager") + name.eq_ignore_ascii_case("profile") || name.eq_ignore_ascii_case("p") + || name.eq_ignore_ascii_case("profilemanager") } else { false } } fn find_binary(name: &str) -> Option { - env::var("PATH") - .ok() - .and_then(|path_env| { - for mut path in env::split_paths(&*path_env) { - path.push(name); - if path.exists() { - return Some(path) - } + env::var("PATH").ok().and_then(|path_env| { + for mut path in env::split_paths(&*path_env) { + path.push(name); + if path.exists() { + return Some(path); } - None - }) + } + None + }) } #[cfg(target_os = "linux")] @@ -310,19 +320,24 @@ pub mod platform { pub fn firefox_default_path() -> Option { if let Some(path) = find_binary("firefox-bin") { - return Some(path) + return Some(path); } let home = env::home_dir(); for &(prefix_home, trial_path) in [ - (false, "/Applications/Firefox.app/Contents/MacOS/firefox-bin"), - (true, "Applications/Firefox.app/Contents/MacOS/firefox-bin")].iter() { + ( + false, + "/Applications/Firefox.app/Contents/MacOS/firefox-bin", + ), + (true, "Applications/Firefox.app/Contents/MacOS/firefox-bin"), + ].iter() + { let path = match (home.as_ref(), prefix_home) { (Some(ref home_dir), true) => home_dir.join(trial_path), (None, true) => continue, - (_, false) => PathBuf::from(trial_path) + (_, false) => PathBuf::from(trial_path), }; if path.exists() { - return Some(path) + return Some(path); } } None @@ -345,7 +360,7 @@ pub mod platform { let opt_path = firefox_registry_path().unwrap_or(None); if let Some(path) = opt_path { if path.exists() { - return Some(path) + return Some(path); } }; find_binary("firefox.exe") @@ -355,9 +370,10 @@ pub mod platform { let hklm = RegKey::predef(HKEY_LOCAL_MACHINE); for subtree_key in ["SOFTWARE", "SOFTWARE\\WOW6432Node"].iter() { let subtree = try!(hklm.open_subkey_with_flags(subtree_key, KEY_READ)); - let mozilla_org = match subtree.open_subkey_with_flags("mozilla.org\\Mozilla", KEY_READ) { + let mozilla_org = match subtree.open_subkey_with_flags("mozilla.org\\Mozilla", KEY_READ) + { Ok(val) => val, - Err(_) => continue + Err(_) => continue, }; let current_version: String = try!(mozilla_org.get_value("CurrentVersion")); let mozilla = try!(subtree.open_subkey_with_flags("Mozilla", KEY_READ)); @@ -372,7 +388,7 @@ pub mod platform { if let Ok(bin_subtree) = mozilla.open_subkey_with_flags(bin_key, KEY_READ) { let path: Result = bin_subtree.get_value("PathToExe"); if let Ok(path) = path { - return Ok(Some(PathBuf::from(path))) + return Ok(Some(PathBuf::from(path))); } } } @@ -402,7 +418,7 @@ pub mod platform { #[cfg(test)] mod tests { - use super::{parse_arg_name, is_profile_arg}; + use super::{is_profile_arg, parse_arg_name}; fn parse(arg: &str, name: Option<&str>) { let result = parse_arg_name(arg); From f510e7a8d82d632b7392f45d9a574bb8e4f74e1a Mon Sep 17 00:00:00 2001 From: James Graham Date: Tue, 19 Dec 2017 18:35:52 +0000 Subject: [PATCH 3/3] fixup! Update the runner creation API to be a builder. The profile has to be kept alive over the lifetime of the process. --- src/runner.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/runner.rs b/src/runner.rs index 27462a1..1586cb1 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -98,6 +98,7 @@ impl From for RunnerError { #[derive(Debug)] pub struct FirefoxProcess { process: Child, + profile: Profile } impl RunnerProcess for FirefoxProcess { @@ -220,7 +221,10 @@ impl Runner for FirefoxRunner { info!("Running command: {:?}", cmd); let process = cmd.spawn()?; - Ok(FirefoxProcess { process: process }) + Ok(FirefoxProcess { + process: process, + profile: self.profile + }) } }