refactor: Remove executor trait

src/bin/cargo/commands/fix.rs

@@ -67,8 +67,6 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
         args.get_one::<String>("profile").map(String::as_str),
         Some("test")
     );
-    let mode = CompileMode::Check { test };
-
     // Unlike other commands default `cargo fix` to all targets to fix as much
     // code as we can.
     let root_manifest = args.root_manifest(gctx)?;
@@ -79,7 +77,12 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
     let lockfile_path = args.lockfile_path(gctx)?;
     ws.set_requested_lockfile_path(lockfile_path.clone());
 
-    let mut opts = args.compile_options(gctx, mode, Some(&ws), ProfileChecking::LegacyTestOnly)?;
+    let mut opts = args.compile_options(
+        gctx,
+        CompileMode::Check { test },
+        Some(&ws),
+        ProfileChecking::LegacyTestOnly,
+    )?;
 
     if !opts.filter.is_specific() {
         // cargo fix with no target selection implies `--all-targets`.

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,57 +334,46 @@ fn rustc(
         if build_plan {
             state.build_plan(buildkey, rustc.clone(), outputs.clone());
         } else {
-            let result = exec
-                .exec(
-                    &rustc,
+            let on_stdout_line = &mut |line: &str| on_stdout_line(state, line, package_id, &target);
+            let on_stderr_line = &mut |line: &str| {
+                on_stderr_line(
+                    state,
+                    line,
                     package_id,
+                    &manifest_path,
                     &target,
-                    mode,
-                    &mut |line| on_stdout_line(state, line, package_id, &target),
-                    &mut |line| {
-                        on_stderr_line(
-                            state,
-                            line,
-                            package_id,
-                            &manifest_path,
-                            &target,
-                            &mut output_options,
-                        )
-                    },
+                    &mut output_options,
                 )
-                .map_err(|e| {
-                    if output_options.errors_seen == 0 {
-                        // If we didn't expect an error, do not require --verbose to fail.
-                        // This is intended to debug
-                        // https://github.com/rust-lang/crater/issues/733, where we are seeing
-                        // Cargo exit unsuccessfully while seeming to not show any errors.
-                        e
-                    } else {
-                        verbose_if_simple_exit_code(e)
-                    }
-                })
-                .with_context(|| {
-                    // adapted from rustc_errors/src/lib.rs
-                    let warnings = match output_options.warnings_seen {
-                        0 => String::new(),
-                        1 => "; 1 warning emitted".to_string(),
-                        count => format!("; {} warnings emitted", count),
-                    };
-                    let errors = match output_options.errors_seen {
-                        0 => String::new(),
-                        1 => " due to 1 previous error".to_string(),
-                        count => format!(" due to {} previous errors", count),
-                    };
-                    let name = descriptive_pkg_name(&name, &target, &mode);
-                    format!("could not compile {name}{errors}{warnings}")
-                });
+            };
+
+            if let Err(mut err) = rustc.exec_with_streaming(on_stdout_line, on_stderr_line, false) {
+                // If we didn't expect an error, do not require --verbose to fail.
+                // This is intended to debug
+                // https://github.com/rust-lang/crater/issues/733, where we are seeing
+                // Cargo exit unsuccessfully while seeming to not show any errors.
+                if output_options.errors_seen != 0 {
+                    err = verbose_if_simple_exit_code(err);
+                }
 
-            if let Err(e) = result {
                 if let Some(diagnostic) = failed_scrape_diagnostic {
                     state.warning(diagnostic)?;
                 }
 
-                return Err(e);
+                // adapted from rustc_errors/src/lib.rs
+                let warnings = match output_options.warnings_seen {
+                    0 => String::new(),
+                    1 => "; 1 warning emitted".to_string(),
+                    count => format!("; {} warnings emitted", count),
+                };
+                let errors = match output_options.errors_seen {
+                    0 => String::new(),
+                    1 => " due to 1 previous error".to_string(),
+                    count => format!(" due to {} previous errors", count),
+                };
+                let name = descriptive_pkg_name(&name, &target, &mode);
+                let err = err.context(format!("could not compile {name}{errors}{warnings}"));
+
+                return Err(err);
             }
 
             // Exec should never return with success *and* generate an error.

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::{apply_env_config, 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};
@@ -338,9 +337,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.