Merge pull request #20 from Zeegomo/actions

Author: Pietro Albini

CHANGELOG.md

@@ -5,9 +5,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Added
+
+- New struct `cmd::ProcessLinesActions` to expose actions available while reading live output from a command.
+
 ### Changed
 
-- The file `.cargo/config` will be removed before starting the build 
+- **BREAKING**: `Command::process_lines` now accepts a `FnMut(&str, &mut ProcessLinesActions)`.
+- The file `.cargo/config` will be removed before starting the build.
 
 ## [0.6.1] - 2020-05-04
 

src/cmd/mod.rs

@@ -1,14 +1,17 @@
 //! Command execution and sandboxing.
 
+mod process_lines_actions;
 mod sandbox;
 
+pub use process_lines_actions::ProcessLinesActions;
 pub use sandbox::*;
 
 use crate::native;
 use crate::workspace::Workspace;
 use failure::{Error, Fail};
 use futures::{future, Future, Stream};
 use log::{error, info};
+use process_lines_actions::InnerState;
 use std::convert::AsRef;
 use std::env::consts::EXE_SUFFIX;
 use std::ffi::{OsStr, OsString};
@@ -125,7 +128,7 @@ pub struct Command<'w, 'pl> {
     binary: Binary,
     args: Vec<OsString>,
     env: Vec<(OsString, OsString)>,
-    process_lines: Option<&'pl mut dyn FnMut(&str)>,
+    process_lines: Option<&'pl mut dyn FnMut(&str, &mut ProcessLinesActions)>,
     cd: Option<PathBuf>,
     timeout: Option<Duration>,
     no_output_timeout: Option<Duration>,
@@ -240,7 +243,7 @@ impl<'w, 'pl> Command<'w, 'pl> {
     /// let mut ice = false;
     /// Command::new(&workspace, "cargo")
     ///     .args(&["build", "--all"])
-    ///     .process_lines(&mut |line| {
+    ///     .process_lines(&mut |line, _| {
     ///         if line.contains("internal compiler error") {
     ///             ice = true;
     ///         }
@@ -249,7 +252,7 @@ impl<'w, 'pl> Command<'w, 'pl> {
     /// # Ok(())
     /// # }
     /// ```
-    pub fn process_lines(mut self, f: &'pl mut dyn FnMut(&str)) -> Self {
+    pub fn process_lines(mut self, f: &'pl mut dyn FnMut(&str, &mut ProcessLinesActions)) -> Self {
         self.process_lines = Some(f);
         self
     }
@@ -480,7 +483,7 @@ impl OutputKind {
 
 fn log_command(
     mut cmd: StdCommand,
-    mut process_lines: Option<&mut dyn FnMut(&str)>,
+    mut process_lines: Option<&mut dyn FnMut(&str, &mut ProcessLinesActions)>,
     capture: bool,
     timeout: Option<Duration>,
     no_output_timeout: Option<Duration>,
@@ -512,6 +515,7 @@ fn log_command(
         .map(|line| (OutputKind::Stderr, line));
 
     let start = Instant::now();
+    let mut actions = ProcessLinesActions::new();
 
     let output = stdout
         .select(stderr)
@@ -533,21 +537,31 @@ fn log_command(
                 return future::err(Error::from(CommandError::Timeout(timeout.as_secs())));
             }
 
+            if let Some(f) = &mut process_lines {
+                f(&line, &mut actions);
+            }
+            // this is done here to avoid duplicating the output line
+            let lines = match actions.take_lines() {
+                InnerState::Removed => Vec::new(),
+                InnerState::Original => vec![line],
+                InnerState::Replaced(new_lines) => new_lines,
+            };
+
             if log_output {
-                info!("[{}] {}", kind.prefix(), line);
+                for line in &lines {
+                    info!("[{}] {}", kind.prefix(), line);
+                }
             }
-            future::ok((kind, line))
+
+            future::ok((kind, lines))
         })
         .fold(
             (Vec::new(), Vec::new()),
-            move |mut res, (kind, line)| -> Result<_, Error> {
-                if let Some(f) = &mut process_lines {
-                    f(&line);
-                }
+            move |mut res, (kind, mut lines)| -> Result<_, Error> {
                 if capture {
                     match kind {
-                        OutputKind::Stdout => res.0.push(line),
-                        OutputKind::Stderr => res.1.push(line),
+                        OutputKind::Stdout => res.0.append(&mut lines),
+                        OutputKind::Stderr => res.1.append(&mut lines),
                     }
                 }
                 Ok(res)

src/cmd/process_lines_actions.rs

@@ -0,0 +1,89 @@
+use std::default::Default;
+
+#[cfg_attr(test, derive(Debug, PartialEq))]
+pub(super) enum InnerState {
+    Removed,
+    Original,
+    Replaced(Vec<String>),
+}
+
+impl Default for InnerState {
+    fn default() -> Self {
+        InnerState::Original
+    }
+}
+
+/// Represents actions that are available while reading live output from a process.
+///
+/// This will be available inside the function you provide to [`Command::process_lines`](struct.Command.html#method.process_lines)
+pub struct ProcessLinesActions {
+    state: InnerState,
+}
+
+impl<'a> ProcessLinesActions {
+    pub(super) fn new() -> Self {
+        ProcessLinesActions {
+            state: InnerState::default(),
+        }
+    }
+
+    pub(super) fn take_lines(&mut self) -> InnerState {
+        std::mem::take(&mut self.state)
+    }
+
+    /// Replace last read line from output with the lines provided.
+    ///
+    /// The new lines will be logged instead of the original line.
+    pub fn replace_with_lines(&mut self, new_lines: impl Iterator<Item = &'a str>) {
+        self.state = InnerState::Replaced(new_lines.map(|str| str.to_string()).collect());
+    }
+
+    /// Remove last read line from output.
+    ///
+    /// This means that the line will not be logged.
+    pub fn remove_line(&mut self) {
+        self.state = InnerState::Removed;
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::InnerState;
+    use super::ProcessLinesActions;
+    #[test]
+    fn test_replace() {
+        let mut actions = ProcessLinesActions::new();
+
+        actions.replace_with_lines("ipsum".split("\n"));
+        assert_eq!(
+            actions.take_lines(),
+            InnerState::Replaced(vec!["ipsum".to_string()])
+        );
+
+        actions.replace_with_lines("lorem ipsum dolor".split(" "));
+        assert_eq!(
+            actions.take_lines(),
+            InnerState::Replaced(vec![
+                "lorem".to_string(),
+                "ipsum".to_string(),
+                "dolor".to_string()
+            ])
+        );
+
+        // assert last input is discarded
+        assert_eq!(actions.take_lines(), InnerState::Original);
+    }
+
+    #[test]
+    fn test_remove() {
+        let mut actions = ProcessLinesActions::new();
+        actions.remove_line();
+        assert_eq!(actions.take_lines(), InnerState::Removed);
+    }
+
+    #[test]
+    fn test_no_actions() {
+        let mut actions = ProcessLinesActions::new();
+        assert_eq!(actions.take_lines(), InnerState::Original);
+    }
+}

src/cmd/sandbox.rs

@@ -1,4 +1,4 @@
-use crate::cmd::{Command, CommandError};
+use crate::cmd::{Command, CommandError, ProcessLinesActions};
 use crate::Workspace;
 use failure::Error;
 use log::{error, info};
@@ -262,7 +262,7 @@ impl SandboxBuilder {
         workspace: &Workspace,
         timeout: Option<Duration>,
         no_output_timeout: Option<Duration>,
-        process_lines: Option<&mut dyn FnMut(&str)>,
+        process_lines: Option<&mut dyn FnMut(&str, &mut ProcessLinesActions)>,
     ) -> Result<(), Error> {
         let container = self.create(workspace)?;
 
@@ -324,7 +324,7 @@ impl Container<'_> {
         &self,
         timeout: Option<Duration>,
         no_output_timeout: Option<Duration>,
-        process_lines: Option<&mut dyn FnMut(&str)>,
+        process_lines: Option<&mut dyn FnMut(&str, &mut ProcessLinesActions)>,
     ) -> Result<(), Error> {
         let mut cmd = Command::new(self.workspace, "docker")
             .args(&["start", "-a", &self.id])

src/crates/git.rs

@@ -1,5 +1,5 @@
 use super::CrateTrait;
-use crate::cmd::Command;
+use crate::cmd::{Command, ProcessLinesActions};
 use crate::prepare::PrepareError;
 use crate::Workspace;
 use failure::{Error, ResultExt};
@@ -83,7 +83,7 @@ impl CrateTrait for GitRepo {
         //    fata: credential helper '{path}' told us to quit
         //
         let mut private_repository = false;
-        let mut detect_private_repositories = |line: &str| {
+        let mut detect_private_repositories = |line: &str, _actions: &mut ProcessLinesActions| {
             if line.starts_with("fatal: credential helper") && line.ends_with("told us to quit") {
                 private_repository = true;
             }

src/prepare.rs

@@ -109,7 +109,7 @@ impl<'a> Prepare<'a> {
         }
         let res = cmd
             .cd(self.source_dir)
-            .process_lines(&mut |line| {
+            .process_lines(&mut |line, _| {
                 if line.contains("failed to select a version for the requirement") {
                     yanked_deps = true;
                 }
@@ -130,7 +130,7 @@ impl<'a> Prepare<'a> {
         let res = Command::new(self.workspace, self.toolchain.cargo())
             .args(&["fetch", "--locked", "--manifest-path", "Cargo.toml"])
             .cd(&self.source_dir)
-            .process_lines(&mut |line| {
+            .process_lines(&mut |line, _| {
                 if line.ends_with(
                     "Cargo.lock needs to be updated but --locked was passed to prevent this",
                 ) {

src/toolchain.rs

@@ -324,7 +324,7 @@ impl Toolchain {
         let result = Command::new(workspace, &RUSTUP)
             .args(&[thing.as_str(), "list", "--installed", "--toolchain", name])
             .log_output(false)
-            .process_lines(&mut |line| {
+            .process_lines(&mut |line, _| {
                 if line.starts_with("error: toolchain ") && line.ends_with(" is not installed") {
                     not_installed = true;
                 }

tests/buildtest/crates/hello-world/src/main.rs

@@ -1,3 +1,4 @@
 fn main() {
     println!("Hello, world!");
+    println!("Hello, world again!");
 }

tests/buildtest/mod.rs

@@ -1,6 +1,6 @@
 use failure::Error;
 use log::LevelFilter;
-use rustwide::cmd::SandboxBuilder;
+use rustwide::cmd::{ProcessLinesActions, SandboxBuilder};
 
 #[macro_use]
 mod runner;
@@ -17,6 +17,9 @@ fn test_hello_world() {
             })?;
 
             assert!(storage.to_string().contains("[stdout] Hello, world!\n"));
+            assert!(storage
+                .to_string()
+                .contains("[stdout] Hello, world again!\n"));
             Ok(())
         })?;
         Ok(())
@@ -32,9 +35,12 @@ fn test_process_lines() {
             rustwide::logging::capture(&storage, || -> Result<_, Error> {
                 build
                     .cargo()
-                    .process_lines(&mut |line: &str| {
-                        if line.contains("Hello, world!") {
+                    .process_lines(&mut |line: &str, actions: &mut ProcessLinesActions| {
+                        if line.contains("Hello, world again!") {
                             ex = true;
+                            actions.replace_with_lines(line.split(","));
+                        } else if line.contains("Hello, world!") {
+                            actions.remove_line();
                         }
                     })
                     .args(&["run"])
@@ -43,6 +49,9 @@ fn test_process_lines() {
             })?;
 
             assert!(ex);
+            assert!(!storage.to_string().contains("[stdout] Hello, world!\n"));
+            assert!(storage.to_string().contains("[stdout]  world again!\n"));
+            assert!(storage.to_string().contains("[stdout] Hello\n"));
             Ok(())
         })?;
         Ok(())