Skip to content

Commit

Permalink
Refactor metadata generation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alexcrichton committed Nov 28, 2016
1 parent 3568be9 commit 0cc39aa
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 148 deletions.
27 changes: 7 additions & 20 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -159,7 +158,6 @@ pub struct Target {
kind: TargetKind,
name: String,
src_path: PathBuf,
metadata: Option<Metadata>,
tested: bool,
benched: bool,
doc: bool,
Expand Down Expand Up @@ -279,7 +277,6 @@ impl Target {
kind: TargetKind::Bin,
name: String::new(),
src_path: PathBuf::new(),
metadata: None,
doc: false,
doctest: false,
harness: true,
Expand All @@ -289,40 +286,35 @@ impl Target {
}
}

pub fn lib_target(name: &str, crate_targets: Vec<LibKind>,
src_path: &Path,
metadata: Metadata) -> Target {
pub fn lib_target(name: &str,
crate_targets: Vec<LibKind>,
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<Metadata>) -> 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<Metadata>) -> 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,
Expand All @@ -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()
}
Expand All @@ -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 }
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 1 addition & 5 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();

Expand Down
23 changes: 1 addition & 22 deletions src/cargo/core/package_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -118,12 +118,6 @@ impl From<PackageIdError> for Box<CargoError> {
fn from(t: PackageIdError) -> Box<CargoError> { 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<T: ToSemver>(name: &str, version: T,
sid: &SourceId) -> CargoResult<PackageId> {
Expand All @@ -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<String>) -> PackageId {
PackageId {
inner: Arc::new(PackageIdInner {
Expand All @@ -169,14 +156,6 @@ impl PackageId {
}
}

impl Metadata {
pub fn mix<T: Hash>(&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)?;
Expand Down
5 changes: 2 additions & 3 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
91 changes: 44 additions & 47 deletions src/cargo/ops/cargo_rustc/context.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -53,6 +56,9 @@ struct TargetInfo {
cfg: Option<Vec<Cfg>>,
}

#[derive(Clone)]
pub struct Metadata(u64);

impl<'a, 'cfg> Context<'a, 'cfg> {
pub fn new(ws: &Workspace<'cfg>,
resolve: &'a Resolve,
Expand Down Expand Up @@ -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<Metadata> {
pub fn target_metadata(&mut self, unit: &Unit) -> Option<Metadata> {
// 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
Expand All @@ -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),
}
}
Expand All @@ -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);
Expand Down Expand Up @@ -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<Vec<(PathBuf, Option<PathBuf>, bool)>> {
let out_dir = self.out_dir(unit);
let stem = self.file_stem(unit);
Expand Down Expand Up @@ -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)
}
}
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_rustc/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}

Expand Down Expand Up @@ -653,7 +653,7 @@ fn mtime_if_fresh<I>(output: &Path, paths: I) -> Option<FileTime>
}
}

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
Expand Down
Loading

0 comments on commit 0cc39aa

Please sign in to comment.