Skip to content

Commit

Permalink
New metabuild strategy using custom src_path enum.
Browse files Browse the repository at this point in the history
- Use new enum `TargertSourcePath` for Target::src_path to make it explicit that metabuild has a special path.
- `cargo metadata` now skips the metabuild Target.
- JSON artifacts include the true path to the metabuild source file. This may not be the best solution, but it's unclear what it should be, and I would prefer to avoid breaking the output. Alternatively it could just not emit anything? I'm not completely familiar with the use case of these artifact messages.
- Place the file in `target/.metabuild/metabuild-pkgname-HASH.rs` instead of in the debug/release directory.  Its contents do not depend on the profile.
- Fix bug in write_if_changed.
- More tests.
  • Loading branch information
ehuss committed Aug 24, 2018
1 parent e685bbf commit ecc87b1
Show file tree
Hide file tree
Showing 13 changed files with 463 additions and 140 deletions.
9 changes: 0 additions & 9 deletions src/cargo/core/compiler/context/compilation_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,6 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
self.layout(unit.kind).build().join(dir).join("out")
}

pub fn metabuild_path(&self, unit: &Unit<'a>) -> PathBuf {
let metadata = self.metadata(unit).expect("metabuild metadata");
self.layout(unit.kind).metabuild().join(format!(
"metabuild-{}-{}.rs",
unit.pkg.name(),
metadata
))
}

/// Returns the file stem for a given target/profile combo (with metadata)
pub fn file_stem(&self, unit: &Unit<'a>) -> String {
match self.metas[unit] {
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,8 @@ fn prepare_metabuild<'a, 'cfg>(
}
output.push("}\n".to_string());
let output = output.join("");
let path = cx.files().metabuild_path(unit);
let path = unit.pkg.manifest().metabuild_path(cx.bcx.ws.target_dir());
fs::create_dir_all(path.parent().unwrap())?;
paths::write_if_changed(path, &output)?;
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ fn calculate<'a, 'cfg>(
profile: profile_hash,
// Note that .0 is hashed here, not .1 which is the cwd. That doesn't
// actually affect the output artifact so there's no need to hash it.
path: util::hash_u64(&super::path_args(cx, unit).0),
path: util::hash_u64(&super::path_args(&cx.bcx, unit).0),
features: format!("{:?}", bcx.resolve.features_sorted(unit.pkg.package_id())),
deps,
local: vec![local],
Expand Down
7 changes: 0 additions & 7 deletions src/cargo/core/compiler/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ pub struct Layout {
deps: PathBuf,
native: PathBuf,
build: PathBuf,
metabuild: PathBuf,
incremental: PathBuf,
fingerprint: PathBuf,
examples: PathBuf,
Expand Down Expand Up @@ -113,7 +112,6 @@ impl Layout {
deps: root.join("deps"),
native: root.join("native"),
build: root.join("build"),
metabuild: root.join(".metabuild"),
incremental: root.join("incremental"),
fingerprint: root.join(".fingerprint"),
examples: root.join("examples"),
Expand Down Expand Up @@ -165,7 +163,6 @@ impl Layout {
mkdir(&self.fingerprint)?;
mkdir(&self.examples)?;
mkdir(&self.build)?;
mkdir(&self.metabuild)?;

return Ok(());

Expand Down Expand Up @@ -205,8 +202,4 @@ impl Layout {
pub fn build(&self) -> &Path {
&self.build
}
/// Fetch the metabuild path.
pub fn metabuild(&self) -> &Path {
&self.metabuild
}
}
25 changes: 15 additions & 10 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::sync::Arc;
use same_file::is_same_file;
use serde_json;

use core::manifest::TargetSourcePath;
use core::profiles::{Lto, Profile};
use core::shell::ColorChoice;
use core::{PackageId, Target};
Expand Down Expand Up @@ -390,7 +391,6 @@ fn link_targets<'a, 'cfg>(
let outputs = cx.outputs(unit)?;
let export_dir = cx.files().export_dir();
let package_id = unit.pkg.package_id().clone();
let target = unit.target.clone();
let profile = unit.profile;
let unit_mode = unit.mode;
let features = bcx.resolve
Expand All @@ -399,6 +399,12 @@ fn link_targets<'a, 'cfg>(
.map(|s| s.to_owned())
.collect();
let json_messages = bcx.build_config.json_messages();
let mut target = unit.target.clone();
if let TargetSourcePath::Metabuild = target.src_path() {
// Give it something to serialize.
let path = unit.pkg.manifest().metabuild_path(cx.bcx.ws.target_dir());
target.set_src_path(TargetSourcePath::Path(path));
}

Ok(Work::new(move |_| {
// If we're a "root crate", e.g. the target of this compilation, then we
Expand Down Expand Up @@ -582,7 +588,7 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
let mut rustdoc = cx.compilation.rustdoc_process(unit.pkg, unit.target)?;
rustdoc.inherit_jobserver(&cx.jobserver);
rustdoc.arg("--crate-name").arg(&unit.target.crate_name());
add_path_args(cx, unit, &mut rustdoc);
add_path_args(bcx, unit, &mut rustdoc);
add_cap_lints(bcx, unit, &mut rustdoc);
add_color(bcx, &mut rustdoc);

Expand Down Expand Up @@ -666,13 +672,12 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult
//
// The first returned value here is the argument to pass to rustc, and the
// second is the cwd that rustc should operate in.
fn path_args<'a, 'cfg>(cx: &Context<'a, 'cfg>, unit: &Unit) -> (PathBuf, PathBuf) {
let ws_root = cx.bcx.ws.root();
// TODO: this is a hack
fn path_args(bcx: &BuildContext, unit: &Unit) -> (PathBuf, PathBuf) {
let ws_root = bcx.ws.root();
let src = if unit.target.is_custom_build() && unit.pkg.manifest().metabuild().is_some() {
cx.files().metabuild_path(unit)
unit.pkg.manifest().metabuild_path(bcx.ws.target_dir())
} else {
unit.target.src_path().to_path_buf()
unit.target.src_path().path().to_path_buf()
};
assert!(src.is_absolute());
if let Ok(path) = src.strip_prefix(ws_root) {
Expand All @@ -681,8 +686,8 @@ fn path_args<'a, 'cfg>(cx: &Context<'a, 'cfg>, unit: &Unit) -> (PathBuf, PathBuf
(src, unit.pkg.root().to_path_buf())
}

fn add_path_args<'a, 'cfg>(cx: &Context<'a, 'cfg>, unit: &Unit, cmd: &mut ProcessBuilder) {
let (arg, cwd) = path_args(cx, unit);
fn add_path_args(bcx: &BuildContext, unit: &Unit, cmd: &mut ProcessBuilder) {
let (arg, cwd) = path_args(bcx, unit);
cmd.arg(arg);
cmd.cwd(cwd);
}
Expand Down Expand Up @@ -741,7 +746,7 @@ fn build_base_args<'a, 'cfg>(

cmd.arg("--crate-name").arg(&unit.target.crate_name());

add_path_args(cx, unit, cmd);
add_path_args(bcx, unit, cmd);
add_color(bcx, cmd);
add_error_format(bcx, cmd);

Expand Down
133 changes: 99 additions & 34 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![allow(deprecated)] // for SipHasher

use std::collections::{BTreeMap, HashMap};
use std::fmt;
use std::hash::{Hash, Hasher};
use std::hash::{Hash, Hasher, SipHasher};
use std::path::{Path, PathBuf};
use std::rc::Rc;

Expand All @@ -15,7 +17,7 @@ use core::{Dependency, PackageId, PackageIdSpec, SourceId, Summary};
use core::{Edition, Feature, Features, WorkspaceConfig};
use util::errors::*;
use util::toml::TomlManifest;
use util::Config;
use util::{Config, Filesystem};

pub enum EitherManifest {
Real(Manifest),
Expand Down Expand Up @@ -191,7 +193,7 @@ pub struct Target {
// as it's absolute currently and is otherwise a little too brittle for
// causing rebuilds. Instead the hash for the path that we send to the
// compiler is handled elsewhere.
src_path: NonHashedPathBuf,
src_path: TargetSourcePath,
required_features: Option<Vec<String>>,
tested: bool,
benched: bool,
Expand All @@ -203,19 +205,50 @@ pub struct Target {
}

#[derive(Clone, PartialEq, Eq)]
struct NonHashedPathBuf {
path: PathBuf,
pub enum TargetSourcePath {
Path(PathBuf),
Metabuild,
}

impl TargetSourcePath {
pub fn path(&self) -> &Path {
match self {
TargetSourcePath::Path(path) => path.as_ref(),
TargetSourcePath::Metabuild => panic!("metabuild not expected"),
}
}

pub fn is_path(&self) -> bool {
match self {
TargetSourcePath::Path(_) => true,
_ => false,
}
}
}

impl Hash for NonHashedPathBuf {
impl Hash for TargetSourcePath {
fn hash<H: Hasher>(&self, _: &mut H) {
// ...
}
}

impl fmt::Debug for NonHashedPathBuf {
impl fmt::Debug for TargetSourcePath {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
self.path.fmt(f)
match self {
TargetSourcePath::Path(path) => path.fmt(f),
TargetSourcePath::Metabuild => "metabuild".fmt(f),
}
}
}

impl From<PathBuf> for TargetSourcePath {
fn from(path: PathBuf) -> Self {
assert!(
path.is_absolute(),
"`{}` is not absolute",
path.display()
);
TargetSourcePath::Path(path)
}
}

Expand All @@ -240,7 +273,7 @@ impl ser::Serialize for Target {
kind: &self.kind,
crate_types: self.rustc_crate_types(),
name: &self.name,
src_path: &self.src_path.path,
src_path: &self.src_path.path().to_path_buf(),
edition: &self.edition.to_string(),
required_features: self
.required_features
Expand All @@ -254,34 +287,43 @@ compact_debug! {
impl fmt::Debug for Target {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let (default, default_name) = {
let src = self.src_path().to_path_buf();
match &self.kind {
TargetKind::Lib(kinds) => {
(
Target::lib_target(
&self.name,
kinds.clone(),
src.clone(),
Edition::Edition2015,
self.src_path().path().to_path_buf(),
self.edition,
),
format!("lib_target({:?}, {:?}, {:?})",
self.name, kinds, src),
format!("lib_target({:?}, {:?}, {:?}, {:?})",
self.name, kinds, self.src_path, self.edition),
)
}
TargetKind::CustomBuild => {
(
Target::custom_build_target(
&self.name,
src.clone(),
Edition::Edition2015,
),
format!("custom_build_target({:?}, {:?})",
self.name, src),
)
match self.src_path {
TargetSourcePath::Path(ref path) => {
(
Target::custom_build_target(
&self.name,
path.to_path_buf(),
self.edition,
),
format!("custom_build_target({:?}, {:?}, {:?})",
self.name, path, self.edition),
)
}
TargetSourcePath::Metabuild => {
(
Target::metabuild_target(&self.name),
format!("metabuild_target({:?})", self.name),
)
}
}
}
_ => (
Target::with_path(src.clone(), Edition::Edition2015),
format!("with_path({:?})", src),
Target::new(self.src_path.clone(), self.edition),
format!("with_path({:?}, {:?})", self.src_path, self.edition),
),
}
};
Expand Down Expand Up @@ -471,6 +513,16 @@ impl Manifest {
pub fn metabuild(&self) -> Option<&Vec<String>> {
self.metabuild.as_ref()
}

pub fn metabuild_path(&self, target_dir: Filesystem) -> PathBuf {
let mut hasher = SipHasher::new_with_keys(0, 0);
self.package_id().hash(&mut hasher);
let hash = hasher.finish();
target_dir
.into_path_unlocked()
.join(".metabuild")
.join(format!("metabuild-{}-{:016x}.rs", self.name(), hash))
}
}

impl VirtualManifest {
Expand Down Expand Up @@ -515,16 +567,11 @@ impl VirtualManifest {
}

impl Target {
fn with_path(src_path: PathBuf, edition: Edition) -> Target {
assert!(
src_path.is_absolute(),
"`{}` is not absolute",
src_path.display()
);
fn new(src_path: TargetSourcePath, edition: Edition) -> Target {
Target {
kind: TargetKind::Bin,
name: String::new(),
src_path: NonHashedPathBuf { path: src_path },
src_path,
required_features: None,
doc: false,
doctest: false,
Expand All @@ -536,6 +583,10 @@ impl Target {
}
}

fn with_path(src_path: PathBuf, edition: Edition) -> Target {
Target::new(TargetSourcePath::from(src_path), edition)
}

pub fn lib_target(
name: &str,
crate_targets: Vec<LibKind>,
Expand Down Expand Up @@ -582,6 +633,17 @@ impl Target {
}
}

pub fn metabuild_target(name: &str) -> Target {
Target {
kind: TargetKind::CustomBuild,
name: name.to_string(),
for_host: true,
benched: false,
tested: false,
..Target::new(TargetSourcePath::Metabuild, Edition::Edition2015)
}
}

pub fn example_target(
name: &str,
crate_targets: Vec<LibKind>,
Expand Down Expand Up @@ -641,8 +703,11 @@ impl Target {
pub fn crate_name(&self) -> String {
self.name.replace("-", "_")
}
pub fn src_path(&self) -> &Path {
&self.src_path.path
pub fn src_path(&self) -> &TargetSourcePath {
&self.src_path
}
pub fn set_src_path(&mut self, src_path: TargetSourcePath) {
self.src_path = src_path;
}
pub fn required_features(&self) -> Option<&Vec<String>> {
self.required_features.as_ref()
Expand Down
Loading

0 comments on commit ecc87b1

Please sign in to comment.