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

Add more diagnostics to smooth edition transition #5824

Merged
merged 2 commits into from
Jul 31, 2018
Merged
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
7 changes: 7 additions & 0 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ pub struct Context<'a, 'cfg: 'a> {
pub links: Links<'a>,
pub used_in_plugin: HashSet<Unit<'a>>,
pub jobserver: Client,
primary_packages: HashSet<&'a PackageId>,
unit_dependencies: HashMap<Unit<'a>, Vec<Unit<'a>>>,
files: Option<CompilationFiles<'a, 'cfg>>,
}
Expand Down Expand Up @@ -129,6 +130,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
jobserver,
build_script_overridden: HashSet::new(),

primary_packages: HashSet::new(),
unit_dependencies: HashMap::new(),
files: None,
})
Expand Down Expand Up @@ -321,6 +323,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
Some(target) => Some(Layout::new(self.bcx.ws, Some(target), dest)?),
None => None,
};
self.primary_packages.extend(units.iter().map(|u| u.pkg.package_id()));

build_unit_dependencies(units, self.bcx, &mut self.unit_dependencies)?;
self.build_used_in_plugin_map(units)?;
Expand Down Expand Up @@ -487,6 +490,10 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
let dir = self.files().layout(unit.kind).incremental().display();
Ok(vec!["-C".to_string(), format!("incremental={}", dir)])
}

pub fn is_primary_package(&self, unit: &Unit<'a>) -> bool {
self.primary_packages.contains(unit.pkg.package_id())
}
}

#[derive(Default)]
Expand Down
5 changes: 3 additions & 2 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use handle_error;
use util::{internal, profile, CargoResult, CargoResultExt, ProcessBuilder};
use util::{Config, DependencyQueue, Dirty, Fresh, Freshness};
use util::{Progress, ProgressStyle};
use util::diagnostic_server;
use util::diagnostic_server::{self, DiagnosticPrinter};

use super::job::Job;
use super::{BuildContext, BuildPlan, CompileMode, Context, Kind, Unit};
Expand Down Expand Up @@ -200,6 +200,7 @@ impl<'a> JobQueue<'a> {
let mut tokens = Vec::new();
let mut queue = Vec::new();
let build_plan = cx.bcx.build_config.build_plan;
let mut print = DiagnosticPrinter::new(cx.bcx.config);
trace!("queue: {:#?}", self.queue);

// Iteratively execute the entire dependency graph. Each turn of the
Expand Down Expand Up @@ -311,7 +312,7 @@ impl<'a> JobQueue<'a> {
}
}
Message::FixDiagnostic(msg) => {
msg.print_to(cx.bcx.config)?;
print.print(&msg)?;
}
Message::Finish(key, result) => {
info!("end: {:?}", key);
Expand Down
7 changes: 5 additions & 2 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,14 @@ fn compile<'a, 'cfg: 'a>(
}

fn rustc<'a, 'cfg>(
mut cx: &mut Context<'a, 'cfg>,
cx: &mut Context<'a, 'cfg>,
unit: &Unit<'a>,
exec: &Arc<Executor>,
) -> CargoResult<Work> {
let mut rustc = prepare_rustc(cx, &unit.target.rustc_crate_types(), unit)?;
if cx.is_primary_package(unit) {
rustc.env("CARGO_PRIMARY_PACKAGE", "1");
}
let build_plan = cx.bcx.build_config.build_plan;

let name = unit.pkg.name().to_string();
Expand Down Expand Up @@ -195,7 +198,7 @@ fn rustc<'a, 'cfg>(
} else {
root.join(&cx.files().file_stem(unit))
}.with_extension("d");
let dep_info_loc = fingerprint::dep_info_loc(&mut cx, unit);
let dep_info_loc = fingerprint::dep_info_loc(cx, unit);

rustc.args(&cx.bcx.rustflags_args(unit)?);
let json_messages = cx.bcx.build_config.json_messages();
Expand Down
165 changes: 134 additions & 31 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::collections::{HashMap, HashSet, BTreeSet};
use std::env;
use std::fs::File;
use std::io::Write;
use std::path::Path;
use std::ffi::OsString;
use std::fs;
use std::path::{Path, PathBuf};
use std::process::{self, Command, ExitStatus};
use std::str;

Expand All @@ -21,6 +21,7 @@ use util::paths;

const FIX_ENV: &str = "__CARGO_FIX_PLZ";
const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE";
const EDITION_ENV: &str = "__CARGO_FIX_EDITION";

pub struct FixOptions<'a> {
pub edition: Option<&'a str>,
Expand Down Expand Up @@ -48,9 +49,10 @@ pub fn fix(ws: &Workspace, opts: &mut FixOptions) -> CargoResult<()> {
}

if let Some(edition) = opts.edition {
opts.compile_opts.build_config.extra_rustc_args.push("-W".to_string());
let lint_name = format!("rust-{}-compatibility", edition);
opts.compile_opts.build_config.extra_rustc_args.push(lint_name);
opts.compile_opts.build_config.extra_rustc_env.push((
EDITION_ENV.to_string(),
edition.to_string(),
));
}
opts.compile_opts.build_config.cargo_as_rustc_wrapper = true;
*opts.compile_opts.build_config.rustfix_diagnostic_server.borrow_mut() =
Expand Down Expand Up @@ -115,14 +117,8 @@ pub fn fix_maybe_exec_rustc() -> CargoResult<bool> {
Err(_) => return Ok(false),
};

// Try to figure out what we're compiling by looking for a rust-like file
// that exists.
let filename = env::args()
.skip(1)
.filter(|s| s.ends_with(".rs"))
.find(|s| Path::new(s).exists());

trace!("cargo-fix as rustc got file {:?}", filename);
let args = FixArgs::get();
trace!("cargo-fix as rustc got file {:?}", args.file);
let rustc = env::var_os("RUSTC").expect("failed to find RUSTC env var");

// Our goal is to fix only the crates that the end user is interested in.
Expand All @@ -133,10 +129,10 @@ pub fn fix_maybe_exec_rustc() -> CargoResult<bool> {
// compiling a Rust file and it *doesn't* have an absolute filename. That's
// not the best heuristic but matches what Cargo does today at least.
let mut fixes = FixedCrate::default();
if let Some(path) = filename {
if !Path::new(&path).is_absolute() {
if let Some(path) = &args.file {
if env::var("CARGO_PRIMARY_PACKAGE").is_ok() {
trace!("start rustfixing {:?}", path);
fixes = rustfix_crate(&lock_addr, rustc.as_ref(), &path)?;
fixes = rustfix_crate(&lock_addr, rustc.as_ref(), path, &args)?;
}
}

Expand All @@ -148,11 +144,10 @@ pub fn fix_maybe_exec_rustc() -> CargoResult<bool> {
// If we didn't actually make any changes then we can immediately exec the
// new rustc, and otherwise we capture the output to hide it in the scenario
// that we have to back it all out.
let mut cmd = Command::new(&rustc);
cmd.args(env::args().skip(1));
cmd.arg("--cap-lints=warn");
cmd.arg("--error-format=json");
if !fixes.original_files.is_empty() {
let mut cmd = Command::new(&rustc);
args.apply(&mut cmd);
cmd.arg("--error-format=json");
let output = cmd.output().context("failed to spawn rustc")?;

if output.status.success() {
Expand All @@ -173,17 +168,15 @@ pub fn fix_maybe_exec_rustc() -> CargoResult<bool> {
// below to recompile again.
if !output.status.success() {
for (k, v) in fixes.original_files {
File::create(&k)
.and_then(|mut f| f.write_all(v.as_bytes()))
fs::write(&k, v)
.with_context(|_| format!("failed to write file `{}`", k))?;
}
log_failed_fix(&output.stderr)?;
}
}

let mut cmd = Command::new(&rustc);
cmd.args(env::args().skip(1));
cmd.arg("--cap-lints=warn");
args.apply(&mut cmd);
exit_with(cmd.status().context("failed to spawn rustc")?);
}

Expand All @@ -193,9 +186,12 @@ struct FixedCrate {
original_files: HashMap<String, String>,
}

fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &str)
fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &Path, args: &FixArgs)
-> Result<FixedCrate, Error>
{
args.verify_not_preparing_for_enabled_edition()?;
args.warn_if_preparing_probably_inert()?;

// If not empty, filter by these lints
//
// TODO: Implement a way to specify this
Expand All @@ -210,8 +206,8 @@ fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &str)
let _lock = LockServerClient::lock(&lock_addr.parse()?, filename)?;

let mut cmd = Command::new(&rustc);
cmd.args(env::args().skip(1));
cmd.arg("--error-format=json").arg("--cap-lints=warn");
cmd.arg("--error-format=json");
args.apply(&mut cmd);
let output = cmd.output()
.with_context(|_| format!("failed to execute `{}`", rustc.display()))?;

Expand Down Expand Up @@ -280,7 +276,8 @@ fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &str)

debug!(
"collected {} suggestions for `{}`",
num_suggestion, filename
num_suggestion,
filename.display(),
);

let mut original_files = HashMap::with_capacity(file_map.len());
Expand Down Expand Up @@ -311,8 +308,7 @@ fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &str)
continue;
}
Ok(new_code) => {
File::create(&file)
.and_then(|mut f| f.write_all(new_code.as_bytes()))
fs::write(&file, new_code)
.with_context(|_| format!("failed to write file `{}`", file))?;
original_files.insert(file, code);
}
Expand Down Expand Up @@ -369,3 +365,110 @@ fn log_failed_fix(stderr: &[u8]) -> Result<(), Error> {

Ok(())
}

#[derive(Default)]
struct FixArgs {
file: Option<PathBuf>,
prepare_for_edition: Option<String>,
enabled_edition: Option<String>,
other: Vec<OsString>,
}

impl FixArgs {
fn get() -> FixArgs {
let mut ret = FixArgs::default();
for arg in env::args_os().skip(1) {
let path = PathBuf::from(arg);
if path.extension().and_then(|s| s.to_str()) == Some("rs") {
if path.exists() {
ret.file = Some(path);
continue
}
}
if let Some(s) = path.to_str() {
let prefix = "--edition=";
if s.starts_with(prefix) {
ret.enabled_edition = Some(s[prefix.len()..].to_string());
continue
}
}
ret.other.push(path.into());
}
if let Ok(s) = env::var(EDITION_ENV) {
ret.prepare_for_edition = Some(s);
}
return ret
}

fn apply(&self, cmd: &mut Command) {
if let Some(path) = &self.file {
cmd.arg(path);
}
cmd.args(&self.other)
.arg("--cap-lints=warn");
if let Some(edition) = &self.enabled_edition {
cmd.arg("--edition").arg(edition);
}
if let Some(prepare_for) = &self.prepare_for_edition {
cmd.arg("-W").arg(format!("rust-{}-compatibility", prepare_for));
}
}

/// Verify that we're not both preparing for an enabled edition and enabling
/// the edition.
///
/// This indicates that `cargo fix --prepare-for` is being executed out of
/// order with enabling the edition itself, meaning that we wouldn't
/// actually be able to fix anything! If it looks like this is happening
/// then yield an error to the user, indicating that this is happening.
fn verify_not_preparing_for_enabled_edition(&self) -> CargoResult<()> {
let edition = match &self.prepare_for_edition {
Some(s) => s,
None => return Ok(()),
};
let enabled = match &self.enabled_edition {
Some(s) => s,
None => return Ok(()),
};
if edition != enabled {
return Ok(())
}
let path = match &self.file {
Some(s) => s,
None => return Ok(()),
};

Message::EditionAlreadyEnabled {
file: path.display().to_string(),
edition: edition.to_string(),
}.post()?;

process::exit(1);
}

fn warn_if_preparing_probably_inert(&self) -> CargoResult<()> {
let edition = match &self.prepare_for_edition {
Some(s) => s,
None => return Ok(()),
};
let path = match &self.file {
Some(s) => s,
None => return Ok(()),
};
let contents = match fs::read_to_string(path) {
Ok(s) => s,
Err(_) => return Ok(())
};

let feature_name = format!("rust_{}_preview", edition);
if contents.contains(&feature_name) {
return Ok(())
}
Message::PreviewNotFound {
file: path.display().to_string(),
edition: edition.to_string(),
}.post()?;

Ok(())
}
}
Loading