From 0cc39aaef919bf8d32779d85c2948ca4d9fd39d6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 21 Nov 2016 10:05:55 -0800 Subject: [PATCH 1/2] Refactor metadata generation Remove generation all the way in manifest-parsing and defer it until we actually need it during compilation. Additionally remove lots of weird logic that's no longer necessary that we're hashing quite a few fields. --- src/cargo/core/manifest.rs | 27 ++----- src/cargo/core/mod.rs | 2 +- src/cargo/core/package.rs | 6 +- src/cargo/core/package_id.rs | 23 +----- src/cargo/ops/cargo_clean.rs | 5 +- src/cargo/ops/cargo_rustc/context.rs | 91 ++++++++++++------------ src/cargo/ops/cargo_rustc/fingerprint.rs | 4 +- src/cargo/ops/cargo_rustc/mod.rs | 17 +++-- src/cargo/util/toml.rs | 54 ++++---------- 9 files changed, 81 insertions(+), 148 deletions(-) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index f5f4217da81..93fac21fdf9 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -6,7 +6,6 @@ use rustc_serialize::{Encoder, Encodable}; use core::{Dependency, PackageId, Summary, SourceId, PackageIdSpec}; use core::WorkspaceConfig; -use core::package_id::Metadata; pub enum EitherManifest { Real(Manifest), @@ -159,7 +158,6 @@ pub struct Target { kind: TargetKind, name: String, src_path: PathBuf, - metadata: Option, tested: bool, benched: bool, doc: bool, @@ -279,7 +277,6 @@ impl Target { kind: TargetKind::Bin, name: String::new(), src_path: PathBuf::new(), - metadata: None, doc: false, doctest: false, harness: true, @@ -289,40 +286,35 @@ impl Target { } } - pub fn lib_target(name: &str, crate_targets: Vec, - src_path: &Path, - metadata: Metadata) -> Target { + pub fn lib_target(name: &str, + crate_targets: Vec, + src_path: &Path) -> Target { Target { kind: TargetKind::Lib(crate_targets), name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: Some(metadata), doctest: true, doc: true, ..Target::blank() } } - pub fn bin_target(name: &str, src_path: &Path, - metadata: Option) -> Target { + pub fn bin_target(name: &str, src_path: &Path) -> Target { Target { kind: TargetKind::Bin, name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: metadata, doc: true, ..Target::blank() } } /// Builds a `Target` corresponding to the `build = "build.rs"` entry. - pub fn custom_build_target(name: &str, src_path: &Path, - metadata: Option) -> Target { + pub fn custom_build_target(name: &str, src_path: &Path) -> Target { Target { kind: TargetKind::CustomBuild, name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: metadata, for_host: true, benched: false, tested: false, @@ -340,25 +332,21 @@ impl Target { } } - pub fn test_target(name: &str, src_path: &Path, - metadata: Metadata) -> Target { + pub fn test_target(name: &str, src_path: &Path) -> Target { Target { kind: TargetKind::Test, name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: Some(metadata), benched: false, ..Target::blank() } } - pub fn bench_target(name: &str, src_path: &Path, - metadata: Metadata) -> Target { + pub fn bench_target(name: &str, src_path: &Path) -> Target { Target { kind: TargetKind::Bench, name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: Some(metadata), tested: false, ..Target::blank() } @@ -367,7 +355,6 @@ impl Target { pub fn name(&self) -> &str { &self.name } pub fn crate_name(&self) -> String { self.name.replace("-", "_") } pub fn src_path(&self) -> &Path { &self.src_path } - pub fn metadata(&self) -> Option<&Metadata> { self.metadata.as_ref() } pub fn kind(&self) -> &TargetKind { &self.kind } pub fn tested(&self) -> bool { self.tested } pub fn harness(&self) -> bool { self.harness } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 305b6ed7c74..e663dcdf270 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -2,7 +2,7 @@ pub use self::dependency::{Dependency, DependencyInner}; pub use self::manifest::{Manifest, Target, TargetKind, Profile, LibKind, Profiles}; pub use self::manifest::{EitherManifest, VirtualManifest}; pub use self::package::{Package, PackageSet}; -pub use self::package_id::{PackageId, Metadata}; +pub use self::package_id::PackageId; pub use self::package_id_spec::PackageIdSpec; pub use self::registry::Registry; pub use self::resolver::Resolve; diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 03ed7d88729..11e53cf2c9a 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -7,7 +7,7 @@ use std::path::{Path, PathBuf}; use semver::Version; use core::{Dependency, Manifest, PackageId, SourceId, Target, TargetKind}; -use core::{Summary, Metadata, SourceMap}; +use core::{Summary, SourceMap}; use ops; use util::{CargoResult, Config, LazyCell, ChainError, internal, human, lev_distance}; use rustc_serialize::{Encoder,Encodable}; @@ -94,10 +94,6 @@ impl Package { self.targets().iter().any(|t| t.is_custom_build()) } - pub fn generate_metadata(&self) -> Metadata { - self.package_id().generate_metadata() - } - pub fn find_closest_target(&self, target: &str, kind: TargetKind) -> Option<&Target> { let targets = self.targets(); diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 8f1992a733e..d9224bae180 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -9,7 +9,7 @@ use regex::Regex; use rustc_serialize::{Encodable, Encoder, Decodable, Decoder}; use semver; -use util::{CargoResult, CargoError, short_hash, ToSemver}; +use util::{CargoResult, CargoError, ToSemver}; use core::source::SourceId; /// Identifier for a specific version of a package in a specific source. @@ -118,12 +118,6 @@ impl From for Box { fn from(t: PackageIdError) -> Box { Box::new(t) } } -#[derive(PartialEq, Eq, Hash, Clone, RustcEncodable, Debug)] -pub struct Metadata { - pub metadata: String, - pub extra_filename: String -} - impl PackageId { pub fn new(name: &str, version: T, sid: &SourceId) -> CargoResult { @@ -141,13 +135,6 @@ impl PackageId { pub fn version(&self) -> &semver::Version { &self.inner.version } pub fn source_id(&self) -> &SourceId { &self.inner.source_id } - pub fn generate_metadata(&self) -> Metadata { - let metadata = short_hash(self); - let extra_filename = format!("-{}", metadata); - - Metadata { metadata: metadata, extra_filename: extra_filename } - } - pub fn with_precise(&self, precise: Option) -> PackageId { PackageId { inner: Arc::new(PackageIdInner { @@ -169,14 +156,6 @@ impl PackageId { } } -impl Metadata { - pub fn mix(&mut self, t: &T) { - let new_metadata = short_hash(&(&self.metadata, t)); - self.extra_filename = format!("-{}", new_metadata); - self.metadata = new_metadata; - } -} - impl fmt::Display for PackageId { fn fmt(&self, f: &mut Formatter) -> fmt::Result { write!(f, "{} v{}", self.inner.name, self.inner.version)?; diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 99b45f7c20a..08f9b78c1d5 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -73,9 +73,8 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> { cx.probe_target_info(&units)?; for unit in units.iter() { - let layout = cx.layout(unit); - rm_rf(&layout.proxy().fingerprint(&unit.pkg))?; - rm_rf(&layout.build(&unit.pkg))?; + rm_rf(&cx.layout(unit).proxy().fingerprint(&unit.pkg))?; + rm_rf(&cx.layout(unit).build(&unit.pkg))?; for (src, link_dst, _) in cx.target_filenames(&unit)? { rm_rf(&src)?; diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index d95b7ee75c6..0d4dc8aef03 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -1,12 +1,15 @@ +#![allow(deprecated)] + use std::collections::{HashSet, HashMap, BTreeSet}; use std::env; +use std::fmt; +use std::hash::{Hasher, Hash, SipHasher}; use std::path::{Path, PathBuf}; use std::str::{self, FromStr}; use std::sync::Arc; - use core::{Package, PackageId, PackageSet, Resolve, Target, Profile}; -use core::{TargetKind, Profiles, Metadata, Dependency, Workspace}; +use core::{TargetKind, Profiles, Dependency, Workspace}; use core::dependency::Kind as DepKind; use util::{CargoResult, ChainError, internal, Config, profile, Cfg, human}; @@ -53,6 +56,9 @@ struct TargetInfo { cfg: Option>, } +#[derive(Clone)] +pub struct Metadata(u64); + impl<'a, 'cfg> Context<'a, 'cfg> { pub fn new(ws: &Workspace<'cfg>, resolve: &'a Resolve, @@ -311,7 +317,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// 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. - pub fn target_metadata(&self, unit: &Unit) -> Option { + pub fn target_metadata(&mut self, unit: &Unit) -> Option { // No metadata for dylibs because of a couple issues // - OSX encodes the dylib name in the executable // - Windows rustc multiple files of which we can't easily link all of them @@ -336,56 +342,41 @@ impl<'a, 'cfg> Context<'a, 'cfg> { return None; } - let metadata = unit.target.metadata().cloned().map(|mut m| { - if let Some(features) = self.resolve.features(unit.pkg.package_id()) { - let mut feat_vec: Vec<&String> = features.iter().collect(); - feat_vec.sort(); - for feat in feat_vec { - m.mix(feat); - } - } - m.mix(unit.profile); - m - }); - let mut pkg_metadata = { - let mut m = unit.pkg.generate_metadata(); - if let Some(features) = self.resolve.features(unit.pkg.package_id()) { + let mut hasher = SipHasher::new_with_keys(0, 0); + + // Unique metadata per (name, source, version) triple. This'll allow us + // to pull crates from anywhere w/o worrying about conflicts + unit.pkg.package_id().hash(&mut hasher); + + // Also mix in enabled features to our metadata. This'll ensure that + // when changing feature sets each lib is separately cached. + match self.resolve.features(unit.pkg.package_id()) { + Some(features) => { let mut feat_vec: Vec<&String> = features.iter().collect(); feat_vec.sort(); - for feat in feat_vec { - m.mix(feat); - } + feat_vec.hash(&mut hasher); } - m.mix(unit.profile); - m - }; - - if unit.target.is_lib() && unit.profile.test { - // Libs and their tests are built in parallel, so we need to make - // sure that their metadata is different. - metadata.map(|mut m| { - m.mix(&"test"); - m - }) - } else if unit.target.is_bin() && unit.profile.test { - // Make sure that the name of this test executable doesn't - // conflict with a library that has the same name and is - // being tested - pkg_metadata.mix(&format!("bin-{}", unit.target.name())); - Some(pkg_metadata) - } else if unit.pkg.package_id().source_id().is_path() && - !unit.profile.test { - Some(pkg_metadata) - } else { - metadata + None => Vec::<&String>::new().hash(&mut hasher), } + + // Throw in the profile we're compiling with. This helps caching + // panic=abort and panic=unwind artifacts, additionally with various + // settings like debuginfo and whatnot. + unit.profile.hash(&mut hasher); + + // Finally throw in the target name/kind. This ensures that concurrent + // compiles of targets in the same crate don't collide. + unit.target.name().hash(&mut hasher); + unit.target.kind().hash(&mut hasher); + + Some(Metadata(hasher.finish())) } /// Returns the file stem for a given target/profile combo (with metadata) - pub fn file_stem(&self, unit: &Unit) -> String { + pub fn file_stem(&mut self, unit: &Unit) -> String { match self.target_metadata(unit) { - Some(ref metadata) => format!("{}{}", unit.target.crate_name(), - metadata.extra_filename), + Some(ref metadata) => format!("{}-{}", unit.target.crate_name(), + metadata), None => self.bin_stem(unit), } } @@ -407,7 +398,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// Returns an Option because in some cases we don't want to link /// (eg a dependent lib) - pub fn link_stem(&self, unit: &Unit) -> Option<(PathBuf, String)> { + pub fn link_stem(&mut self, unit: &Unit) -> Option<(PathBuf, String)> { let src_dir = self.out_dir(unit); let bin_stem = self.bin_stem(unit); let file_stem = self.file_stem(unit); @@ -441,7 +432,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// filename: filename rustc compiles to. (Often has metadata suffix). /// link_dst: Optional file to link/copy the result to (without metadata suffix) /// linkable: Whether possible to link against file (eg it's a library) - pub fn target_filenames(&self, unit: &Unit) + pub fn target_filenames(&mut self, unit: &Unit) -> CargoResult, bool)>> { let out_dir = self.out_dir(unit); let stem = self.file_stem(unit); @@ -870,3 +861,9 @@ fn env_args(config: &Config, Ok(Vec::new()) } + +impl fmt::Display for Metadata { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{:016x}", self.0) + } +} diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 6cc8f88c105..952ccac7830 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -531,7 +531,7 @@ pub fn dir(cx: &Context, unit: &Unit) -> PathBuf { } /// Returns the (old, new) location for the dep info file of a target. -pub fn dep_info_loc(cx: &Context, unit: &Unit) -> PathBuf { +pub fn dep_info_loc(cx: &mut Context, unit: &Unit) -> PathBuf { dir(cx, unit).join(&format!("dep-{}", filename(cx, unit))) } @@ -653,7 +653,7 @@ fn mtime_if_fresh(output: &Path, paths: I) -> Option } } -fn filename(cx: &Context, unit: &Unit) -> String { +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 diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 02e1088303b..73f3fed96a5 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -421,7 +421,7 @@ fn add_plugin_deps(rustc: &mut ProcessBuilder, Ok(()) } -fn prepare_rustc(cx: &Context, +fn prepare_rustc(cx: &mut Context, crate_types: Vec<&str>, unit: &Unit) -> CargoResult { let mut base = cx.compilation.rustc_process(unit.pkg)?; @@ -509,7 +509,7 @@ fn root_path(cx: &Context, unit: &Unit) -> PathBuf { } } -fn build_base_args(cx: &Context, +fn build_base_args(cx: &mut Context, cmd: &mut ProcessBuilder, unit: &Unit, crate_types: &[&str]) { @@ -610,8 +610,8 @@ fn build_base_args(cx: &Context, match cx.target_metadata(unit) { Some(m) => { - cmd.arg("-C").arg(&format!("metadata={}", m.metadata)); - cmd.arg("-C").arg(&format!("extra-filename={}", m.extra_filename)); + cmd.arg("-C").arg(&format!("metadata={}", m)); + cmd.arg("-C").arg(&format!("extra-filename=-{}", m)); } None => { cmd.arg("-C").arg(&format!("metadata={}", short_hash(unit.pkg))); @@ -645,17 +645,16 @@ fn build_plugin_args(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) { opt(cmd, "-C", "linker=", cx.linker(unit.kind).map(|s| s.as_ref())); } -fn build_deps_args(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) +fn build_deps_args(cmd: &mut ProcessBuilder, cx: &mut Context, unit: &Unit) -> CargoResult<()> { - let layout = cx.layout(unit); cmd.arg("-L").arg(&{ let mut deps = OsString::from("dependency="); - deps.push(layout.deps()); + deps.push(cx.layout(unit).deps()); deps }); if unit.pkg.has_custom_build() { - cmd.env("OUT_DIR", &layout.build_out(unit.pkg)); + cmd.env("OUT_DIR", &cx.layout(unit).build_out(unit.pkg)); } for unit in cx.dep_targets(unit)?.iter() { @@ -666,7 +665,7 @@ fn build_deps_args(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) return Ok(()); - fn link_to(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) + fn link_to(cmd: &mut ProcessBuilder, cx: &mut Context, unit: &Unit) -> CargoResult<()> { for (dst, _link_dst, linkable) in cx.target_filenames(unit)? { if !linkable { diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index c5d70b7098b..5e5dc93a669 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -14,7 +14,6 @@ use core::{Summary, Manifest, Target, Dependency, DependencyInner, PackageId}; use core::{EitherManifest, VirtualManifest}; use core::dependency::{Kind, Platform}; use core::manifest::{LibKind, Profile, ManifestMetadata}; -use core::package_id::Metadata; use sources::CRATES_IO; use util::{self, CargoResult, human, ToUrl, ToSemver, ChainError, Config}; @@ -437,7 +436,6 @@ impl TomlManifest { } let pkgid = project.to_package_id(source_id)?; - let metadata = pkgid.generate_metadata(); // If we have no lib at all, use the inferred lib if available // If we have a lib with a path, we're done @@ -550,8 +548,7 @@ impl TomlManifest { new_build, &examples, &tests, - &benches, - &metadata); + &benches); if targets.is_empty() { debug!("manifest has no build targets"); @@ -1064,8 +1061,7 @@ fn normalize(lib: &Option, custom_build: Option, examples: &[TomlExampleTarget], tests: &[TomlTestTarget], - benches: &[TomlBenchTarget], - metadata: &Metadata) -> Vec { + benches: &[TomlBenchTarget]) -> Vec { fn configure(toml: &TomlTarget, target: &mut Target) { let t2 = target.clone(); target.set_tested(toml.test.unwrap_or(t2.tested())) @@ -1080,9 +1076,7 @@ fn normalize(lib: &Option, }); } - fn lib_target(dst: &mut Vec, - l: &TomlLibTarget, - metadata: &Metadata) { + fn lib_target(dst: &mut Vec, l: &TomlLibTarget) { let path = l.path.clone().unwrap_or( PathValue::Path(Path::new("src").join(&format!("{}.rs", l.name()))) ); @@ -1095,14 +1089,8 @@ fn normalize(lib: &Option, } }; - // Binaries, examples, etc, may link to this library. Their crate names - // have a high likelihood to being the same as ours, however, so we need - // some extra metadata in our name to ensure symbols won't collide. - let mut metadata = metadata.clone(); - metadata.mix(&"lib"); let mut target = Target::lib_target(&l.name(), crate_types, - &path.to_path(), - metadata); + &path.to_path()); configure(l, &mut target); dst.push(target); } @@ -1113,8 +1101,7 @@ fn normalize(lib: &Option, let path = bin.path.clone().unwrap_or_else(|| { PathValue::Path(default(bin)) }); - let mut target = Target::bin_target(&bin.name(), &path.to_path(), - None); + let mut target = Target::bin_target(&bin.name(), &path.to_path()); configure(bin, &mut target); dst.push(target); } @@ -1124,7 +1111,7 @@ fn normalize(lib: &Option, let name = format!("build-script-{}", cmd.file_stem().and_then(|s| s.to_str()).unwrap_or("")); - dst.push(Target::custom_build_target(&name, cmd, None)); + dst.push(Target::custom_build_target(&name, cmd)); } fn example_targets(dst: &mut Vec, @@ -1141,40 +1128,29 @@ fn normalize(lib: &Option, } } - fn test_targets(dst: &mut Vec, tests: &[TomlTestTarget], - metadata: &Metadata, + fn test_targets(dst: &mut Vec, + tests: &[TomlTestTarget], default: &mut FnMut(&TomlTestTarget) -> PathBuf) { for test in tests.iter() { let path = test.path.clone().unwrap_or_else(|| { PathValue::Path(default(test)) }); - // make sure this metadata is different from any same-named libs. - let mut metadata = metadata.clone(); - metadata.mix(&format!("test-{}", test.name())); - - let mut target = Target::test_target(&test.name(), &path.to_path(), - metadata); + let mut target = Target::test_target(&test.name(), &path.to_path()); configure(test, &mut target); dst.push(target); } } - fn bench_targets(dst: &mut Vec, benches: &[TomlBenchTarget], - metadata: &Metadata, + fn bench_targets(dst: &mut Vec, + benches: &[TomlBenchTarget], default: &mut FnMut(&TomlBenchTarget) -> PathBuf) { for bench in benches.iter() { let path = bench.path.clone().unwrap_or_else(|| { PathValue::Path(default(bench)) }); - // make sure this metadata is different from any same-named libs. - let mut metadata = metadata.clone(); - metadata.mix(&format!("bench-{}", bench.name())); - - let mut target = Target::bench_target(&bench.name(), - &path.to_path(), - metadata); + let mut target = Target::bench_target(&bench.name(), &path.to_path()); configure(bench, &mut target); dst.push(target); } @@ -1183,7 +1159,7 @@ fn normalize(lib: &Option, let mut ret = Vec::new(); if let Some(ref lib) = *lib { - lib_target(&mut ret, lib, metadata); + lib_target(&mut ret, lib); bin_targets(&mut ret, bins, &mut |bin| Path::new("src").join("bin") .join(&format!("{}.rs", bin.name()))); @@ -1201,7 +1177,7 @@ fn normalize(lib: &Option, &mut |ex| Path::new("examples") .join(&format!("{}.rs", ex.name()))); - test_targets(&mut ret, tests, metadata, &mut |test| { + test_targets(&mut ret, tests, &mut |test| { if test.name() == "test" { Path::new("src").join("test.rs") } else { @@ -1209,7 +1185,7 @@ fn normalize(lib: &Option, } }); - bench_targets(&mut ret, benches, metadata, &mut |bench| { + bench_targets(&mut ret, benches, &mut |bench| { if bench.name() == "bench" { Path::new("src").join("bench.rs") } else { From 41579bada5c7091e2be1ece45d7f7e02e62fd6b3 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 21 Nov 2016 12:32:10 -0800 Subject: [PATCH 2/2] Apply new fingerprinting to build dir outputs We now much more aggressively cache the output of the compiler based on feature sets and profile configuration. Unfortunately we forgot to extend this caching to build script output directories as well so this commit ensures that build script outputs are cached the same way with a directory per configuration of features and output settings. Closes #3302 --- src/cargo/ops/cargo_clean.rs | 13 +++-- src/cargo/ops/cargo_rustc/context.rs | 62 ++++++++++++++++----- src/cargo/ops/cargo_rustc/custom_build.rs | 22 ++++---- src/cargo/ops/cargo_rustc/fingerprint.rs | 15 ++---- src/cargo/ops/cargo_rustc/layout.rs | 65 ++--------------------- src/cargo/ops/cargo_rustc/mod.rs | 32 +++++------ src/cargo/ops/mod.rs | 4 +- tests/build-script.rs | 46 ++++++++++++++++ 8 files changed, 140 insertions(+), 119 deletions(-) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 08f9b78c1d5..fd270b79aba 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -73,10 +73,17 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> { cx.probe_target_info(&units)?; for unit in units.iter() { - rm_rf(&cx.layout(unit).proxy().fingerprint(&unit.pkg))?; - rm_rf(&cx.layout(unit).build(&unit.pkg))?; + rm_rf(&cx.fingerprint_dir(unit))?; + if unit.target.is_custom_build() { + if unit.profile.run_custom_build { + rm_rf(&cx.build_script_out_dir(unit))?; + } else { + rm_rf(&cx.build_script_dir(unit))?; + } + continue + } - for (src, link_dst, _) in cx.target_filenames(&unit)? { + for (src, link_dst, _) in cx.target_filenames(unit)? { rm_rf(&src)?; if let Some(dst) = link_dst { rm_rf(&dst)?; diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 0d4dc8aef03..c0a92d643ec 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -11,12 +11,12 @@ use std::sync::Arc; use core::{Package, PackageId, PackageSet, Resolve, Target, Profile}; use core::{TargetKind, Profiles, Dependency, Workspace}; use core::dependency::Kind as DepKind; -use util::{CargoResult, ChainError, internal, Config, profile, Cfg, human}; +use util::{self, CargoResult, ChainError, internal, Config, profile, Cfg, human}; use super::TargetConfig; use super::custom_build::{BuildState, BuildScripts}; use super::fingerprint::Fingerprint; -use super::layout::{Layout, LayoutProxy}; +use super::layout::Layout; use super::links::Links; use super::{Kind, Compilation, BuildConfig}; @@ -278,23 +278,61 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } /// Returns the appropriate directory layout for either a plugin or not. - pub fn layout(&self, unit: &Unit) -> LayoutProxy { - let primary = unit.pkg.package_id() == &self.current_package; - match unit.kind { - Kind::Host => LayoutProxy::new(&self.host, primary), - Kind::Target => LayoutProxy::new(self.target.as_ref() - .unwrap_or(&self.host), - primary), + fn layout(&self, kind: Kind) -> &Layout { + match kind { + Kind::Host => &self.host, + Kind::Target => self.target.as_ref().unwrap_or(&self.host) } } + /// Returns the directories where Rust crate dependencies are found for the + /// specified unit. + pub fn deps_dir(&self, unit: &Unit) -> &Path { + self.layout(unit.kind).deps() + } + + /// Returns the directory for the specified unit where fingerprint + /// information is stored. + pub fn fingerprint_dir(&mut self, unit: &Unit) -> PathBuf { + let dir = self.pkg_dir(unit); + self.layout(unit.kind).fingerprint().join(dir) + } + + /// Returns the appropriate directory layout for either a plugin or not. + pub fn build_script_dir(&mut self, unit: &Unit) -> PathBuf { + assert!(unit.target.is_custom_build()); + assert!(!unit.profile.run_custom_build); + let dir = self.pkg_dir(unit); + self.layout(Kind::Host).build().join(dir) + } + + /// Returns the appropriate directory layout for either a plugin or not. + pub fn build_script_out_dir(&mut self, unit: &Unit) -> PathBuf { + assert!(unit.target.is_custom_build()); + assert!(unit.profile.run_custom_build); + let dir = self.pkg_dir(unit); + self.layout(unit.kind).build().join(dir).join("out") + } + /// Returns the appropriate output directory for the specified package and /// target. - pub fn out_dir(&self, unit: &Unit) -> PathBuf { + pub fn out_dir(&mut self, unit: &Unit) -> PathBuf { if unit.profile.doc { - self.layout(unit).doc_root() + self.layout(unit.kind).root().parent().unwrap().join("doc") + } else if unit.target.is_custom_build() { + self.build_script_dir(unit) + } else if unit.target.is_example() { + self.layout(unit.kind).examples().to_path_buf() } else { - self.layout(unit).out_dir(unit) + self.deps_dir(unit).to_path_buf() + } + } + + fn pkg_dir(&mut self, unit: &Unit) -> String { + let name = unit.pkg.package_id().name(); + match self.target_metadata(unit) { + Some(meta) => format!("{}-{}", name, meta), + None => format!("{}-{}", name, util::short_hash(unit.pkg)), } } diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 375a6ea434c..eb62f38c522 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -82,11 +82,12 @@ pub fn prepare<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult<(Work, Work)> { - let host_unit = Unit { kind: Kind::Host, ..*unit }; - let (script_output, build_output) = { - (cx.layout(&host_unit).build(unit.pkg), - cx.layout(unit).build_out(unit.pkg)) - }; + let dependencies = cx.dep_run_custom_build(unit)?; + let build_script_unit = dependencies.iter().find(|d| { + !d.profile.run_custom_build && d.target.is_custom_build() + }).expect("running a script not depending on an actual script"); + let script_output = cx.build_script_dir(build_script_unit); + let build_output = cx.build_script_out_dir(unit); // Building the command to execute let to_exec = script_output.join(unit.target.name()); @@ -150,7 +151,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // This information will be used at build-time later on to figure out which // sorts of variables need to be discovered at that time. let lib_deps = { - cx.dep_run_custom_build(unit)?.iter().filter_map(|unit| { + dependencies.iter().filter_map(|unit| { if unit.profile.run_custom_build { Some((unit.pkg.manifest().links().unwrap().to_string(), unit.pkg.package_id().clone())) @@ -177,8 +178,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) }; cx.build_explicit_deps.insert(*unit, (output_file.clone(), rerun_if_changed)); - fs::create_dir_all(&cx.layout(&host_unit).build(unit.pkg))?; - fs::create_dir_all(&cx.layout(unit).build(unit.pkg))?; + fs::create_dir_all(&script_output)?; + fs::create_dir_all(&build_output)?; // Prepare the unit of "dirty work" which will actually run the custom build // command. @@ -336,9 +337,8 @@ impl BuildOutput { match key { "rustc-flags" => { - let (libs, links) = - BuildOutput::parse_rustc_flags(value, &whence) - ?; + let (libs, links) = + BuildOutput::parse_rustc_flags(value, &whence)?; library_links.extend(links.into_iter()); library_paths.extend(libs.into_iter()); } diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 952ccac7830..3d1f274498f 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -48,7 +48,7 @@ pub fn prepare_target<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult { let _p = profile::start(format!("fingerprint: {} / {}", unit.pkg.package_id(), unit.target.name())); - let new = dir(cx, unit); + let new = cx.fingerprint_dir(unit); let loc = new.join(&filename(cx, unit)); debug!("fingerprint at: {}", loc.display()); @@ -426,7 +426,7 @@ pub fn prepare_build_cmd<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult { let _p = profile::start(format!("fingerprint build cmd: {}", unit.pkg.package_id())); - let new = dir(cx, unit); + let new = cx.fingerprint_dir(unit); let loc = new.join("build"); debug!("fingerprint at: {}", loc.display()); @@ -507,13 +507,13 @@ fn write_fingerprint(loc: &Path, fingerprint: &Fingerprint) -> CargoResult<()> { debug!("write fingerprint: {}", loc.display()); paths::write(&loc, util::to_hex(hash).as_bytes())?; paths::write(&loc.with_extension("json"), - json::encode(&fingerprint).unwrap().as_bytes())?; + json::encode(&fingerprint).unwrap().as_bytes())?; Ok(()) } /// Prepare work for when a package starts to build pub fn prepare_init(cx: &mut Context, unit: &Unit) -> CargoResult<()> { - let new1 = dir(cx, unit); + let new1 = cx.fingerprint_dir(unit); let new2 = new1.clone(); if fs::metadata(&new1).is_err() { @@ -525,14 +525,9 @@ pub fn prepare_init(cx: &mut Context, unit: &Unit) -> CargoResult<()> { Ok(()) } -/// Return the (old, new) location for fingerprints for a package -pub fn dir(cx: &Context, unit: &Unit) -> PathBuf { - cx.layout(unit).proxy().fingerprint(unit.pkg) -} - /// Returns the (old, new) location for the dep info file of a target. pub fn dep_info_loc(cx: &mut Context, unit: &Unit) -> PathBuf { - dir(cx, unit).join(&format!("dep-{}", filename(cx, unit))) + cx.fingerprint_dir(unit).join(&format!("dep-{}", filename(cx, unit))) } fn compare_old_fingerprint(loc: &Path, new_fingerprint: &Fingerprint) diff --git a/src/cargo/ops/cargo_rustc/layout.rs b/src/cargo/ops/cargo_rustc/layout.rs index cf0cec2df02..2b5cd7514d8 100644 --- a/src/cargo/ops/cargo_rustc/layout.rs +++ b/src/cargo/ops/cargo_rustc/layout.rs @@ -49,10 +49,8 @@ use std::fs; use std::io; use std::path::{PathBuf, Path}; -use core::{Package, Workspace}; +use core::Workspace; use util::{Config, FileLock, CargoResult, Filesystem, human}; -use util::hex::short_hash; -use super::Unit; pub struct Layout { root: PathBuf, @@ -64,11 +62,6 @@ pub struct Layout { _lock: FileLock, } -pub struct LayoutProxy<'a> { - root: &'a Layout, - primary: bool, -} - impl Layout { pub fn new(ws: &Workspace, triple: Option<&str>, @@ -127,58 +120,6 @@ impl Layout { pub fn deps(&self) -> &Path { &self.deps } pub fn examples(&self) -> &Path { &self.examples } pub fn root(&self) -> &Path { &self.root } - - pub fn fingerprint(&self, package: &Package) -> PathBuf { - self.fingerprint.join(&self.pkg_dir(package)) - } - - pub fn build(&self, package: &Package) -> PathBuf { - self.build.join(&self.pkg_dir(package)) - } - - pub fn build_out(&self, package: &Package) -> PathBuf { - self.build(package).join("out") - } - - fn pkg_dir(&self, pkg: &Package) -> String { - format!("{}-{}", pkg.name(), short_hash(pkg)) - } -} - -impl<'a> LayoutProxy<'a> { - pub fn new(root: &'a Layout, primary: bool) -> LayoutProxy<'a> { - LayoutProxy { - root: root, - primary: primary, - } - } - - pub fn root(&self) -> &'a Path { - if self.primary {self.root.dest()} else {self.root.deps()} - } - pub fn deps(&self) -> &'a Path { self.root.deps() } - - pub fn examples(&self) -> &'a Path { self.root.examples() } - - pub fn build(&self, pkg: &Package) -> PathBuf { self.root.build(pkg) } - - pub fn build_out(&self, pkg: &Package) -> PathBuf { self.root.build_out(pkg) } - - pub fn proxy(&self) -> &'a Layout { self.root } - - pub fn out_dir(&self, unit: &Unit) -> PathBuf { - if unit.target.is_custom_build() { - self.build(unit.pkg) - } else if unit.target.is_example() { - self.examples().to_path_buf() - } else { - self.deps().to_path_buf() - } - } - - pub fn doc_root(&self) -> PathBuf { - // the "root" directory ends in 'debug' or 'release', and we want it to - // end in 'doc' instead - self.root.root().parent().unwrap().join("doc") - } + pub fn fingerprint(&self) -> &Path { &self.fingerprint } + pub fn build(&self) -> &Path { &self.build } } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 73f3fed96a5..c9d3198d2af 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -18,7 +18,6 @@ use self::job_queue::JobQueue; pub use self::compilation::Compilation; pub use self::context::{Context, Unit}; -pub use self::layout::{Layout, LayoutProxy}; pub use self::custom_build::{BuildOutput, BuildMap, BuildScripts}; mod compilation; @@ -104,12 +103,6 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>, queue.execute(&mut cx)?; for unit in units.iter() { - let out_dir = cx.layout(unit).build_out(unit.pkg) - .display().to_string(); - cx.compilation.extra_env.entry(unit.pkg.package_id().clone()) - .or_insert(Vec::new()) - .push(("OUT_DIR".to_string(), out_dir)); - for (dst, link_dst, _linkable) in cx.target_filenames(unit)? { let bindst = match link_dst { Some(link_dst) => link_dst, @@ -131,6 +124,13 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>, // Include immediate lib deps as well for unit in cx.dep_targets(unit)?.iter() { + if unit.profile.run_custom_build { + let out_dir = cx.build_script_out_dir(unit).display().to_string(); + cx.compilation.extra_env.entry(unit.pkg.package_id().clone()) + .or_insert(Vec::new()) + .push(("OUT_DIR".to_string(), out_dir)); + } + let pkgid = unit.pkg.package_id(); if !unit.target.is_lib() { continue } if unit.profile.doc { continue } @@ -183,8 +183,7 @@ fn compile<'a, 'cfg: 'a>(cx: &mut Context<'a, 'cfg>, let (dirty, fresh, freshness) = if unit.profile.run_custom_build { custom_build::prepare(cx, unit)? } else { - let (freshness, dirty, fresh) = fingerprint::prepare_target(cx, - unit)?; + let (freshness, dirty, fresh) = fingerprint::prepare_target(cx, unit)?; let work = if unit.profile.doc { rustdoc(cx, unit)? } else { @@ -465,10 +464,6 @@ fn rustdoc(cx: &mut Context, unit: &Unit) -> CargoResult { build_deps_args(&mut rustdoc, cx, unit)?; - if unit.pkg.has_custom_build() { - rustdoc.env("OUT_DIR", &cx.layout(unit).build_out(unit.pkg)); - } - rustdoc.args(&cx.rustdocflags_args(unit)?); let name = unit.pkg.name().to_string(); @@ -624,7 +619,7 @@ fn build_base_args(cx: &mut Context, } -fn build_plugin_args(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) { +fn build_plugin_args(cmd: &mut ProcessBuilder, cx: &mut Context, unit: &Unit) { fn opt(cmd: &mut ProcessBuilder, key: &str, prefix: &str, val: Option<&OsStr>) { if let Some(val) = val { @@ -649,15 +644,14 @@ fn build_deps_args(cmd: &mut ProcessBuilder, cx: &mut Context, unit: &Unit) -> CargoResult<()> { cmd.arg("-L").arg(&{ let mut deps = OsString::from("dependency="); - deps.push(cx.layout(unit).deps()); + deps.push(cx.deps_dir(unit)); deps }); - if unit.pkg.has_custom_build() { - cmd.env("OUT_DIR", &cx.layout(unit).build_out(unit.pkg)); - } - for unit in cx.dep_targets(unit)?.iter() { + if unit.profile.run_custom_build { + cmd.env("OUT_DIR", &cx.build_script_out_dir(unit)); + } if unit.target.linkable() && !unit.profile.doc { link_to(cmd, cx, unit)?; } diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index ff4583c569a..902c72e3007 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -2,8 +2,8 @@ pub use self::cargo_clean::{clean, CleanOptions}; pub use self::cargo_compile::{compile, compile_ws, resolve_dependencies, CompileOptions}; pub use self::cargo_compile::{CompileFilter, CompileMode, MessageFormat}; pub use self::cargo_read_manifest::{read_manifest,read_package,read_packages}; -pub use self::cargo_rustc::{compile_targets, Compilation, Layout, Kind, Unit}; -pub use self::cargo_rustc::{Context, LayoutProxy}; +pub use self::cargo_rustc::{compile_targets, Compilation, Kind, Unit}; +pub use self::cargo_rustc::Context; pub use self::cargo_rustc::{BuildOutput, BuildConfig, TargetConfig}; pub use self::cargo_run::run; pub use self::cargo_install::{install, install_list, uninstall}; diff --git a/tests/build-script.rs b/tests/build-script.rs index e6bcef0fbe9..dfccd32201f 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -2288,3 +2288,49 @@ fn cfg_env_vars_available() { assert_that(build.cargo_process("bench"), execs().with_status(0)); } + +#[test] +fn switch_features_rerun() { + let build = project("builder") + .file("Cargo.toml", r#" + [package] + name = "builder" + version = "0.0.1" + authors = [] + build = "build.rs" + + [features] + foo = [] + "#) + .file("src/main.rs", r#" + fn main() { + println!(include_str!(concat!(env!("OUT_DIR"), "/output"))); + } + "#) + .file("build.rs", r#" + use std::env; + use std::fs::File; + use std::io::Write; + use std::path::Path; + + fn main() { + let out_dir = env::var_os("OUT_DIR").unwrap(); + let out_dir = Path::new(&out_dir).join("output"); + let mut f = File::create(&out_dir).unwrap(); + + if env::var_os("CARGO_FEATURE_FOO").is_some() { + f.write_all(b"foo").unwrap(); + } else { + f.write_all(b"bar").unwrap(); + } + } + "#); + build.build(); + + assert_that(build.cargo("run").arg("-v").arg("--features=foo"), + execs().with_status(0).with_stdout("foo\n")); + assert_that(build.cargo("run").arg("-v"), + execs().with_status(0).with_stdout("bar\n")); + assert_that(build.cargo("run").arg("-v").arg("--features=foo"), + execs().with_status(0).with_stdout("foo\n")); +}