From 7a06be149c114bffb7c65732eca09f3060afd96f Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 26 Apr 2020 13:20:44 -0700 Subject: [PATCH 1/5] Add CrateType to replace LibKind. --- .../compiler/build_context/target_info.rs | 40 +++--- src/cargo/core/compiler/compilation.rs | 2 +- .../compiler/context/compilation_files.rs | 25 ++-- src/cargo/core/compiler/custom_build.rs | 2 +- src/cargo/core/compiler/mod.rs | 14 ++- src/cargo/core/compiler/unit.rs | 8 +- src/cargo/core/compiler/unit_dependencies.rs | 4 +- src/cargo/core/manifest.rs | 118 ++++-------------- src/cargo/core/mod.rs | 2 +- src/cargo/ops/cargo_compile.rs | 4 +- src/cargo/util/toml/mod.rs | 2 +- src/cargo/util/toml/targets.rs | 13 +- 12 files changed, 89 insertions(+), 145 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 17ea6c6411e..698a9e4080f 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -1,4 +1,4 @@ -use crate::core::compiler::{BuildOutput, CompileKind, CompileTarget}; +use crate::core::compiler::{BuildOutput, CompileKind, CompileTarget, CrateType}; use crate::core::{Dependency, TargetKind, Workspace}; use crate::util::config::{Config, StringList, TargetConfig}; use crate::util::{CargoResult, CargoResultExt, ProcessBuilder, Rustc}; @@ -25,7 +25,7 @@ pub struct TargetInfo { /// `Some((prefix, suffix))`, for example `libcargo.so` would be /// `Some(("lib", ".so")). The value is `None` if the crate type is not /// supported. - crate_types: RefCell>>, + crate_types: RefCell>>, /// `cfg` information extracted from `rustc --print=cfg`. cfg: Vec, /// Path to the sysroot. @@ -123,10 +123,16 @@ impl TargetInfo { } let crate_type_process = process.clone(); - const KNOWN_CRATE_TYPES: &[&str] = - &["bin", "rlib", "dylib", "cdylib", "staticlib", "proc-macro"]; + const KNOWN_CRATE_TYPES: &[CrateType] = &[ + CrateType::Bin, + CrateType::Rlib, + CrateType::Dylib, + CrateType::Cdylib, + CrateType::Staticlib, + CrateType::ProcMacro, + ]; for crate_type in KNOWN_CRATE_TYPES.iter() { - process.arg("--crate-type").arg(crate_type); + process.arg("--crate-type").arg(crate_type.as_str()); } process.arg("--print=sysroot"); @@ -140,7 +146,7 @@ impl TargetInfo { let mut map = HashMap::new(); for crate_type in KNOWN_CRATE_TYPES { let out = parse_crate_type(crate_type, &process, &output, &error, &mut lines)?; - map.insert(crate_type.to_string(), out); + map.insert(crate_type.clone(), out); } let line = match lines.next() { @@ -228,13 +234,19 @@ impl TargetInfo { /// Returns `None` if the target does not support the given crate type. pub fn file_types( &self, - crate_type: &str, + crate_type: &CrateType, flavor: FileFlavor, kind: &TargetKind, target_triple: &str, ) -> CargoResult>> { + let crate_type = if *crate_type == CrateType::Lib { + CrateType::Rlib + } else { + crate_type.clone() + }; + let mut crate_types = self.crate_types.borrow_mut(); - let entry = crate_types.entry(crate_type.to_string()); + let entry = crate_types.entry(crate_type.clone()); let crate_type_info = match entry { Entry::Occupied(o) => &*o.into_mut(), Entry::Vacant(v) => { @@ -255,7 +267,7 @@ impl TargetInfo { // See rust-lang/cargo#4500. if target_triple.ends_with("-windows-msvc") - && crate_type.ends_with("dylib") + && (crate_type == CrateType::Dylib || crate_type == CrateType::Cdylib) && suffix == ".dll" { ret.push(FileType { @@ -265,7 +277,7 @@ impl TargetInfo { should_replace_hyphens: false, }) } else if target_triple.ends_with("windows-gnu") - && crate_type.ends_with("dylib") + && (crate_type == CrateType::Dylib || crate_type == CrateType::Cdylib) && suffix == ".dll" { // LD can link DLL directly, but LLD requires the import library. @@ -278,7 +290,7 @@ impl TargetInfo { } // See rust-lang/cargo#4535. - if target_triple.starts_with("wasm32-") && crate_type == "bin" && suffix == ".js" { + if target_triple.starts_with("wasm32-") && crate_type == CrateType::Bin && suffix == ".js" { ret.push(FileType { suffix: ".wasm".to_string(), prefix: prefix.clone(), @@ -319,10 +331,10 @@ impl TargetInfo { Ok(Some(ret)) } - fn discover_crate_type(&self, crate_type: &str) -> CargoResult> { + fn discover_crate_type(&self, crate_type: &CrateType) -> CargoResult> { let mut process = self.crate_type_process.clone(); - process.arg("--crate-type").arg(crate_type); + process.arg("--crate-type").arg(crate_type.as_str()); let output = process.exec_with_output().chain_err(|| { format!( @@ -353,7 +365,7 @@ impl TargetInfo { /// This function can not handle more than one file per type (with wasm32-unknown-emscripten, there /// are two files for bin (`.wasm` and `.js`)). fn parse_crate_type( - crate_type: &str, + crate_type: &CrateType, cmd: &ProcessBuilder, output: &str, error: &str, diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index efcaff1355e..7324618ff45 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -167,7 +167,7 @@ impl<'cfg> Compilation<'cfg> { } for crate_type in unit.target.rustc_crate_types() { - p.arg("--crate-type").arg(crate_type); + p.arg("--crate-type").arg(crate_type.as_str()); } Ok(p) diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 9fc2f4117a4..b2c48d4d7cd 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -9,7 +9,7 @@ use lazycell::LazyCell; use log::info; use super::{BuildContext, CompileKind, Context, FileFlavor, Layout}; -use crate::core::compiler::{CompileMode, CompileTarget, Unit}; +use crate::core::compiler::{CompileMode, CompileTarget, CrateType, Unit}; use crate::core::{Target, TargetKind, Workspace}; use crate::util::{self, CargoResult}; @@ -274,7 +274,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { let info = bcx.target_data.info(kind); let file_types = info .file_types( - "bin", + &CrateType::Bin, FileFlavor::Normal, &TargetKind::Bin, bcx.target_data.short_name(&kind), @@ -416,12 +416,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { let info = bcx.target_data.info(unit.kind); let file_stem = self.file_stem(unit); - let mut add = |crate_type: &str, flavor: FileFlavor| -> CargoResult<()> { - let crate_type = if crate_type == "lib" { - "rlib" - } else { - crate_type - }; + let mut add = |crate_type: &CrateType, flavor: FileFlavor| -> CargoResult<()> { let file_types = info.file_types( crate_type, flavor, @@ -465,22 +460,22 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { } Ok(()) }; - match *unit.target.kind() { + match unit.target.kind() { TargetKind::Bin | TargetKind::CustomBuild | TargetKind::ExampleBin | TargetKind::Bench | TargetKind::Test => { - add("bin", FileFlavor::Normal)?; + add(&CrateType::Bin, FileFlavor::Normal)?; } TargetKind::Lib(..) | TargetKind::ExampleLib(..) if unit.mode.is_any_test() => { - add("bin", FileFlavor::Normal)?; + add(&CrateType::Bin, FileFlavor::Normal)?; } - TargetKind::ExampleLib(ref kinds) | TargetKind::Lib(ref kinds) => { - for kind in kinds { + TargetKind::ExampleLib(crate_types) | TargetKind::Lib(crate_types) => { + for crate_type in crate_types { add( - kind.crate_type(), - if kind.linkable() { + crate_type, + if crate_type.is_linkable() { FileFlavor::Linkable { rmeta: false } } else { FileFlavor::Normal diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 38af4e7d2a6..6b642392018 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -738,7 +738,7 @@ pub fn build_map(cx: &mut Context<'_, '_>) -> CargoResult<()> { if dep_unit.target.for_host() { ret.plugins.extend(dep_scripts.to_link.iter().cloned()); - } else if dep_unit.target.linkable() { + } else if dep_unit.target.is_linkable() { for &(pkg, metadata) in dep_scripts.to_link.iter() { add_to_link(&mut ret, pkg, metadata); } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index dd2dbea39ea..a24aaf0ab9b 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -4,6 +4,7 @@ mod build_plan; mod compilation; mod compile_kind; mod context; +mod crate_type; mod custom_build; mod fingerprint; mod job; @@ -35,6 +36,7 @@ use self::build_plan::BuildPlan; pub use self::compilation::{Compilation, Doctest}; pub use self::compile_kind::{CompileKind, CompileTarget}; pub use self::context::{Context, Metadata}; +pub use self::crate_type::CrateType; pub use self::custom_build::{BuildOutput, BuildScriptOutputs, BuildScripts}; pub use self::job::Freshness; use self::job::{Job, Work}; @@ -532,7 +534,7 @@ where fn prepare_rustc( cx: &mut Context<'_, '_>, - crate_types: &[&str], + crate_types: &[CrateType], unit: &Unit, ) -> CargoResult { let is_primary = cx.is_primary_package(unit); @@ -734,7 +736,7 @@ fn build_base_args( cx: &mut Context<'_, '_>, cmd: &mut ProcessBuilder, unit: &Unit, - crate_types: &[&str], + crate_types: &[CrateType], ) -> CargoResult<()> { assert!(!unit.mode.is_run_custom_build()); @@ -764,7 +766,7 @@ fn build_base_args( if !test { for crate_type in crate_types.iter() { - cmd.arg("--crate-type").arg(crate_type); + cmd.arg("--crate-type").arg(crate_type.as_str()); } } @@ -780,7 +782,7 @@ fn build_base_args( } let prefer_dynamic = (unit.target.for_host() && !unit.target.is_custom_build()) - || (crate_types.contains(&"dylib") && bcx.ws.members().any(|p| *p != unit.pkg)); + || (crate_types.contains(&CrateType::Dylib) && bcx.ws.members().any(|p| *p != unit.pkg)); if prefer_dynamic { cmd.arg("-C").arg("prefer-dynamic"); } @@ -984,7 +986,7 @@ fn build_deps_args( // error in the future (see PR #4797). if !deps .iter() - .any(|dep| !dep.unit.mode.is_doc() && dep.unit.target.linkable()) + .any(|dep| !dep.unit.mode.is_doc() && dep.unit.target.is_linkable()) { if let Some(dep) = deps .iter() @@ -1088,7 +1090,7 @@ pub fn extern_args( }; for dep in deps { - if dep.unit.target.linkable() && !dep.unit.mode.is_doc() { + if dep.unit.target.is_linkable() && !dep.unit.mode.is_doc() { link_to(dep, dep.extern_crate_name, dep.noprelude)?; } } diff --git a/src/cargo/core/compiler/unit.rs b/src/cargo/core/compiler/unit.rs index 84565a6517e..a7f80321d41 100644 --- a/src/cargo/core/compiler/unit.rs +++ b/src/cargo/core/compiler/unit.rs @@ -1,5 +1,5 @@ -use crate::core::compiler::{CompileKind, CompileMode}; -use crate::core::manifest::{LibKind, Target, TargetKind}; +use crate::core::compiler::{CompileKind, CompileMode, CrateType}; +use crate::core::manifest::{Target, TargetKind}; use crate::core::{profiles::Profile, InternedString, Package}; use crate::util::hex::short_hash; use crate::util::Config; @@ -178,9 +178,9 @@ impl UnitInterner { // // At some point in the future, it would be nice to have a // first-class way of overriding or specifying crate-types. - (true, TargetKind::Lib(crate_types)) if crate_types.contains(&LibKind::Dylib) => { + (true, TargetKind::Lib(crate_types)) if crate_types.contains(&CrateType::Dylib) => { let mut new_target = Target::clone(target); - new_target.set_kind(TargetKind::Lib(vec![LibKind::Rlib])); + new_target.set_kind(TargetKind::Lib(vec![CrateType::Rlib])); new_target } _ => target.clone(), diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 57cb6c68d06..da2ee2660cf 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -476,7 +476,7 @@ fn maybe_lib( unit.pkg .targets() .iter() - .find(|t| t.linkable()) + .find(|t| t.is_linkable()) .map(|t| { let mode = check_or_build_mode(unit.mode, t); new_unit_dep( @@ -681,7 +681,7 @@ fn connect_run_custom_build_deps(unit_dependencies: &mut UnitGraph) { // Only deps with `links`. .filter(|other| { other.unit.pkg != unit.pkg - && other.unit.target.linkable() + && other.unit.target.is_linkable() && other.unit.pkg.manifest().links().is_some() }) // Get the RunCustomBuild for other lib. diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 646708e5901..6e96cdf4c3d 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -10,6 +10,7 @@ use serde::ser; use serde::Serialize; use url::Url; +use crate::core::compiler::CrateType; use crate::core::interning::InternedString; use crate::core::resolver::ResolveBehavior; use crate::core::{Dependency, PackageId, PackageIdSpec, SourceId, Summary}; @@ -96,73 +97,13 @@ pub struct ManifestMetadata { pub links: Option, } -#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] -pub enum LibKind { - Lib, - Rlib, - Dylib, - ProcMacro, - Other(String), -} - -impl LibKind { - /// Returns the argument suitable for `--crate-type` to pass to rustc. - pub fn crate_type(&self) -> &str { - match *self { - LibKind::Lib => "lib", - LibKind::Rlib => "rlib", - LibKind::Dylib => "dylib", - LibKind::ProcMacro => "proc-macro", - LibKind::Other(ref s) => s, - } - } - - pub fn linkable(&self) -> bool { - match *self { - LibKind::Lib | LibKind::Rlib | LibKind::Dylib | LibKind::ProcMacro => true, - LibKind::Other(..) => false, - } - } - - pub fn requires_upstream_objects(&self) -> bool { - match *self { - // "lib" == "rlib" and is a compilation that doesn't actually - // require upstream object files to exist, only upstream metadata - // files. As a result, it doesn't require upstream artifacts - LibKind::Lib | LibKind::Rlib => false, - - // Everything else, however, is some form of "linkable output" or - // something that requires upstream object files. - _ => true, - } - } -} - -impl fmt::Debug for LibKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.crate_type().fmt(f) - } -} - -impl<'a> From<&'a String> for LibKind { - fn from(string: &'a String) -> Self { - match string.as_ref() { - "lib" => LibKind::Lib, - "rlib" => LibKind::Rlib, - "dylib" => LibKind::Dylib, - "proc-macro" => LibKind::ProcMacro, - s => LibKind::Other(s.to_string()), - } - } -} - #[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] pub enum TargetKind { - Lib(Vec), + Lib(Vec), Bin, Test, Bench, - ExampleLib(Vec), + ExampleLib(Vec), ExampleBin, CustomBuild, } @@ -173,8 +114,8 @@ impl ser::Serialize for TargetKind { S: ser::Serializer, { use self::TargetKind::*; - match *self { - Lib(ref kinds) => s.collect_seq(kinds.iter().map(LibKind::crate_type)), + match self { + Lib(kinds) => s.collect_seq(kinds.iter().map(|t| t.to_string())), Bin => ["bin"].serialize(s), ExampleBin | ExampleLib(_) => ["example"].serialize(s), Test => ["test"].serialize(s), @@ -303,7 +244,7 @@ struct SerializedTarget<'a> { kind: &'a TargetKind, /// Corresponds to `--crate-type` compiler attribute. /// See https://doc.rust-lang.org/reference/linkage.html - crate_types: Vec<&'a str>, + crate_types: Vec, name: &'a str, src_path: Option<&'a PathBuf>, edition: &'a str, @@ -659,7 +600,7 @@ impl Target { pub fn lib_target( name: &str, - crate_targets: Vec, + crate_targets: Vec, src_path: PathBuf, edition: Edition, ) -> Target { @@ -712,15 +653,12 @@ impl Target { pub fn example_target( name: &str, - crate_targets: Vec, + crate_targets: Vec, src_path: PathBuf, required_features: Option>, edition: Edition, ) -> Target { - let kind = if crate_targets.is_empty() - || crate_targets - .iter() - .all(|t| *t == LibKind::Other("bin".into())) + let kind = if crate_targets.is_empty() || crate_targets.iter().all(|t| *t == CrateType::Bin) { TargetKind::ExampleBin } else { @@ -812,9 +750,9 @@ impl Target { pub fn doctestable(&self) -> bool { match self.kind() { - TargetKind::Lib(ref kinds) => kinds - .iter() - .any(|k| *k == LibKind::Rlib || *k == LibKind::Lib || *k == LibKind::ProcMacro), + TargetKind::Lib(ref kinds) => kinds.iter().any(|k| { + *k == CrateType::Rlib || *k == CrateType::Lib || *k == CrateType::ProcMacro + }), _ => false, } } @@ -836,29 +774,25 @@ impl Target { pub fn is_dylib(&self) -> bool { match self.kind() { - TargetKind::Lib(libs) => libs.iter().any(|l| *l == LibKind::Dylib), + TargetKind::Lib(libs) => libs.iter().any(|l| *l == CrateType::Dylib), _ => false, } } pub fn is_cdylib(&self) -> bool { - let libs = match self.kind() { - TargetKind::Lib(libs) => libs, - _ => return false, - }; - libs.iter().any(|l| match *l { - LibKind::Other(ref s) => s == "cdylib", + match self.kind() { + TargetKind::Lib(libs) => libs.iter().any(|l| *l == CrateType::Cdylib), _ => false, - }) + } } /// Returns whether this target produces an artifact which can be linked /// into a Rust crate. /// /// This only returns true for certain kinds of libraries. - pub fn linkable(&self) -> bool { + pub fn is_linkable(&self) -> bool { match self.kind() { - TargetKind::Lib(kinds) => kinds.iter().any(|k| k.linkable()), + TargetKind::Lib(kinds) => kinds.iter().any(|k| k.is_linkable()), _ => false, } } @@ -900,25 +834,23 @@ impl Target { } /// Returns the arguments suitable for `--crate-type` to pass to rustc. - pub fn rustc_crate_types(&self) -> Vec<&str> { + pub fn rustc_crate_types(&self) -> Vec { match self.kind() { - TargetKind::Lib(ref kinds) | TargetKind::ExampleLib(ref kinds) => { - kinds.iter().map(LibKind::crate_type).collect() - } + TargetKind::Lib(kinds) | TargetKind::ExampleLib(kinds) => kinds.clone(), TargetKind::CustomBuild | TargetKind::Bench | TargetKind::Test | TargetKind::ExampleBin - | TargetKind::Bin => vec!["bin"], + | TargetKind::Bin => vec![CrateType::Bin], } } pub fn can_lto(&self) -> bool { match self.kind() { - TargetKind::Lib(ref v) => { - !v.contains(&LibKind::Rlib) - && !v.contains(&LibKind::Dylib) - && !v.contains(&LibKind::Lib) + TargetKind::Lib(v) => { + !v.contains(&CrateType::Rlib) + && !v.contains(&CrateType::Dylib) + && !v.contains(&CrateType::Lib) } _ => true, } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index b73c546493b..3d664d153b7 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -5,7 +5,7 @@ pub use self::features::{ pub use self::features::{CliUnstable, Edition, Feature, Features}; pub use self::interning::InternedString; pub use self::manifest::{EitherManifest, VirtualManifest}; -pub use self::manifest::{LibKind, Manifest, Target, TargetKind}; +pub use self::manifest::{Manifest, Target, TargetKind}; pub use self::package::{Package, PackageSet}; pub use self::package_id::PackageId; pub use self::package_id_spec::PackageIdSpec; diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 762da60b02b..24ce6f901bb 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -832,9 +832,11 @@ fn generate_targets( for proposal in filter_targets(packages, Target::is_lib, false, mode) { let Proposal { target, pkg, .. } = proposal; if mode.is_doc_test() && !target.doctestable() { + let types = target.rustc_crate_types(); + let types_str: Vec<&str> = types.iter().map(|t| t.as_str()).collect(); ws.config().shell().warn(format!( "doc tests are not supported for crate type(s) `{}` in package `{}`", - target.rustc_crate_types().join(", "), + types_str.join(", "), pkg.name() ))?; } else { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 77e0dbb8385..c8cceecde87 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -14,7 +14,7 @@ use serde::{Deserialize, Serialize}; use url::Url; use crate::core::dependency::DepKind; -use crate::core::manifest::{LibKind, ManifestMetadata, TargetSourcePath, Warnings}; +use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings}; use crate::core::resolver::ResolveBehavior; use crate::core::{Dependency, InternedString, Manifest, PackageId, Summary, Target}; use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace}; diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index fee2b6691c0..b383adee102 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -15,9 +15,10 @@ use std::fs::{self, DirEntry}; use std::path::{Path, PathBuf}; use super::{ - LibKind, PathValue, StringOrBool, StringOrVec, TomlBenchTarget, TomlBinTarget, - TomlExampleTarget, TomlLibTarget, TomlManifest, TomlTarget, TomlTestTarget, + PathValue, StringOrBool, StringOrVec, TomlBenchTarget, TomlBinTarget, TomlExampleTarget, + TomlLibTarget, TomlManifest, TomlTarget, TomlTestTarget, }; +use crate::core::compiler::CrateType; use crate::core::{Edition, Feature, Features, Target}; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::restricted_names; @@ -222,15 +223,15 @@ fn clean_lib( if kinds.len() > 1 { anyhow::bail!("cannot mix `proc-macro` crate type with others"); } - vec![LibKind::ProcMacro] + vec![CrateType::ProcMacro] } (_, Some(true), Some(true)) => { anyhow::bail!("`lib.plugin` and `lib.proc-macro` cannot both be `true`") } (Some(kinds), _, _) => kinds.iter().map(|s| s.into()).collect(), - (None, Some(true), _) => vec![LibKind::Dylib], - (None, _, Some(true)) => vec![LibKind::ProcMacro], - (None, _, _) => vec![LibKind::Lib], + (None, Some(true), _) => vec![CrateType::Dylib], + (None, _, Some(true)) => vec![CrateType::ProcMacro], + (None, _, _) => vec![CrateType::Lib], }; let mut target = Target::lib_target(&lib.name(), crate_types, path, edition); From eac3b66bd457c6beb4295b077871300e290b2448 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 5 May 2020 14:20:56 -0700 Subject: [PATCH 2/5] Rework how Cargo computes the rustc file outputs. --- src/cargo/core/compiler/build_context/mod.rs | 2 +- .../compiler/build_context/target_info.rs | 241 ++++++++--- .../compiler/context/compilation_files.rs | 383 +++++++----------- src/cargo/core/compiler/context/mod.rs | 4 +- src/cargo/core/compiler/crate_type.rs | 97 +++++ src/cargo/core/compiler/fingerprint.rs | 18 +- src/cargo/core/compiler/layout.rs | 7 +- src/cargo/core/compiler/mod.rs | 54 +-- src/cargo/core/compiler/output_depinfo.rs | 2 +- src/cargo/core/manifest.rs | 29 +- src/cargo/ops/mod.rs | 2 +- tests/testsuite/bench.rs | 2 +- tests/testsuite/build_plan.rs | 4 +- tests/testsuite/cache_messages.rs | 20 + tests/testsuite/dep_info.rs | 16 +- tests/testsuite/metabuild.rs | 6 +- tests/testsuite/out_dir.rs | 4 +- tests/testsuite/test.rs | 2 +- 18 files changed, 516 insertions(+), 377 deletions(-) create mode 100644 src/cargo/core/compiler/crate_type.rs diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index 8dac81cffd2..ac7d6eca295 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -10,7 +10,7 @@ use std::collections::HashMap; use std::path::PathBuf; mod target_info; -pub use self::target_info::{FileFlavor, RustcTargetData, TargetInfo}; +pub use self::target_info::{FileFlavor, FileType, RustcTargetData, TargetInfo}; /// The build context, containing all information about a build task. /// diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 698a9e4080f..d3bab3365bf 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -1,5 +1,5 @@ -use crate::core::compiler::{BuildOutput, CompileKind, CompileTarget, CrateType}; -use crate::core::{Dependency, TargetKind, Workspace}; +use crate::core::compiler::{BuildOutput, CompileKind, CompileMode, CompileTarget, CrateType}; +use crate::core::{Dependency, Target, TargetKind, Workspace}; use crate::util::config::{Config, StringList, TargetConfig}; use crate::util::{CargoResult, CargoResultExt, ProcessBuilder, Rustc}; use cargo_platform::{Cfg, CfgExpr}; @@ -49,41 +49,73 @@ pub struct TargetInfo { pub enum FileFlavor { /// Not a special file type. Normal, - /// Like `Normal`, but not directly executable + /// Like `Normal`, but not directly executable. + /// For example, a `.wasm` file paired with the "normal" `.js` file. Auxiliary, /// Something you can link against (e.g., a library). - Linkable { rmeta: bool }, + Linkable, + /// An `.rmeta` Rust metadata file. + Rmeta, /// Piece of external debug information (e.g., `.dSYM`/`.pdb` file). DebugInfo, } /// Type of each file generated by a Unit. +#[derive(Debug)] pub struct FileType { /// The kind of file. pub flavor: FileFlavor, + /// The crate-type that generates this file. + /// + /// `None` for things that aren't associated with a specific crate type, + /// for example `rmeta` files. + pub crate_type: Option, /// The suffix for the file (for example, `.rlib`). /// This is an empty string for executables on Unix-like platforms. suffix: String, /// The prefix for the file (for example, `lib`). /// This is an empty string for things like executables. prefix: String, - /// Flag to convert hyphen to underscore. - /// - /// wasm bin targets will generate two files in deps such as - /// "web-stuff.js" and "web_stuff.wasm". Note the different usages of "-" - /// and "_". This flag indicates that the stem "web-stuff" should be - /// converted to "web_stuff". + /// Flag to convert hyphen to underscore when uplifting. should_replace_hyphens: bool, } impl FileType { - pub fn filename(&self, stem: &str) -> String { - let stem = if self.should_replace_hyphens { - stem.replace("-", "_") + /// The filename for this FileType crated by rustc. + pub fn output_filename(&self, target: &Target, metadata: Option<&str>) -> String { + match metadata { + Some(metadata) => format!( + "{}{}-{}{}", + self.prefix, + target.crate_name(), + metadata, + self.suffix + ), + None => format!("{}{}{}", self.prefix, target.crate_name(), self.suffix), + } + } + + /// The filename for this FileType that Cargo should use when "uplifting" + /// it to the destination directory. + pub fn uplift_filename(&self, target: &Target) -> String { + let name = if self.should_replace_hyphens { + target.crate_name() } else { - stem.to_string() + target.name().to_string() }; - format!("{}{}{}", self.prefix, stem, self.suffix) + format!("{}{}{}", self.prefix, name, self.suffix) + } + + /// Creates a new instance representing a `.rmeta` file. + pub fn new_rmeta() -> FileType { + // Note that even binaries use the `lib` prefix. + FileType { + flavor: FileFlavor::Rmeta, + crate_type: None, + suffix: ".rmeta".to_string(), + prefix: "lib".to_string(), + should_replace_hyphens: true, + } } } @@ -232,11 +264,10 @@ impl TargetInfo { /// Returns the list of file types generated by the given crate type. /// /// Returns `None` if the target does not support the given crate type. - pub fn file_types( + fn file_types( &self, crate_type: &CrateType, flavor: FileFlavor, - kind: &TargetKind, target_triple: &str, ) -> CargoResult>> { let crate_type = if *crate_type == CrateType::Lib { @@ -262,58 +293,95 @@ impl TargetInfo { suffix: suffix.clone(), prefix: prefix.clone(), flavor, - should_replace_hyphens: false, + crate_type: Some(crate_type.clone()), + should_replace_hyphens: crate_type != CrateType::Bin, }]; - // See rust-lang/cargo#4500. - if target_triple.ends_with("-windows-msvc") - && (crate_type == CrateType::Dylib || crate_type == CrateType::Cdylib) - && suffix == ".dll" - { - ret.push(FileType { - suffix: ".dll.lib".to_string(), - prefix: prefix.clone(), - flavor: FileFlavor::Normal, - should_replace_hyphens: false, - }) - } else if target_triple.ends_with("windows-gnu") - && (crate_type == CrateType::Dylib || crate_type == CrateType::Cdylib) - && suffix == ".dll" - { - // LD can link DLL directly, but LLD requires the import library. - ret.push(FileType { - suffix: ".dll.a".to_string(), - prefix: "lib".to_string(), - flavor: FileFlavor::Normal, - should_replace_hyphens: false, - }) + // Window shared library import/export files. + if crate_type.is_dynamic() { + if target_triple.ends_with("-windows-msvc") { + assert!(suffix == ".dll"); + // See https://docs.microsoft.com/en-us/cpp/build/reference/working-with-import-libraries-and-export-files + // for more information about DLL import/export files. + ret.push(FileType { + suffix: ".dll.lib".to_string(), + prefix: prefix.clone(), + flavor: FileFlavor::Auxiliary, + crate_type: Some(crate_type.clone()), + should_replace_hyphens: true, + }); + // NOTE: lld does not produce these + ret.push(FileType { + suffix: ".dll.exp".to_string(), + prefix: prefix.clone(), + flavor: FileFlavor::Auxiliary, + crate_type: Some(crate_type.clone()), + should_replace_hyphens: true, + }); + } else if target_triple.ends_with("windows-gnu") { + assert!(suffix == ".dll"); + // See https://cygwin.com/cygwin-ug-net/dll.html for more + // information about GNU import libraries. + // LD can link DLL directly, but LLD requires the import library. + ret.push(FileType { + suffix: ".dll.a".to_string(), + prefix: "lib".to_string(), + flavor: FileFlavor::Auxiliary, + crate_type: Some(crate_type.clone()), + should_replace_hyphens: true, + }) + } } - // See rust-lang/cargo#4535. if target_triple.starts_with("wasm32-") && crate_type == CrateType::Bin && suffix == ".js" { + // emscripten binaries generate a .js file, which loads a .wasm + // file. ret.push(FileType { suffix: ".wasm".to_string(), prefix: prefix.clone(), flavor: FileFlavor::Auxiliary, + crate_type: Some(crate_type.clone()), + // Name `foo-bar` will generate a `foo_bar.js` and + // `foo_bar.wasm`. Cargo will translate the underscore and + // copy `foo_bar.js` to `foo-bar.js`. However, the wasm + // filename is embedded in the .js file with an underscore, so + // it should not contain hyphens. should_replace_hyphens: true, - }) + }); + // And a map file for debugging. This is only emitted with debug=2 + // (-g4 for emcc). + ret.push(FileType { + suffix: ".wasm.map".to_string(), + prefix: prefix.clone(), + flavor: FileFlavor::DebugInfo, + crate_type: Some(crate_type.clone()), + should_replace_hyphens: true, + }); } - // See rust-lang/cargo#4490, rust-lang/cargo#4960. - // Only uplift debuginfo for binaries. - // - Tests are run directly from `target/debug/deps/` with the - // metadata hash still in the filename. - // - Examples are only uplifted for apple because the symbol file - // needs to match the executable file name to be found (i.e., it - // needs to remove the hash in the filename). On Windows, the path - // to the .pdb with the hash is embedded in the executable. + // Handle separate debug files. let is_apple = target_triple.contains("-apple-"); - if *kind == TargetKind::Bin || (*kind == TargetKind::ExampleBin && is_apple) { + if matches!( + crate_type, + CrateType::Bin | CrateType::Dylib | CrateType::Cdylib | CrateType::ProcMacro + ) { if is_apple { + let suffix = if crate_type == CrateType::Bin { + ".dSYM".to_string() + } else { + ".dylib.dSYM".to_string() + }; ret.push(FileType { - suffix: ".dSYM".to_string(), + suffix, prefix: prefix.clone(), flavor: FileFlavor::DebugInfo, + crate_type: Some(crate_type.clone()), + // macOS tools like lldb use all sorts of magic to locate + // dSYM files. See https://lldb.llvm.org/use/symbols.html + // for some details. It seems like a `.dSYM` located next + // to the executable with the same name is one method. The + // dSYM should have the same hyphens as the executable for + // the names to match. should_replace_hyphens: false, }) } else if target_triple.ends_with("-msvc") { @@ -321,8 +389,13 @@ impl TargetInfo { suffix: ".pdb".to_string(), prefix: prefix.clone(), flavor: FileFlavor::DebugInfo, - // rustc calls the linker with underscores, and the - // filename is embedded in the executable. + crate_type: Some(crate_type.clone()), + // The absolute path to the pdb file is embedded in the + // executable. If the exe/pdb pair is moved to another + // machine, then debuggers will look in the same directory + // of the exe with the original pdb filename. Since the + // original name contains underscores, they need to be + // preserved. should_replace_hyphens: true, }) } @@ -353,6 +426,62 @@ impl TargetInfo { &mut output.lines(), )?) } + + /// Returns all the file types generated by rustc for the given mode/target_kind. + /// + /// The first value is a Vec of file types generated, the second value is + /// a list of CrateTypes that are not supported by the given target. + pub fn rustc_outputs( + &self, + mode: CompileMode, + target_kind: &TargetKind, + target_triple: &str, + ) -> CargoResult<(Vec, Vec)> { + match mode { + CompileMode::Build => self.calc_rustc_outputs(target_kind, target_triple), + CompileMode::Test | CompileMode::Bench => { + match self.file_types(&CrateType::Bin, FileFlavor::Normal, target_triple)? { + Some(fts) => Ok((fts, Vec::new())), + None => Ok((Vec::new(), vec![CrateType::Bin])), + } + } + CompileMode::Check { .. } => Ok((vec![FileType::new_rmeta()], Vec::new())), + CompileMode::Doc { .. } | CompileMode::Doctest | CompileMode::RunCustomBuild => { + panic!("asked for rustc output for non-rustc mode") + } + } + } + + fn calc_rustc_outputs( + &self, + target_kind: &TargetKind, + target_triple: &str, + ) -> CargoResult<(Vec, Vec)> { + let mut unsupported = Vec::new(); + let mut result = Vec::new(); + let crate_types = target_kind.rustc_crate_types(); + for crate_type in &crate_types { + let flavor = if crate_type.is_linkable() { + FileFlavor::Linkable + } else { + FileFlavor::Normal + }; + let file_types = self.file_types(&crate_type, flavor, target_triple)?; + match file_types { + Some(types) => { + result.extend(types); + } + None => { + unsupported.push(crate_type.clone()); + } + } + } + if !result.is_empty() && !crate_types.iter().any(|ct| ct.requires_upstream_objects()) { + // Only add rmeta if pipelining. + result.push(FileType::new_rmeta()); + } + Ok((result, unsupported)) + } } /// Takes rustc output (using specialized command line args), and calculates the file prefix and @@ -537,9 +666,7 @@ pub struct RustcTargetData { host_info: TargetInfo, /// Build information for targets that we're building for. This will be - /// empty if the `--target` flag is not passed, and currently also only ever - /// has at most one entry, but eventually we'd like to support multi-target - /// builds with Cargo. + /// empty if the `--target` flag is not passed. target_config: HashMap, target_info: HashMap, } diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index b2c48d4d7cd..09a8bf0587b 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -9,7 +9,7 @@ use lazycell::LazyCell; use log::info; use super::{BuildContext, CompileKind, Context, FileFlavor, Layout}; -use crate::core::compiler::{CompileMode, CompileTarget, CrateType, Unit}; +use crate::core::compiler::{CompileMode, CompileTarget, CrateType, FileType, Unit}; use crate::core::{Target, TargetKind, Workspace}; use crate::util::{self, CargoResult}; @@ -85,10 +85,7 @@ pub struct CompilationFiles<'a, 'cfg> { roots: Vec, ws: &'a Workspace<'cfg>, /// Metadata hash to use for each unit. - /// - /// `None` if the unit should not use a metadata data hash (like rustdoc, - /// or some dylibs). - metas: HashMap>, + metas: HashMap, /// For each Unit, a list all files produced. outputs: HashMap>>>, } @@ -151,15 +148,18 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { } } - /// Gets the metadata for a target in a specific profile. - /// We build to the path `"{filename}-{target_metadata}"`. - /// We use a linking step to link/copy to a predictable filename - /// like `target/debug/libfoo.{a,so,rlib}` and such. + /// Gets the metadata for the given unit. + /// + /// See module docs for more details. /// - /// Returns `None` if the unit should not use a metadata data hash (like + /// Returns `None` if the unit should not use a metadata hash (like /// rustdoc, or some dylibs). - pub fn metadata(&self, unit: &Unit) -> Option { - self.metas[unit] + pub fn metadata(&self, bcx: &BuildContext<'_, '_>, unit: &Unit) -> Option { + if should_use_metadata(bcx, unit) { + Some(self.metas[unit]) + } else { + None + } } /// Gets the short hash based only on the `PackageId`. @@ -169,8 +169,8 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { util::short_hash(&hashable) } - /// Returns the appropriate output directory for the specified package and - /// target. + /// Returns the directory where the artifacts for the given unit are + /// initially created. pub fn out_dir(&self, unit: &Unit) -> PathBuf { if unit.mode.is_doc() { self.layout(unit.kind).doc().to_path_buf() @@ -191,12 +191,11 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { } /// Directory name to use for a package in the form `NAME-HASH`. + /// + /// The hash is unique per Unit. pub fn pkg_dir(&self, unit: &Unit) -> String { let name = unit.pkg.package_id().name(); - match self.metas[unit] { - Some(ref meta) => format!("{}-{}", name, meta), - None => format!("{}-{}", name, self.target_short_hash(unit)), - } + format!("{}-{}", name, self.metas[unit]) } /// Returns the root of the build output tree for the host @@ -252,14 +251,6 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { self.build_script_run_dir(unit).join("out") } - /// Returns the file stem for a given target/profile combo (with metadata). - pub fn file_stem(&self, unit: &Unit) -> String { - match self.metas[unit] { - Some(ref metadata) => format!("{}-{}", unit.target.crate_name(), metadata), - None => self.bin_stem(unit), - } - } - /// Returns the path to the executable binary for the given bin target. /// /// This should only to be used when a `Unit` is not available. @@ -272,13 +263,12 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { assert!(target.is_bin()); let dest = self.layout(kind).dest(); let info = bcx.target_data.info(kind); - let file_types = info - .file_types( - &CrateType::Bin, - FileFlavor::Normal, + let (file_types, _) = info + .rustc_outputs( + CompileMode::Build, &TargetKind::Bin, bcx.target_data.short_name(&kind), - )? + ) .expect("target must support `bin`"); let file_type = file_types @@ -286,10 +276,12 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { .find(|file_type| file_type.flavor == FileFlavor::Normal) .expect("target must support `bin`"); - Ok(dest.join(file_type.filename(target.name()))) + Ok(dest.join(file_type.uplift_filename(target))) } /// Returns the filenames that the given unit will generate. + /// + /// Note: It is not guaranteed that all of the files will be generated. pub(super) fn outputs( &self, unit: &Unit, @@ -300,57 +292,50 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { .map(Arc::clone) } - /// Returns the bin filename for a given target, without extension and metadata. - fn bin_stem(&self, unit: &Unit) -> String { - if unit.target.allows_dashes() { - unit.target.name().to_string() - } else { - unit.target.crate_name() + /// Returns the path where the output for the given unit and FileType + /// should be uplifted to. + /// + /// Returns `None` if the unit shouldn't be uplifted (for example, a + /// dependent rlib). + fn uplift_to(&self, unit: &Unit, file_type: &FileType, from_path: &Path) -> Option { + // Tests, check, doc, etc. should not be uplifted. + if unit.mode != CompileMode::Build || file_type.flavor == FileFlavor::Rmeta { + return None; + } + // Only uplift: + // - Binaries: The user always wants to see these, even if they are + // implicitly built (for example for integration tests). + // - dylibs: This ensures that the dynamic linker pulls in all the + // latest copies (even if the dylib was built from a previous cargo + // build). There are complex reasons for this, see #8139, #6167, #6162. + // - Things directly requested from the command-line (the "roots"). + // This one is a little questionable for rlibs (see #6131), but is + // historically how Cargo has operated. This is primarily useful to + // give the user access to staticlibs and cdylibs. + if !unit.target.is_bin() + && !unit.target.is_custom_build() + && file_type.crate_type != Some(CrateType::Dylib) + && !self.roots.contains(unit) + { + return None; } - } - /// Returns a tuple `(hard_link_dir, filename_stem)` for the primary - /// output file for the given unit. - /// - /// `hard_link_dir` is the directory where the file should be hard-linked - /// ("uplifted") to. For example, `/path/to/project/target`. - /// - /// `filename_stem` is the base filename without an extension. - /// - /// This function returns it in two parts so the caller can add - /// prefix/suffix to filename separately. - /// - /// Returns an `Option` because in some cases we don't want to link - /// (eg a dependent lib). - fn link_stem(&self, unit: &Unit) -> Option<(PathBuf, String)> { - let out_dir = self.out_dir(unit); - let bin_stem = self.bin_stem(unit); // Stem without metadata. - let file_stem = self.file_stem(unit); // Stem with metadata. - - // We currently only lift files up from the `deps` directory. If - // it was compiled into something like `example/` or `doc/` then - // we don't want to link it up. - if out_dir.ends_with("deps") { - // Don't lift up library dependencies. - if unit.target.is_bin() || self.roots.contains(unit) || unit.target.is_dylib() { - Some(( - out_dir.parent().unwrap().to_owned(), - if unit.mode.is_any_test() { - file_stem - } else { - bin_stem - }, - )) - } else { - None - } - } else if bin_stem == file_stem { - None - } else if out_dir.ends_with("examples") || out_dir.parent().unwrap().ends_with("build") { - Some((out_dir, bin_stem)) + let filename = file_type.uplift_filename(&unit.target); + let uplift_path = if unit.target.is_example() { + // Examples live in their own little world. + self.layout(unit.kind).examples().join(filename) + } else if unit.target.is_custom_build() { + self.build_script_dir(unit).join(filename) } else { - None + self.layout(unit.kind).dest().join(filename) + }; + if from_path == uplift_path { + // This can happen with things like examples that reside in the + // same directory, do not have a metadata hash (like on Windows), + // and do not have hyphens. + return None; } + Some(uplift_path) } fn calc_outputs( @@ -359,18 +344,6 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { bcx: &BuildContext<'a, 'cfg>, ) -> CargoResult>> { let ret = match unit.mode { - CompileMode::Check { .. } => { - // This may be confusing. rustc outputs a file named `lib*.rmeta` - // for both libraries and binaries. - let file_stem = self.file_stem(unit); - let path = self.out_dir(unit).join(format!("lib{}.rmeta", file_stem)); - vec![OutputFile { - path, - hardlink: None, - export_path: None, - flavor: FileFlavor::Linkable { rmeta: false }, - }] - } CompileMode::Doc { .. } => { let path = self .out_dir(unit) @@ -394,131 +367,77 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { // but Cargo does not know about that. vec![] } - CompileMode::Test | CompileMode::Build | CompileMode::Bench => { - self.calc_outputs_rustc(unit, bcx)? - } + CompileMode::Test + | CompileMode::Build + | CompileMode::Bench + | CompileMode::Check { .. } => self.calc_outputs_rustc(unit, bcx)?, }; info!("Target filenames: {:?}", ret); Ok(Arc::new(ret)) } + /// Computes the actual, full pathnames for all the files generated by rustc. + /// + /// The `OutputFile` also contains the paths where those files should be + /// "uplifted" to. fn calc_outputs_rustc( &self, unit: &Unit, bcx: &BuildContext<'a, 'cfg>, ) -> CargoResult> { - let mut ret = Vec::new(); - let mut unsupported = Vec::new(); - let out_dir = self.out_dir(unit); - let link_stem = self.link_stem(unit); + let info = bcx.target_data.info(unit.kind); - let file_stem = self.file_stem(unit); - - let mut add = |crate_type: &CrateType, flavor: FileFlavor| -> CargoResult<()> { - let file_types = info.file_types( - crate_type, - flavor, - unit.target.kind(), - bcx.target_data.short_name(&unit.kind), - )?; - - match file_types { - Some(types) => { - for file_type in types { - let path = out_dir.join(file_type.filename(&file_stem)); - // Don't create hardlink for tests - let hardlink = if unit.mode.is_any_test() { - None - } else { - link_stem - .as_ref() - .map(|&(ref ld, ref ls)| ld.join(file_type.filename(ls))) - }; - let export_path = if unit.target.is_custom_build() { - None - } else { - self.export_dir.as_ref().and_then(|export_dir| { - hardlink - .as_ref() - .map(|hardlink| export_dir.join(hardlink.file_name().unwrap())) - }) - }; - ret.push(OutputFile { - path, - hardlink, - export_path, - flavor: file_type.flavor, - }); - } - } - // Not supported; don't worry about it. - None => { - unsupported.push(crate_type.to_string()); - } - } - Ok(()) - }; - match unit.target.kind() { - TargetKind::Bin - | TargetKind::CustomBuild - | TargetKind::ExampleBin - | TargetKind::Bench - | TargetKind::Test => { - add(&CrateType::Bin, FileFlavor::Normal)?; - } - TargetKind::Lib(..) | TargetKind::ExampleLib(..) if unit.mode.is_any_test() => { - add(&CrateType::Bin, FileFlavor::Normal)?; - } - TargetKind::ExampleLib(crate_types) | TargetKind::Lib(crate_types) => { - for crate_type in crate_types { - add( - crate_type, - if crate_type.is_linkable() { - FileFlavor::Linkable { rmeta: false } - } else { - FileFlavor::Normal - }, - )?; - } - let path = out_dir.join(format!("lib{}.rmeta", file_stem)); - if !unit.requires_upstream_objects() { - ret.push(OutputFile { - path, - hardlink: None, - export_path: None, - flavor: FileFlavor::Linkable { rmeta: true }, - }); - } - } - } - if ret.is_empty() { + let triple = bcx.target_data.short_name(&unit.kind); + let (file_types, unsupported) = + info.rustc_outputs(unit.mode, unit.target.kind(), triple)?; + if file_types.is_empty() { if !unsupported.is_empty() { + let unsupported_strs: Vec<_> = unsupported.iter().map(|ct| ct.as_str()).collect(); anyhow::bail!( "cannot produce {} for `{}` as the target `{}` \ does not support these crate types", - unsupported.join(", "), + unsupported_strs.join(", "), unit.pkg, - bcx.target_data.short_name(&unit.kind), + triple, ) } anyhow::bail!( "cannot compile `{}` as the target `{}` does not \ support any of the output crate types", unit.pkg, - bcx.target_data.short_name(&unit.kind), + triple, ); } - Ok(ret) + + // Convert FileType to OutputFile. + let mut outputs = Vec::new(); + for file_type in file_types { + let meta = self.metadata(bcx, unit).map(|m| m.to_string()); + let path = out_dir.join(file_type.output_filename(&unit.target, meta.as_deref())); + let hardlink = self.uplift_to(unit, &file_type, &path); + let export_path = if unit.target.is_custom_build() { + None + } else { + self.export_dir.as_ref().and_then(|export_dir| { + hardlink + .as_ref() + .map(|hardlink| export_dir.join(hardlink.file_name().unwrap())) + }) + }; + outputs.push(OutputFile { + path, + hardlink, + export_path, + flavor: file_type.flavor, + }); + } + Ok(outputs) } } -fn metadata_of( - unit: &Unit, - cx: &Context<'_, '_>, - metas: &mut HashMap>, -) -> Option { +fn metadata_of(unit: &Unit, cx: &Context<'_, '_>, metas: &mut HashMap) -> Metadata { if !metas.contains_key(unit) { let meta = compute_metadata(unit, cx, metas); metas.insert(unit.clone(), meta); @@ -532,48 +451,9 @@ fn metadata_of( fn compute_metadata( unit: &Unit, cx: &Context<'_, '_>, - metas: &mut HashMap>, -) -> Option { - if unit.mode.is_doc_test() { - // Doc tests do not have metadata. - return None; - } - // No metadata for dylibs because of a couple issues: - // - macOS encodes the dylib name in the executable, - // - Windows rustc multiple files of which we can't easily link all of them. - // - // No metadata for bin because of an issue: - // - wasm32 rustc/emcc encodes the `.wasm` name in the `.js` (rust-lang/cargo#4535). - // - msvc: The path to the PDB is embedded in the executable, and we don't - // want the PDB path to include the hash in it. - // - // Two exceptions: - // 1) Upstream dependencies (we aren't exporting + need to resolve name conflict), - // 2) `__CARGO_DEFAULT_LIB_METADATA` env var. - // - // Note, however, that the compiler's build system at least wants - // path dependencies (eg libstd) to have hashes in filenames. To account for - // that we have an extra hack here which reads the - // `__CARGO_DEFAULT_LIB_METADATA` environment variable and creates a - // hash in the filename if that's present. - // - // This environment variable should not be relied on! It's - // just here for rustbuild. We need a more principled method - // doing this eventually. + metas: &mut HashMap, +) -> Metadata { let bcx = &cx.bcx; - let __cargo_default_lib_metadata = env::var("__CARGO_DEFAULT_LIB_METADATA"); - let short_name = bcx.target_data.short_name(&unit.kind); - if !(unit.mode.is_any_test() || unit.mode.is_check()) - && (unit.target.is_dylib() - || unit.target.is_cdylib() - || (unit.target.is_executable() && short_name.starts_with("wasm32-")) - || (unit.target.is_executable() && short_name.contains("msvc"))) - && unit.pkg.package_id().source_id().is_path() - && __cargo_default_lib_metadata.is_err() - { - return None; - } - let mut hasher = SipHasher::new(); // This is a generic version number that can be changed to make @@ -633,7 +513,7 @@ fn compute_metadata( // 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 { + if let Ok(ref channel) = env::var("__CARGO_DEFAULT_LIB_METADATA") { channel.hash(&mut hasher); } @@ -646,7 +526,7 @@ fn compute_metadata( // with user dependencies. unit.is_std.hash(&mut hasher); - Some(Metadata(hasher.finish())) + Metadata(hasher.finish()) } fn hash_rustc_version(bcx: &BuildContext<'_, '_>, hasher: &mut SipHasher) { @@ -680,3 +560,46 @@ fn hash_rustc_version(bcx: &BuildContext<'_, '_>, hasher: &mut SipHasher) { // the future when cranelift sees more use, and people want to switch // between different backends without recompiling. } + +/// Returns whether or not this unit should use a metadata hash. +fn should_use_metadata(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool { + if unit.mode.is_doc_test() { + // Doc tests do not have metadata. + return false; + } + if unit.mode.is_any_test() || unit.mode.is_check() { + // These always use metadata. + return true; + } + // No metadata in these cases: + // + // - dylibs: + // - macOS encodes the dylib name in the executable, so it can't be renamed. + // - TODO: Are there other good reasons? If not, maybe this should be macos specific? + // - Windows MSVC executables: The path to the PDB is embedded in the + // executable, and we don't want the PDB path to include the hash in it. + // - wasm32 executables: When using emscripten, the path to the .wasm file + // is embedded in the .js file, so we don't want the hash in there. + // TODO: Is this necessary for wasm32-unknown-unknown? + // + // This is only done for local packages, as we don't expect to export + // dependencies. + // + // The __CARGO_DEFAULT_LIB_METADATA env var is used to override this to + // force metadata in the hash. This is only used for building libstd. For + // example, if libstd is placed in a common location, we don't want a file + // named /usr/lib/libstd.so which could conflict with other rustc + // installs. TODO: Is this still a realistic concern? + // See https://github.com/rust-lang/cargo/issues/3005 + let short_name = bcx.target_data.short_name(&unit.kind); + if (unit.target.is_dylib() + || unit.target.is_cdylib() + || (unit.target.is_executable() && short_name.starts_with("wasm32-")) + || (unit.target.is_executable() && short_name.contains("msvc"))) + && unit.pkg.package_id().source_id().is_path() + && env::var("__CARGO_DEFAULT_LIB_METADATA").is_err() + { + return false; + } + true +} diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index ae5c1382947..318dc8655b4 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -262,7 +262,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// Returns the executable for the specified unit (if any). pub fn get_executable(&mut self, unit: &Unit) -> CargoResult> { for output in self.outputs(unit)?.iter() { - if output.flavor == FileFlavor::DebugInfo { + if output.flavor != FileFlavor::Normal { continue; } @@ -377,7 +377,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { pub fn get_run_build_script_metadata(&self, unit: &Unit) -> Metadata { assert!(unit.mode.is_run_custom_build()); self.files() - .metadata(unit) + .metadata(self.bcx, unit) .expect("build script should always have hash") } diff --git a/src/cargo/core/compiler/crate_type.rs b/src/cargo/core/compiler/crate_type.rs new file mode 100644 index 00000000000..fd0126cbe83 --- /dev/null +++ b/src/cargo/core/compiler/crate_type.rs @@ -0,0 +1,97 @@ +use std::fmt; + +#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum CrateType { + Bin, + Lib, + Rlib, + Dylib, + Cdylib, + Staticlib, + ProcMacro, + Other(String), +} + +impl CrateType { + pub fn as_str(&self) -> &str { + match self { + CrateType::Bin => "bin", + CrateType::Lib => "lib", + CrateType::Rlib => "rlib", + CrateType::Dylib => "dylib", + CrateType::Cdylib => "cdylib", + CrateType::Staticlib => "staticlib", + CrateType::ProcMacro => "proc-macro", + CrateType::Other(s) => s, + } + } + + pub fn is_linkable(&self) -> bool { + match self { + CrateType::Lib | CrateType::Rlib | CrateType::Dylib | CrateType::ProcMacro => true, + CrateType::Bin | CrateType::Cdylib | CrateType::Staticlib | CrateType::Other(..) => { + false + } + } + } + + pub fn is_dynamic(&self) -> bool { + match self { + CrateType::Dylib | CrateType::Cdylib | CrateType::ProcMacro => true, + CrateType::Lib + | CrateType::Rlib + | CrateType::Bin + | CrateType::Staticlib + | CrateType::Other(..) => false, + } + } + + pub fn requires_upstream_objects(&self) -> bool { + match self { + // "lib" == "rlib" and is a compilation that doesn't actually + // require upstream object files to exist, only upstream metadata + // files. As a result, it doesn't require upstream artifacts + CrateType::Lib | CrateType::Rlib => false, + + // Everything else, however, is some form of "linkable output" or + // something that requires upstream object files. + _ => true, + } + } +} + +impl fmt::Display for CrateType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_str().fmt(f) + } +} + +impl<'a> From<&'a String> for CrateType { + fn from(s: &'a String) -> Self { + match s.as_str() { + "bin" => CrateType::Bin, + "lib" => CrateType::Lib, + "rlib" => CrateType::Rlib, + "dylib" => CrateType::Dylib, + "cdylib" => CrateType::Cdylib, + "staticlib" => CrateType::Staticlib, + "procmacro" => CrateType::ProcMacro, + _ => CrateType::Other(s.clone()), + } + } +} + +impl fmt::Debug for CrateType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.to_string().fmt(f) + } +} + +impl serde::Serialize for CrateType { + fn serialize(&self, s: S) -> Result + where + S: serde::ser::Serializer, + { + self.to_string().serialize(s) + } +} diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index 598abdec8e1..eaf88e2ee5c 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -104,8 +104,9 @@ //! - A "dep-info" file which contains a list of source filenames for the //! target. See below for details. //! - An `invoked.timestamp` file whose filesystem mtime is updated every time -//! the Unit is built. This is an experimental feature used for cleaning -//! unused artifacts. +//! the Unit is built. This is used for capturing the time when the build +//! starts, to detect if files are changed in the middle of the build. See +//! below for more details. //! //! Note that some units are a little different. A Unit for *running* a build //! script or for `rustdoc` does not have a dep-info file (it's not @@ -352,7 +353,7 @@ pub fn prepare_target(cx: &mut Context<'_, '_>, unit: &Unit, force: bool) -> Car )); let bcx = cx.bcx; let new = cx.files().fingerprint_dir(unit); - let loc = new.join(&filename(cx, unit)); + let loc = new.join(&filename(unit)); debug!("fingerprint at: {}", loc.display()); @@ -1523,7 +1524,7 @@ pub fn prepare_init(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<()> { pub fn dep_info_loc(cx: &mut Context<'_, '_>, unit: &Unit) -> PathBuf { cx.files() .fingerprint_dir(unit) - .join(&format!("dep-{}", filename(cx, unit))) + .join(&format!("dep-{}", filename(unit))) } /// Returns an absolute path that target directory. @@ -1679,11 +1680,8 @@ where None } -fn filename(cx: &mut Context<'_, '_>, unit: &Unit) -> String { - // file_stem includes metadata hash. Thus we have a different - // fingerprint for every metadata hash version. This works because - // even if the package is fresh, we'll still link the fresh target - let file_stem = cx.files().file_stem(unit); +/// Returns the filename used for the fingerprint file. +fn filename(unit: &Unit) -> String { let kind = unit.target.kind().description(); let flavor = if unit.mode.is_any_test() { "test-" @@ -1694,7 +1692,7 @@ fn filename(cx: &mut Context<'_, '_>, unit: &Unit) -> String { } else { "" }; - format!("{}{}-{}", flavor, kind, file_stem) + format!("{}{}-{}", flavor, kind, unit.target.name()) } #[repr(u8)] diff --git a/src/cargo/core/compiler/layout.rs b/src/cargo/core/compiler/layout.rs index a896d5be38b..c9f8fcedff0 100644 --- a/src/cargo/core/compiler/layout.rs +++ b/src/cargo/core/compiler/layout.rs @@ -26,16 +26,17 @@ //! # packages //! .fingerprint/ //! # Each package is in a separate directory. +//! # Note that different target kinds have different filename prefixes. //! $pkgname-$META/ //! # Set of source filenames for this package. -//! dep-lib-$pkgname-$META +//! dep-lib-$targetname //! # Timestamp when this package was last built. //! invoked.timestamp //! # The fingerprint hash. -//! lib-$pkgname-$META +//! lib-$targetname //! # Detailed information used for logging the reason why //! # something is being recompiled. -//! lib-$pkgname-$META.json +//! lib-$targetname.json //! # The console output from the compiler. This is cached //! # so that warnings can be redisplayed for "fresh" units. //! output diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index a24aaf0ab9b..ef404c5e989 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -31,7 +31,7 @@ use lazycell::LazyCell; use log::debug; pub use self::build_config::{BuildConfig, CompileMode, MessageFormat}; -pub use self::build_context::{BuildContext, FileFlavor, RustcTargetData, TargetInfo}; +pub use self::build_context::{BuildContext, FileFlavor, FileType, RustcTargetData, TargetInfo}; use self::build_plan::BuildPlan; pub use self::compilation::{Compilation, Doctest}; pub use self::compile_kind::{CompileKind, CompileTarget}; @@ -192,17 +192,12 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc) -> Car // don't pass the `-l` flags. let pass_l_flag = unit.target.is_lib() || !unit.pkg.targets().iter().any(|t| t.is_lib()); let pass_cdylib_link_args = unit.target.is_cdylib(); - let do_rename = unit.target.allows_dashes() && !unit.mode.is_any_test(); - let real_name = unit.target.name().to_string(); - let crate_name = unit.target.crate_name(); - // Rely on `target_filenames` iterator as source of truth rather than rederiving filestem. - let rustc_dep_info_loc = if do_rename && cx.files().metadata(unit).is_none() { - root.join(&crate_name) - } else { - root.join(&cx.files().file_stem(unit)) - } - .with_extension("d"); + let dep_info_name = match cx.files().metadata(cx.bcx, unit) { + Some(metadata) => format!("{}-{}.d", unit.target.crate_name(), metadata), + None => format!("{}.d", unit.target.crate_name()), + }; + let rustc_dep_info_loc = root.join(dep_info_name); let dep_info_loc = fingerprint::dep_info_loc(cx, unit); rustc.args(cx.bcx.rustflags_args(unit)); @@ -294,20 +289,6 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc) -> Car .chain_err(|| format!("could not compile `{}`.", name))?; } - if do_rename && real_name != crate_name { - let dst = &outputs[0].path; - let src = dst.with_file_name( - dst.file_name() - .unwrap() - .to_str() - .unwrap() - .replace(&real_name, &crate_name), - ); - if src.exists() && src.file_name() != dst.file_name() { - fs::rename(&src, &dst).chain_err(|| format!("could not rename crate {:?}", src))?; - } - } - if rustc_dep_info_loc.exists() { fingerprint::translate_dep_info( &rustc_dep_info_loc, @@ -888,7 +869,7 @@ fn build_base_args( cmd.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); } - match cx.files().metadata(unit) { + match cx.files().metadata(cx.bcx, unit) { Some(m) => { cmd.arg("-C").arg(&format!("metadata={}", m)); cmd.arg("-C").arg(&format!("extra-filename=-{}", m)); @@ -1069,19 +1050,18 @@ pub fn extern_args( }; let outputs = cx.outputs(&dep.unit)?; - let mut outputs = outputs.iter().filter_map(|output| match output.flavor { - FileFlavor::Linkable { rmeta } => Some((output, rmeta)), - _ => None, - }); - - if cx.only_requires_rmeta(unit, &dep.unit) { - let (output, _rmeta) = outputs - .find(|(_output, rmeta)| *rmeta) - .expect("failed to find rlib dep for pipelined dep"); + + if cx.only_requires_rmeta(unit, &dep.unit) || dep.unit.mode.is_check() { + // Example: rlib dependency for an rlib, rmeta is all that is required. + let output = outputs + .iter() + .find(|output| output.flavor == FileFlavor::Rmeta) + .expect("failed to find rmeta dep for pipelined dep"); pass(&output.path); } else { - for (output, rmeta) in outputs { - if !rmeta { + // Example: a bin needs `rlib` for dependencies, it cannot use rmeta. + for output in outputs.iter() { + if output.flavor == FileFlavor::Linkable { pass(&output.path); } } diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index 284cb338cd5..3a9fac2c6d4 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/src/cargo/core/compiler/output_depinfo.rs @@ -132,7 +132,7 @@ pub fn output_depinfo(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<()> for output in cx .outputs(unit)? .iter() - .filter(|o| o.flavor != FileFlavor::DebugInfo) + .filter(|o| !matches!(o.flavor, FileFlavor::DebugInfo | FileFlavor::Auxiliary)) { if let Some(ref link_dst) = output.hardlink { let output_path = link_dst.with_extension("d"); diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 6e96cdf4c3d..994603aca57 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -164,6 +164,18 @@ impl TargetKind { _ => true, } } + + /// Returns the arguments suitable for `--crate-type` to pass to rustc. + pub fn rustc_crate_types(&self) -> Vec { + match self { + TargetKind::Lib(kinds) | TargetKind::ExampleLib(kinds) => kinds.clone(), + TargetKind::CustomBuild + | TargetKind::Bench + | TargetKind::Test + | TargetKind::ExampleBin + | TargetKind::Bin => vec![CrateType::Bin], + } + } } /// Information about a binary, a library, an example, etc. that is part of the @@ -757,14 +769,6 @@ impl Target { } } - /// Whether or not this target allows dashes in its filename. - /// - /// Rustc will always emit filenames with underscores, and Cargo will - /// rename them back to dashes if this is true. - pub fn allows_dashes(&self) -> bool { - self.is_bin() || self.is_example() || self.is_custom_build() - } - pub fn is_lib(&self) -> bool { match self.kind() { TargetKind::Lib(_) => true, @@ -835,14 +839,7 @@ impl Target { /// Returns the arguments suitable for `--crate-type` to pass to rustc. pub fn rustc_crate_types(&self) -> Vec { - match self.kind() { - TargetKind::Lib(kinds) | TargetKind::ExampleLib(kinds) => kinds.clone(), - TargetKind::CustomBuild - | TargetKind::Bench - | TargetKind::Test - | TargetKind::ExampleBin - | TargetKind::Bin => vec![CrateType::Bin], - } + self.kind().rustc_crate_types() } pub fn can_lto(&self) -> bool { diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index eed36c7867f..dc45fc41dcb 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -1,6 +1,6 @@ pub use self::cargo_clean::{clean, CleanOptions}; pub use self::cargo_compile::{ - compile, compile_with_exec, compile_ws, resolve_all_features, CompileOptions, + compile, compile_with_exec, compile_ws, create_bcx, resolve_all_features, CompileOptions, }; pub use self::cargo_compile::{CompileFilter, FilterRule, LibRule, Packages}; pub use self::cargo_doc::{doc, DocOptions}; diff --git a/tests/testsuite/bench.rs b/tests/testsuite/bench.rs index 225d5c9625e..b4d7de19aa8 100644 --- a/tests/testsuite/bench.rs +++ b/tests/testsuite/bench.rs @@ -1614,7 +1614,7 @@ fn json_artifact_includes_executable_for_benchmark() { { "executable": "[..]/foo/target/release/deps/benchmark-[..][EXE]", "features": [], - "filenames": [ "[..]/foo/target/release/deps/benchmark-[..][EXE]" ], + "filenames": "{...}", "fresh": false, "package_id": "foo 0.0.1 ([..])", "profile": "{...}", diff --git a/tests/testsuite/build_plan.rs b/tests/testsuite/build_plan.rs index 092ff90e637..a6e8eaac8dd 100644 --- a/tests/testsuite/build_plan.rs +++ b/tests/testsuite/build_plan.rs @@ -154,9 +154,7 @@ fn cargo_build_plan_build_script() { "env": "{...}", "kind": null, "links": "{...}", - "outputs": [ - "[..]/foo/target/debug/build/[..]/build_script_build-[..]" - ], + "outputs": "{...}", "package_name": "foo", "package_version": "0.5.0", "program": "rustc", diff --git a/tests/testsuite/cache_messages.rs b/tests/testsuite/cache_messages.rs index 0503306b943..feaeff7e62a 100644 --- a/tests/testsuite/cache_messages.rs +++ b/tests/testsuite/cache_messages.rs @@ -491,3 +491,23 @@ fn rustc_workspace_wrapper() { .with_stdout_does_not_contain("WRAPPER CALLED: rustc --crate-name foo src/lib.rs [..]") .run(); } + +#[cargo_test] +fn wacky_hashless_fingerprint() { + // On Windows, executables don't have hashes. This checks for a bad + // assumption that caused bad caching. + let p = project() + .file("src/bin/a.rs", "fn main() { let unused = 1; }") + .file("src/bin/b.rs", "fn main() {}") + .build(); + p.cargo("build --bin b") + .with_stderr_does_not_contain("[..]unused[..]") + .run(); + p.cargo("build --bin a") + .with_stderr_contains("[..]unused[..]") + .run(); + // This should not pick up the cache from `a`. + p.cargo("build --bin b") + .with_stderr_does_not_contain("[..]unused[..]") + .run(); +} diff --git a/tests/testsuite/dep_info.rs b/tests/testsuite/dep_info.rs index 886ef862cc4..4223fb31198 100644 --- a/tests/testsuite/dep_info.rs +++ b/tests/testsuite/dep_info.rs @@ -272,13 +272,13 @@ fn relative_depinfo_paths_ws() { assert_deps_contains( &p, - "target/debug/.fingerprint/pm-*/dep-lib-pm-*", + "target/debug/.fingerprint/pm-*/dep-lib-pm", &[(1, "src/lib.rs"), (2, "debug/deps/libpmdep-*.rlib")], ); assert_deps_contains( &p, - &format!("target/{}/debug/.fingerprint/foo-*/dep-bin-foo*", host), + &format!("target/{}/debug/.fingerprint/foo-*/dep-bin-foo", host), &[ (1, "src/main.rs"), ( @@ -296,7 +296,7 @@ fn relative_depinfo_paths_ws() { assert_deps_contains( &p, - "target/debug/.fingerprint/foo-*/dep-build-script-build_script_build-*", + "target/debug/.fingerprint/foo-*/dep-build-script-build-script-build", &[(1, "build.rs"), (2, "debug/deps/libbdep-*.rlib")], ); @@ -400,13 +400,13 @@ fn relative_depinfo_paths_no_ws() { assert_deps_contains( &p, - "target/debug/.fingerprint/pm-*/dep-lib-pm-*", + "target/debug/.fingerprint/pm-*/dep-lib-pm", &[(1, "src/lib.rs"), (2, "debug/deps/libpmdep-*.rlib")], ); assert_deps_contains( &p, - "target/debug/.fingerprint/foo-*/dep-bin-foo*", + "target/debug/.fingerprint/foo-*/dep-bin-foo", &[ (1, "src/main.rs"), ( @@ -424,7 +424,7 @@ fn relative_depinfo_paths_no_ws() { assert_deps_contains( &p, - "target/debug/.fingerprint/foo-*/dep-build-script-build_script_build-*", + "target/debug/.fingerprint/foo-*/dep-build-script-build-script-build", &[(1, "build.rs"), (2, "debug/deps/libbdep-*.rlib")], ); @@ -461,7 +461,7 @@ fn reg_dep_source_not_tracked() { assert_deps( &p, - "target/debug/.fingerprint/regdep-*/dep-lib-regdep-*", + "target/debug/.fingerprint/regdep-*/dep-lib-regdep", |info_path, entries| { for (kind, path) in entries { if *kind == 1 { @@ -513,7 +513,7 @@ fn canonical_path() { assert_deps_contains( &p, - "target/debug/.fingerprint/foo-*/dep-lib-foo-*", + "target/debug/.fingerprint/foo-*/dep-lib-foo", &[(1, "src/lib.rs"), (2, "debug/deps/libregdep-*.rmeta")], ); } diff --git a/tests/testsuite/metabuild.rs b/tests/testsuite/metabuild.rs index ece3c7aece8..77f4843aa3a 100644 --- a/tests/testsuite/metabuild.rs +++ b/tests/testsuite/metabuild.rs @@ -497,7 +497,7 @@ fn metabuild_build_plan() { "compile_mode": "build", "kind": null, "deps": [0, 1], - "outputs": ["[..]/target/debug/build/foo-[..]/metabuild_foo-[..][EXE]"], + "outputs": "{...}", "links": "{...}", "program": "rustc", "args": "{...}", @@ -686,9 +686,7 @@ fn metabuild_json_artifact() { { "executable": null, "features": [], - "filenames": [ - "[..]/foo/target/debug/build/foo-[..]/metabuild-foo[EXE]" - ], + "filenames": "{...}", "fresh": false, "package_id": "foo [..]", "profile": "{...}", diff --git a/tests/testsuite/out_dir.rs b/tests/testsuite/out_dir.rs index 15b26f61980..91365137c9a 100644 --- a/tests/testsuite/out_dir.rs +++ b/tests/testsuite/out_dir.rs @@ -90,8 +90,8 @@ fn dynamic_library_with_debug() { check_dir_contents( &p.root().join("out"), &["libfoo.so"], - &["libfoo.dylib"], - &["foo.dll", "foo.dll.lib"], + &["libfoo.dylib", "libfoo.dylib.dSYM"], + &["foo.dll", "foo.dll.exp", "foo.dll.lib", "foo.pdb"], &["foo.dll", "libfoo.dll.a"], ); } diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index 7f1561e3372..68fb5bc8182 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -3360,7 +3360,7 @@ fn json_artifact_includes_test_flag() { "name":"foo", "src_path":"[..]lib.rs" }, - "filenames":["[..]/foo-[..]"], + "filenames":"{...}", "fresh": false } From a8997cbc0f68427d5d00560594b48ad6f4bd683b Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 5 May 2020 14:22:06 -0700 Subject: [PATCH 3/5] Implement new `clean -p` using globs. --- src/cargo/core/compiler/mod.rs | 1 + src/cargo/ops/cargo_clean.rs | 274 ++++++++++++++++----------------- tests/testsuite/clean.rs | 168 +++++++++++++++++++- 3 files changed, 301 insertions(+), 142 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index ef404c5e989..315ec0bc644 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -41,6 +41,7 @@ pub use self::custom_build::{BuildOutput, BuildScriptOutputs, BuildScripts}; pub use self::job::Freshness; use self::job::{Job, Work}; use self::job_queue::{JobQueue, JobState}; +pub(crate) use self::layout::Layout; use self::output_depinfo::output_depinfo; use self::unit_graph::UnitDep; pub use crate::core::compiler::unit::{Unit, UnitInterner}; diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 0a4166940e3..b9761dbeff0 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -1,22 +1,12 @@ -use crate::core::InternedString; -use std::collections::HashMap; -use std::fs; -use std::path::Path; - -use crate::core::compiler::unit_dependencies; -use crate::core::compiler::BuildContext; -use crate::core::compiler::{ - BuildConfig, CompileKind, CompileMode, Context, RustcTargetData, UnitInterner, -}; -use crate::core::profiles::{Profiles, UnitFor}; -use crate::core::resolver::features::HasDevUnits; -use crate::core::resolver::ResolveOpts; -use crate::core::{PackageIdSpec, Workspace}; +use crate::core::compiler::{CompileKind, CompileMode, Layout, RustcTargetData}; +use crate::core::profiles::Profiles; +use crate::core::{InternedString, PackageIdSpec, Workspace}; use crate::ops; -use crate::ops::resolve::WorkspaceResolve; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::paths; use crate::util::Config; +use std::fs; +use std::path::Path; pub struct CleanOptions<'a> { pub config: &'a Config, @@ -61,135 +51,130 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { if opts.spec.is_empty() { return rm_rf(&target_dir.into_path_unlocked(), config); } - let mut build_config = BuildConfig::new(config, Some(1), &opts.targets, CompileMode::Build)?; - build_config.requested_profile = opts.requested_profile; - let target_data = RustcTargetData::new(ws, &build_config.requested_kinds)?; - // Resolve for default features. In the future, `cargo clean` should be rewritten - // so that it doesn't need to guess filename hashes. - let resolve_opts = ResolveOpts::new( - /*dev_deps*/ true, - &[], - /*all features*/ false, - /*default*/ true, - ); - let specs = opts - .spec - .iter() - .map(|spec| PackageIdSpec::parse(spec)) - .collect::>>()?; - let ws_resolve = ops::resolve_ws_with_opts( - ws, - &target_data, - &build_config.requested_kinds, - &resolve_opts, - &specs, - HasDevUnits::Yes, - )?; - let WorkspaceResolve { - pkg_set, - targeted_resolve: resolve, - resolved_features: features, - .. - } = ws_resolve; - - let interner = UnitInterner::new(); - let mut units = Vec::new(); - - for spec in opts.spec.iter() { - // Translate the spec to a Package - let pkgid = resolve.query(spec)?; - let pkg = pkg_set.get_one(pkgid)?; - - // Generate all relevant `Unit` targets for this package - for target in pkg.targets() { - for kind in build_config - .requested_kinds - .iter() - .chain(Some(&CompileKind::Host)) - { - for mode in CompileMode::all_modes() { - for unit_for in UnitFor::all_values() { - let profile = if mode.is_run_custom_build() { - profiles.get_profile_run_custom_build(&profiles.get_profile( - pkg.package_id(), - ws.is_member(pkg), - /*is_local*/ true, - *unit_for, - CompileMode::Build, - )) - } else { - profiles.get_profile( - pkg.package_id(), - ws.is_member(pkg), - /*is_local*/ true, - *unit_for, - *mode, - ) - }; - // Use unverified here since this is being more - // exhaustive than what is actually needed. - let features_for = unit_for.map_to_features_for(); - let features = features - .activated_features_unverified(pkg.package_id(), features_for) - .unwrap_or_default(); - units.push(interner.intern( - pkg, target, profile, *kind, *mode, features, /*is_std*/ false, - )); - } - } - } + + // Clean specific packages. + let requested_kinds = CompileKind::from_requested_targets(config, &opts.targets)?; + let target_data = RustcTargetData::new(ws, &requested_kinds)?; + let (pkg_set, resolve) = ops::resolve_ws(ws)?; + let prof_dir_name = profiles.get_dir_name(); + let host_layout = Layout::new(ws, None, &prof_dir_name)?; + // Convert requested kinds to a Vec of layouts. + let target_layouts: Vec<(CompileKind, Layout)> = requested_kinds + .into_iter() + .filter_map(|kind| match kind { + CompileKind::Target(target) => match Layout::new(ws, Some(target), &prof_dir_name) { + Ok(layout) => Some(Ok((kind, layout))), + Err(e) => Some(Err(e)), + }, + CompileKind::Host => None, + }) + .collect::>()?; + // A Vec of layouts. This is a little convoluted because there can only be + // one host_layout. + let layouts = if opts.targets.is_empty() { + vec![(CompileKind::Host, &host_layout)] + } else { + target_layouts + .iter() + .map(|(kind, layout)| (*kind, layout)) + .collect() + }; + // Create a Vec that also includes the host for things that need to clean both. + let layouts_with_host: Vec<(CompileKind, &Layout)> = + std::iter::once((CompileKind::Host, &host_layout)) + .chain(layouts.iter().map(|(k, l)| (*k, *l))) + .collect(); + + // Cleaning individual rustdoc crates is currently not supported. + // For example, the search index would need to be rebuilt to fully + // remove it (otherwise you're left with lots of broken links). + // Doc tests produce no output. + + // Get Packages for the specified specs. + let mut packages = Vec::new(); + for spec_str in opts.spec.iter() { + // Translate the spec to a Package. + let spec = PackageIdSpec::parse(spec_str)?; + if spec.version().is_some() { + config.shell().warn(&format!( + "version qualifier in `-p {}` is ignored, \ + cleaning all versions of `{}` found", + spec_str, + spec.name() + ))?; + } + if spec.url().is_some() { + config.shell().warn(&format!( + "url qualifier in `-p {}` ignored, \ + cleaning all versions of `{}` found", + spec_str, + spec.name() + ))?; } + let matches: Vec<_> = resolve.iter().filter(|id| spec.matches(*id)).collect(); + if matches.is_empty() { + anyhow::bail!("package ID specification `{}` matched no packages", spec); + } + packages.extend(pkg_set.get_many(matches)?); } - let unit_graph = unit_dependencies::build_unit_dependencies( - ws, - &pkg_set, - &resolve, - &features, - None, - &units, - &Default::default(), - build_config.mode, - &target_data, - &profiles, - &interner, - )?; - let extra_args = HashMap::new(); - let bcx = BuildContext::new( - ws, - pkg_set, - &build_config, - profiles, - extra_args, - target_data, - units, - unit_graph, - )?; - let mut cx = Context::new(&bcx)?; - cx.prepare_units()?; - - for unit in &bcx.roots { - if unit.mode.is_doc() || unit.mode.is_doc_test() { - // Cleaning individual rustdoc crates is currently not supported. - // For example, the search index would need to be rebuilt to fully - // remove it (otherwise you're left with lots of broken links). - // Doc tests produce no output. - continue; + for pkg in packages { + let pkg_dir = format!("{}-*", pkg.name()); + + // Clean fingerprints. + for (_, layout) in &layouts_with_host { + rm_rf_glob(&layout.fingerprint().join(&pkg_dir), config)?; } - rm_rf(&cx.files().fingerprint_dir(unit), config)?; - if unit.target.is_custom_build() { - if unit.mode.is_run_custom_build() { - rm_rf(&cx.files().build_script_out_dir(unit), config)?; - } else { - rm_rf(&cx.files().build_script_dir(unit), config)?; + + for target in pkg.targets() { + if target.is_custom_build() { + // Get both the build_script_build and the output directory. + for (_, layout) in &layouts_with_host { + rm_rf_glob(&layout.build().join(&pkg_dir), config)?; + } + continue; } - continue; - } + let crate_name = target.crate_name(); + for &mode in &[ + CompileMode::Build, + CompileMode::Test, + CompileMode::Check { test: false }, + ] { + for (compile_kind, layout) in &layouts { + let triple = target_data.short_name(compile_kind); + + let (file_types, _unsupported) = target_data + .info(*compile_kind) + .rustc_outputs(mode, target.kind(), triple)?; + let (dir, uplift_dir) = if target.is_example() { + (layout.examples(), layout.examples()) + } else { + (layout.deps(), layout.dest()) + }; + for file_type in file_types { + // Some files include a hash in the filename, some don't. + let hashed_name = file_type.output_filename(target, Some("*")); + let unhashed_name = file_type.output_filename(target, None); + rm_rf_glob(&dir.join(&hashed_name), config)?; + rm_rf(&dir.join(&unhashed_name), config)?; + // Remove dep-info file generated by rustc. It is not tracked in + // file_types. It does not have a prefix. + let hashed_dep_info = dir.join(format!("{}-*.d", crate_name)); + let unhashed_dep_info = dir.join(format!("{}.d", crate_name)); + rm_rf_glob(&hashed_dep_info, config)?; + rm_rf(&unhashed_dep_info, config)?; - for output in cx.outputs(unit)?.iter() { - rm_rf(&output.path, config)?; - if let Some(ref dst) = output.hardlink { - rm_rf(dst, config)?; + // Remove the uplifted copy. + let uplifted_path = uplift_dir.join(file_type.uplift_filename(target)); + rm_rf(&uplifted_path, config)?; + // Dep-info generated by Cargo itself. + let dep_info = uplifted_path.with_extension("d"); + rm_rf(&dep_info, config)?; + } + // TODO: what to do about build_script_build? + let incremental = layout.incremental().join(format!("{}-*", crate_name)); + rm_rf_glob(&incremental, config)?; + } } } } @@ -197,8 +182,19 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { Ok(()) } +fn rm_rf_glob(pattern: &Path, config: &Config) -> CargoResult<()> { + // TODO: Display utf8 warning to user? Or switch to globset? + let pattern = pattern + .to_str() + .ok_or_else(|| anyhow::anyhow!("expected utf-8 path"))?; + for path in glob::glob(pattern)? { + rm_rf(&path?, config)?; + } + Ok(()) +} + fn rm_rf(path: &Path, config: &Config) -> CargoResult<()> { - let m = fs::metadata(path); + let m = fs::symlink_metadata(path); if m.as_ref().map(|s| s.is_dir()).unwrap_or(false) { config .shell() diff --git a/tests/testsuite/clean.rs b/tests/testsuite/clean.rs index 32f3b29b539..673dc4969a1 100644 --- a/tests/testsuite/clean.rs +++ b/tests/testsuite/clean.rs @@ -1,9 +1,10 @@ //! Tests for the `cargo clean` command. -use std::env; - +use cargo_test_support::paths::CargoPathExt; use cargo_test_support::registry::Package; -use cargo_test_support::{basic_bin_manifest, basic_manifest, git, main_file, project}; +use cargo_test_support::{basic_bin_manifest, basic_manifest, git, main_file, project, rustc_host}; +use std::env; +use std::path::Path; #[cargo_test] fn cargo_clean_simple() { @@ -291,6 +292,7 @@ fn clean_verbose() { [REMOVING] [..] [REMOVING] [..] [REMOVING] [..] +[REMOVING] [..] ", ) .run(); @@ -319,3 +321,163 @@ fn clean_remove_rlib_rmeta() { assert!(!p.target_debug_dir().join("libfoo.rlib").exists()); assert!(!rmeta.exists()); } + +#[cargo_test] +fn package_cleans_all_the_things() { + // -p cleans everything + // Use dashes everywhere to make sure dash/underscore stuff is handled. + for crate_type in &["rlib", "dylib", "cdylib", "staticlib", "proc-macro"] { + // Try each crate type individually since the behavior changes when + // they are combined. + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo-bar" + version = "0.1.0" + + [lib] + crate-type = ["{}"] + "#, + crate_type + ), + ) + .file("src/lib.rs", "") + .build(); + p.cargo("build").run(); + p.cargo("clean -p foo-bar").run(); + assert_all_clean(&p.build_dir()); + } + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo-bar" + version = "0.1.0" + edition = "2018" + + [lib] + crate-type = ["rlib", "dylib", "staticlib"] + + [[example]] + name = "foo-ex-rlib" + crate-type = ["rlib"] + test = true + + [[example]] + name = "foo-ex-cdylib" + crate-type = ["cdylib"] + test = true + + [[example]] + name = "foo-ex-bin" + test = true + "#, + ) + .file("src/lib.rs", "") + .file("src/main.rs", "fn main() {}") + .file("src/bin/other-main.rs", "fn main() {}") + .file("examples/foo-ex-rlib.rs", "") + .file("examples/foo-ex-cdylib.rs", "") + .file("examples/foo-ex-bin.rs", "fn main() {}") + .file("tests/foo-test.rs", "") + .file("benches/foo-bench.rs", "") + .file("build.rs", "fn main() {}") + .build(); + + p.cargo("build --all-targets") + .env("CARGO_INCREMENTAL", "1") + .run(); + p.cargo("test --all-targets") + .env("CARGO_INCREMENTAL", "1") + .run(); + p.cargo("check --all-targets") + .env("CARGO_INCREMENTAL", "1") + .run(); + p.cargo("clean -p foo-bar").run(); + assert_all_clean(&p.build_dir()); + + // Try some targets. + p.cargo("build --all-targets --target") + .arg(rustc_host()) + .run(); + p.cargo("clean -p foo-bar --target").arg(rustc_host()).run(); + assert_all_clean(&p.build_dir()); +} + +// Ensures that all files for the package have been deleted. +fn assert_all_clean(build_dir: &Path) { + let walker = walkdir::WalkDir::new(build_dir).into_iter(); + for entry in walker.filter_entry(|e| { + let path = e.path(); + // This is a known limitation, clean can't differentiate between + // the different build scripts from different packages. + !(path + .file_name() + .unwrap() + .to_str() + .unwrap() + .starts_with("build_script_build") + && path + .parent() + .unwrap() + .file_name() + .unwrap() + .to_str() + .unwrap() + == "incremental") + }) { + let entry = entry.unwrap(); + let path = entry.path(); + if let ".rustc_info.json" | ".cargo-lock" = path.file_name().unwrap().to_str().unwrap() { + continue; + } + if path.is_symlink() || path.is_file() { + panic!("{:?} was not cleaned", path); + } + } +} + +#[cargo_test] +fn clean_spec_multiple() { + // clean -p foo where foo matches multiple versions + Package::new("bar", "1.0.0").publish(); + Package::new("bar", "2.0.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar1 = {version="1.0", package="bar"} + bar2 = {version="2.0", package="bar"} + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build").run(); + p.cargo("clean -p bar:1.0.0") + .with_stderr( + "warning: version qualifier in `-p bar:1.0.0` is ignored, \ + cleaning all versions of `bar` found", + ) + .run(); + let mut walker = walkdir::WalkDir::new(p.build_dir()) + .into_iter() + .filter_map(|e| e.ok()) + .filter(|e| { + let n = e.file_name().to_str().unwrap(); + n.starts_with("bar") || n.starts_with("libbar") + }); + if let Some(e) = walker.next() { + panic!("{:?} was not cleaned", e.path()); + } +} From bd5f563856295e455f943207809ff85adf0d1401 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 6 May 2020 09:36:07 -0700 Subject: [PATCH 4/5] clean -p: call `get_many` once. --- src/cargo/ops/cargo_clean.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index b9761dbeff0..a8b39c4b5b9 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -91,7 +91,7 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { // Doc tests produce no output. // Get Packages for the specified specs. - let mut packages = Vec::new(); + let mut pkg_ids = Vec::new(); for spec_str in opts.spec.iter() { // Translate the spec to a Package. let spec = PackageIdSpec::parse(spec_str)?; @@ -115,8 +115,9 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { if matches.is_empty() { anyhow::bail!("package ID specification `{}` matched no packages", spec); } - packages.extend(pkg_set.get_many(matches)?); + pkg_ids.extend(matches); } + let packages = pkg_set.get_many(pkg_ids)?; for pkg in packages { let pkg_dir = format!("{}-*", pkg.name()); From 7438770be8d1d02b6569d22ee5e38b4b63eea66b Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 6 May 2020 12:06:39 -0700 Subject: [PATCH 5/5] Revert always computing filename Metadata. When a unit does not have Metadata, the computation of fingerprints depends on reusing the same fingerprint file to detect if the output needs to be rebuilt. The previous change that put each unit's fingerprint into a separate directory was wrong, and broke change detection in this case (for example, executables on MSVC). Instead, use a different approach to deal with compiler output caching by using the same naming convention as the fingerprint file itself. --- .../compiler/context/compilation_files.rs | 59 ++++++++++++++----- src/cargo/core/compiler/context/mod.rs | 2 +- src/cargo/core/compiler/fingerprint.rs | 27 ++------- src/cargo/core/compiler/layout.rs | 2 +- src/cargo/core/compiler/mod.rs | 4 +- tests/testsuite/cache_messages.rs | 27 +++++++-- 6 files changed, 73 insertions(+), 48 deletions(-) diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 09a8bf0587b..aef7e69dc28 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -85,7 +85,7 @@ pub struct CompilationFiles<'a, 'cfg> { roots: Vec, ws: &'a Workspace<'cfg>, /// Metadata hash to use for each unit. - metas: HashMap, + metas: HashMap>, /// For each Unit, a list all files produced. outputs: HashMap>>>, } @@ -154,12 +154,8 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { /// /// Returns `None` if the unit should not use a metadata hash (like /// rustdoc, or some dylibs). - pub fn metadata(&self, bcx: &BuildContext<'_, '_>, unit: &Unit) -> Option { - if should_use_metadata(bcx, unit) { - Some(self.metas[unit]) - } else { - None - } + pub fn metadata(&self, unit: &Unit) -> Option { + self.metas[unit] } /// Gets the short hash based only on the `PackageId`. @@ -192,10 +188,14 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { /// Directory name to use for a package in the form `NAME-HASH`. /// - /// The hash is unique per Unit. - pub fn pkg_dir(&self, unit: &Unit) -> String { + /// Note that some units may share the same directory, so care should be + /// taken in those cases! + fn pkg_dir(&self, unit: &Unit) -> String { let name = unit.pkg.package_id().name(); - format!("{}-{}", name, self.metas[unit]) + match self.metas[unit] { + Some(ref meta) => format!("{}-{}", name, meta), + None => format!("{}-{}", name, self.target_short_hash(unit)), + } } /// Returns the root of the build output tree for the host @@ -220,9 +220,29 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { self.layout(unit.kind).fingerprint().join(dir) } + /// Returns the path for a file in the fingerprint directory. + /// + /// The "prefix" should be something to distinguish the file from other + /// files in the fingerprint directory. + pub fn fingerprint_file_path(&self, unit: &Unit, prefix: &str) -> PathBuf { + // Different targets need to be distinguished in the + let kind = unit.target.kind().description(); + let flavor = if unit.mode.is_any_test() { + "test-" + } else if unit.mode.is_doc() { + "doc-" + } else if unit.mode.is_run_custom_build() { + "run-" + } else { + "" + }; + let name = format!("{}{}{}-{}", prefix, flavor, kind, unit.target.name()); + self.fingerprint_dir(unit).join(name) + } + /// Path where compiler output is cached. pub fn message_cache_path(&self, unit: &Unit) -> PathBuf { - self.fingerprint_dir(unit).join("output") + self.fingerprint_file_path(unit, "output-") } /// Returns the directory where a compiled build script is stored. @@ -414,7 +434,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { // Convert FileType to OutputFile. let mut outputs = Vec::new(); for file_type in file_types { - let meta = self.metadata(bcx, unit).map(|m| m.to_string()); + let meta = self.metadata(unit).map(|m| m.to_string()); let path = out_dir.join(file_type.output_filename(&unit.target, meta.as_deref())); let hardlink = self.uplift_to(unit, &file_type, &path); let export_path = if unit.target.is_custom_build() { @@ -437,7 +457,11 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { } } -fn metadata_of(unit: &Unit, cx: &Context<'_, '_>, metas: &mut HashMap) -> Metadata { +fn metadata_of( + unit: &Unit, + cx: &Context<'_, '_>, + metas: &mut HashMap>, +) -> Option { if !metas.contains_key(unit) { let meta = compute_metadata(unit, cx, metas); metas.insert(unit.clone(), meta); @@ -451,9 +475,12 @@ fn metadata_of(unit: &Unit, cx: &Context<'_, '_>, metas: &mut HashMap, - metas: &mut HashMap, -) -> Metadata { + metas: &mut HashMap>, +) -> Option { let bcx = &cx.bcx; + if !should_use_metadata(bcx, unit) { + return None; + } let mut hasher = SipHasher::new(); // This is a generic version number that can be changed to make @@ -526,7 +553,7 @@ fn compute_metadata( // with user dependencies. unit.is_std.hash(&mut hasher); - Metadata(hasher.finish()) + Some(Metadata(hasher.finish())) } fn hash_rustc_version(bcx: &BuildContext<'_, '_>, hasher: &mut SipHasher) { diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 318dc8655b4..60f2b8af34d 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -377,7 +377,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { pub fn get_run_build_script_metadata(&self, unit: &Unit) -> Metadata { assert!(unit.mode.is_run_custom_build()); self.files() - .metadata(self.bcx, unit) + .metadata(unit) .expect("build script should always have hash") } diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index eaf88e2ee5c..145a622bb1f 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -43,8 +43,9 @@ //! The `Metadata` hash is a hash added to the output filenames to isolate //! each unit. See the documentation in the `compilation_files` module for //! more details. NOTE: Not all output files are isolated via filename hashes -//! (like dylibs), but the fingerprint directory always has the `Metadata` -//! hash in its directory name. +//! (like dylibs). The fingerprint directory uses a hash, but sometimes units +//! share the same fingerprint directory (when they don't have Metadata) so +//! care should be taken to handle this! //! //! Fingerprints and Metadata are similar, and track some of the same things. //! The Metadata contains information that is required to keep Units separate. @@ -352,8 +353,7 @@ pub fn prepare_target(cx: &mut Context<'_, '_>, unit: &Unit, force: bool) -> Car unit.target.name() )); let bcx = cx.bcx; - let new = cx.files().fingerprint_dir(unit); - let loc = new.join(&filename(unit)); + let loc = cx.files().fingerprint_file_path(unit, ""); debug!("fingerprint at: {}", loc.display()); @@ -1522,9 +1522,7 @@ pub fn prepare_init(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<()> { /// Returns the location that the dep-info file will show up at for the `unit` /// specified. pub fn dep_info_loc(cx: &mut Context<'_, '_>, unit: &Unit) -> PathBuf { - cx.files() - .fingerprint_dir(unit) - .join(&format!("dep-{}", filename(unit))) + cx.files().fingerprint_file_path(unit, "dep-") } /// Returns an absolute path that target directory. @@ -1680,21 +1678,6 @@ where None } -/// Returns the filename used for the fingerprint file. -fn filename(unit: &Unit) -> String { - let kind = unit.target.kind().description(); - let flavor = if unit.mode.is_any_test() { - "test-" - } else if unit.mode.is_doc() { - "doc-" - } else if unit.mode.is_run_custom_build() { - "run-" - } else { - "" - }; - format!("{}{}-{}", flavor, kind, unit.target.name()) -} - #[repr(u8)] enum DepInfoPathType { // src/, e.g. src/lib.rs diff --git a/src/cargo/core/compiler/layout.rs b/src/cargo/core/compiler/layout.rs index c9f8fcedff0..53c615fbe1d 100644 --- a/src/cargo/core/compiler/layout.rs +++ b/src/cargo/core/compiler/layout.rs @@ -39,7 +39,7 @@ //! lib-$targetname.json //! # The console output from the compiler. This is cached //! # so that warnings can be redisplayed for "fresh" units. -//! output +//! output-lib-$targetname //! //! # This is the root directory for all rustc artifacts except build //! # scripts, examples, and test and bench executables. Almost every diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 315ec0bc644..93922ea7e0e 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -194,7 +194,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc) -> Car let pass_l_flag = unit.target.is_lib() || !unit.pkg.targets().iter().any(|t| t.is_lib()); let pass_cdylib_link_args = unit.target.is_cdylib(); - let dep_info_name = match cx.files().metadata(cx.bcx, unit) { + let dep_info_name = match cx.files().metadata(unit) { Some(metadata) => format!("{}-{}.d", unit.target.crate_name(), metadata), None => format!("{}.d", unit.target.crate_name()), }; @@ -870,7 +870,7 @@ fn build_base_args( cmd.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); } - match cx.files().metadata(cx.bcx, unit) { + match cx.files().metadata(unit) { Some(m) => { cmd.arg("-C").arg(&format!("metadata={}", m)); cmd.arg("-C").arg(&format!("extra-filename=-{}", m)); diff --git a/tests/testsuite/cache_messages.rs b/tests/testsuite/cache_messages.rs index feaeff7e62a..1df15b22764 100644 --- a/tests/testsuite/cache_messages.rs +++ b/tests/testsuite/cache_messages.rs @@ -194,7 +194,7 @@ fn clears_cache_after_fix() { // Fill the cache. p.cargo("check").with_stderr_contains("[..]asdf[..]").run(); let cpath = p - .glob("target/debug/.fingerprint/foo-*/output") + .glob("target/debug/.fingerprint/foo-*/output-*") .next() .unwrap() .unwrap(); @@ -215,7 +215,10 @@ fn clears_cache_after_fix() { ", ) .run(); - assert_eq!(p.glob("target/debug/.fingerprint/foo-*/output").count(), 0); + assert_eq!( + p.glob("target/debug/.fingerprint/foo-*/output-*").count(), + 0 + ); // And again, check the cache is correct. p.cargo("check") @@ -253,7 +256,10 @@ fn rustdoc() { let rustdoc_stderr = as_str(&rustdoc_output.stderr); assert!(rustdoc_stderr.contains("private")); assert!(rustdoc_stderr.contains("\x1b[")); - assert_eq!(p.glob("target/debug/.fingerprint/foo-*/output").count(), 1); + assert_eq!( + p.glob("target/debug/.fingerprint/foo-*/output-*").count(), + 1 + ); // Check the cached output. let rustdoc_output = p @@ -331,14 +337,23 @@ fn doesnt_create_extra_files() { p.cargo("build").run(); - assert_eq!(p.glob("target/debug/.fingerprint/foo-*/output").count(), 0); - assert_eq!(p.glob("target/debug/.fingerprint/dep-*/output").count(), 0); + assert_eq!( + p.glob("target/debug/.fingerprint/foo-*/output-*").count(), + 0 + ); + assert_eq!( + p.glob("target/debug/.fingerprint/dep-*/output-*").count(), + 0 + ); if is_coarse_mtime() { sleep_ms(1000); } p.change_file("src/lib.rs", "fn unused() {}"); p.cargo("build").run(); - assert_eq!(p.glob("target/debug/.fingerprint/foo-*/output").count(), 1); + assert_eq!( + p.glob("target/debug/.fingerprint/foo-*/output-*").count(), + 1 + ); } #[cargo_test]