Auto merge of #6832 - alexcrichton:freshness, r=ehuss

Remove `Freshness` from `DependencyQueue`

Ever since the inception of Cargo and the advent of incremental
compilation at the crate level via Cargo, Cargo has tracked whether it
needs to recompile something at a unit level in its "dependency queue"
which manages when items are ready for execution. Over time we've fixed
lots and lots of bugs related to incremental compilation, and perhaps
one of the most impactful realizations was that the model Cargo started
with fundamentally doesn't handle interrupting Cargo halfway through and
resuming the build later.

The previous model relied upon implicitly propagating "dirtiness" based
on whether the one of the dependencies of a build was rebuilt or not.
This information is not available, however, if Cargo is interrupted and
resumed (or performs a subset of steps and then later performs more).
We've fixed this in a number of places historically but the purpose of
this commit is to put a nail in this coffin once and for all.

Implicit propagation of whether a unit is fresh or dirty is no longer
present at all. Instead Cargo should always know, irrespective of it's
in-memory state, whether a unit needs to be recompiled or not. This
commit actually turns up a few bugs in the test suite, so later commits
will be targeted at fixing this.

Note that this required a good deal of work on the `fingerprint` module
to fix some longstanding bugs (like #6780) and some serious hoops had to
be jumped through for others (like #6779). While these were fallout from
this change they weren't necessarily the primary motivation, but rather
to help make `fingerprints` a bit more straightforward in what's an
already confusing system!

Closes #6780

Author: bors

src/bin/cargo/commands/owner.rs

@@ -7,7 +7,14 @@ pub fn cli() -> App {
         .about("Manage the owners of a crate on the registry")
         .arg(opt("quiet", "No output printed to stdout").short("q"))
         .arg(Arg::with_name("crate"))
-        .arg(multi_opt("add", "LOGIN", "Name of a user or team to invite as an owner").short("a"))
+        .arg(
+            multi_opt(
+                "add",
+                "LOGIN",
+                "Name of a user or team to invite as an owner",
+            )
+            .short("a"),
+        )
         .arg(
             multi_opt(
                 "remove",

src/bin/cargo/commands/test.rs

@@ -19,8 +19,11 @@ pub fn cli() -> App {
                 .last(true),
         )
         .arg(
-            opt("quiet", "Display one character per test instead of one line")
-            .short("q")
+            opt(
+                "quiet",
+                "Display one character per test instead of one line",
+            )
+            .short("q"),
         )
         .arg_targets_all(
             "Test only this package's library unit tests",
@@ -131,9 +134,9 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     } else if test_name.is_some() {
         if let CompileFilter::Default { .. } = compile_opts.filter {
             compile_opts.filter = ops::CompileFilter::new(
-                LibRule::Default, // compile the library, so the unit tests can be run filtered
-                FilterRule::All,    // compile the binaries, so the unit tests in binaries can be run filtered
-                FilterRule::All,    // compile the tests, so the integration tests can be run filtered
+                LibRule::Default,   // compile the library, so the unit tests can be run filtered
+                FilterRule::All, // compile the binaries, so the unit tests in binaries can be run filtered
+                FilterRule::All, // compile the tests, so the integration tests can be run filtered
                 FilterRule::none(), // specify --examples to unit test binaries filtered
                 FilterRule::none(), // specify --benches to unit test benchmarks filtered
             ); // also, specify --doc to run doc tests filtered

src/bin/cargo/commands/version.rs

@@ -3,7 +3,8 @@ use crate::command_prelude::*;
 use crate::cli;
 
 pub fn cli() -> App {
-    subcommand("version").about("Show version information")
+    subcommand("version")
+        .about("Show version information")
         .arg(opt("quiet", "No output printed to stdout").short("q"))
 }
 

src/cargo/core/compiler/build_context/mod.rs

@@ -265,9 +265,7 @@ impl TargetConfig {
                     }
                     "rustc-cdylib-link-arg" => {
                         let args = value.list(k)?;
-                        output
-                            .linker_args
-                            .extend(args.iter().map(|v| v.0.clone()));
+                        output.linker_args.extend(args.iter().map(|v| v.0.clone()));
                     }
                     "rustc-cfg" => {
                         let list = value.list(k)?;

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

@@ -166,8 +166,13 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
         }
     }
 
-    /// Returns the root of the build output tree.
+    /// Returns the root of the build output tree for the target
     pub fn target_root(&self) -> &Path {
+        self.target.as_ref().unwrap_or(&self.host).dest()
+    }
+
+    /// Returns the root of the build output tree for the host
+    pub fn host_root(&self) -> &Path {
         self.host.dest()
     }
 

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

@@ -430,9 +430,10 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                     path.display()
                 )
             };
-        let suggestion = "Consider changing their names to be unique or compiling them separately.\n\
-            This may become a hard error in the future; see \
-            <https://github.com/rust-lang/cargo/issues/6313>.";
+        let suggestion =
+            "Consider changing their names to be unique or compiling them separately.\n\
+             This may become a hard error in the future; see \
+             <https://github.com/rust-lang/cargo/issues/6313>.";
         let report_collision = |unit: &Unit<'_>,
                                 other_unit: &Unit<'_>,
                                 path: &PathBuf|

src/cargo/core/compiler/custom_build.rs

@@ -8,10 +8,10 @@ use std::sync::{Arc, Mutex};
 use crate::core::PackageId;
 use crate::util::errors::{CargoResult, CargoResultExt};
 use crate::util::machine_message;
+use crate::util::Cfg;
 use crate::util::{self, internal, paths, profile};
-use crate::util::{Cfg, Freshness};
 
-use super::job::Work;
+use super::job::{Freshness, Job, Work};
 use super::{fingerprint, Context, Kind, TargetConfig, Unit};
 
 /// Contains the parsed output of a custom build script.
@@ -80,32 +80,19 @@ pub struct BuildDeps {
 /// prepare work for. If the requirement is specified as both the target and the
 /// host platforms it is assumed that the two are equal and the build script is
 /// only run once (not twice).
-pub fn prepare<'a, 'cfg>(
-    cx: &mut Context<'a, 'cfg>,
-    unit: &Unit<'a>,
-) -> CargoResult<(Work, Work, Freshness)> {
+pub fn prepare<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult<Job> {
     let _p = profile::start(format!(
         "build script prepare: {}/{}",
         unit.pkg,
         unit.target.name()
     ));
 
     let key = (unit.pkg.package_id(), unit.kind);
-    let overridden = cx.build_script_overridden.contains(&key);
-    let (work_dirty, work_fresh) = if overridden {
-        (Work::noop(), Work::noop())
-    } else {
-        build_work(cx, unit)?
-    };
 
-    if cx.bcx.build_config.build_plan {
-        Ok((work_dirty, work_fresh, Freshness::Dirty))
+    if cx.build_script_overridden.contains(&key) {
+        fingerprint::prepare_target(cx, unit, false)
     } else {
-        // Now that we've prep'd our work, build the work needed to manage the
-        // fingerprint and then start returning that upwards.
-        let (freshness, dirty, fresh) = fingerprint::prepare_build_cmd(cx, unit)?;
-
-        Ok((work_dirty.then(dirty), work_fresh.then(fresh), freshness))
+        build_work(cx, unit)
     }
 }
 
@@ -125,7 +112,7 @@ fn emit_build_output(output: &BuildOutput, package_id: PackageId) {
     });
 }
 
-fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult<(Work, Work)> {
+fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult<Job> {
     assert!(unit.mode.is_run_custom_build());
     let bcx = &cx.bcx;
     let dependencies = cx.dep_targets(unit);
@@ -248,7 +235,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
     let output_file = script_run_dir.join("output");
     let err_file = script_run_dir.join("stderr");
     let root_output_file = script_run_dir.join("root-output");
-    let host_target_root = cx.files().target_root().to_path_buf();
+    let host_target_root = cx.files().host_root().to_path_buf();
     let all = (
         id,
         pkg_name.clone(),
@@ -260,22 +247,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
     let kind = unit.kind;
     let json_messages = bcx.build_config.json_messages();
     let extra_verbose = bcx.config.extra_verbose();
-
-    // Check to see if the build script has already run, and if it has, keep
-    // track of whether it has told us about some explicit dependencies.
-    let prev_script_out_dir = paths::read_bytes(&root_output_file)
-        .and_then(|bytes| util::bytes2path(&bytes))
-        .unwrap_or_else(|_| script_out_dir.clone());
-
-    let prev_output = BuildOutput::parse_file(
-        &output_file,
-        &pkg_name,
-        &prev_script_out_dir,
-        &script_out_dir,
-    )
-    .ok();
-    let deps = BuildDeps::new(&output_file, prev_output.as_ref());
-    cx.build_explicit_deps.insert(*unit, deps);
+    let (prev_output, prev_script_out_dir) = prev_build_output(cx, unit);
 
     fs::create_dir_all(&script_dir)?;
     fs::create_dir_all(&script_out_dir)?;
@@ -392,7 +364,17 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
         Ok(())
     });
 
-    Ok((dirty, fresh))
+    let mut job = if cx.bcx.build_config.build_plan {
+        Job::new(Work::noop(), Freshness::Dirty)
+    } else {
+        fingerprint::prepare_target(cx, unit, false)?
+    };
+    if job.freshness() == Freshness::Dirty {
+        job.before(dirty);
+    } else {
+        job.before(fresh);
+    }
+    Ok(job)
 }
 
 impl BuildState {
@@ -637,22 +619,20 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, units: &[Unit<'b>]) -> Ca
             return Ok(&out[unit]);
         }
 
-        {
-            let key = unit
-                .pkg
-                .manifest()
-                .links()
-                .map(|l| (l.to_string(), unit.kind));
-            let build_state = &cx.build_state;
-            if let Some(output) = key.and_then(|k| build_state.overrides.get(&k)) {
-                let key = (unit.pkg.package_id(), unit.kind);
-                cx.build_script_overridden.insert(key);
-                build_state
-                    .outputs
-                    .lock()
-                    .unwrap()
-                    .insert(key, output.clone());
-            }
+        let key = unit
+            .pkg
+            .manifest()
+            .links()
+            .map(|l| (l.to_string(), unit.kind));
+        let build_state = &cx.build_state;
+        if let Some(output) = key.and_then(|k| build_state.overrides.get(&k)) {
+            let key = (unit.pkg.package_id(), unit.kind);
+            cx.build_script_overridden.insert(key);
+            build_state
+                .outputs
+                .lock()
+                .unwrap()
+                .insert(key, output.clone());
         }
 
         let mut ret = BuildScripts::default();
@@ -661,6 +641,10 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, units: &[Unit<'b>]) -> Ca
             add_to_link(&mut ret, unit.pkg.package_id(), unit.kind);
         }
 
+        if unit.mode.is_run_custom_build() {
+            parse_previous_explicit_deps(cx, unit)?;
+        }
+
         // We want to invoke the compiler deterministically to be cache-friendly
         // to rustc invocation caching schemes, so be sure to generate the same
         // set of build script dependency orderings via sorting the targets that
@@ -694,4 +678,46 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, units: &[Unit<'b>]) -> Ca
             scripts.to_link.push((pkg, kind));
         }
     }
+
+    fn parse_previous_explicit_deps<'a, 'cfg>(
+        cx: &mut Context<'a, 'cfg>,
+        unit: &Unit<'a>,
+    ) -> CargoResult<()> {
+        let script_run_dir = cx.files().build_script_run_dir(unit);
+        let output_file = script_run_dir.join("output");
+        let (prev_output, _) = prev_build_output(cx, unit);
+        let deps = BuildDeps::new(&output_file, prev_output.as_ref());
+        cx.build_explicit_deps.insert(*unit, deps);
+        Ok(())
+    }
+}
+
+/// Returns the previous parsed `BuildOutput`, if any, from a previous
+/// execution.
+///
+/// Also returns the directory containing the output, typically used later in
+/// processing.
+fn prev_build_output<'a, 'cfg>(
+    cx: &mut Context<'a, 'cfg>,
+    unit: &Unit<'a>,
+) -> (Option<BuildOutput>, PathBuf) {
+    let script_out_dir = cx.files().build_script_out_dir(unit);
+    let script_run_dir = cx.files().build_script_run_dir(unit);
+    let root_output_file = script_run_dir.join("root-output");
+    let output_file = script_run_dir.join("output");
+
+    let prev_script_out_dir = paths::read_bytes(&root_output_file)
+        .and_then(|bytes| util::bytes2path(&bytes))
+        .unwrap_or_else(|_| script_out_dir.clone());
+
+    (
+        BuildOutput::parse_file(
+            &output_file,
+            &unit.pkg.to_string(),
+            &prev_script_out_dir,
+            &script_out_dir,
+        )
+        .ok(),
+        prev_script_out_dir,
+    )
 }