From 6177c6584bccd3d959d5c2739a73eed09083a9a5 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 29 Oct 2020 17:48:09 -0400 Subject: [PATCH 1/4] 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. --- Cargo.toml | 1 + src/bin/cargo/commands/build.rs | 1 + src/bin/cargo/commands/check.rs | 1 + .../describe_future_incompatibilities.rs | 50 ++++++ src/bin/cargo/commands/mod.rs | 3 + src/bin/cargo/commands/rustc.rs | 1 + src/cargo/core/compiler/build_config.rs | 3 + src/cargo/core/compiler/future_incompat.rs | 36 ++++ src/cargo/core/compiler/job_queue.rs | 108 ++++++++++++ src/cargo/core/compiler/mod.rs | 50 ++++-- src/cargo/core/features.rs | 2 + src/cargo/util/command_prelude.rs | 21 +++ tests/testsuite/future_incompat_report.rs | 165 ++++++++++++++++++ tests/testsuite/main.rs | 1 + 14 files changed, 430 insertions(+), 13 deletions(-) create mode 100644 src/bin/cargo/commands/describe_future_incompatibilities.rs create mode 100644 src/cargo/core/compiler/future_incompat.rs create mode 100644 tests/testsuite/future_incompat_report.rs diff --git a/Cargo.toml b/Cargo.toml index b7dc747b80a..85d99c1ae25 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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"] } diff --git a/src/bin/cargo/commands/build.rs b/src/bin/cargo/commands/build.rs index 238d0864808..299f06cc925 100644 --- a/src/bin/cargo/commands/build.rs +++ b/src/bin/cargo/commands/build.rs @@ -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") } diff --git a/src/bin/cargo/commands/check.rs b/src/bin/cargo/commands/check.rs index fec00ad8b7e..8c14395bf73 100644 --- a/src/bin/cargo/commands/check.rs +++ b/src/bin/cargo/commands/check.rs @@ -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") } diff --git a/src/bin/cargo/commands/describe_future_incompatibilities.rs b/src/bin/cargo/commands/describe_future_incompatibilities.rs new file mode 100644 index 00000000000..cf7ca54e71c --- /dev/null +++ b/src/bin/cargo/commands/describe_future_incompatibilities.rs @@ -0,0 +1,50 @@ +use crate::command_prelude::*; +use anyhow::anyhow; +use cargo::core::compiler::future_incompat::{OnDiskReport, FUTURE_INCOMPAT_FILE}; +use cargo::core::nightly_features_allowed; +use cargo::drop_eprint; +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 !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) + .map_err(|e| anyhow!("Failed to read report: {:?}", e))?; + let on_disk_report: OnDiskReport = serde_json::from_str(&file_contents).unwrap(); + + 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(()) +} diff --git a/src/bin/cargo/commands/mod.rs b/src/bin/cargo/commands/mod.rs index eb72c955f05..56ca19bb117 100644 --- a/src/bin/cargo/commands/mod.rs +++ b/src/bin/cargo/commands/mod.rs @@ -6,6 +6,7 @@ pub fn builtin() -> Vec { build::cli(), check::cli(), clean::cli(), + describe_future_incompatibilities::cli(), doc::cli(), fetch::cli(), fix::cli(), @@ -44,6 +45,7 @@ pub fn builtin_exec(cmd: &str) -> Option) -> 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, @@ -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; diff --git a/src/bin/cargo/commands/rustc.rs b/src/bin/cargo/commands/rustc.rs index b8a8952610e..d71d182b31b 100644 --- a/src/bin/cargo/commands/rustc.rs +++ b/src/bin/cargo/commands/rustc.rs @@ -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") } diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index d0f5431a47e..593aaf2076d 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -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, + /// `true` to output a future incompatibility report at the end of the build + pub future_incompat_report: bool, } impl BuildConfig { @@ -80,6 +82,7 @@ impl BuildConfig { primary_unit_rustc: None, rustfix_diagnostic_server: RefCell::new(None), export_dir: None, + future_incompat_report: false, }) } diff --git a/src/cargo/core/compiler/future_incompat.rs b/src/cargo/core/compiler/future_incompat.rs new file mode 100644 index 00000000000..0b3705958ba --- /dev/null +++ b/src/cargo/core/compiler/future_incompat.rs @@ -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, +} + +#[derive(Serialize, Deserialize)] +pub struct FutureBreakageItem { + /// The date at which this lint will become an error. + /// Currently unused + pub future_breakage_date: Option, + /// The original diagnostic emitted by the compiler + pub diagnostic: Diagnostic, +} + +/// A diagnostic emitted by the compiler a 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, +} diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index de9d66ad838..1e7efede83e 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -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::{ @@ -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}; @@ -151,6 +157,8 @@ struct DrainState<'cfg> { /// How many jobs we've finished finished: usize, + show_future_incompat_report: bool, + per_crate_future_incompat_reports: Vec, } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -162,6 +170,11 @@ impl std::fmt::Display for JobId { } } +struct FutureIncompatReportCrate { + package_id: PackageId, + report: Vec, +} + /// 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. /// @@ -228,6 +241,7 @@ enum Message { FixDiagnostic(diagnostic_server::Message), Token(io::Result), Finish(JobId, Artifact, CargoResult<()>), + FutureIncompatReport(JobId, Vec), // This client should get release_raw called on it with one of our tokens NeedsToken(JobId), @@ -282,6 +296,11 @@ impl<'a> JobState<'a> { .push(Message::Finish(self.id, Artifact::Metadata, Ok(()))); } + pub fn future_incompat_report(&self, report: Vec) { + 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. /// @@ -410,6 +429,8 @@ impl<'cfg> JobQueue<'cfg> { pending_queue: Vec::new(), print: DiagnosticPrinter::new(cx.bcx.config), finished: 0, + show_future_incompat_report: cx.bcx.build_config.future_incompat_report, + per_crate_future_incompat_reports: Vec::new(), }; // Create a helper thread for acquiring jobserver tokens @@ -591,6 +612,14 @@ impl<'cfg> DrainState<'cfg> { } } } + Message::FutureIncompatReport(id, report) => { + let unit = self.active[&id].clone(); + self.per_crate_future_incompat_reports + .push(FutureIncompatReportCrate { + package_id: unit.pkg.package_id(), + report, + }); + } Message::Token(acquired_token) => { let token = acquired_token.chain_err(|| "failed to acquire jobserver token")?; self.tokens.push(token); @@ -771,7 +800,9 @@ 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); @@ -779,6 +810,83 @@ impl<'cfg> DrainState<'cfg> { } } + 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| format!("{}", r.package_id)) + .collect::>() + .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 { + drop(cx.bcx.config.shell().warn(&format!( + "Failed to open on-disk future incompat report: {:?}", + e + ))); + } + + if self.show_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, diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 71e3abd3eae..ef08bc942db 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -7,6 +7,7 @@ mod context; mod crate_type; mod custom_build; mod fingerprint; +pub mod future_incompat; mod job; mod job_queue; mod layout; @@ -48,6 +49,7 @@ pub(crate) use self::layout::Layout; pub use self::lto::Lto; use self::output_depinfo::output_depinfo; use self::unit_graph::UnitDep; +use crate::core::compiler::future_incompat::FutureIncompatReport; pub use crate::core::compiler::unit::{Unit, UnitInterner}; use crate::core::manifest::TargetSourcePath; use crate::core::profiles::{PanicStrategy, Profile, Strip}; @@ -172,18 +174,17 @@ fn compile<'cfg>( }; work.then(link_targets(cx, unit, false)?) } else { - let work = if unit.show_warnings(bcx.config) { - replay_output_cache( - unit.pkg.package_id(), - PathBuf::from(unit.pkg.manifest_path()), - &unit.target, - cx.files().message_cache_path(unit), - cx.bcx.build_config.message_format, - cx.bcx.config.shell().err_supports_color(), - ) - } else { - Work::noop() - }; + // We always replay the output cache, + // since it might contain future-incompat-report messages + let work = replay_output_cache( + unit.pkg.package_id(), + PathBuf::from(unit.pkg.manifest_path()), + &unit.target, + cx.files().message_cache_path(unit), + cx.bcx.build_config.message_format, + cx.bcx.config.shell().err_supports_color(), + unit.show_warnings(bcx.config), + ); // Need to link targets on both the dirty and fresh. work.then(link_targets(cx, unit, true)?) }); @@ -924,6 +925,10 @@ fn build_base_args( .env("RUSTC_BOOTSTRAP", "1"); } + if bcx.config.cli_unstable().enable_future_incompat_feature { + cmd.arg("-Z").arg("emit-future-incompat-report"); + } + // Add `CARGO_BIN_` environment variables for building tests. if unit.target.is_test() || unit.target.is_bench() { for bin_target in unit @@ -1128,6 +1133,10 @@ struct OutputOptions { /// of empty files are not created. If this is None, the output will not /// be cached (such as when replaying cached messages). cache_cell: Option<(PathBuf, LazyCell)>, + /// If `true`, display any recorded warning messages. + /// Other types of messages are processed regardless + /// of the value of this flag + show_warnings: bool, } impl OutputOptions { @@ -1143,6 +1152,7 @@ impl OutputOptions { look_for_metadata_directive, color, cache_cell, + show_warnings: true, } } } @@ -1210,6 +1220,11 @@ fn on_stderr_line_inner( } }; + if let Ok(report) = serde_json::from_str::(compiler_message.get()) { + state.future_incompat_report(report.future_incompat_report); + return Ok(true); + } + // Depending on what we're emitting from Cargo itself, we figure out what to // do with this JSON message. match options.format { @@ -1241,7 +1256,9 @@ fn on_stderr_line_inner( .map(|v| String::from_utf8(v).expect("utf8")) .expect("strip should never fail") }; - state.stderr(rendered)?; + if options.show_warnings { + state.stderr(rendered)?; + } return Ok(true); } } @@ -1322,6 +1339,11 @@ fn on_stderr_line_inner( // And failing all that above we should have a legitimate JSON diagnostic // from the compiler, so wrap it in an external Cargo JSON message // indicating which package it came from and then emit it. + + if !options.show_warnings { + return Ok(true); + } + let msg = machine_message::FromCompiler { package_id, manifest_path, @@ -1344,6 +1366,7 @@ fn replay_output_cache( path: PathBuf, format: MessageFormat, color: bool, + show_warnings: bool, ) -> Work { let target = target.clone(); let mut options = OutputOptions { @@ -1351,6 +1374,7 @@ fn replay_output_cache( look_for_metadata_directive: true, color, cache_cell: None, + show_warnings, }; Work::new(move |state| { if !path.exists() { diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 2a2816e9ada..550d50fc84a 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -558,6 +558,7 @@ pub struct CliUnstable { pub extra_link_arg: bool, pub credential_process: bool, pub configurable_env: bool, + pub enable_future_incompat_feature: bool, } const STABILIZED_COMPILE_PROGRESS: &str = "The progress bar is now always \ @@ -754,6 +755,7 @@ impl CliUnstable { "config-profile" => stabilized_warn(k, "1.43", STABILIZED_CONFIG_PROFILE), "crate-versions" => stabilized_warn(k, "1.47", STABILIZED_CRATE_VERSIONS), "package-features" => stabilized_warn(k, "1.51", STABILIZED_PACKAGE_FEATURES), + "future-incompat-report" => self.enable_future_incompat_feature = parse_empty(k, v)?, _ => bail!("unknown `-Z` flag specified: {}", k), } diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 3d7268caac5..b449691aade 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -219,6 +219,13 @@ pub trait AppExt: Sized { "Ignore `rust-version` specification in packages (unstable)", )) } + + fn arg_future_incompat_report(self) -> Self { + self._arg(opt( + "future-incompat-report", + "Ouputs a future incompatibility report at the end of the build (unstable)", + )) + } } impl AppExt for App { @@ -462,6 +469,7 @@ pub trait ArgMatchesExt { build_config.requested_profile = self.get_profile_name(config, "dev", profile_checking)?; build_config.build_plan = self._is_present("build-plan"); build_config.unit_graph = self._is_present("unit-graph"); + build_config.future_incompat_report = self._is_present("future-incompat-report"); if build_config.build_plan { config .cli_unstable() @@ -472,6 +480,19 @@ pub trait ArgMatchesExt { .cli_unstable() .fail_if_stable_opt("--unit-graph", 8002)?; } + if build_config.future_incompat_report { + config + .cli_unstable() + // TODO: Tracking issue + .fail_if_stable_opt("--future-incompat-report", 0)?; + + if !config.cli_unstable().enable_future_incompat_feature { + anyhow::bail!( + "Usage of `--future-incompat-report` requires `-Z future-incompat-report`" + ) + } + } + let opts = CompileOptions { build_config, features: self._values_of("features"), diff --git a/tests/testsuite/future_incompat_report.rs b/tests/testsuite/future_incompat_report.rs new file mode 100644 index 00000000000..76472b15e8b --- /dev/null +++ b/tests/testsuite/future_incompat_report.rs @@ -0,0 +1,165 @@ +/// Tests for future-incompat-report messages +use cargo_test_support::registry::Package; +use cargo_test_support::{basic_manifest, is_nightly, project}; + +#[cargo_test] +fn no_output_on_stable() { + let p = project() + .file("Cargo.toml", &basic_manifest("foo", "0.0.0")) + .file("src/main.rs", "fn main() { [true].into_iter(); }") + .build(); + + p.cargo("build") + .with_stderr_contains(" = note: `#[warn(array_into_iter)]` on by default") + .with_stderr_does_not_contain("warning: the following crates contain code that will be rejected by a future version of Rust: `foo` v0.0.0") + .run(); +} + +#[cargo_test] +fn gate_future_incompat_report() { + let p = project() + .file("Cargo.toml", &basic_manifest("foo", "0.0.0")) + .file("src/main.rs", "fn main() { [true].into_iter(); }") + .build(); + + p.cargo("build --future-incompat-report") + .with_stderr_contains("error: the `--future-incompat-report` flag is unstable[..]") + .with_status(101) + .run(); + + // Both `-Z future-incompat-report` and `-Z unstable-opts` are required + p.cargo("build --future-incompat-report -Z future-incompat-report") + .masquerade_as_nightly_cargo() + .with_stderr_contains("error: the `--future-incompat-report` flag is unstable[..]") + .with_status(101) + .run(); + + p.cargo("build --future-incompat-report -Z unstable-options") + .masquerade_as_nightly_cargo() + .with_stderr_contains( + "error: Usage of `--future-incompat-report` requires `-Z future-incompat-report`", + ) + .with_status(101) + .run(); + + p.cargo("describe-future-incompatibilities --id foo") + .with_stderr_contains( + "error: `cargo describe-future-incompatibilities` can only be used on the nightly channel" + ) + .with_status(101) + .run(); +} + +#[cargo_test] +fn test_single_crate() { + if !is_nightly() { + return; + } + + let p = project() + .file("Cargo.toml", &basic_manifest("foo", "0.0.0")) + .file("src/main.rs", "fn main() { [true].into_iter(); }") + .build(); + + for command in &["build", "check", "rustc"] { + p.cargo(command).arg("-Zfuture-incompat-report") + .masquerade_as_nightly_cargo() + .with_stderr_contains(" = note: `#[warn(array_into_iter)]` on by default") + .with_stderr_contains("warning: the following crates contain code that will be rejected by a future version of Rust: foo v0.0.0 [..]") + .with_stderr_does_not_contain("[..] future incompatibility lints:") + .run(); + + p.cargo(command).arg("-Zfuture-incompat-report").arg("-Zunstable-options").arg("--future-incompat-report") + .masquerade_as_nightly_cargo() + .with_stderr_contains(" = note: `#[warn(array_into_iter)]` on by default") + .with_stderr_contains("warning: the following crates contain code that will be rejected by a future version of Rust: foo v0.0.0 [..]") + .with_stderr_contains("The crate `foo v0.0.0 ([..])` currently triggers the following future incompatibility lints:") + .run(); + } +} + +#[cargo_test] +fn test_multi_crate() { + if !is_nightly() { + return; + } + + Package::new("first-dep", "0.0.1") + .file("src/lib.rs", "fn foo() { [25].into_iter(); }") + .publish(); + Package::new("second-dep", "0.0.2") + .file("src/lib.rs", "fn foo() { ['a'].into_iter(); }") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.0" + + [dependencies] + first-dep = "*" + second-dep = "*" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + for command in &["build", "check", "rustc"] { + p.cargo(&format!("{} -Z future-incompat-report", command)) + .masquerade_as_nightly_cargo() + .with_stderr_does_not_contain(" = note: `#[warn(array_into_iter)]` on by default") + .with_stderr_contains("warning: the following crates contain code that will be rejected by a future version of Rust: first-dep v0.0.1, second-dep v0.0.2") + .with_stderr_does_not_contain("The crate `foo` v0.0.0 currently triggers the following future incompatibility lints:") + .run(); + + p.cargo("describe-future-incompatibilities -Z future-incompat-report --id bad-id") + .masquerade_as_nightly_cargo() + .with_stderr_does_not_contain(" = note: `#[warn(array_into_iter)]` on by default") + .with_stderr_contains("error: Expected an id of [..]") + .with_stderr_does_not_contain("The crate `first-dep v0.0.1` currently triggers the following future incompatibility lints:") + .with_stderr_does_not_contain("The crate `second-dep v0.0.2` currently triggers the following future incompatibility lints:") + .with_status(101) + .run(); + + p.cargo(&format!("{} -Z unstable-options -Z future-incompat-report --future-incompat-report", command)) + .masquerade_as_nightly_cargo() + .with_stderr_does_not_contain(" = note: `#[warn(array_into_iter)]` on by default") + .with_stderr_contains("warning: the following crates contain code that will be rejected by a future version of Rust: first-dep v0.0.1, second-dep v0.0.2") + .with_stderr_contains("The crate `first-dep v0.0.1` currently triggers the following future incompatibility lints:") + .with_stderr_contains("The crate `second-dep v0.0.2` currently triggers the following future incompatibility lints:") + .run(); + } + + // Test that passing the correct id via '--id' doesn't generate a warning message + let output = p + .cargo("build -Z future-incompat-report") + .masquerade_as_nightly_cargo() + .exec_with_output() + .unwrap(); + + // Extract the 'id' from the stdout. We are looking + // for the id in a line of the form "run `cargo describe-future-incompatibilities --id 721d0666-81b6-4765-84fc-fd2832328324`" + // which is generated by Cargo to tell the user what command to run + // This is just to test that passing the id suppresses the warning mesasge. Any users needing + // access to the report from a shell script should use the `--future-incompat-report` flag + let stderr = std::str::from_utf8(&output.stderr).unwrap(); + + // Find '--id ' in the output + let mut iter = stderr.split(" "); + iter.find(|w| *w == "--id").unwrap(); + let id = iter + .next() + .unwrap_or_else(|| panic!("Unexpected output:\n{}", stderr)); + // Strip off the trailing '`' included in the output + let id: String = id.chars().take_while(|c| *c != '`').collect(); + + p.cargo(&format!("describe-future-incompatibilities -Z future-incompat-report --id {}", id)) + .masquerade_as_nightly_cargo() + .with_stderr_does_not_contain("warning: Expected an id of [..]") + .with_stderr_contains("The crate `first-dep v0.0.1` currently triggers the following future incompatibility lints:") + .with_stderr_contains("The crate `second-dep v0.0.2` currently triggers the following future incompatibility lints:") + .run(); +} diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 1f1dd634c04..1d4c967b99d 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -52,6 +52,7 @@ mod features_namespaced; mod fetch; mod fix; mod freshness; +mod future_incompat_report; mod generate_lockfile; mod git; mod git_auth; From f03d47ce4b3e541fb857308f8d12939602eb739d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 4 Mar 2021 14:04:36 -0500 Subject: [PATCH 2/4] Address review comments --- .../describe_future_incompatibilities.rs | 17 +++++++--- src/cargo/core/compiler/future_incompat.rs | 2 +- src/cargo/core/compiler/job_queue.rs | 33 ++++++++++--------- src/doc/src/reference/unstable.md | 8 +++++ tests/testsuite/future_incompat_report.rs | 19 +++++------ 5 files changed, 46 insertions(+), 33 deletions(-) diff --git a/src/bin/cargo/commands/describe_future_incompatibilities.rs b/src/bin/cargo/commands/describe_future_incompatibilities.rs index cf7ca54e71c..8a8371d69b2 100644 --- a/src/bin/cargo/commands/describe_future_incompatibilities.rs +++ b/src/bin/cargo/commands/describe_future_incompatibilities.rs @@ -1,8 +1,8 @@ use crate::command_prelude::*; use anyhow::anyhow; use cargo::core::compiler::future_incompat::{OnDiskReport, FUTURE_INCOMPAT_FILE}; -use cargo::core::nightly_features_allowed; use cargo::drop_eprint; +use cargo::util::CargoResultExt; use std::io::Read; pub fn cli() -> App { @@ -19,7 +19,7 @@ pub fn cli() -> App { } pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { - if !nightly_features_allowed() { + if !config.nightly_features_allowed { return Err(anyhow!( "`cargo describe-future-incompatibilities` can only be used on the nightly channel" ) @@ -37,12 +37,19 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { report_file .file() .read_to_string(&mut file_contents) - .map_err(|e| anyhow!("Failed to read report: {:?}", e))?; - let on_disk_report: OnDiskReport = serde_json::from_str(&file_contents).unwrap(); + .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()); + 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); diff --git a/src/cargo/core/compiler/future_incompat.rs b/src/cargo/core/compiler/future_incompat.rs index 0b3705958ba..0a74aa5f110 100644 --- a/src/cargo/core/compiler/future_incompat.rs +++ b/src/cargo/core/compiler/future_incompat.rs @@ -15,7 +15,7 @@ pub struct FutureBreakageItem { pub diagnostic: Diagnostic, } -/// A diagnostic emitted by the compiler a a JSON message. +/// A diagnostic emitted by the compiler as a JSON message. /// We only care about the 'rendered' field #[derive(Serialize, Deserialize)] pub struct Diagnostic { diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index 1e7efede83e..6df97baabbc 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -157,7 +157,6 @@ struct DrainState<'cfg> { /// How many jobs we've finished finished: usize, - show_future_incompat_report: bool, per_crate_future_incompat_reports: Vec, } @@ -429,7 +428,6 @@ impl<'cfg> JobQueue<'cfg> { pending_queue: Vec::new(), print: DiagnosticPrinter::new(cx.bcx.config), finished: 0, - show_future_incompat_report: cx.bcx.build_config.future_incompat_report, per_crate_future_incompat_reports: Vec::new(), }; @@ -613,12 +611,9 @@ impl<'cfg> DrainState<'cfg> { } } Message::FutureIncompatReport(id, report) => { - let unit = self.active[&id].clone(); + let package_id = self.active[&id].pkg.package_id(); self.per_crate_future_incompat_reports - .push(FutureIncompatReportCrate { - package_id: unit.pkg.package_id(), - report, - }); + .push(FutureIncompatReportCrate { package_id, report }); } Message::Token(acquired_token) => { let token = acquired_token.chain_err(|| "failed to acquire jobserver token")?; @@ -820,12 +815,14 @@ impl<'cfg> DrainState<'cfg> { let crates_and_versions = self .per_crate_future_incompat_reports .iter() - .map(|r| format!("{}", r.package_id)) + .map(|r| r.package_id.to_string()) .collect::>() .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))); + 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(); @@ -837,7 +834,10 @@ impl<'cfg> DrainState<'cfg> { .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)); + 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 @@ -868,13 +868,14 @@ impl<'cfg> DrainState<'cfg> { }) .err(); if let Some(e) = err { - drop(cx.bcx.config.shell().warn(&format!( - "Failed to open on-disk future incompat report: {:?}", - e - ))); + crate::display_warning_with_error( + "failed to write on-disk future incompat report", + &e, + &mut cx.bcx.config.shell(), + ); } - if self.show_future_incompat_report { + 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) diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index fb248427a68..fe116146bc1 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -1122,3 +1122,11 @@ The 2021 edition will set the default [resolver version] to "2". } })(); + +### future incompat report +* RFC: [#2834](https://github.com/rust-lang/rfcs/blob/master/text/2834-cargo-report-future-incompat.md) +* rustc Tracking Issue: [#71249](https://github.com/rust-lang/rust/issues/71249) + +The `-Z future-incompat-report` flag enables the creation of a future-incompat report +for all dependencies. This makes users aware if any of their crate's dependencies +might stop compiling with a future version of Rust. diff --git a/tests/testsuite/future_incompat_report.rs b/tests/testsuite/future_incompat_report.rs index 76472b15e8b..0bb0edac1eb 100644 --- a/tests/testsuite/future_incompat_report.rs +++ b/tests/testsuite/future_incompat_report.rs @@ -11,7 +11,7 @@ fn no_output_on_stable() { p.cargo("build") .with_stderr_contains(" = note: `#[warn(array_into_iter)]` on by default") - .with_stderr_does_not_contain("warning: the following crates contain code that will be rejected by a future version of Rust: `foo` v0.0.0") + .with_stderr_does_not_contain("[..]crates[..]") .run(); } @@ -66,7 +66,7 @@ fn test_single_crate() { .masquerade_as_nightly_cargo() .with_stderr_contains(" = note: `#[warn(array_into_iter)]` on by default") .with_stderr_contains("warning: the following crates contain code that will be rejected by a future version of Rust: foo v0.0.0 [..]") - .with_stderr_does_not_contain("[..] future incompatibility lints:") + .with_stderr_does_not_contain("[..]incompatibility[..]") .run(); p.cargo(command).arg("-Zfuture-incompat-report").arg("-Zunstable-options").arg("--future-incompat-report") @@ -108,25 +108,23 @@ fn test_multi_crate() { .build(); for command in &["build", "check", "rustc"] { - p.cargo(&format!("{} -Z future-incompat-report", command)) + p.cargo(command).arg("-Zfuture-incompat-report") .masquerade_as_nightly_cargo() - .with_stderr_does_not_contain(" = note: `#[warn(array_into_iter)]` on by default") + .with_stderr_does_not_contain("[..]array_into_iter[..]") .with_stderr_contains("warning: the following crates contain code that will be rejected by a future version of Rust: first-dep v0.0.1, second-dep v0.0.2") - .with_stderr_does_not_contain("The crate `foo` v0.0.0 currently triggers the following future incompatibility lints:") + // Check that we don't have the 'triggers' message shown at the bottom of this loop + .with_stderr_does_not_contain("[..]triggers[..]") .run(); p.cargo("describe-future-incompatibilities -Z future-incompat-report --id bad-id") .masquerade_as_nightly_cargo() - .with_stderr_does_not_contain(" = note: `#[warn(array_into_iter)]` on by default") .with_stderr_contains("error: Expected an id of [..]") - .with_stderr_does_not_contain("The crate `first-dep v0.0.1` currently triggers the following future incompatibility lints:") - .with_stderr_does_not_contain("The crate `second-dep v0.0.2` currently triggers the following future incompatibility lints:") + .with_stderr_does_not_contain("[..]triggers[..]") .with_status(101) .run(); - p.cargo(&format!("{} -Z unstable-options -Z future-incompat-report --future-incompat-report", command)) + p.cargo(command).arg("-Zunstable-options").arg("-Zfuture-incompat-report").arg("--future-incompat-report") .masquerade_as_nightly_cargo() - .with_stderr_does_not_contain(" = note: `#[warn(array_into_iter)]` on by default") .with_stderr_contains("warning: the following crates contain code that will be rejected by a future version of Rust: first-dep v0.0.1, second-dep v0.0.2") .with_stderr_contains("The crate `first-dep v0.0.1` currently triggers the following future incompatibility lints:") .with_stderr_contains("The crate `second-dep v0.0.2` currently triggers the following future incompatibility lints:") @@ -158,7 +156,6 @@ fn test_multi_crate() { p.cargo(&format!("describe-future-incompatibilities -Z future-incompat-report --id {}", id)) .masquerade_as_nightly_cargo() - .with_stderr_does_not_contain("warning: Expected an id of [..]") .with_stderr_contains("The crate `first-dep v0.0.1` currently triggers the following future incompatibility lints:") .with_stderr_contains("The crate `second-dep v0.0.2` currently triggers the following future incompatibility lints:") .run(); From 9ea35036802bb87e605e0606091b1675f3b492e5 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 5 Mar 2021 14:01:01 -0800 Subject: [PATCH 3/4] Fix some minor formatting issues. --- src/bin/cargo/commands/describe_future_incompatibilities.rs | 4 ++-- tests/testsuite/future_incompat_report.rs | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/bin/cargo/commands/describe_future_incompatibilities.rs b/src/bin/cargo/commands/describe_future_incompatibilities.rs index 8a8371d69b2..dd1b5f2b10b 100644 --- a/src/bin/cargo/commands/describe_future_incompatibilities.rs +++ b/src/bin/cargo/commands/describe_future_incompatibilities.rs @@ -44,8 +44,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { 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.", + "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 ) diff --git a/tests/testsuite/future_incompat_report.rs b/tests/testsuite/future_incompat_report.rs index 0bb0edac1eb..403c8566e38 100644 --- a/tests/testsuite/future_incompat_report.rs +++ b/tests/testsuite/future_incompat_report.rs @@ -1,4 +1,5 @@ -/// Tests for future-incompat-report messages +//! Tests for future-incompat-report messages + use cargo_test_support::registry::Package; use cargo_test_support::{basic_manifest, is_nightly, project}; @@ -139,7 +140,7 @@ fn test_multi_crate() { .unwrap(); // Extract the 'id' from the stdout. We are looking - // for the id in a line of the form "run `cargo describe-future-incompatibilities --id 721d0666-81b6-4765-84fc-fd2832328324`" + // for the id in a line of the form "run `cargo describe-future-incompatibilities --id yZ7S`" // which is generated by Cargo to tell the user what command to run // This is just to test that passing the id suppresses the warning mesasge. Any users needing // access to the report from a shell script should use the `--future-incompat-report` flag From 139ed73f536d73467161cf2b88b2ed173ce52a99 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 5 Mar 2021 14:05:52 -0800 Subject: [PATCH 4/4] Add future-incompat tracking issue number. --- src/cargo/util/command_prelude.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index b449691aade..d27fbfb972c 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -484,7 +484,7 @@ pub trait ArgMatchesExt { config .cli_unstable() // TODO: Tracking issue - .fail_if_stable_opt("--future-incompat-report", 0)?; + .fail_if_stable_opt("--future-incompat-report", 9241)?; if !config.cli_unstable().enable_future_incompat_feature { anyhow::bail!(