Skip to content

Commit

Permalink
Auto merge of #8021 - ehuss:remove-cfg-from-options, r=alexcrichton
Browse files Browse the repository at this point in the history
Remove Config from CompileOptions.

This removes Config from CompileOptions. This removes the lifetime parameters, which I think simplifies things slightly (with the drawback that Config now needs to be passed as a parameter to a few functions).
  • Loading branch information
bors committed Mar 20, 2020
2 parents 4c7a638 + e4918c4 commit 7dec94d
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 86 deletions.
1 change: 1 addition & 0 deletions src/bin/cargo/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
ops::install_list(root, config)?;
} else {
ops::install(
config,
root,
krates,
source,
Expand Down
19 changes: 7 additions & 12 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ use crate::util::{closest_msg, profile, CargoResult};

/// Contains information about how a package should be compiled.
#[derive(Debug)]
pub struct CompileOptions<'a> {
pub config: &'a Config,
pub struct CompileOptions {
/// Configuration information for a rustc build
pub build_config: BuildConfig,
/// Extra features to build for the root package
Expand Down Expand Up @@ -79,10 +78,9 @@ pub struct CompileOptions<'a> {
pub export_dir: Option<PathBuf>,
}

impl<'a> CompileOptions<'a> {
pub fn new(config: &'a Config, mode: CompileMode) -> CargoResult<CompileOptions<'a>> {
impl<'a> CompileOptions {
pub fn new(config: &Config, mode: CompileMode) -> CargoResult<CompileOptions> {
Ok(CompileOptions {
config,
build_config: BuildConfig::new(config, None, &None, mode)?,
features: Vec::new(),
all_features: false,
Expand Down Expand Up @@ -242,10 +240,7 @@ pub enum CompileFilter {
},
}

pub fn compile<'a>(
ws: &Workspace<'a>,
options: &CompileOptions<'a>,
) -> CargoResult<Compilation<'a>> {
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)
}
Expand All @@ -254,7 +249,7 @@ pub fn compile<'a>(
/// calls and add custom logic. `compile` uses `DefaultExecutor` which just passes calls through.
pub fn compile_with_exec<'a>(
ws: &Workspace<'a>,
options: &CompileOptions<'a>,
options: &CompileOptions,
exec: &Arc<dyn Executor>,
) -> CargoResult<Compilation<'a>> {
ws.emit_warnings()?;
Expand All @@ -263,11 +258,10 @@ pub fn compile_with_exec<'a>(

pub fn compile_ws<'a>(
ws: &Workspace<'a>,
options: &CompileOptions<'a>,
options: &CompileOptions,
exec: &Arc<dyn Executor>,
) -> CargoResult<Compilation<'a>> {
let CompileOptions {
config,
ref build_config,
ref spec,
ref features,
Expand All @@ -280,6 +274,7 @@ pub fn compile_ws<'a>(
rustdoc_document_private_items,
ref export_dir,
} = *options;
let config = ws.config();

match build_config.mode {
CompileMode::Test
Expand Down
8 changes: 4 additions & 4 deletions src/cargo/ops/cargo_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ use std::process::Command;

/// Strongly typed options for the `cargo doc` command.
#[derive(Debug)]
pub struct DocOptions<'a> {
pub struct DocOptions {
/// Whether to attempt to open the browser after compiling the docs
pub open_result: bool,
/// Options to pass through to the compiler
pub compile_opts: ops::CompileOptions<'a>,
pub compile_opts: ops::CompileOptions,
}

/// Main method for `cargo doc`.
pub fn doc(ws: &Workspace<'_>, options: &DocOptions<'_>) -> CargoResult<()> {
pub fn doc(ws: &Workspace<'_>, options: &DocOptions) -> CargoResult<()> {
let specs = options.compile_opts.spec.to_package_id_specs(ws)?;
let opts = ResolveOpts::new(
/*dev_deps*/ true,
Expand Down Expand Up @@ -85,7 +85,7 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions<'_>) -> CargoResult<()> {
.join(&name)
.join("index.html");
if path.exists() {
let mut shell = options.compile_opts.config.shell();
let mut shell = ws.config().shell();
shell.status("Opening", path.display())?;
open_docs(&path, &mut shell)?;
}
Expand Down
20 changes: 11 additions & 9 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,22 @@ impl Drop for Transaction {
}

pub fn install(
config: &Config,
root: Option<&str>,
krates: Vec<&str>,
source_id: SourceId,
from_cwd: bool,
vers: Option<&str>,
opts: &ops::CompileOptions<'_>,
opts: &ops::CompileOptions,
force: bool,
no_track: bool,
) -> CargoResult<()> {
let root = resolve_root(root, opts.config)?;
let map = SourceConfigMap::new(opts.config)?;
let root = resolve_root(root, config)?;
let map = SourceConfigMap::new(config)?;

let (installed_anything, scheduled_error) = if krates.len() <= 1 {
install_one(
config,
&root,
&map,
krates.into_iter().next(),
Expand All @@ -69,6 +71,7 @@ pub fn install(
let root = root.clone();
let map = map.clone();
match install_one(
config,
&root,
&map,
Some(krate),
Expand All @@ -82,7 +85,7 @@ pub fn install(
) {
Ok(()) => succeeded.push(krate),
Err(e) => {
crate::display_error(&e, &mut opts.config.shell());
crate::display_error(&e, &mut config.shell());
failed.push(krate)
}
}
Expand All @@ -100,7 +103,7 @@ pub fn install(
));
}
if !succeeded.is_empty() || !failed.is_empty() {
opts.config.shell().status("Summary", summary.join(" "))?;
config.shell().status("Summary", summary.join(" "))?;
}

(!succeeded.is_empty(), !failed.is_empty())
Expand All @@ -117,7 +120,7 @@ pub fn install(
}
}

opts.config.shell().warn(&format!(
config.shell().warn(&format!(
"be sure to add `{}` to your PATH to be \
able to run the installed binaries",
dst.display()
Expand All @@ -132,19 +135,18 @@ pub fn install(
}

fn install_one(
config: &Config,
root: &Filesystem,
map: &SourceConfigMap<'_>,
krate: Option<&str>,
source_id: SourceId,
from_cwd: bool,
vers: Option<&str>,
opts: &ops::CompileOptions<'_>,
opts: &ops::CompileOptions,
force: bool,
no_track: bool,
is_first_install: bool,
) -> CargoResult<()> {
let config = opts.config;

let pkg = if source_id.is_git() {
select_pkg(
GitSource::new(source_id, config)?,
Expand Down
1 change: 0 additions & 1 deletion src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,6 @@ fn run_verify(ws: &Workspace<'_>, tar: &FileLock, opts: &PackageOpts<'_>) -> Car
ops::compile_with_exec(
&ws,
&ops::CompileOptions {
config,
build_config: BuildConfig::new(config, opts.jobs, &opts.target, CompileMode::Build)?,
features: opts.features.clone(),
no_default_features: opts.no_default_features,
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::util::{CargoResult, ProcessError};

pub fn run(
ws: &Workspace<'_>,
options: &ops::CompileOptions<'_>,
options: &ops::CompileOptions,
args: &[OsString],
) -> CargoResult<Option<ProcessError>> {
let config = ws.config();
Expand Down
31 changes: 14 additions & 17 deletions src/cargo/ops/cargo_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,32 @@ use crate::core::shell::Verbosity;
use crate::core::Workspace;
use crate::ops;
use crate::util::errors::CargoResult;
use crate::util::{CargoTestError, ProcessError, Test};
use crate::util::{CargoTestError, Config, ProcessError, Test};

pub struct TestOptions<'a> {
pub compile_opts: ops::CompileOptions<'a>,
pub struct TestOptions {
pub compile_opts: ops::CompileOptions,
pub no_run: bool,
pub no_fail_fast: bool,
}

pub fn run_tests(
ws: &Workspace<'_>,
options: &TestOptions<'_>,
options: &TestOptions,
test_args: &[&str],
) -> CargoResult<Option<CargoTestError>> {
let compilation = compile_tests(ws, options)?;

if options.no_run {
return Ok(None);
}
let (test, mut errors) = run_unit_tests(options, test_args, &compilation)?;
let (test, mut errors) = run_unit_tests(ws.config(), options, test_args, &compilation)?;

// If we have an error and want to fail fast, then return.
if !errors.is_empty() && !options.no_fail_fast {
return Ok(Some(CargoTestError::new(test, errors)));
}

let (doctest, docerrors) = run_doc_tests(options, test_args, &compilation)?;
let (doctest, docerrors) = run_doc_tests(ws.config(), options, test_args, &compilation)?;
let test = if docerrors.is_empty() { test } else { doctest };
errors.extend(docerrors);
if errors.is_empty() {
Expand All @@ -42,7 +42,7 @@ pub fn run_tests(

pub fn run_benches(
ws: &Workspace<'_>,
options: &TestOptions<'_>,
options: &TestOptions,
args: &[&str],
) -> CargoResult<Option<CargoTestError>> {
let compilation = compile_tests(ws, options)?;
Expand All @@ -54,18 +54,15 @@ pub fn run_benches(
let mut args = args.to_vec();
args.push("--bench");

let (test, errors) = run_unit_tests(options, &args, &compilation)?;
let (test, errors) = run_unit_tests(ws.config(), options, &args, &compilation)?;

match errors.len() {
0 => Ok(None),
_ => Ok(Some(CargoTestError::new(test, errors))),
}
}

fn compile_tests<'a>(
ws: &Workspace<'a>,
options: &TestOptions<'a>,
) -> CargoResult<Compilation<'a>> {
fn compile_tests<'a>(ws: &Workspace<'a>, options: &TestOptions) -> CargoResult<Compilation<'a>> {
let mut compilation = ops::compile(ws, &options.compile_opts)?;
compilation
.tests
Expand All @@ -75,12 +72,12 @@ fn compile_tests<'a>(

/// Runs the unit and integration tests of a package.
fn run_unit_tests(
options: &TestOptions<'_>,
config: &Config,
options: &TestOptions,
test_args: &[&str],
compilation: &Compilation<'_>,
) -> CargoResult<(Test, Vec<ProcessError>)> {
let config = options.compile_opts.config;
let cwd = options.compile_opts.config.cwd();
let cwd = config.cwd();

let mut errors = Vec::new();

Expand Down Expand Up @@ -133,12 +130,12 @@ fn run_unit_tests(
}

fn run_doc_tests(
options: &TestOptions<'_>,
config: &Config,
options: &TestOptions,
test_args: &[&str],
compilation: &Compilation<'_>,
) -> CargoResult<(Test, Vec<ProcessError>)> {
let mut errors = Vec::new();
let config = options.compile_opts.config;

// The unstable doctest-xcompile feature enables both per-target-ignores and
// cross-compiling doctests. As a side effect, this feature also gates running
Expand Down
13 changes: 4 additions & 9 deletions src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl InstallTracker {
dst: &Path,
pkg: &Package,
force: bool,
opts: &CompileOptions<'_>,
opts: &CompileOptions,
target: &str,
_rustc: &str,
) -> CargoResult<(Freshness, BTreeMap<String, Option<PackageId>>)> {
Expand Down Expand Up @@ -267,7 +267,7 @@ impl InstallTracker {
package: &Package,
bins: &BTreeSet<String>,
version_req: Option<String>,
opts: &CompileOptions<'_>,
opts: &CompileOptions,
target: &str,
rustc: &str,
) {
Expand Down Expand Up @@ -401,7 +401,7 @@ impl CrateListingV2 {
pkg: &Package,
bins: &BTreeSet<String>,
version_req: Option<String>,
opts: &CompileOptions<'_>,
opts: &CompileOptions,
target: &str,
rustc: &str,
) {
Expand Down Expand Up @@ -490,12 +490,7 @@ impl InstallInfo {
/// Determine if this installation is "up to date", or if it needs to be reinstalled.
///
/// This does not do Package/Source/Version checking.
fn is_up_to_date(
&self,
opts: &CompileOptions<'_>,
target: &str,
exes: &BTreeSet<String>,
) -> bool {
fn is_up_to_date(&self, opts: &CompileOptions, target: &str, exes: &BTreeSet<String>) -> bool {
self.features == feature_set(&opts.features)
&& self.all_features == opts.all_features
&& self.no_default_features == opts.no_default_features
Expand Down
12 changes: 5 additions & 7 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,9 @@ use rustfix::{self, CodeFix};

use crate::core::Workspace;
use crate::ops::{self, CompileOptions};
use crate::util;
use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer};
use crate::util::errors::CargoResult;
use crate::util::ProcessBuilder;
use crate::util::{self, Config, ProcessBuilder};
use crate::util::{existing_vcs_repo, LockServer, LockServerClient};

const FIX_ENV: &str = "__CARGO_FIX_PLZ";
Expand All @@ -69,15 +68,15 @@ pub struct FixOptions<'a> {
pub edition: bool,
pub prepare_for: Option<&'a str>,
pub idioms: bool,
pub compile_opts: CompileOptions<'a>,
pub compile_opts: CompileOptions,
pub allow_dirty: bool,
pub allow_no_vcs: bool,
pub allow_staged: bool,
pub broken_code: bool,
}

pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions<'_>) -> CargoResult<()> {
check_version_control(opts)?;
check_version_control(ws.config(), opts)?;

// Spin up our lock server, which our subprocesses will use to synchronize fixes.
let lock_server = LockServer::new()?;
Expand Down Expand Up @@ -116,7 +115,7 @@ pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions<'_>) -> CargoResult<()> {
server.configure(&mut wrapper);
}

let rustc = opts.compile_opts.config.load_global_rustc(Some(ws))?;
let rustc = ws.config().load_global_rustc(Some(ws))?;
wrapper.arg(&rustc.path);

// primary crates are compiled using a cargo subprocess to do extra work of applying fixes and
Expand All @@ -127,11 +126,10 @@ pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions<'_>) -> CargoResult<()> {
Ok(())
}

fn check_version_control(opts: &FixOptions<'_>) -> CargoResult<()> {
fn check_version_control(config: &Config, opts: &FixOptions<'_>) -> CargoResult<()> {
if opts.allow_no_vcs {
return Ok(());
}
let config = opts.compile_opts.config;
if !existing_vcs_repo(config.cwd(), config.cwd()) {
anyhow::bail!(
"no VCS found for this package and `cargo fix` can potentially \
Expand Down
Loading

0 comments on commit 7dec94d

Please sign in to comment.