Merge #349

349: Refactoring. r=therealprof a=reitermarkus



Co-authored-by: Markus Reiter <me@reitermark.us>

Cargo.toml

@@ -7,7 +7,7 @@ keywords = ["cross", "compilation", "testing", "tool"]
 license = "MIT OR Apache-2.0"
 name = "cross"
 repository = "https://github.com/rust-embedded/cross"
-version = "0.1.16"
+version = "0.2.0-alpha.1"
 edition = "2018"
 
 [dependencies]

src/cargo.rs

@@ -19,15 +19,15 @@ pub enum Subcommand {
 }
 
 impl Subcommand {
-    pub fn needs_docker(&self) -> bool {
-        match *self {
+    pub fn needs_docker(self) -> bool {
+        match self {
             Subcommand::Other => false,
             _ => true,
         }
     }
 
-    pub fn needs_interpreter(&self) -> bool {
-        match *self {
+    pub fn needs_interpreter(self) -> bool {
+        match self {
             Subcommand::Run | Subcommand::Test | Subcommand::Bench => true,
             _ => false,
         }

src/cli.rs

@@ -36,8 +36,8 @@ pub fn parse(target_list: &TargetList) -> Args {
     }
 
     Args {
-        all: all,
+        all,
         subcommand: sc,
-        target: target,
+        target,
     }
 }

src/docker.rs

@@ -134,12 +134,12 @@ pub fn run(target: &Target,
     }
 
     if let Ok(value) = env::var("DOCKER_OPTS") {
-        let opts: Vec<&str> = value.split(" ").collect();
+        let opts: Vec<&str> = value.split(' ').collect();
         docker.args(&opts);
     }
 
     docker
-        .args(&["-e", &format!("CROSS_RUNNER={}", runner.unwrap_or_else(|| String::new()))])
+        .args(&["-e", &format!("CROSS_RUNNER={}", runner.unwrap_or_else(String::new))])
         .args(&["-v", &format!("{}:/xargo:Z", xargo_dir.display())])
         .args(&["-v", &format!("{}:/cargo:Z", cargo_dir.display())])
         // Prevent `bin` from being mounted inside the Docker container.

src/extensions.rs

@@ -3,53 +3,50 @@ use std::process::{Command, ExitStatus};
 use crate::errors::*;
 
 pub trait CommandExt {
+    fn print_verbose(&self, verbose: bool);
+    fn status_result(&self, status: ExitStatus) -> Result<()>;
     fn run(&mut self, verbose: bool) -> Result<()>;
     fn run_and_get_status(&mut self, verbose: bool) -> Result<ExitStatus>;
     fn run_and_get_stdout(&mut self, verbose: bool) -> Result<String>;
 }
 
 impl CommandExt for Command {
-    /// Runs the command to completion
-    fn run(&mut self, verbose: bool) -> Result<()> {
-        let status = self.run_and_get_status(verbose)?;
+    fn print_verbose(&self, verbose: bool) {
+        if verbose {
+            println!("+ {:?}", self);
+        }
+    }
 
+    fn status_result(&self, status: ExitStatus) -> Result<()> {
         if status.success() {
             Ok(())
         } else {
-            Err(format!("`{:?}` failed with exit code: {:?}",
-                        self,
-                        status.code()))?
+            Err(format!("`{:?}` failed with exit code: {:?}", self, status.code()).into())
         }
     }
 
     /// Runs the command to completion
-    fn run_and_get_status(&mut self, verbose: bool) -> Result<ExitStatus> {
-        if verbose {
-            println!("+ {:?}", self);
-        }
+    fn run(&mut self, verbose: bool) -> Result<()> {
+        let status = self.run_and_get_status(verbose)?;
+        self.status_result(status)
+    }
 
+    /// Runs the command to completion
+    fn run_and_get_status(&mut self, verbose: bool) -> Result<ExitStatus> {
+        self.print_verbose(verbose);
         self.status()
             .chain_err(|| format!("couldn't execute `{:?}`", self))
     }
 
     /// Runs the command to completion and returns its stdout
     fn run_and_get_stdout(&mut self, verbose: bool) -> Result<String> {
-        if verbose {
-            println!("+ {:?}", self);
-        }
-
+        self.print_verbose(verbose);
         let out = self.output()
             .chain_err(|| format!("couldn't execute `{:?}`", self))?;
 
-        if out.status.success() {
-            Ok(String::from_utf8(out.stdout).chain_err(|| {
-                    format!("`{:?}` output was not UTF-8",
-                            self)
-                })?)
-        } else {
-            Err(format!("`{:?}` failed with exit code: {:?}",
-                        self,
-                        out.status.code()))?
-        }
+        self.status_result(out.status)?;
+
+        Ok(String::from_utf8(out.stdout)
+            .chain_err(|| format!("`{:?}` output was not UTF-8", self))?)
     }
 }

src/interpreter.rs

@@ -7,7 +7,7 @@ use crate::Target;
 /// Checks if the interpreters have been registered in the host system
 pub fn is_registered(target: &Target) -> Result<bool> {
     if file::read("/proc/sys/fs/binfmt_misc/status")?.trim() != "enabled" {
-        Err("host system doesn't have binfmt_misc support")?
+        return Err("host system doesn't have binfmt_misc support".into());
     }
 
     let ok = if target.is_windows() {

src/main.rs

@@ -225,7 +225,7 @@ fn run() -> Result<ExitStatus> {
 
         if host.is_supported(args.target.as_ref()) {
             let target = args.target
-                .unwrap_or(Target::from(host.triple(), &target_list));
+                .unwrap_or_else(|| Target::from(host.triple(), &target_list));
             let toml = toml(&root)?;
 
             let sysroot = rustc::sysroot(&host, &target, verbose)?;
@@ -254,10 +254,9 @@ fn run() -> Result<ExitStatus> {
                 rustup::install_component("rust-src", toolchain, verbose)?;
             }
 
-            if args.subcommand.map(|sc| sc == Subcommand::Clippy).unwrap_or(false) {
-                if !rustup::component_is_installed("clippy", toolchain, verbose)? {
-                    rustup::install_component("clippy", toolchain, verbose)?;
-                }
+            if args.subcommand.map(|sc| sc == Subcommand::Clippy).unwrap_or(false) &&
+                !rustup::component_is_installed("clippy", toolchain, verbose)? {
+                rustup::install_component("clippy", toolchain, verbose)?;
             }
 
             let needs_interpreter = args.subcommand.map(|sc| sc.needs_interpreter()).unwrap_or(false);

src/rustc.rs

@@ -28,13 +28,13 @@ impl VersionMetaExt for VersionMeta {
     }
 
     fn needs_interpreter(&self) -> bool {
-        !(self.semver >= Version {
+        self.semver < Version {
             major: 1,
             minor: 19,
             patch: 0,
             pre: vec![],
             build: vec![],
-        })
+        }
     }
 }