Auto merge of #13361 - weihanglo:fingerprint, r=epage

refactor: remove unnecessary Option in `Freshness::Dirty`

### What does this PR try to resolve?

This avoids reading and parsing JSON fingerprint files if the detailed
rebuild reason is not requested.

This also encodes `Freshness::Dirty(None)` better with a new variant `DirtyReason::FreshBuild`.

Author: bors

src/cargo/core/compiler/custom_build.rs

@@ -34,6 +34,7 @@
 use super::{fingerprint, Context, Job, Unit, Work};
 use crate::core::compiler::artifact;
 use crate::core::compiler::context::Metadata;
+use crate::core::compiler::fingerprint::DirtyReason;
 use crate::core::compiler::job_queue::JobState;
 use crate::core::{profiles::ProfileRoot, PackageId, Target};
 use crate::util::errors::CargoResult;
@@ -608,7 +609,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
     });
 
     let mut job = if cx.bcx.build_config.build_plan {
-        Job::new_dirty(Work::noop(), None)
+        Job::new_dirty(Work::noop(), DirtyReason::FreshBuild)
     } else {
         fingerprint::prepare_target(cx, unit, false)?
     };

src/cargo/core/compiler/fingerprint/dirty_reason.rs

@@ -77,6 +77,8 @@ pub enum DirtyReason {
     FsStatusOutdated(FsStatus),
     NothingObvious,
     Forced,
+    /// First time to build something.
+    FreshBuild,
 }
 
 trait ShellExt {
@@ -131,6 +133,11 @@ impl fmt::Display for After {
 }
 
 impl DirtyReason {
+    /// Whether a build is dirty because it is a fresh build being kicked off.
+    pub fn is_fresh_build(&self) -> bool {
+        matches!(self, DirtyReason::FreshBuild)
+    }
+
     fn after(old_time: FileTime, new_time: FileTime, what: &'static str) -> After {
         After {
             old_time,
@@ -257,6 +264,7 @@ impl DirtyReason {
                 s.dirty_because(unit, "the fingerprint comparison turned up nothing obvious")
             }
             DirtyReason::Forced => s.dirty_because(unit, "forced"),
+            DirtyReason::FreshBuild => s.dirty_because(unit, "fresh build"),
         }
     }
 }

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

@@ -415,43 +415,27 @@ pub fn prepare_target(cx: &mut Context<'_, '_>, unit: &Unit, force: bool) -> Car
     // information about failed comparisons to aid in debugging.
     let fingerprint = calculate(cx, unit)?;
     let mtime_on_use = cx.bcx.config.cli_unstable().mtime_on_use;
-    let compare = compare_old_fingerprint(&loc, &*fingerprint, mtime_on_use);
-    log_compare(unit, &compare);
+    let dirty_reason = compare_old_fingerprint(unit, &loc, &*fingerprint, mtime_on_use, force);
 
-    // If our comparison failed or reported dirty (e.g., we're going to trigger
-    // a rebuild of this crate), then we also ensure the source of the crate
-    // passes all verification checks before we build it.
+    let Some(dirty_reason) = dirty_reason else {
+        return Ok(Job::new_fresh());
+    };
+
+    // We're going to rebuild, so ensure the source of the crate passes all
+    // verification checks before we build it.
     //
     // The `Source::verify` method is intended to allow sources to execute
     // pre-build checks to ensure that the relevant source code is all
     // up-to-date and as expected. This is currently used primarily for
     // directory sources which will use this hook to perform an integrity check
     // on all files in the source to ensure they haven't changed. If they have
     // changed then an error is issued.
-    if compare
-        .as_ref()
-        .map(|dirty| dirty.is_some())
-        .unwrap_or(true)
-    {
-        let source_id = unit.pkg.package_id().source_id();
-        let sources = bcx.packages.sources();
-        let source = sources
-            .get(source_id)
-            .ok_or_else(|| internal("missing package source"))?;
-        source.verify(unit.pkg.package_id())?;
-    }
-
-    let dirty_reason = match compare {
-        Ok(None) => {
-            if force {
-                Some(DirtyReason::Forced)
-            } else {
-                return Ok(Job::new_fresh());
-            }
-        }
-        Ok(reason) => reason,
-        Err(_) => None,
-    };
+    let source_id = unit.pkg.package_id().source_id();
+    let sources = bcx.packages.sources();
+    let source = sources
+        .get(source_id)
+        .ok_or_else(|| internal("missing package source"))?;
+    source.verify(unit.pkg.package_id())?;
 
     // Clear out the old fingerprint file if it exists. This protects when
     // compilation is interrupted leaving a corrupt file. For example, a
@@ -1752,19 +1736,52 @@ fn target_root(cx: &Context<'_, '_>) -> PathBuf {
 /// If dirty, it then restores the detailed information
 /// from the fingerprint JSON file, and provides an rich dirty reason.
 fn compare_old_fingerprint(
+    unit: &Unit,
     old_hash_path: &Path,
     new_fingerprint: &Fingerprint,
     mtime_on_use: bool,
-) -> CargoResult<Option<DirtyReason>> {
-    let old_fingerprint_short = paths::read(old_hash_path)?;
-
+    forced: bool,
+) -> Option<DirtyReason> {
     if mtime_on_use {
         // update the mtime so other cleaners know we used it
         let t = FileTime::from_system_time(SystemTime::now());
         debug!("mtime-on-use forcing {:?} to {}", old_hash_path, t);
         paths::set_file_time_no_err(old_hash_path, t);
     }
 
+    let compare = _compare_old_fingerprint(old_hash_path, new_fingerprint);
+
+    match compare.as_ref() {
+        Ok(None) => {}
+        Ok(Some(reason)) => {
+            info!(
+                "fingerprint dirty for {}/{:?}/{:?}",
+                unit.pkg, unit.mode, unit.target,
+            );
+            info!("    dirty: {reason:?}");
+        }
+        Err(e) => {
+            info!(
+                "fingerprint error for {}/{:?}/{:?}",
+                unit.pkg, unit.mode, unit.target,
+            );
+            info!("    err: {e:?}");
+        }
+    }
+
+    match compare {
+        Ok(None) if forced => Some(DirtyReason::Forced),
+        Ok(reason) => reason,
+        Err(_) => Some(DirtyReason::FreshBuild),
+    }
+}
+
+fn _compare_old_fingerprint(
+    old_hash_path: &Path,
+    new_fingerprint: &Fingerprint,
+) -> CargoResult<Option<DirtyReason>> {
+    let old_fingerprint_short = paths::read(old_hash_path)?;
+
     let new_hash = new_fingerprint.hash_u64();
 
     if util::to_hex(new_hash) == old_fingerprint_short && new_fingerprint.fs_status.up_to_date() {
@@ -1785,29 +1802,6 @@ fn compare_old_fingerprint(
     Ok(Some(new_fingerprint.compare(&old_fingerprint)))
 }
 
-/// Logs the result of fingerprint comparison.
-///
-/// TODO: Obsolete and mostly superseded by [`DirtyReason`]. Could be removed.
-fn log_compare(unit: &Unit, compare: &CargoResult<Option<DirtyReason>>) {
-    match compare {
-        Ok(None) => {}
-        Ok(Some(reason)) => {
-            info!(
-                "fingerprint dirty for {}/{:?}/{:?}",
-                unit.pkg, unit.mode, unit.target,
-            );
-            info!("    dirty: {reason:?}");
-        }
-        Err(e) => {
-            info!(
-                "fingerprint error for {}/{:?}/{:?}",
-                unit.pkg, unit.mode, unit.target,
-            );
-            info!("    err: {e:?}");
-        }
-    }
-}
-
 /// Parses Cargo's internal [`EncodedDepInfo`] structure that was previously
 /// serialized to disk.
 ///

src/cargo/core/compiler/job_queue/job.rs

@@ -60,7 +60,7 @@ impl Job {
     }
 
     /// Creates a new job representing a unit of work.
-    pub fn new_dirty(work: Work, dirty_reason: Option<DirtyReason>) -> Job {
+    pub fn new_dirty(work: Work, dirty_reason: DirtyReason) -> Job {
         Job {
             work,
             fresh: Freshness::Dirty(dirty_reason),
@@ -100,7 +100,7 @@ impl fmt::Debug for Job {
 #[derive(Debug, Clone)]
 pub enum Freshness {
     Fresh,
-    Dirty(Option<DirtyReason>),
+    Dirty(DirtyReason),
 }
 
 impl Freshness {

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

@@ -1110,10 +1110,10 @@ impl<'cfg> DrainState<'cfg> {
             // Any dirty stage which runs at least one command gets printed as
             // being a compiled package.
             Dirty(dirty_reason) => {
-                if let Some(reason) = dirty_reason {
+                if !dirty_reason.is_fresh_build() {
                     config
                         .shell()
-                        .verbose(|shell| reason.present_to(shell, unit, ws_root))?;
+                        .verbose(|shell| dirty_reason.present_to(shell, unit, ws_root))?;
                 }
 
                 if unit.mode.is_doc() {

src/cargo/core/compiler/mod.rs

@@ -183,7 +183,7 @@ fn compile<'cfg>(
         // We run these targets later, so this is just a no-op for now.
         Job::new_fresh()
     } else if build_plan {
-        Job::new_dirty(rustc(cx, unit, &exec.clone())?, None)
+        Job::new_dirty(rustc(cx, unit, &exec.clone())?, DirtyReason::FreshBuild)
     } else {
         let force = exec.force_rebuild(unit) || force_rebuild;
         let mut job = fingerprint::prepare_target(cx, unit, force)?;

src/cargo/ops/common_for_install_and_uninstall.rs

@@ -175,7 +175,7 @@ impl InstallTracker {
         // Check if any tracked exe's are already installed.
         let duplicates = self.find_duplicates(dst, &exes);
         if force || duplicates.is_empty() {
-            return Ok((Freshness::Dirty(Some(DirtyReason::Forced)), duplicates));
+            return Ok((Freshness::Dirty(DirtyReason::Forced), duplicates));
         }
         // Check if all duplicates come from packages of the same name. If
         // there are duplicates from other packages, then --force will be
@@ -205,7 +205,7 @@ impl InstallTracker {
             let source_id = pkg.package_id().source_id();
             if source_id.is_path() {
                 // `cargo install --path ...` is always rebuilt.
-                return Ok((Freshness::Dirty(Some(DirtyReason::Forced)), duplicates));
+                return Ok((Freshness::Dirty(DirtyReason::Forced), duplicates));
             }
             let is_up_to_date = |dupe_pkg_id| {
                 let info = self
@@ -229,7 +229,7 @@ impl InstallTracker {
             if matching_duplicates.iter().all(is_up_to_date) {
                 Ok((Freshness::Fresh, duplicates))
             } else {
-                Ok((Freshness::Dirty(Some(DirtyReason::Forced)), duplicates))
+                Ok((Freshness::Dirty(DirtyReason::Forced), duplicates))
             }
         } else {
             // Format the error message.