diff --git a/src/bin/cargo/commands/fix.rs b/src/bin/cargo/commands/fix.rs index 38bd75497a4..7b1b5258594 100644 --- a/src/bin/cargo/commands/fix.rs +++ b/src/bin/cargo/commands/fix.rs @@ -67,8 +67,6 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { args.get_one::("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`. diff --git a/src/cargo/core/compiler/build_runner/mod.rs b/src/cargo/core/compiler/build_runner/mod.rs index 1a55d211068..66bd187ae50 100644 --- a/src/cargo/core/compiler/build_runner/mod.rs +++ b/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) -> CargoResult> { + pub fn compile(mut self) -> CargoResult> { // 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 diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 277039685d6..fce8025a355 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/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, 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, -) -> CargoResult { +fn rustc(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult { 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. diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index b059be395ec..0af781cabac 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/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> { - let exec: Arc = 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, -) -> CargoResult> { 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, ) -> CargoResult> { 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() } } diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 6e5222810dd..1bea00317c8 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/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 = 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()); diff --git a/src/cargo/ops/cargo_package/verify.rs b/src/cargo/ops/cargo_package/verify.rs index 794e5d30581..672e0214703 100644 --- a/src/cargo/ops/cargo_package/verify.rs +++ b/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 = 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. diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index 1d5d5a94009..1489e457a24 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -4,27 +4,27 @@ //! diagnostics with suggested fixes that can be applied to the files on the //! filesystem, and validate that those changes didn't break anything. //! -//! Cargo begins by launching a `LockServer` thread in the background to +//! Cargo begins by launching a [`LockServer`] thread in the background to //! listen for network connections to coordinate locking when multiple targets //! are built simultaneously. It ensures each package has only one fix running //! at once. //! -//! The `RustfixDiagnosticServer` is launched in a background thread (in +//! The [`RustfixDiagnosticServer`] is launched in a background thread (in //! `JobQueue`) to listen for network connections to coordinate displaying //! messages to the user on the console (so that multiple processes don't try //! to print at the same time). //! //! Cargo begins a normal `cargo check` operation with itself set as a proxy -//! for rustc by setting `primary_unit_rustc` in the build config. When +//! for rustc by setting [`BuildConfig::primary_unit_rustc`] in the build config. When //! cargo launches rustc to check a crate, it is actually launching itself. -//! The `FIX_ENV_INTERNAL` environment variable is set so that cargo knows it is in -//! fix-proxy-mode. +//! The `FIX_ENV_INTERNAL` environment variable is set to the value of the [`LockServer`]'s +//! address so that cargo knows it is in fix-proxy-mode. //! //! Each proxied cargo-as-rustc detects it is in fix-proxy-mode (via `FIX_ENV_INTERNAL` //! environment variable in `main`) and does the following: //! -//! - Acquire a lock from the `LockServer` from the master cargo process. -//! - Launches the real rustc (`rustfix_and_fix`), looking at the JSON output +//! - Acquire a lock from the [`LockServer`] from the master cargo process. +//! - Launches the real rustc ([`rustfix_and_fix`]), looking at the JSON output //! for suggested fixes. //! - Uses the `rustfix` crate to apply the suggestions to the files on the //! file system. @@ -35,6 +35,7 @@ //! applied cleanly, rustc is run again to verify the suggestions didn't //! break anything. The change will be backed out if it fails (unless //! `--broken-code` is used). +//! [BuildConfig]: cargo::core::compiler::build_config use std::collections::{BTreeSet, HashMap, HashSet}; use std::ffi::OsString; diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index 7d35f952995..cee32d422de 100644 --- a/src/cargo/ops/mod.rs +++ b/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}; diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index 0b388978d69..51c0d037342 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -2068,7 +2068,7 @@ impl ConfigError { ConfigError { error: anyhow::Error::from(self) .context(format!("could not load config key `{}`", key)), - definition: definition, + definition, } } } diff --git a/src/cargo/util/lockserver.rs b/src/cargo/util/lockserver.rs index 3dbb2126e64..99a4ebf243b 100644 --- a/src/cargo/util/lockserver.rs +++ b/src/cargo/util/lockserver.rs @@ -22,6 +22,7 @@ use anyhow::{Context, Error}; use crate::util::network::LOCALHOST; +// pub struct LockServer { listener: TcpListener, addr: SocketAddr,