refactor: Remove Executor trait

crates/cargo-util/src/process_builder.rs

@@ -436,6 +436,15 @@ impl ProcessBuilder {
         Ok(output)
     }
 
+    pub fn execute(
+        &self,
+        on_stdout_line: &mut dyn FnMut(&str) -> Result<()>,
+        on_stderr_line: &mut dyn FnMut(&str) -> Result<()>,
+    ) -> Result<()> {
+        self.exec_with_streaming(on_stdout_line, on_stderr_line, false)
+            .map(drop)
+    }
+
     /// Builds the command with an `@<path>` argfile that contains all the
     /// arguments. This is primarily served for rustc/rustdoc command family.
     fn build_command_with_argfile(&self) -> io::Result<(Command, NamedTempFile)> {

src/cargo/core/compiler/build_runner/mod.rs

@@ -21,9 +21,7 @@ use super::job_queue::JobQueue;
 use super::layout::Layout;
 use super::lto::Lto;
 use super::unit_graph::UnitDep;
-use super::{
-    BuildContext, Compilation, CompileKind, CompileMode, Executor, FileFlavor, RustDocFingerprint,
-};
+use super::{BuildContext, Compilation, CompileKind, CompileMode, FileFlavor, RustDocFingerprint};
 
 mod compilation_files;
 use self::compilation_files::CompilationFiles;
@@ -157,7 +155,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
     ///
     /// [`ops::cargo_compile`]: crate::ops::cargo_compile
     #[tracing::instrument(skip_all)]
-    pub fn compile(mut self, exec: &Arc<dyn Executor>) -> CargoResult<Compilation<'gctx>> {
+    pub fn compile(mut self) -> CargoResult<Compilation<'gctx>> {
         // A shared lock is held during the duration of the build since rustc
         // needs to read from the `src` cache, and we don't want other
         // commands modifying the `src` cache while it is running.
@@ -189,7 +187,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
 
         for unit in &self.bcx.roots {
             let force_rebuild = self.bcx.build_config.force_rebuild;
-            super::compile(&mut self, &mut queue, &mut plan, unit, exec, force_rebuild)?;
+            super::compile(&mut self, &mut queue, &mut plan, unit, force_rebuild)?;
         }
 
         // Now that we've got the full job queue and we've done all our

src/cargo/core/compiler/mod.rs

@@ -104,53 +104,7 @@ use rustfix::diagnostics::Applicability;
 
 const RUSTDOC_CRATE_VERSION_FLAG: &str = "--crate-version";
 
-/// A glorified callback for executing calls to rustc. Rather than calling rustc
-/// directly, we'll use an `Executor`, giving clients an opportunity to intercept
-/// the build calls.
-pub trait Executor: Send + Sync + 'static {
-    /// Called after a rustc process invocation is prepared up-front for a given
-    /// unit of work (may still be modified for runtime-known dependencies, when
-    /// the work is actually executed).
-    fn init(&self, _build_runner: &BuildRunner<'_, '_>, _unit: &Unit) {}
-
-    /// In case of an `Err`, Cargo will not continue with the build process for
-    /// this package.
-    fn exec(
-        &self,
-        cmd: &ProcessBuilder,
-        id: PackageId,
-        target: &Target,
-        mode: CompileMode,
-        on_stdout_line: &mut dyn FnMut(&str) -> CargoResult<()>,
-        on_stderr_line: &mut dyn FnMut(&str) -> CargoResult<()>,
-    ) -> CargoResult<()>;
-
-    /// Queried when queuing each unit of work. If it returns true, then the
-    /// unit will always be rebuilt, independent of whether it needs to be.
-    fn force_rebuild(&self, _unit: &Unit) -> bool {
-        false
-    }
-}
-
-/// A `DefaultExecutor` calls rustc without doing anything else. It is Cargo's
-/// default behaviour.
-#[derive(Copy, Clone)]
-pub struct DefaultExecutor;
-
-impl Executor for DefaultExecutor {
-    fn exec(
-        &self,
-        cmd: &ProcessBuilder,
-        _id: PackageId,
-        _target: &Target,
-        _mode: CompileMode,
-        on_stdout_line: &mut dyn FnMut(&str) -> CargoResult<()>,
-        on_stderr_line: &mut dyn FnMut(&str) -> CargoResult<()>,
-    ) -> CargoResult<()> {
-        cmd.exec_with_streaming(on_stdout_line, on_stderr_line, false)
-            .map(drop)
-    }
-}
+//    cmd.exec_with_streaming(on_stdout_line, on_stderr_line, false)
 
 /// Builds up and enqueue a list of pending jobs onto the `job` queue.
 ///
@@ -161,13 +115,12 @@ impl Executor for DefaultExecutor {
 /// Note that **no actual work is executed as part of this**, that's all done
 /// next as part of [`JobQueue::execute`] function which will run everything
 /// in order with proper parallelism.
-#[tracing::instrument(skip(build_runner, jobs, plan, exec))]
+#[tracing::instrument(skip(build_runner, jobs, plan))]
 fn compile<'gctx>(
     build_runner: &mut BuildRunner<'_, 'gctx>,
     jobs: &mut JobQueue<'gctx>,
     plan: &mut BuildPlan,
     unit: &Unit,
-    exec: &Arc<dyn Executor>,
     force_rebuild: bool,
 ) -> CargoResult<()> {
     let bcx = build_runner.bcx;
@@ -186,18 +139,14 @@ fn compile<'gctx>(
         // We run these targets later, so this is just a no-op for now.
         Job::new_fresh()
     } else if build_plan {
-        Job::new_dirty(
-            rustc(build_runner, unit, &exec.clone())?,
-            DirtyReason::FreshBuild,
-        )
+        Job::new_dirty(rustc(build_runner, unit)?, DirtyReason::FreshBuild)
     } else {
-        let force = exec.force_rebuild(unit) || force_rebuild;
-        let mut job = fingerprint::prepare_target(build_runner, unit, force)?;
+        let mut job = fingerprint::prepare_target(build_runner, unit, force_rebuild)?;
         job.before(if job.freshness().is_dirty() {
             let work = if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
                 rustdoc(build_runner, unit)?
             } else {
-                rustc(build_runner, unit, exec)?
+                rustc(build_runner, unit)?
             };
             work.then(link_targets(build_runner, unit, false)?)
         } else {
@@ -224,7 +173,7 @@ fn compile<'gctx>(
     // Be sure to compile all dependencies of this target as well.
     let deps = Vec::from(build_runner.unit_deps(unit)); // Create vec due to mutable borrow.
     for dep in deps {
-        compile(build_runner, jobs, plan, &dep.unit, exec, false)?;
+        compile(build_runner, jobs, plan, &dep.unit, false)?;
     }
     if build_plan {
         plan.add(build_runner, unit)?;
@@ -255,11 +204,7 @@ fn make_failed_scrape_diagnostic(
 }
 
 /// Creates a unit of work invoking `rustc` for building the `unit`.
-fn rustc(
-    build_runner: &mut BuildRunner<'_, '_>,
-    unit: &Unit,
-    exec: &Arc<dyn Executor>,
-) -> CargoResult<Work> {
+fn rustc(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult<Work> {
     let mut rustc = prepare_rustc(build_runner, unit)?;
     let build_plan = build_runner.bcx.build_config.build_plan;
 
@@ -293,9 +238,6 @@ fn rustc(
     let target = Target::clone(&unit.target);
     let mode = unit.mode;
 
-    exec.init(build_runner, unit);
-    let exec = exec.clone();
-
     let root_output = build_runner.files().host_dest().to_path_buf();
     let target_dir = build_runner.bcx.ws.target_dir().into_path_unlocked();
     let pkg_root = unit.pkg.root().to_path_buf();
@@ -392,12 +334,8 @@ fn rustc(
         if build_plan {
             state.build_plan(buildkey, rustc.clone(), outputs.clone());
         } else {
-            let result = exec
-                .exec(
-                    &rustc,
-                    package_id,
-                    &target,
-                    mode,
+            let result = rustc
+                .execute(
                     &mut |line| on_stdout_line(state, line, package_id, &target),
                     &mut |line| {
                         on_stderr_line(

src/cargo/ops/cargo_compile/mod.rs

@@ -37,14 +37,13 @@
 
 use std::collections::{HashMap, HashSet};
 use std::hash::{Hash, Hasher};
-use std::sync::Arc;
 
 use crate::core::compiler::unit_dependencies::build_unit_dependencies;
 use crate::core::compiler::unit_graph::{self, UnitDep, UnitGraph};
+use crate::core::compiler::UnitInterner;
 use crate::core::compiler::{standard_lib, CrateType, TargetInfo};
 use crate::core::compiler::{BuildConfig, BuildContext, BuildRunner, Compilation};
 use crate::core::compiler::{CompileKind, CompileMode, CompileTarget, RustcTargetData, Unit};
-use crate::core::compiler::{DefaultExecutor, Executor, UnitInterner};
 use crate::core::profiles::Profiles;
 use crate::core::resolver::features::{self, CliFeatures, FeaturesFor};
 use crate::core::resolver::{HasDevUnits, Resolve};
@@ -122,35 +121,27 @@ impl CompileOptions {
 
 /// Compiles!
 ///
-/// This uses the [`DefaultExecutor`]. To use a custom [`Executor`], see [`compile_with_exec`].
+/// See [`ops::cargo_compile`] for a higher-level view of the compile process.
+/// [`ops::cargo_compile`]: crate::ops::cargo_compile
 pub fn compile<'a>(ws: &Workspace<'a>, options: &CompileOptions) -> CargoResult<Compilation<'a>> {
-    let exec: Arc<dyn Executor> = Arc::new(DefaultExecutor);
-    compile_with_exec(ws, options, &exec)
-}
-
-/// Like [`compile`] but allows specifying a custom [`Executor`]
-/// that will be able to intercept build calls and add custom logic.
-///
-/// [`compile`] uses [`DefaultExecutor`] which just passes calls through.
-pub fn compile_with_exec<'a>(
-    ws: &Workspace<'a>,
-    options: &CompileOptions,
-    exec: &Arc<dyn Executor>,
-) -> CargoResult<Compilation<'a>> {
     ws.emit_warnings()?;
-    let compilation = compile_ws(ws, options, exec)?;
+
+    let compilation = compile_without_warnings(ws, options)?;
+
     if ws.gctx().warning_handling()? == WarningHandling::Deny && compilation.warning_count > 0 {
         anyhow::bail!("warnings are denied by `build.warnings` configuration")
     }
     Ok(compilation)
 }
 
-/// Like [`compile_with_exec`] but without warnings from manifest parsing.
+/// Like [`compile`] but without warnings from manifest parsing.
+///
+/// See [`ops::cargo_compile`] for a higher-level view of the compile process.
+/// [`ops::cargo_compile`]: crate::ops::cargo_compile
 #[tracing::instrument(skip_all)]
-pub fn compile_ws<'a>(
+pub fn compile_without_warnings<'a>(
     ws: &Workspace<'a>,
     options: &CompileOptions,
-    exec: &Arc<dyn Executor>,
 ) -> CargoResult<Compilation<'a>> {
     let interner = UnitInterner::new();
     let bcx = create_bcx(ws, options, &interner)?;
@@ -163,7 +154,7 @@ pub fn compile_ws<'a>(
     if options.build_config.dry_run {
         build_runner.dry_run()
     } else {
-        build_runner.compile(exec)
+        build_runner.compile()
     }
 }
 

src/cargo/ops/cargo_install.rs

@@ -1,9 +1,8 @@
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::path::{Path, PathBuf};
-use std::sync::Arc;
 use std::{env, fs};
 
-use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, UnitOutput};
+use crate::core::compiler::{CompileKind, UnitOutput};
 use crate::core::{Dependency, Edition, Package, PackageId, SourceId, Target, Workspace};
 use crate::ops::{common_for_install_and_uninstall::*, FilterRule};
 use crate::ops::{CompileFilter, Packages};
@@ -337,9 +336,8 @@ impl<'gctx> InstallablePackage<'gctx> {
 
         self.check_yanked_install()?;
 
-        let exec: Arc<dyn Executor> = Arc::new(DefaultExecutor);
         self.opts.build_config.dry_run = dry_run;
-        let compile = ops::compile_ws(&self.ws, &self.opts, &exec).with_context(|| {
+        let compile = ops::compile_without_warnings(&self.ws, &self.opts).with_context(|| {
             if let Some(td) = td_opt.take() {
                 // preserve the temporary directory, so the user can inspect it
                 drop(td.into_path());

src/cargo/ops/cargo_package/verify.rs

@@ -7,7 +7,6 @@ use std::io::prelude::*;
 use std::io::SeekFrom;
 use std::path::Path;
 use std::path::PathBuf;
-use std::sync::Arc;
 
 use anyhow::Context as _;
 use cargo_util::paths;
@@ -16,8 +15,6 @@ use tar::Archive;
 
 use crate::core::compiler::BuildConfig;
 use crate::core::compiler::CompileMode;
-use crate::core::compiler::DefaultExecutor;
-use crate::core::compiler::Executor;
 use crate::core::Feature;
 use crate::core::Package;
 use crate::core::SourceId;
@@ -85,8 +82,7 @@ pub fn run_verify(
         None
     };
 
-    let exec: Arc<dyn Executor> = Arc::new(DefaultExecutor);
-    ops::compile_with_exec(
+    ops::compile(
         &ws,
         &ops::CompileOptions {
             build_config: BuildConfig::new(
@@ -107,7 +103,6 @@ pub fn run_verify(
             rustdoc_document_private_items: false,
             honor_rust_version: None,
         },
-        &exec,
     )?;
 
     // Check that `build.rs` didn't modify any files in the `src` directory.

src/cargo/ops/mod.rs

@@ -2,7 +2,7 @@ use crate::sources::CRATES_IO_DOMAIN;
 
 pub use self::cargo_clean::{clean, CleanContext, CleanOptions};
 pub use self::cargo_compile::{
-    compile, compile_with_exec, compile_ws, create_bcx, print, resolve_all_features, CompileOptions,
+    compile, compile_without_warnings, create_bcx, print, resolve_all_features, CompileOptions,
 };
 pub use self::cargo_compile::{CompileFilter, FilterRule, LibRule, Packages};
 pub use self::cargo_doc::{doc, DocOptions, OutputFormat};

src/cargo/util/context/mod.rs

@@ -2075,7 +2075,7 @@ impl ConfigError {
         ConfigError {
             error: anyhow::Error::from(self)
                 .context(format!("could not load config key `{}`", key)),
-            definition: definition,
+            definition,
         }
     }
 }