From 44931d76aba0603dcf8a8dd47111c8e1e26cfba5 Mon Sep 17 00:00:00 2001 From: ranger-ross Date: Sat, 21 Dec 2024 17:30:53 +0900 Subject: [PATCH] Moved manifest metadata tracking from fingerprint to dep info --- src/cargo/core/compiler/compilation.rs | 34 +---- .../core/compiler/fingerprint/dep_info.rs | 6 +- src/cargo/core/compiler/fingerprint/mod.rs | 35 +++-- src/cargo/core/manifest.rs | 92 ++++++++++++ tests/testsuite/freshness.rs | 133 +++++++++++++----- tests/testsuite/freshness_checksum.rs | 53 +------ 6 files changed, 218 insertions(+), 135 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 7531f0fa61b3..3b988cee34b4 100644 --- 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 index e8628f34ebfd..1579b6ff70f7 100644 --- 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/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index ac29b0d15d9e..dc61e6965cf1 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -378,6 +378,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 +392,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 +614,6 @@ pub struct Fingerprint { memoized_hash: Mutex>, /// RUSTFLAGS/RUSTDOCFLAGS environment variable value (or config value). rustflags: Vec, - /// 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 +829,12 @@ impl LocalFingerprint { &self, mtime_cache: &mut HashMap, checksum_cache: &mut HashMap, - pkg_root: &Path, + pkg: &Package, target_root: &Path, cargo_exe: &Path, gctx: &GlobalContext, ) -> CargoResult> { + let pkg_root = pkg.root(); match self { // We need to parse `dep_info`, learn about the crate's dependencies. // @@ -848,7 +847,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 +940,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 +1002,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 +1146,14 @@ impl Fingerprint { &mut self, mtime_cache: &mut HashMap, checksum_cache: &mut HashMap, - 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 +1254,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 +1284,6 @@ impl hash::Hash for Fingerprint { profile, ref deps, ref local, - metadata, config, compile_kind, ref rustflags, @@ -1294,7 +1298,6 @@ impl hash::Hash for Fingerprint { path, profile, &*local, - metadata, config, compile_kind, rustflags, @@ -1445,7 +1448,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 +1532,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 +1560,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 index d63dbd61de36..230d9bf0e4fd 100644 --- 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, } +/// "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 { + 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 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), diff --git a/tests/testsuite/freshness.rs b/tests/testsuite/freshness.rs index d2a6135e8a55..5b3042f721f9 100644 --- a/tests/testsuite/freshness.rs +++ b/tests/testsuite/freshness.rs @@ -953,7 +953,7 @@ new desc "#]]) .with_stderr_data(str![[r#" -[DIRTY] foo v0.0.1 ([ROOT]/foo): the metadata changed +[DIRTY] foo v0.0.1 ([ROOT]/foo): the environment variable CARGO_PKG_DESCRIPTION changed [COMPILING] foo v0.0.1 ([ROOT]/foo) [RUNNING] `rustc [..] [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s @@ -2113,49 +2113,114 @@ fn simulated_docker_deps_stay_cached() { #[cargo_test] fn metadata_change_invalidates() { - let p = project() - .file( - "Cargo.toml", + // (key, value, value-updated, env-var-name) + let scenarios = [ + ( + "description", + r#""foo""#, + r#""foo_updated""#, + "CARGO_PKG_DESCRIPTION", + ), + ( + "homepage", + r#""foo""#, + r#""foo_updated""#, + "CARGO_PKG_HOMEPAGE", + ), + ( + "repository", + r#""foo""#, + r#""foo_updated""#, + "CARGO_PKG_REPOSITORY", + ), + ( + "license", + r#""foo""#, + r#""foo_updated""#, + "CARGO_PKG_LICENSE", + ), + ( + "license-file", + r#""foo""#, + r#""foo_updated""#, + "CARGO_PKG_LICENSE_FILE", + ), + ( + "authors", + r#"["foo"]"#, + r#"["foo_updated"]"#, + "CARGO_PKG_AUTHORS", + ), + ( + "rust-version", + r#""1.0.0""#, + r#""1.0.1""#, + "CARGO_PKG_RUST_VERSION", + ), + ("readme", r#""foo""#, r#""foo_updated""#, "CARGO_PKG_README"), + ]; + let base_cargo_toml = r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + "#; + + let p = project().build(); + for (key, value, value_updated, env_var) in scenarios { + p.change_file("Cargo.toml", base_cargo_toml); + p.change_file( + "src/main.rs", + &format!( + r#" + fn main() {{ + let output = env!("{env_var}"); + println!("{{output}}"); + }} + "# + ), + ); + + // Compile the first time + p.cargo("build").run(); + + // Update the manifest, rebuild, and verify the build was invalided + p.change_file("Cargo.toml", &format!("{base_cargo_toml}\n{key} = {value}")); + p.cargo("build -v") + .with_stderr_data(format!( + r#"[DIRTY] foo v0.1.0 ([ROOT]/foo): the environment variable {env_var} changed +[COMPILING] foo v0.1.0 ([ROOT]/foo) +[RUNNING] `rustc [..] +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +"# + )) + .run(); + + // Remove references to the metadata and rebuild + p.change_file( + "src/main.rs", r#" - [package] - name = "foo" - version = "0.1.0" - edition = "2015" + fn main() { + println!("foo"); + } "#, - ) - .file("src/lib.rs", "") - .build(); + ); + p.cargo("build").run(); - p.cargo("build").run(); + // Update the manifest value and verify the build is NOT invalidated. + p.change_file( + "Cargo.toml", + &format!("{base_cargo_toml}\n{key} = {value_updated}"), + ); - for attr in &[ - "authors = [\"foo\"]", - "description = \"desc\"", - "homepage = \"https://example.com\"", - "repository =\"https://example.com\"", - ] { - let mut file = OpenOptions::new() - .write(true) - .append(true) - .open(p.root().join("Cargo.toml")) - .unwrap(); - writeln!(file, "{}", attr).unwrap(); - p.cargo("build") + p.cargo("build -v") .with_stderr_data(str![[r#" -[COMPILING] foo v0.1.0 ([ROOT]/foo) +[FRESH] foo v0.1.0 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) .run(); } - p.cargo("build -v") - .with_stderr_data(str![[r#" -[FRESH] foo v0.1.0 ([ROOT]/foo) -[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s - -"#]]) - .run(); - assert_eq!(p.glob("target/debug/deps/libfoo-*.rlib").count(), 1); } #[cargo_test] diff --git a/tests/testsuite/freshness_checksum.rs b/tests/testsuite/freshness_checksum.rs index 5c4b88fdbc30..1ae2ba1cd2cb 100644 --- a/tests/testsuite/freshness_checksum.rs +++ b/tests/testsuite/freshness_checksum.rs @@ -1099,7 +1099,7 @@ new desc "#]]) .with_stderr_data(str![[r#" -[DIRTY] foo v0.0.1 ([ROOT]/foo): the metadata changed +[DIRTY] foo v0.0.1 ([ROOT]/foo): the environment variable CARGO_PKG_DESCRIPTION changed [COMPILING] foo v0.0.1 ([ROOT]/foo) [RUNNING] `rustc [..] [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s @@ -1994,57 +1994,6 @@ fn script_fails_stay_dirty() { .run(); } -#[cargo_test(nightly, reason = "requires -Zchecksum-hash-algorithm")] -fn metadata_change_invalidates() { - let p = project() - .file( - "Cargo.toml", - r#" - [package] - name = "foo" - version = "0.1.0" - edition = "2015" - "#, - ) - .file("src/lib.rs", "") - .build(); - - p.cargo("build -Zchecksum-freshness") - .masquerade_as_nightly_cargo(&["checksum-freshness"]) - .run(); - - for attr in &[ - "authors = [\"foo\"]", - "description = \"desc\"", - "homepage = \"https://example.com\"", - "repository =\"https://example.com\"", - ] { - let mut file = OpenOptions::new() - .write(true) - .append(true) - .open(p.root().join("Cargo.toml")) - .unwrap(); - writeln!(file, "{}", attr).unwrap(); - p.cargo("build -Zchecksum-freshness") - .masquerade_as_nightly_cargo(&["checksum-freshness"]) - .with_stderr_data(str![[r#" -[COMPILING] foo v0.1.0 ([ROOT]/foo) -[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s - -"#]]) - .run(); - } - p.cargo("build -Zchecksum-freshness -v") - .masquerade_as_nightly_cargo(&["checksum-freshness"]) - .with_stderr_data(str![[r#" -[FRESH] foo v0.1.0 ([ROOT]/foo) -[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s - -"#]]) - .run(); - assert_eq!(p.glob("target/debug/deps/libfoo-*.rlib").count(), 1); -} - #[cargo_test(nightly, reason = "requires -Zchecksum-hash-algorithm")] fn edition_change_invalidates() { const MANIFEST: &str = r#"