Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core::compiler: remove Executor trait #11428

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,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;
Expand Down Expand Up @@ -126,7 +124,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {

/// Starts compilation, waits for it to finish, and returns information
/// about the result of compilation.
pub fn compile(mut self, exec: &Arc<dyn Executor>) -> CargoResult<Compilation<'cfg>> {
pub fn compile(mut self) -> CargoResult<Compilation<'cfg>> {
let mut queue = JobQueue::new(self.bcx);
let mut plan = BuildPlan::new();
let build_plan = self.bcx.build_config.build_plan;
Expand Down Expand Up @@ -156,7 +154,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
// function which will run everything in order with proper
// parallelism.
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
Expand Down
127 changes: 36 additions & 91 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,60 +90,11 @@ impl LinkType {
}
}

/// 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, _cx: &Context<'_, '_>, _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)
}
}

fn compile<'cfg>(
cx: &mut Context<'_, 'cfg>,
jobs: &mut JobQueue<'cfg>,
plan: &mut BuildPlan,
unit: &Unit,
exec: &Arc<dyn Executor>,
force_rebuild: bool,
) -> CargoResult<()> {
let bcx = cx.bcx;
Expand All @@ -163,15 +114,14 @@ fn compile<'cfg>(
// 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(cx, unit, &exec.clone())?)
Job::new_dirty(rustc(cx, unit)?)
} else {
let force = exec.force_rebuild(unit) || force_rebuild;
let mut job = fingerprint::prepare_target(cx, unit, force)?;
let mut job = fingerprint::prepare_target(cx, unit, force_rebuild)?;
job.before(if job.freshness() == Freshness::Dirty {
let work = if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
rustdoc(cx, unit)?
} else {
rustc(cx, unit, exec)?
rustc(cx, unit)?
};
work.then(link_targets(cx, unit, false)?)
} else {
Expand All @@ -198,7 +148,7 @@ fn compile<'cfg>(
// Be sure to compile all dependencies of this target as well.
let deps = Vec::from(cx.unit_deps(unit)); // Create vec due to mutable borrow.
for dep in deps {
compile(cx, jobs, plan, &dep.unit, exec, false)?;
compile(cx, jobs, plan, &dep.unit, false)?;
}
if build_plan {
plan.add(cx, unit)?;
Expand All @@ -207,7 +157,7 @@ fn compile<'cfg>(
Ok(())
}

fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> CargoResult<Work> {
fn rustc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
let mut rustc = prepare_rustc(cx, &unit.target.rustc_crate_types(), unit)?;
let build_plan = cx.bcx.build_config.build_plan;

Expand Down Expand Up @@ -248,10 +198,6 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
let mut output_options = OutputOptions::new(cx, unit);
let package_id = unit.pkg.package_id();
let target = Target::clone(&unit.target);
let mode = unit.mode;

exec.init(cx, unit);
let exec = exec.clone();

let root_output = cx.files().host_dest().to_path_buf();
let target_dir = cx.bcx.ws.target_dir().into_path_unlocked();
Expand Down Expand Up @@ -339,38 +285,37 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
if build_plan {
state.build_plan(buildkey, rustc.clone(), outputs.clone());
} else {
exec.exec(
&rustc,
package_id,
&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,
)
},
)
.map_err(verbose_if_simple_exit_code)
.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 previous error".to_string(),
count => format!(" due to {} previous errors", count),
};
format!("could not compile `{}`{}{}", name, errors, warnings)
})?;
rustc
.exec_with_streaming(
&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,
)
},
false,
)
.map(drop)
.map_err(verbose_if_simple_exit_code)
.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 previous error".to_string(),
count => format!(" due to {} previous errors", count),
};
format!("could not compile `{}`{}{}", name, errors, warnings)
})?;
// Exec should never return with success *and* generate an error.
debug_assert_eq!(output_options.errors_seen, 0);
}
Expand Down
25 changes: 3 additions & 22 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,14 @@
use std::collections::{HashMap, HashSet};
use std::fmt::Write;
use std::hash::{Hash, Hasher};
use std::sync::Arc;

use crate::core::compiler::rustdoc::RustdocScrapeExamples;
use crate::core::compiler::unit_dependencies::{build_unit_dependencies, IsArtifact};
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, Compilation, Context};
use crate::core::compiler::{CompileKind, CompileMode, CompileTarget, RustcTargetData, Unit};
use crate::core::compiler::{DefaultExecutor, Executor, UnitInterner};
use crate::core::profiles::{Profiles, UnitFor};
use crate::core::resolver::features::{self, CliFeatures, FeaturesFor};
use crate::core::resolver::{HasDevUnits, Resolve};
Expand Down Expand Up @@ -115,32 +114,14 @@ impl CompileOptions {
}
}

/// Compiles!
///
/// This uses the [`DefaultExecutor`]. To use a custom [`Executor`], see [`compile_with_exec`].
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()?;
compile_ws(ws, options, exec)
compile_ws(ws, options)
}

/// Like [`compile_with_exec`] but without warnings from manifest parsing.
pub fn compile_ws<'a>(
ws: &Workspace<'a>,
options: &CompileOptions,
exec: &Arc<dyn Executor>,
) -> CargoResult<Compilation<'a>> {
let interner = UnitInterner::new();
let bcx = create_bcx(ws, options, &interner)?;
Expand All @@ -150,7 +131,7 @@ pub fn compile_ws<'a>(
}
let _p = profile::start("compiling");
let cx = Context::new(&bcx)?;
cx.compile(exec)
cx.compile()
}

/// Executes `rustc --print <VALUE>`.
Expand Down
6 changes: 2 additions & 4 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
@@ -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, Freshness, UnitOutput};
use crate::core::compiler::{CompileKind, Freshness, UnitOutput};
use crate::core::{Dependency, Edition, Package, PackageId, Source, SourceId, Workspace};
use crate::ops::CompileFilter;
use crate::ops::{common_for_install_and_uninstall::*, FilterRule};
Expand Down Expand Up @@ -296,8 +295,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {

self.check_yanked_install()?;

let exec: Arc<dyn Executor> = Arc::new(DefaultExecutor);
let compile = ops::compile_ws(&self.ws, self.opts, &exec).with_context(|| {
let compile = ops::compile_ws(&self.ws, self.opts).with_context(|| {
if let Some(td) = td_opt.take() {
// preserve the temporary directory, so the user can inspect it
td.into_path();
Expand Down
7 changes: 2 additions & 5 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ use std::io::prelude::*;
use std::io::SeekFrom;
use std::path::{Path, PathBuf};
use std::rc::Rc;
use std::sync::Arc;
use std::task::Poll;

use crate::core::compiler::{BuildConfig, CompileMode, DefaultExecutor, Executor};
use crate::core::compiler::{BuildConfig, CompileMode};
use crate::core::resolver::CliFeatures;
use crate::core::{Feature, Shell, Verbosity, Workspace};
use crate::core::{Package, PackageId, PackageSet, Resolve, SourceId};
Expand Down Expand Up @@ -835,8 +834,7 @@ 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(
Expand All @@ -857,7 +855,6 @@ fn run_verify(
rustdoc_document_private_items: false,
honor_rust_version: true,
},
&exec,
)?;

// Check that `build.rs` didn't modify any files in the `src` directory.
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::sources::CRATES_IO_DOMAIN;

pub use self::cargo_clean::{clean, CleanOptions};
pub use self::cargo_compile::{
compile, compile_with_exec, compile_ws, create_bcx, print, resolve_all_features, CompileOptions,
compile, compile_ws, create_bcx, print, resolve_all_features, CompileOptions,
};
pub use self::cargo_compile::{CompileFilter, FilterRule, LibRule, Packages};
pub use self::cargo_doc::{doc, DocOptions};
Expand Down