Stabilize cache-messages

Author: ehuss

src/bin/cargo/cli.rs

@@ -35,7 +35,6 @@ Available unstable (nightly-only) flags:
     -Z unstable-options -- Allow the usage of unstable options such as --registry
     -Z config-profile   -- Read profiles from .cargo/config files
     -Z install-upgrade  -- `cargo install` will upgrade instead of failing
-    -Z cache-messages   -- Cache compiler messages
     -Z timings          -- Display concurrency information
     -Z doctest-xcompile -- Compile and run doctests for non-host target using runner config
 

src/bin/cargo/commands/clippy.rs

@@ -79,7 +79,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     }
 
     compile_opts.build_config.primary_unit_rustc = Some(wrapper);
-    compile_opts.build_config.force_rebuild = true;
 
     ops::compile(&ws, &compile_opts)?;
     Ok(())

src/cargo/core/compiler/build_config.rs

@@ -43,8 +43,6 @@ pub struct BuildConfig {
     /// An optional override of the rustc path for primary units only
     pub primary_unit_rustc: Option<ProcessBuilder>,
     pub rustfix_diagnostic_server: RefCell<Option<RustfixDiagnosticServer>>,
-    /// Whether or not Cargo should cache compiler output on disk.
-    cache_messages: bool,
 }
 
 impl BuildConfig {
@@ -101,15 +99,9 @@ impl BuildConfig {
             build_plan: false,
             primary_unit_rustc: None,
             rustfix_diagnostic_server: RefCell::new(None),
-            cache_messages: config.cli_unstable().cache_messages,
         })
     }
 
-    /// Whether or not Cargo should cache compiler messages on disk.
-    pub fn cache_messages(&self) -> bool {
-        self.cache_messages
-    }
-
     /// Whether or not the *user* wants JSON output. Whether or not rustc
     /// actually uses JSON is decided in `add_error_format`.
     pub fn emit_json(&self) -> bool {

src/cargo/core/compiler/context/compilation_files.rs

@@ -553,6 +553,14 @@ fn compute_metadata<'a, 'cfg>(
 
     bcx.rustc.verbose_version.hash(&mut hasher);
 
+    if cx.is_primary_package(unit) {
+        // This is primarily here for clippy. This ensures that the clippy
+        // artifacts are separate from the `check` ones.
+        if let Some(proc) = &cx.bcx.build_config.primary_unit_rustc {
+            proc.get_program().hash(&mut hasher);
+        }
+    }
+
     // Seed the contents of `__CARGO_DEFAULT_LIB_METADATA` to the hasher if present.
     // This should be the release channel, to get a different hash for each channel.
     if let Ok(ref channel) = __cargo_default_lib_metadata {

src/cargo/core/compiler/fingerprint.rs

@@ -1083,11 +1083,23 @@ fn calculate_normal<'a, 'cfg>(
     // Fill out a bunch more information that we'll be tracking typically
     // hashed to take up less space on disk as we just need to know when things
     // change.
-    let extra_flags = if unit.mode.is_doc() {
+    let mut extra_flags = if unit.mode.is_doc() {
         cx.bcx.rustdocflags_args(unit)
     } else {
         cx.bcx.rustflags_args(unit)
-    };
+    }
+    .to_vec();
+    if cx.is_primary_package(unit) {
+        // This is primarily here for clippy arguments.
+        if let Some(proc) = &cx.bcx.build_config.primary_unit_rustc {
+            let args = proc
+                .get_args()
+                .iter()
+                .map(|s| s.to_string_lossy().to_string());
+            extra_flags.extend(args);
+        }
+    }
+
     let profile_hash = util::hash_u64((&unit.profile, unit.mode, cx.bcx.extra_args_for(unit)));
     // Include metadata since it is exposed as environment variables.
     let m = unit.pkg.manifest().metadata();
@@ -1104,7 +1116,7 @@ fn calculate_normal<'a, 'cfg>(
         local: Mutex::new(local),
         memoized_hash: Mutex::new(None),
         metadata,
-        rustflags: extra_flags.to_vec(),
+        rustflags: extra_flags,
         fs_status: FsStatus::Stale,
         outputs,
     })

src/cargo/core/compiler/layout.rs

@@ -36,6 +36,9 @@
 //!                 # Detailed information used for logging the reason why
 //!                 # something is being recompiled.
 //!                 lib-$pkgname-$META.json
+//!                 # The console output from the compiler. This is cached
+//!                 # so that warnings can be redisplayed for "fresh" units.
+//!                 output
 //!
 //!         # This is the root directory for all rustc artifacts except build
 //!         # scripts, examples, and test and bench executables. Almost every

src/cargo/core/compiler/mod.rs

@@ -136,9 +136,7 @@ fn compile<'a, 'cfg: 'a>(
             };
             work.then(link_targets(cx, unit, false)?)
         } else {
-            let work = if cx.bcx.build_config.cache_messages()
-                && cx.bcx.show_warnings(unit.pkg.package_id())
-            {
+            let work = if cx.bcx.show_warnings(unit.pkg.package_id()) {
                 replay_output_cache(
                     unit.pkg.package_id(),
                     unit.target,
@@ -663,79 +661,31 @@ fn add_cap_lints(bcx: &BuildContext<'_, '_>, unit: &Unit<'_>, cmd: &mut ProcessB
 
 /// Add error-format flags to the command.
 ///
-/// This is somewhat odd right now, but the general overview is that if
-/// `-Zcache-messages` or `pipelined` is enabled then Cargo always uses JSON
-/// output. This has several benefits, such as being easier to parse, handles
-/// changing formats (for replaying cached messages), ensures atomic output (so
-/// messages aren't interleaved), etc.
-///
-/// It is intended in the future that Cargo *always* uses the JSON output (by
-/// turning on cache-messages by default), and this function can be simplified.
+/// Cargo always uses JSON output. This has several benefits, such as being
+/// easier to parse, handles changing formats (for replaying cached messages),
+/// ensures atomic output (so messages aren't interleaved), allows for
+/// intercepting messages like rmeta artifacts, etc. rustc includes a
+/// "rendered" field in the JSON message with the message properly formatted,
+/// which Cargo will extract and display to the user.
 fn add_error_format_and_color(
     cx: &Context<'_, '_>,
     cmd: &mut ProcessBuilder,
     pipelined: bool,
 ) -> CargoResult<()> {
-    // If this unit is producing a required rmeta file then we need to know
-    // when the rmeta file is ready so we can signal to the rest of Cargo that
-    // it can continue dependent compilations. To do this we are currently
-    // required to switch the compiler into JSON message mode, but we still
-    // want to present human readable errors as well. (this rabbit hole just
-    // goes and goes)
-    //
-    // All that means is that if we're not already in JSON mode we need to
-    // switch to JSON mode, ensure that rustc error messages can be rendered
-    // prettily, and then when parsing JSON messages from rustc we need to
-    // internally understand that we should extract the `rendered` field and
-    // present it if we can.
-    if cx.bcx.build_config.cache_messages() || pipelined {
-        cmd.arg("--error-format=json");
-        let mut json = String::from("--json=diagnostic-rendered-ansi");
-        if pipelined {
-            json.push_str(",artifacts");
-        }
-        match cx.bcx.build_config.message_format {
-            MessageFormat::Short | MessageFormat::Json { short: true, .. } => {
-                json.push_str(",diagnostic-short");
-            }
-            _ => {}
-        }
-        cmd.arg(json);
-    } else {
-        let mut color = true;
-        match cx.bcx.build_config.message_format {
-            MessageFormat::Human => (),
-            MessageFormat::Json {
-                ansi,
-                short,
-                render_diagnostics,
-            } => {
-                cmd.arg("--error-format").arg("json");
-                // If ansi is explicitly requested, enable it. If we're
-                // rendering diagnostics ourselves then also enable it because
-                // we'll figure out what to do with the colors later.
-                if ansi || render_diagnostics {
-                    cmd.arg("--json=diagnostic-rendered-ansi");
-                }
-                if short {
-                    cmd.arg("--json=diagnostic-short");
-                }
-                color = false;
-            }
-            MessageFormat::Short => {
-                cmd.arg("--error-format").arg("short");
-            }
-        }
-
-        if color {
-            let color = if cx.bcx.config.shell().supports_color() {
-                "always"
-            } else {
-                "never"
-            };
-            cmd.args(&["--color", color]);
+    cmd.arg("--error-format=json");
+    let mut json = String::from("--json=diagnostic-rendered-ansi");
+    if pipelined {
+        // Pipelining needs to know when rmeta files are finished. Tell rustc
+        // to emit a message that cargo will intercept.
+        json.push_str(",artifacts");
+    }
+    match cx.bcx.build_config.message_format {
+        MessageFormat::Short | MessageFormat::Json { short: true, .. } => {
+            json.push_str(",diagnostic-short");
         }
+        _ => {}
     }
+    cmd.arg(json);
     Ok(())
 }
 
@@ -1058,22 +1008,19 @@ struct OutputOptions {
     color: bool,
     /// Where to write the JSON messages to support playback later if the unit
     /// is fresh. The file is created lazily so that in the normal case, lots
-    /// of empty files are not created. This is None if caching is disabled.
+    /// 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<File>)>,
 }
 
 impl OutputOptions {
     fn new<'a>(cx: &Context<'a, '_>, unit: &Unit<'a>) -> OutputOptions {
         let look_for_metadata_directive = cx.rmeta_required(unit);
         let color = cx.bcx.config.shell().supports_color();
-        let cache_cell = if cx.bcx.build_config.cache_messages() {
-            let path = cx.files().message_cache_path(unit);
-            // Remove old cache, ignore ENOENT, which is the common case.
-            drop(fs::remove_file(&path));
-            Some((path, LazyCell::new()))
-        } else {
-            None
-        };
+        let path = cx.files().message_cache_path(unit);
+        // Remove old cache, ignore ENOENT, which is the common case.
+        drop(fs::remove_file(&path));
+        let cache_cell = Some((path, LazyCell::new()));
         OutputOptions {
             format: cx.bcx.build_config.message_format,
             look_for_metadata_directive,
@@ -1100,23 +1047,35 @@ fn on_stderr_line(
     target: &Target,
     options: &mut OutputOptions,
 ) -> CargoResult<()> {
-    // Check if caching is enabled.
-    if let Some((path, cell)) = &mut options.cache_cell {
-        // Cache the output, which will be replayed later when Fresh.
-        let f = cell.try_borrow_mut_with(|| File::create(path))?;
-        debug_assert!(!line.contains('\n'));
-        f.write_all(line.as_bytes())?;
-        f.write_all(&[b'\n'])?;
+    if on_stderr_line_inner(state, line, package_id, target, options)? {
+        // Check if caching is enabled.
+        if let Some((path, cell)) = &mut options.cache_cell {
+            // Cache the output, which will be replayed later when Fresh.
+            let f = cell.try_borrow_mut_with(|| File::create(path))?;
+            debug_assert!(!line.contains('\n'));
+            f.write_all(line.as_bytes())?;
+            f.write_all(&[b'\n'])?;
+        }
     }
+    Ok(())
+}
 
+/// Returns true if the line should be cached.
+fn on_stderr_line_inner(
+    state: &JobState<'_>,
+    line: &str,
+    package_id: PackageId,
+    target: &Target,
+    options: &mut OutputOptions,
+) -> CargoResult<bool> {
     // We primarily want to use this function to process JSON messages from
     // rustc. The compiler should always print one JSON message per line, and
     // otherwise it may have other output intermingled (think RUST_LOG or
     // something like that), so skip over everything that doesn't look like a
     // JSON message.
     if !line.starts_with('{') {
         state.stderr(line.to_string());
-        return Ok(());
+        return Ok(true);
     }
 
     let mut compiler_message: Box<serde_json::value::RawValue> = match serde_json::from_str(line) {
@@ -1128,7 +1087,7 @@ fn on_stderr_line(
         Err(e) => {
             debug!("failed to parse json: {:?}", e);
             state.stderr(line.to_string());
-            return Ok(());
+            return Ok(true);
         }
     };
 
@@ -1164,14 +1123,13 @@ fn on_stderr_line(
                         .expect("strip should never fail")
                 };
                 state.stderr(rendered);
-                return Ok(());
+                return Ok(true);
             }
         }
 
-        // Remove color information from the rendered string. When pipelining is
-        // enabled and/or when cached messages are enabled we're always asking
-        // for ANSI colors from rustc, so unconditionally postprocess here and
-        // remove ansi color codes.
+        // Remove color information from the rendered string if color is not
+        // enabled. Cargo always asks for ANSI colors from rustc. This allows
+        // cached replay to enable/disable colors without re-invoking rustc.
         MessageFormat::Json { ansi: false, .. } => {
             #[derive(serde::Deserialize, serde::Serialize)]
             struct CompilerMessage {
@@ -1213,7 +1171,7 @@ fn on_stderr_line(
                 log::debug!("looks like metadata finished early!");
                 state.rmeta_produced();
             }
-            return Ok(());
+            return Ok(false);
         }
     }
 
@@ -1231,7 +1189,7 @@ fn on_stderr_line(
     // instead. We want the stdout of Cargo to always be machine parseable as
     // stderr has our colorized human-readable messages.
     state.stdout(msg);
-    Ok(())
+    Ok(true)
 }
 
 fn replay_output_cache(
@@ -1244,7 +1202,7 @@ fn replay_output_cache(
     let target = target.clone();
     let mut options = OutputOptions {
         format,
-        look_for_metadata_directive: false,
+        look_for_metadata_directive: true,
         color,
         cache_cell: None,
     };

src/cargo/core/features.rs

@@ -335,7 +335,6 @@ pub struct CliUnstable {
     pub dual_proc_macros: bool,
     pub mtime_on_use: bool,
     pub install_upgrade: bool,
-    pub cache_messages: bool,
     pub named_profiles: bool,
     pub binary_dep_depinfo: bool,
     pub build_std: Option<Vec<String>>,
@@ -393,7 +392,6 @@ impl CliUnstable {
             // can also be set in .cargo/config or with and ENV
             "mtime-on-use" => self.mtime_on_use = true,
             "install-upgrade" => self.install_upgrade = true,
-            "cache-messages" => self.cache_messages = true,
             "named-profiles" => self.named_profiles = true,
             "binary-dep-depinfo" => self.binary_dep_depinfo = true,
             "build-std" => {

src/doc/src/reference/unstable.md

@@ -23,11 +23,11 @@ index each time.
 * Original Issue: [#6477](https://github.com/rust-lang/cargo/pull/6477)
 * Cache usage meta tracking issue: [#7150](https://github.com/rust-lang/cargo/issues/7150)
 
-The `-Z mtime-on-use` flag is an experiment to have Cargo update the mtime of 
-used files to make it easier for tools like cargo-sweep to detect which files 
+The `-Z mtime-on-use` flag is an experiment to have Cargo update the mtime of
+used files to make it easier for tools like cargo-sweep to detect which files
 are stale. For many workflows this needs to be set on *all* invocations of cargo.
 To make this more practical setting the `unstable.mtime_on_use` flag in `.cargo/config`
-or the corresponding ENV variable will apply the `-Z mtime-on-use` to all 
+or the corresponding ENV variable will apply the `-Z mtime-on-use` to all
 invocations of nightly cargo. (the config flag is ignored by stable)
 
 ### avoid-dev-deps
@@ -323,25 +323,6 @@ my_dep = { version = "1.2.3", public = true }
 private_dep = "2.0.0" # Will be 'private' by default
 ```
 
-### cache-messages
-* Tracking Issue: [#6986](https://github.com/rust-lang/cargo/issues/6986)
-
-The `cache-messages` feature causes Cargo to cache the messages generated by
-the compiler. This is primarily useful if a crate compiles successfully with
-warnings. Previously, re-running Cargo would not display any output. With the
-`cache-messages` feature, it will quickly redisplay the previous warnings.
-
-```
-cargo +nightly check -Z cache-messages
-```
-
-This works with any command that runs the compiler (`build`, `check`, `test`,
-etc.).
-
-This also changes the way Cargo interacts with the compiler, helping to
-prevent interleaved messages when multiple crates attempt to display a message
-at the same time.
-
 ### build-std
 * Tracking Repository: https://github.com/rust-lang/wg-cargo-std-aware