Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metabuild (RFC 2196) #5628

Merged
merged 4 commits into from
Aug 24, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
New metabuild strategy using custom src_path enum.
- 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
commit ecc87b1795ddb181fede9a2fb6813e47e227c0f8
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