Move ProcessBuilder to cargo-util.

Author: ehuss

.github/workflows/main.yml

@@ -66,6 +66,7 @@ jobs:
     - run: cargo test --features 'deny-warnings'
     - run: cargo test --features 'deny-warnings' -p cargo-test-support
     - run: cargo test -p cargo-platform
+    - run: cargo test -p cargo-util
     - run: cargo test --manifest-path crates/mdman/Cargo.toml
     - run: cargo build --manifest-path crates/credential/cargo-credential-1password/Cargo.toml
     - run: cargo build --manifest-path crates/credential/cargo-credential-gnome-secret/Cargo.toml

Cargo.toml

@@ -56,7 +56,6 @@ semver = { version = "0.10", features = ["serde"] }
 serde = { version = "1.0.123", features = ["derive"] }
 serde_ignored = "0.1.0"
 serde_json = { version = "1.0.30", features = ["raw_value"] }
-shell-escape = "0.1.4"
 strip-ansi-escapes = "0.1.0"
 tar = { version = "0.4.26", default-features = false }
 tempfile = "3.0"
@@ -80,7 +79,6 @@ rand = "0.8.3"
 core-foundation = { version = "0.9.0", features = ["mac_os_10_7_support"] }
 
 [target.'cfg(windows)'.dependencies]
-miow = "0.3.6"
 fwdansi = "1.1.0"
 
 [target.'cfg(windows)'.dependencies.winapi]

crates/cargo-test-support/Cargo.toml

@@ -11,6 +11,7 @@ doctest = false
 [dependencies]
 cargo = { path = "../.." }
 cargo-test-macro = { path = "../cargo-test-macro" }
+cargo-util = { path = "../cargo-util" }
 filetime = "0.2"
 flate2 = { version = "1.0", default-features = false, features = ["zlib"] }
 git2 = "0.13.16"

crates/cargo-test-support/src/cross_compile.rs

@@ -10,8 +10,8 @@
 //! These tests are all disabled on rust-lang/rust's CI, but run in Cargo's CI.
 
 use crate::{basic_manifest, main_file, project};
-use cargo::util::ProcessError;
 use cargo::CargoResult;
+use cargo_util::ProcessError;
 use std::env;
 use std::fmt::Write;
 use std::process::{Command, Output};

crates/cargo-test-support/src/lib.rs

@@ -15,7 +15,8 @@ use std::process::{Command, Output};
 use std::str;
 use std::time::{self, Duration};
 
-use cargo::util::{is_ci, CargoResult, ProcessBuilder, ProcessError, Rustc};
+use cargo::util::{is_ci, CargoResult, Rustc};
+use cargo_util::{ProcessBuilder, ProcessError};
 use serde_json::{self, Value};
 use url::Url;
 
@@ -1569,12 +1570,12 @@ pub fn is_nightly() -> bool {
             .with(|r| r.verbose_version.contains("-nightly") || r.verbose_version.contains("-dev"))
 }
 
-pub fn process<T: AsRef<OsStr>>(t: T) -> cargo::util::ProcessBuilder {
+pub fn process<T: AsRef<OsStr>>(t: T) -> ProcessBuilder {
     _process(t.as_ref())
 }
 
-fn _process(t: &OsStr) -> cargo::util::ProcessBuilder {
-    let mut p = cargo::util::process(t);
+fn _process(t: &OsStr) -> ProcessBuilder {
+    let mut p = ProcessBuilder::new(t);
 
     // In general just clear out all cargo-specific configuration already in the
     // environment. Our tests all assume a "default configuration" unless
@@ -1643,7 +1644,7 @@ pub trait ChannelChanger: Sized {
     fn masquerade_as_nightly_cargo(&mut self) -> &mut Self;
 }
 
-impl ChannelChanger for cargo::util::ProcessBuilder {
+impl ChannelChanger for ProcessBuilder {
     fn masquerade_as_nightly_cargo(&mut self) -> &mut Self {
         self.env("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS", "nightly")
     }

crates/cargo-util/Cargo.toml

@@ -9,3 +9,11 @@ repository = "https://github.com/rust-lang/cargo"
 description = "Miscellaneous support code used by Cargo."
 
 [dependencies]
+anyhow = "1.0"
+jobserver = "0.1.21"
+libc = "0.2"
+shell-escape = "0.1.4"
+
+[target.'cfg(windows)'.dependencies]
+miow = "0.3.6"
+winapi = { version = "0.3", features = ["consoleapi", "minwindef"] }

crates/cargo-util/LICENSE-APACHE

@@ -0,0 +1 @@
+../../LICENSE-APACHE

crates/cargo-util/LICENSE-MIT

@@ -0,0 +1 @@
+../../LICENSE-MIT

crates/cargo-util/src/lib.rs

@@ -1 +1,9 @@
 //! Miscellaneous support code used by Cargo.
+
+pub use self::read2::read2;
+pub use process_builder::ProcessBuilder;
+pub use process_error::{exit_status_to_string, is_simple_exit_code, ProcessError};
+
+mod process_builder;
+mod process_error;
+mod read2;

src/cargo/util/process_builder.rs crates/cargo-util/src/process_builder.rs

@@ -1,5 +1,6 @@
-use crate::util::{process_error, read2, CargoResult, CargoResultExt};
-use anyhow::bail;
+use crate::process_error::ProcessError;
+use crate::read2;
+use anyhow::{bail, Context, Result};
 use jobserver::Client;
 use shell_escape::escape;
 use std::collections::BTreeMap;
@@ -10,7 +11,7 @@ use std::iter::once;
 use std::path::Path;
 use std::process::{Command, Output, Stdio};
 
-/// A builder object for an external process, similar to `std::process::Command`.
+/// A builder object for an external process, similar to [`std::process::Command`].
 #[derive(Clone, Debug)]
 pub struct ProcessBuilder {
     /// The program to execute.
@@ -21,10 +22,10 @@ pub struct ProcessBuilder {
     env: BTreeMap<String, Option<OsString>>,
     /// The directory to run the program from.
     cwd: Option<OsString>,
-    /// The `make` jobserver. See the [jobserver crate][jobserver_docs] for
+    /// The `make` jobserver. See the [jobserver crate] for
     /// more information.
     ///
-    /// [jobserver_docs]: https://docs.rs/jobserver/0.1.6/jobserver/
+    /// [jobserver crate]: https://docs.rs/jobserver/
     jobserver: Option<Client>,
     /// `true` to include environment variable in display.
     display_env_vars: bool,
@@ -58,6 +59,18 @@ impl fmt::Display for ProcessBuilder {
 }
 
 impl ProcessBuilder {
+    /// Creates a new [`ProcessBuilder`] with the given executable path.
+    pub fn new<T: AsRef<OsStr>>(cmd: T) -> ProcessBuilder {
+        ProcessBuilder {
+            program: cmd.as_ref().to_os_string(),
+            args: Vec::new(),
+            cwd: None,
+            env: BTreeMap::new(),
+            jobserver: None,
+            display_env_vars: false,
+        }
+    }
+
     /// (chainable) Sets the executable for the process.
     pub fn program<T: AsRef<OsStr>>(&mut self, program: T) -> &mut ProcessBuilder {
         self.program = program.as_ref().to_os_string();
@@ -149,16 +162,16 @@ impl ProcessBuilder {
     }
 
     /// Runs the process, waiting for completion, and mapping non-success exit codes to an error.
-    pub fn exec(&self) -> CargoResult<()> {
+    pub fn exec(&self) -> Result<()> {
         let mut command = self.build_command();
-        let exit = command.status().chain_err(|| {
-            process_error(&format!("could not execute process {}", self), None, None)
+        let exit = command.status().with_context(|| {
+            ProcessError::new(&format!("could not execute process {}", self), None, None)
         })?;
 
         if exit.success() {
             Ok(())
         } else {
-            Err(process_error(
+            Err(ProcessError::new(
                 &format!("process didn't exit successfully: {}", self),
                 Some(exit),
                 None,
@@ -182,22 +195,22 @@ impl ProcessBuilder {
     /// include our child process. If the child terminates then we'll reap them in Cargo
     /// pretty quickly, and if the child handles the signal then we won't terminate
     /// (and we shouldn't!) until the process itself later exits.
-    pub fn exec_replace(&self) -> CargoResult<()> {
+    pub fn exec_replace(&self) -> Result<()> {
         imp::exec_replace(self)
     }
 
     /// Executes the process, returning the stdio output, or an error if non-zero exit status.
-    pub fn exec_with_output(&self) -> CargoResult<Output> {
+    pub fn exec_with_output(&self) -> Result<Output> {
         let mut command = self.build_command();
 
-        let output = command.output().chain_err(|| {
-            process_error(&format!("could not execute process {}", self), None, None)
+        let output = command.output().with_context(|| {
+            ProcessError::new(&format!("could not execute process {}", self), None, None)
         })?;
 
         if output.status.success() {
             Ok(output)
         } else {
-            Err(process_error(
+            Err(ProcessError::new(
                 &format!("process didn't exit successfully: {}", self),
                 Some(output.status),
                 Some(&output),
@@ -217,10 +230,10 @@ impl ProcessBuilder {
     /// output.
     pub fn exec_with_streaming(
         &self,
-        on_stdout_line: &mut dyn FnMut(&str) -> CargoResult<()>,
-        on_stderr_line: &mut dyn FnMut(&str) -> CargoResult<()>,
+        on_stdout_line: &mut dyn FnMut(&str) -> Result<()>,
+        on_stderr_line: &mut dyn FnMut(&str) -> Result<()>,
         capture_output: bool,
-    ) -> CargoResult<Output> {
+    ) -> Result<Output> {
         let mut stdout = Vec::new();
         let mut stderr = Vec::new();
 
@@ -274,7 +287,9 @@ impl ProcessBuilder {
             })?;
             child.wait()
         })()
-        .chain_err(|| process_error(&format!("could not execute process {}", self), None, None))?;
+        .with_context(|| {
+            ProcessError::new(&format!("could not execute process {}", self), None, None)
+        })?;
         let output = Output {
             status,
             stdout,
@@ -284,14 +299,14 @@ impl ProcessBuilder {
         {
             let to_print = if capture_output { Some(&output) } else { None };
             if let Some(e) = callback_error {
-                let cx = process_error(
+                let cx = ProcessError::new(
                     &format!("failed to parse process output: {}", self),
                     Some(output.status),
                     to_print,
                 );
                 bail!(anyhow::Error::new(cx).context(e));
             } else if !output.status.success() {
-                bail!(process_error(
+                bail!(ProcessError::new(
                     &format!("process didn't exit successfully: {}", self),
                     Some(output.status),
                     to_print,
@@ -333,9 +348,9 @@ impl ProcessBuilder {
     /// # Examples
     ///
     /// ```rust
-    /// use cargo::util::{ProcessBuilder, process};
+    /// use cargo_util::ProcessBuilder;
     /// // Running this would execute `rustc`
-    /// let cmd: ProcessBuilder = process("rustc");
+    /// let cmd = ProcessBuilder::new("rustc");
     ///
     /// // Running this will execute `sccache rustc`
     /// let cmd = cmd.wrapped(Some("sccache"));
@@ -360,28 +375,16 @@ impl ProcessBuilder {
     }
 }
 
-/// A helper function to create a `ProcessBuilder`.
-pub fn process<T: AsRef<OsStr>>(cmd: T) -> ProcessBuilder {
-    ProcessBuilder {
-        program: cmd.as_ref().to_os_string(),
-        args: Vec::new(),
-        cwd: None,
-        env: BTreeMap::new(),
-        jobserver: None,
-        display_env_vars: false,
-    }
-}
-
 #[cfg(unix)]
 mod imp {
-    use crate::util::{process_error, ProcessBuilder};
-    use crate::CargoResult;
+    use super::{ProcessBuilder, ProcessError};
+    use anyhow::Result;
     use std::os::unix::process::CommandExt;
 
-    pub fn exec_replace(process_builder: &ProcessBuilder) -> CargoResult<()> {
+    pub fn exec_replace(process_builder: &ProcessBuilder) -> Result<()> {
         let mut command = process_builder.build_command();
         let error = command.exec();
-        Err(anyhow::Error::from(error).context(process_error(
+        Err(anyhow::Error::from(error).context(ProcessError::new(
             &format!("could not execute process {}", process_builder),
             None,
             None,
@@ -391,8 +394,8 @@ mod imp {
 
 #[cfg(windows)]
 mod imp {
-    use crate::util::{process_error, ProcessBuilder};
-    use crate::CargoResult;
+    use super::{ProcessBuilder, ProcessError};
+    use anyhow::Result;
     use winapi::shared::minwindef::{BOOL, DWORD, FALSE, TRUE};
     use winapi::um::consoleapi::SetConsoleCtrlHandler;
 
@@ -401,10 +404,10 @@ mod imp {
         TRUE
     }
 
-    pub fn exec_replace(process_builder: &ProcessBuilder) -> CargoResult<()> {
+    pub fn exec_replace(process_builder: &ProcessBuilder) -> Result<()> {
         unsafe {
             if SetConsoleCtrlHandler(Some(ctrlc_handler), TRUE) == FALSE {
-                return Err(process_error("Could not set Ctrl-C handler.", None, None).into());
+                return Err(ProcessError::new("Could not set Ctrl-C handler.", None, None).into());
             }
         }