Moved manifest metadata tracking from fingerprint to dep info

ranger-ross · ranger-ross · commit 6c040320d676 · 2024-12-23T23:02:32.000+09:00
diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs
@@ -351,7 +351,11 @@ impl<'gctx> Compilation<'gctx> {
             }
         }
 
+        // Add manifest metadata related keys
         let metadata = pkg.manifest().metadata();
+        for (key, value) in metadata.emitted_env_vars() {
+            cmd.env(key, value);
+        }
 
         let cargo_exe = self.gctx.cargo_exe()?;
         cmd.env(crate::CARGO_ENV, cargo_exe);
@@ -360,7 +364,6 @@ impl<'gctx> Compilation<'gctx> {
         // crate properties which might require rebuild upon change
         // consider adding the corresponding properties to the hash
         // in BuildContext::target_metadata()
-        let rust_version = pkg.rust_version().as_ref().map(ToString::to_string);
         cmd.env("CARGO_MANIFEST_DIR", pkg.root())
             .env("CARGO_MANIFEST_PATH", pkg.manifest_path())
             .env("CARGO_PKG_VERSION_MAJOR", &pkg.version().major.to_string())
@@ -369,35 +372,6 @@ impl<'gctx> Compilation<'gctx> {
             .env("CARGO_PKG_VERSION_PRE", pkg.version().pre.as_str())
             .env("CARGO_PKG_VERSION", &pkg.version().to_string())
             .env("CARGO_PKG_NAME", &*pkg.name())
-            .env(
-                "CARGO_PKG_DESCRIPTION",
-                metadata.description.as_ref().unwrap_or(&String::new()),
-            )
-            .env(
-                "CARGO_PKG_HOMEPAGE",
-                metadata.homepage.as_ref().unwrap_or(&String::new()),
-            )
-            .env(
-                "CARGO_PKG_REPOSITORY",
-                metadata.repository.as_ref().unwrap_or(&String::new()),
-            )
-            .env(
-                "CARGO_PKG_LICENSE",
-                metadata.license.as_ref().unwrap_or(&String::new()),
-            )
-            .env(
-                "CARGO_PKG_LICENSE_FILE",
-                metadata.license_file.as_ref().unwrap_or(&String::new()),
-            )
-            .env("CARGO_PKG_AUTHORS", &pkg.authors().join(":"))
-            .env(
-                "CARGO_PKG_RUST_VERSION",
-                &rust_version.as_deref().unwrap_or_default(),
-            )
-            .env(
-                "CARGO_PKG_README",
-                metadata.readme.as_ref().unwrap_or(&String::new()),
-            )
             .cwd(pkg.root());
 
         apply_env_config(self.gctx, &mut cmd)?;
diff --git a/src/cargo/core/compiler/fingerprint/dep_info.rs b/src/cargo/core/compiler/fingerprint/dep_info.rs
@@ -19,6 +19,7 @@ use cargo_util::paths;
 use cargo_util::ProcessBuilder;
 use cargo_util::Sha256;
 
+use crate::core::manifest::ManifestMetadata;
 use crate::CargoResult;
 use crate::CARGO_ENV;
 
@@ -334,7 +335,10 @@ pub fn translate_dep_info(
     //
     // For cargo#13280, We trace env vars that are defined in the `[env]` config table.
     on_disk_info.env.retain(|(key, _)| {
-        env_config.contains_key(key) || !rustc_cmd.get_envs().contains_key(key) || key == CARGO_ENV
+        ManifestMetadata::is_emittable_env_var(key)
+            || env_config.contains_key(key)
+            || !rustc_cmd.get_envs().contains_key(key)
+            || key == CARGO_ENV
     });
 
     let serialize_path = |file| {
diff --git a/src/cargo/core/compiler/fingerprint/dirty_reason.rs b/src/cargo/core/compiler/fingerprint/dirty_reason.rs
@@ -26,7 +26,6 @@ pub enum DirtyReason {
         old: Vec<String>,
         new: Vec<String>,
     },
-    MetadataChanged,
     ConfigSettingsChanged,
     CompileKindChanged,
     LocalLengthsChanged,
@@ -168,7 +167,6 @@ impl DirtyReason {
                 s.dirty_because(unit, "the profile configuration changed")
             }
             DirtyReason::RustflagsChanged { .. } => s.dirty_because(unit, "the rustflags changed"),
-            DirtyReason::MetadataChanged => s.dirty_because(unit, "the metadata changed"),
             DirtyReason::ConfigSettingsChanged => {
                 s.dirty_because(unit, "the config settings changed")
             }
diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs
@@ -78,7 +78,6 @@
 //! [`CompileKind`] (host/target)              | ✓           | ✓                   | ✓                      | ✓
 //! `__CARGO_DEFAULT_LIB_METADATA`[^4]         |             | ✓                   | ✓                      | ✓
 //! `package_id`                               |             | ✓                   | ✓                      | ✓
-//! authors, description, homepage, repo       | ✓           |                     |                        |
 //! Target src path relative to ws             | ✓           |                     |                        |
 //! Target flags (test/bench/for_host/edition) | ✓           |                     |                        |
 //! -C incremental=… flag                      | ✓           |                     |                        |
@@ -189,6 +188,8 @@
 //! files to learn about environment variables that the rustc compile depends on.
 //! Cargo then later uses this to trigger a recompile if a referenced env var
 //! changes (even if the source didn't change).
+//! This also includes env vars generated from Cargo metadata like `CARGO_PKG_DESCRIPTION`.
+//! (See [`EmittableManifestMetadata`])
 //!
 //! #### dep-info files for build system integration.
 //!
@@ -378,6 +379,7 @@ use std::fs::File;
 use std::hash::{self, Hash, Hasher};
 use std::io::{self};
 use std::path::{Path, PathBuf};
+use std::str::FromStr;
 use std::sync::{Arc, Mutex};
 use std::time::SystemTime;
 
@@ -391,6 +393,7 @@ use serde::{Deserialize, Serialize};
 use tracing::{debug, info};
 
 use crate::core::compiler::unit_graph::UnitDep;
+use crate::core::manifest::EmittableManifestMetadata;
 use crate::core::Package;
 use crate::util;
 use crate::util::errors::CargoResult;
@@ -612,10 +615,6 @@ pub struct Fingerprint {
     memoized_hash: Mutex<Option<u64>>,
     /// RUSTFLAGS/RUSTDOCFLAGS environment variable value (or config value).
     rustflags: Vec<String>,
-    /// Hash of some metadata from the manifest, such as "authors", or
-    /// "description", which are exposed as environment variables during
-    /// compilation.
-    metadata: u64,
     /// Hash of various config settings that change how things are compiled.
     config: u64,
     /// The rustc target. This is only relevant for `.json` files, otherwise
@@ -831,11 +830,12 @@ impl LocalFingerprint {
         &self,
         mtime_cache: &mut HashMap<PathBuf, FileTime>,
         checksum_cache: &mut HashMap<PathBuf, Checksum>,
-        pkg_root: &Path,
+        pkg: &Package,
         target_root: &Path,
         cargo_exe: &Path,
         gctx: &GlobalContext,
     ) -> CargoResult<Option<StaleItem>> {
+        let pkg_root = pkg.root();
         match self {
             // We need to parse `dep_info`, learn about the crate's dependencies.
             //
@@ -848,7 +848,16 @@ impl LocalFingerprint {
                 let Some(info) = parse_dep_info(pkg_root, target_root, &dep_info)? else {
                     return Ok(Some(StaleItem::MissingFile(dep_info)));
                 };
+                let metadata = pkg.manifest().metadata();
                 for (key, previous) in info.env.iter() {
+                    if let Ok(t) = EmittableManifestMetadata::from_str(key.as_str()) {
+                        let value = metadata.emitted_env_var(&t);
+
+                        if Some(value.as_str()) == previous.as_deref() {
+                            continue;
+                        }
+                    }
+
                     let current = if key == CARGO_ENV {
                         Some(cargo_exe.to_str().ok_or_else(|| {
                             format_err!(
@@ -932,7 +941,6 @@ impl Fingerprint {
             local: Mutex::new(Vec::new()),
             memoized_hash: Mutex::new(None),
             rustflags: Vec::new(),
-            metadata: 0,
             config: 0,
             compile_kind: 0,
             fs_status: FsStatus::Stale,
@@ -995,9 +1003,6 @@ impl Fingerprint {
                 new: self.rustflags.clone(),
             };
         }
-        if self.metadata != old.metadata {
-            return DirtyReason::MetadataChanged;
-        }
         if self.config != old.config {
             return DirtyReason::ConfigSettingsChanged;
         }
@@ -1142,13 +1147,14 @@ impl Fingerprint {
         &mut self,
         mtime_cache: &mut HashMap<PathBuf, FileTime>,
         checksum_cache: &mut HashMap<PathBuf, Checksum>,
-        pkg_root: &Path,
+        pkg: &Package,
         target_root: &Path,
         cargo_exe: &Path,
         gctx: &GlobalContext,
     ) -> CargoResult<()> {
         assert!(!self.fs_status.up_to_date());
 
+        let pkg_root = pkg.root();
         let mut mtimes = HashMap::new();
 
         // Get the `mtime` of all outputs. Optionally update their mtime
@@ -1249,7 +1255,7 @@ impl Fingerprint {
             if let Some(item) = local.find_stale_item(
                 mtime_cache,
                 checksum_cache,
-                pkg_root,
+                pkg,
                 target_root,
                 cargo_exe,
                 gctx,
@@ -1279,7 +1285,6 @@ impl hash::Hash for Fingerprint {
             profile,
             ref deps,
             ref local,
-            metadata,
             config,
             compile_kind,
             ref rustflags,
@@ -1294,7 +1299,6 @@ impl hash::Hash for Fingerprint {
             path,
             profile,
             &*local,
-            metadata,
             config,
             compile_kind,
             rustflags,
@@ -1445,7 +1449,7 @@ fn calculate(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult
     fingerprint.check_filesystem(
         &mut build_runner.mtime_cache,
         &mut build_runner.checksum_cache,
-        unit.pkg.root(),
+        &unit.pkg,
         &target_root,
         cargo_exe,
         build_runner.bcx.gctx,
@@ -1529,9 +1533,6 @@ fn calculate_normal(
         build_runner.lto[unit],
         unit.pkg.manifest().lint_rustflags(),
     ));
-    // Include metadata since it is exposed as environment variables.
-    let m = unit.pkg.manifest().metadata();
-    let metadata = util::hash_u64((&m.authors, &m.description, &m.homepage, &m.repository));
     let mut config = StableHasher::new();
     if let Some(linker) = build_runner.compilation.target_linker(unit.kind) {
         linker.hash(&mut config);
@@ -1560,7 +1561,6 @@ fn calculate_normal(
         deps,
         local: Mutex::new(local),
         memoized_hash: Mutex::new(None),
-        metadata,
         config: Hasher::finish(&config),
         compile_kind,
         rustflags: extra_flags,
diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs
@@ -3,6 +3,7 @@ use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
+use std::str::FromStr;
 use std::sync::Arc;
 
 use anyhow::Context as _;
@@ -146,6 +147,97 @@ pub struct ManifestMetadata {
     pub rust_version: Option<RustVersion>,
 }
 
+/// "Emittable" here meaning emitted to rustc as enviromental variables. Usable by `env!()`
+/// If these change we need to trigger a rebuild
+pub enum EmittableManifestMetadata {
+    Description,
+    Homepage,
+    Repository,
+    License,
+    LicenseFile,
+    Authors,
+    RustVersion,
+    Readme,
+}
+
+const EMITTED_MANIFEST_METADATA: [EmittableManifestMetadata; 8] = [
+    EmittableManifestMetadata::Description,
+    EmittableManifestMetadata::Homepage,
+    EmittableManifestMetadata::Repository,
+    EmittableManifestMetadata::License,
+    EmittableManifestMetadata::LicenseFile,
+    EmittableManifestMetadata::Authors,
+    EmittableManifestMetadata::RustVersion,
+    EmittableManifestMetadata::Readme,
+];
+
+impl FromStr for EmittableManifestMetadata {
+    type Err = String;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        Ok(match value {
+            "CARGO_PKG_DESCRIPTION" => EmittableManifestMetadata::Description,
+            "CARGO_PKG_HOMEPAGE" => EmittableManifestMetadata::Homepage,
+            "CARGO_PKG_REPOSITORY" => EmittableManifestMetadata::Repository,
+            "CARGO_PKG_LICENSE" => EmittableManifestMetadata::License,
+            "CARGO_PKG_LICENSE_FILE" => EmittableManifestMetadata::LicenseFile,
+            "CARGO_PKG_AUTHORS" => EmittableManifestMetadata::Authors,
+            "CARGO_PKG_RUST_VERSION" => EmittableManifestMetadata::RustVersion,
+            "CARGO_PKG_README" => EmittableManifestMetadata::Readme,
+            other => return Err(format!("Invalid emittable manifest metadata key {other}")),
+        })
+    }
+}
+
+impl From<EmittableManifestMetadata> for &'static str {
+    fn from(value: EmittableManifestMetadata) -> Self {
+        match value {
+            EmittableManifestMetadata::Description => "CARGO_PKG_DESCRIPTION",
+            EmittableManifestMetadata::Homepage => "CARGO_PKG_HOMEPAGE",
+            EmittableManifestMetadata::Repository => "CARGO_PKG_REPOSITORY",
+            EmittableManifestMetadata::License => "CARGO_PKG_LICENSE",
+            EmittableManifestMetadata::LicenseFile => "CARGO_PKG_LICENSE_FILE",
+            EmittableManifestMetadata::Authors => "CARGO_PKG_AUTHORS",
+            EmittableManifestMetadata::RustVersion => "CARGO_PKG_RUST_VERSION",
+            EmittableManifestMetadata::Readme => "CARGO_PKG_README",
+        }
+    }
+}
+
+impl ManifestMetadata {
+    pub fn is_emittable_env_var(key: &str) -> bool {
+        EmittableManifestMetadata::from_str(key).is_ok()
+    }
+
+    pub fn emitted_env_var<'a>(&'a self, key: &EmittableManifestMetadata) -> String {
+        match key {
+            EmittableManifestMetadata::Description => self.description.clone().unwrap_or_default(),
+            EmittableManifestMetadata::Homepage => self.homepage.clone().unwrap_or_default(),
+            EmittableManifestMetadata::Repository => self.repository.clone().unwrap_or_default(),
+            EmittableManifestMetadata::License => self.license.clone().unwrap_or_default(),
+            EmittableManifestMetadata::LicenseFile => self.license_file.clone().unwrap_or_default(),
+            EmittableManifestMetadata::Authors => self.authors.join(":"),
+            EmittableManifestMetadata::RustVersion => {
+                let rust_version = self.rust_version.as_ref().map(ToString::to_string);
+                rust_version.unwrap_or_default()
+            }
+            EmittableManifestMetadata::Readme => self.readme.clone().unwrap_or_default(),
+        }
+    }
+
+    // k/v pairs of the metadata environmental variables emitted to rustc.
+    pub fn emitted_env_vars(&self) -> Vec<(&'static str, String)> {
+        let mut vars = Vec::with_capacity(EMITTED_MANIFEST_METADATA.len());
+
+        for key in EMITTED_MANIFEST_METADATA {
+            let value = self.emitted_env_var(&key);
+            vars.push((key.into(), value));
+        }
+
+        return vars;
+    }
+}
+
 #[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
 pub enum TargetKind {
     Lib(Vec<CrateType>),
diff --git a/tests/testsuite/freshness.rs b/tests/testsuite/freshness.rs
diff --git a/tests/testsuite/freshness_checksum.rs b/tests/testsuite/freshness_checksum.rs
