Skip to content

Commit

Permalink
Auto merge of #8825 - Aaron1011:feature/report-future-incompat, r=ehuss
Browse files Browse the repository at this point in the history
Implement future incompatibility report support

cc rust-lang/rust#71249

This implements the Cargo side of 'Cargo report future-incompat'

Based on feedback from alexcrichton and est31, I'm implemented this a
flag `--future-compat-report` on `cargo check/build/rustc`, rather than
a separate `cargo describe-future-incompatibilities` command. This
allows us to avoid writing additional information to disk (beyond the
pre-existing recording of rustc command outputs).

This PR contains:

* Gating of all functionality behind `-Z report-future-incompat`.
  Without this flag, all user output is unchanged.
* Passing `-Z emit-future-incompat-report` to rustc when
  `-Z report-future-incompat` is enabled
* Parsing the rustc JSON future incompat report, and displaying it
  it a user-readable format.
* Emitting a warning at the end of a build if any crates had
  future-incompat reports
* A `--future-incompat-report` flag, which shows the full report for
  each affected crate.
* Tests for all of the above.

At the moment, we can use the `array_into_iter` to write a test.
However, we might eventually get to a point where rustc is not currently
emitting future-incompat reports for any lints. What would we want the
cargo tests to do in this situation?

This functionality didn't require any significant internal changes to
Cargo, with one exception: we now process captured command output for
all units, not just ones where we want to display warnings. This may
result in a slightly longer time to run `cargo build/check/rustc` from
a full cache. since we do slightly more work for each upstream
dependency. Doing this seems unavoidable with the current architecture,
since we need to process captured command outputs to detect
any future-incompat-report messages that were emitted.
  • Loading branch information
bors committed Mar 5, 2021
2 parents c694096 + 139ed73 commit 7441e8c
Show file tree
Hide file tree
Showing 15 changed files with 444 additions and 13 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ im-rc = "15.0.0"
# See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`
# for more information.
rustc-workspace-hack = "1.0.0"
rand = "0.8.3"

[target.'cfg(target_os = "macos")'.dependencies]
core-foundation = { version = "0.9.0", features = ["mac_os_10_7_support"] }
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub fn cli() -> App {
.arg_message_format()
.arg_build_plan()
.arg_unit_graph()
.arg_future_incompat_report()
.after_help("Run `cargo help build` for more detailed information.\n")
}

Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub fn cli() -> App {
.arg_ignore_rust_version()
.arg_message_format()
.arg_unit_graph()
.arg_future_incompat_report()
.after_help("Run `cargo help check` for more detailed information.\n")
}

Expand Down
57 changes: 57 additions & 0 deletions src/bin/cargo/commands/describe_future_incompatibilities.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
use crate::command_prelude::*;
use anyhow::anyhow;
use cargo::core::compiler::future_incompat::{OnDiskReport, FUTURE_INCOMPAT_FILE};
use cargo::drop_eprint;
use cargo::util::CargoResultExt;
use std::io::Read;

pub fn cli() -> App {
subcommand("describe-future-incompatibilities")
.arg(
opt(
"id",
"identifier of the report [generated by a Cargo command invocation",
)
.value_name("id")
.required(true),
)
.about("Reports any crates which will eventually stop compiling")
}

pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
if !config.nightly_features_allowed {
return Err(anyhow!(
"`cargo describe-future-incompatibilities` can only be used on the nightly channel"
)
.into());
}

let ws = args.workspace(config)?;
let report_file = ws.target_dir().open_ro(
FUTURE_INCOMPAT_FILE,
ws.config(),
"Future incompatible report",
)?;

let mut file_contents = String::new();
report_file
.file()
.read_to_string(&mut file_contents)
.chain_err(|| "failed to read report")?;
let on_disk_report: OnDiskReport =
serde_json::from_str(&file_contents).chain_err(|| "failed to load report")?;

let id = args.value_of("id").unwrap();
if id != on_disk_report.id {
return Err(anyhow!(
"Expected an id of `{}`, but `{}` was provided on the command line. \
Your report may have been overwritten by a different one.",
on_disk_report.id,
id
)
.into());
}

drop_eprint!(config, "{}", on_disk_report.report);
Ok(())
}
3 changes: 3 additions & 0 deletions src/bin/cargo/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub fn builtin() -> Vec<App> {
build::cli(),
check::cli(),
clean::cli(),
describe_future_incompatibilities::cli(),
doc::cli(),
fetch::cli(),
fix::cli(),
Expand Down Expand Up @@ -44,6 +45,7 @@ pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches<'_>) -> Cli
"build" => build::exec,
"check" => check::exec,
"clean" => clean::exec,
"describe-future-incompatibilities" => describe_future_incompatibilities::exec,
"doc" => doc::exec,
"fetch" => fetch::exec,
"fix" => fix::exec,
Expand Down Expand Up @@ -82,6 +84,7 @@ pub mod bench;
pub mod build;
pub mod check;
pub mod clean;
pub mod describe_future_incompatibilities;
pub mod doc;
pub mod fetch;
pub mod fix;
Expand Down
1 change: 1 addition & 0 deletions src/bin/cargo/commands/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub fn cli() -> App {
.arg_message_format()
.arg_unit_graph()
.arg_ignore_rust_version()
.arg_future_incompat_report()
.after_help("Run `cargo help rustc` for more detailed information.\n")
}

Expand Down
3 changes: 3 additions & 0 deletions src/cargo/core/compiler/build_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub struct BuildConfig {
// Note that, although the cmd-line flag name is `out-dir`, in code we use
// `export_dir`, to avoid confusion with out dir at `target/debug/deps`.
pub export_dir: Option<PathBuf>,
/// `true` to output a future incompatibility report at the end of the build
pub future_incompat_report: bool,
}

impl BuildConfig {
Expand Down Expand Up @@ -80,6 +82,7 @@ impl BuildConfig {
primary_unit_rustc: None,
rustfix_diagnostic_server: RefCell::new(None),
export_dir: None,
future_incompat_report: false,
})
}

Expand Down
36 changes: 36 additions & 0 deletions src/cargo/core/compiler/future_incompat.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use serde::{Deserialize, Serialize};

/// The future incompatibility report, emitted by the compiler as a JSON message.
#[derive(serde::Deserialize)]
pub struct FutureIncompatReport {
pub future_incompat_report: Vec<FutureBreakageItem>,
}

#[derive(Serialize, Deserialize)]
pub struct FutureBreakageItem {
/// The date at which this lint will become an error.
/// Currently unused
pub future_breakage_date: Option<String>,
/// The original diagnostic emitted by the compiler
pub diagnostic: Diagnostic,
}

/// A diagnostic emitted by the compiler as a JSON message.
/// We only care about the 'rendered' field
#[derive(Serialize, Deserialize)]
pub struct Diagnostic {
pub rendered: String,
}

/// The filename in the top-level `target` directory where we store
/// the report
pub const FUTURE_INCOMPAT_FILE: &str = ".future-incompat-report.json";

#[derive(Serialize, Deserialize)]
pub struct OnDiskReport {
// A Cargo-generated id used to detect when a report has been overwritten
pub id: String,
// Cannot be a &str, since Serde needs
// to be able to un-escape the JSON
pub report: String,
}
109 changes: 109 additions & 0 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ use anyhow::format_err;
use crossbeam_utils::thread::Scope;
use jobserver::{Acquired, Client, HelperThread};
use log::{debug, info, trace};
use rand::distributions::Alphanumeric;
use rand::{thread_rng, Rng};

use super::context::OutputFile;
use super::job::{
Expand All @@ -68,7 +70,11 @@ use super::job::{
};
use super::timings::Timings;
use super::{BuildContext, BuildPlan, CompileMode, Context, Unit};
use crate::core::compiler::future_incompat::{
FutureBreakageItem, OnDiskReport, FUTURE_INCOMPAT_FILE,
};
use crate::core::{PackageId, Shell, TargetKind};
use crate::drop_eprint;
use crate::util::diagnostic_server::{self, DiagnosticPrinter};
use crate::util::machine_message::{self, Message as _};
use crate::util::{self, internal, profile};
Expand Down Expand Up @@ -151,6 +157,7 @@ struct DrainState<'cfg> {

/// How many jobs we've finished
finished: usize,
per_crate_future_incompat_reports: Vec<FutureIncompatReportCrate>,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
Expand All @@ -162,6 +169,11 @@ impl std::fmt::Display for JobId {
}
}

struct FutureIncompatReportCrate {
package_id: PackageId,
report: Vec<FutureBreakageItem>,
}

/// A `JobState` is constructed by `JobQueue::run` and passed to `Job::run`. It includes everything
/// necessary to communicate between the main thread and the execution of the job.
///
Expand Down Expand Up @@ -228,6 +240,7 @@ enum Message {
FixDiagnostic(diagnostic_server::Message),
Token(io::Result<Acquired>),
Finish(JobId, Artifact, CargoResult<()>),
FutureIncompatReport(JobId, Vec<FutureBreakageItem>),

// This client should get release_raw called on it with one of our tokens
NeedsToken(JobId),
Expand Down Expand Up @@ -282,6 +295,11 @@ impl<'a> JobState<'a> {
.push(Message::Finish(self.id, Artifact::Metadata, Ok(())));
}

pub fn future_incompat_report(&self, report: Vec<FutureBreakageItem>) {
self.messages
.push(Message::FutureIncompatReport(self.id, report));
}

/// The rustc underlying this Job is about to acquire a jobserver token (i.e., block)
/// on the passed client.
///
Expand Down Expand Up @@ -410,6 +428,7 @@ impl<'cfg> JobQueue<'cfg> {
pending_queue: Vec::new(),
print: DiagnosticPrinter::new(cx.bcx.config),
finished: 0,
per_crate_future_incompat_reports: Vec::new(),
};

// Create a helper thread for acquiring jobserver tokens
Expand Down Expand Up @@ -591,6 +610,11 @@ impl<'cfg> DrainState<'cfg> {
}
}
}
Message::FutureIncompatReport(id, report) => {
let package_id = self.active[&id].pkg.package_id();
self.per_crate_future_incompat_reports
.push(FutureIncompatReportCrate { package_id, report });
}
Message::Token(acquired_token) => {
let token = acquired_token.chain_err(|| "failed to acquire jobserver token")?;
self.tokens.push(token);
Expand Down Expand Up @@ -771,14 +795,99 @@ impl<'cfg> DrainState<'cfg> {
if !cx.bcx.build_config.build_plan {
// It doesn't really matter if this fails.
drop(cx.bcx.config.shell().status("Finished", message));
self.emit_future_incompat(cx);
}

None
} else {
debug!("queue: {:#?}", self.queue);
Some(internal("finished with jobs still left in the queue"))
}
}

fn emit_future_incompat(&mut self, cx: &mut Context<'_, '_>) {
if cx.bcx.config.cli_unstable().enable_future_incompat_feature
&& !self.per_crate_future_incompat_reports.is_empty()
{
self.per_crate_future_incompat_reports
.sort_by_key(|r| r.package_id);

let crates_and_versions = self
.per_crate_future_incompat_reports
.iter()
.map(|r| r.package_id.to_string())
.collect::<Vec<_>>()
.join(", ");

drop(cx.bcx.config.shell().warn(&format!(
"the following crates contain code that will be rejected by a future version of Rust: {}",
crates_and_versions
)));

let mut full_report = String::new();
let mut rng = thread_rng();

// Generate a short ID to allow detecting if a report gets overwritten
let id: String = std::iter::repeat(())
.map(|()| char::from(rng.sample(Alphanumeric)))
.take(4)
.collect();

for report in std::mem::take(&mut self.per_crate_future_incompat_reports) {
full_report.push_str(&format!(
"The crate `{}` currently triggers the following future incompatibility lints:\n",
report.package_id
));
for item in report.report {
let rendered = if cx.bcx.config.shell().err_supports_color() {
item.diagnostic.rendered
} else {
strip_ansi_escapes::strip(&item.diagnostic.rendered)
.map(|v| String::from_utf8(v).expect("utf8"))
.expect("strip should never fail")
};

for line in rendered.lines() {
full_report.push_str(&format!("> {}\n", line));
}
}
}

let report_file = cx.bcx.ws.target_dir().open_rw(
FUTURE_INCOMPAT_FILE,
cx.bcx.config,
"Future incompatibility report",
);
let err = report_file
.and_then(|report_file| {
let on_disk_report = OnDiskReport {
id: id.clone(),
report: full_report.clone(),
};
serde_json::to_writer(report_file, &on_disk_report).map_err(|e| e.into())
})
.err();
if let Some(e) = err {
crate::display_warning_with_error(
"failed to write on-disk future incompat report",
&e,
&mut cx.bcx.config.shell(),
);
}

if cx.bcx.build_config.future_incompat_report {
drop_eprint!(cx.bcx.config, "{}", full_report);
drop(cx.bcx.config.shell().note(
&format!("this report can be shown with `cargo describe-future-incompatibilities -Z future-incompat-report --id {}`", id)
));
} else {
drop(cx.bcx.config.shell().note(
&format!("to see what the problems were, use the option `--future-incompat-report`, or run `cargo describe-future-incompatibilities --id {}`", id)
));
}
}
}

fn handle_error(
&self,
shell: &mut Shell,
Expand Down
Loading

0 comments on commit 7441e8c

Please sign in to comment.