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

Add dep-info generation #3557

Merged
merged 6 commits into from
Jan 28, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
5 changes: 5 additions & 0 deletions src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use util::Freshness;
use self::job::{Job, Work};
use self::job_queue::JobQueue;

use self::output_depinfo::output_depinfo;

pub use self::compilation::Compilation;
pub use self::context::{Context, Unit};
pub use self::custom_build::{BuildOutput, BuildMap, BuildScripts};
Expand All @@ -30,6 +32,7 @@ mod job;
mod job_queue;
mod layout;
mod links;
mod output_depinfo;

#[derive(PartialEq, Eq, Hash, Debug, Clone, Copy, PartialOrd, Ord)]
pub enum Kind { Host, Target }
Expand Down Expand Up @@ -184,6 +187,8 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
.or_insert(HashSet::new())
.extend(feats.iter().map(|feat| format!("feature=\"{}\"", feat)));
}

output_depinfo(&mut cx, unit)?;
}

for (&(ref pkg, _), output) in cx.build_state.outputs.lock().unwrap().iter() {
Expand Down
128 changes: 128 additions & 0 deletions src/cargo/ops/cargo_rustc/output_depinfo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
use std::borrow::ToOwned;
use std::collections::HashSet;
use std::io::{Read, Write};
use std::fs::File;
use std::path::{Path, PathBuf};

use core::{TargetKind};
use ops::{Context, Unit};
use util::{CargoResult, internal, human};

#[derive(Debug)]
struct DepLine {
target: String,
deps: Vec<String>,
}

struct DepFile {
dir: String,
deps: Vec<DepLine>,
}

fn render_filename<P: AsRef<Path>>(path: P, basedir: Option<&str>) -> CargoResult<String> {
let path = path.as_ref();
let relpath = match basedir {
None => path,
Some(base) => match path.strip_prefix(base) {
Ok(relpath) => relpath,
_ => path,
}
};
relpath.to_str().ok_or(internal("path not utf-8")).map(ToOwned::to_owned)
}

fn read_dep_file<P: AsRef<Path>>(path: P) -> CargoResult<DepFile> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this implementation be shared with the fingerprint module? I think the fingerprint module could store information in a side table in the context perhaps to avoid rereading the filesystem here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I shared the implementation, but have not yet implemented a change to stash the info so we can avoid rereading the file system. I don't think that's hard, but for reference here are some measurements from a null build of cargo: a null build takes about 215ms. The time between reading the input fingerprints and writing the output .d is about 400us, repeated twice. Also, for perspective, this only reads the deps for path dependencies. So I think the impact on performance is pretty minimal. I'll happily stash if you wanna chase that down, though.

(However, in looking at the strace, I see I'm losing several ms by not buffering the file writing - fixing that now!)

let mut file = File::open(&path).map_err(|_|
human("error opening ".to_string() + path.as_ref().to_str().unwrap_or("(bad unicode")))?;
let mut contents = String::new();
let _ = file.read_to_string(&mut contents)?;
let mut spl = contents.split('\0');
let dir = spl.next().ok_or(internal("dependency file empty"))?;
let dep_txt = spl.next().ok_or(internal("dependency file missing null byte"))?;
let mut result = Vec::new();
for line in dep_txt.lines() {
let mut line_spl = line.split(": ");
if let Some(target) = line_spl.next() {
if let Some(deps) = line_spl.next() {
let deps = deps.split_whitespace().map(ToOwned::to_owned).collect();
result.push(DepLine {
target: target.to_string(),
deps: deps,
});
}
}
}
Ok(DepFile {
dir: dir.to_string(),
deps: result,
})
}

fn add_deps(depfile: &DepFile, deps: &mut HashSet<PathBuf>) {
let dep_dir = PathBuf::from(&depfile.dir);
for depline in &depfile.deps {
for dep in &depline.deps {
deps.insert(dep_dir.join(dep));
}
}
}

// TODO: probably better to use Context::target_filenames for this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we'll just want to emit one .d file with the same contents for each filename returned from target_filenames

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

fn target_filename(context: &mut Context, unit: &Unit) -> CargoResult<PathBuf> {
let (dir, base) = context.link_stem(&unit).ok_or(internal("can't get link stem"))?;
if unit.target.is_lib() {
Ok(dir.join(["lib", &base, ".rlib"].concat()))
} else {
Ok(dir.join(base))
}
}

fn add_deps_for_unit(deps: &mut HashSet<PathBuf>, context: &mut Context, unit: &Unit)
-> CargoResult<()>
{
// TODO: this is duplicated against filename in fingerprint.rs
let kind = match *unit.target.kind() {
TargetKind::Lib(..) => "lib",
TargetKind::Bin => "bin",
TargetKind::Test => "integration-test",
TargetKind::ExampleBin |
TargetKind::ExampleLib(..) => "example",
TargetKind::Bench => "bench",
TargetKind::CustomBuild => "build-script",
};
let flavor = if unit.profile.test {
"test-"
} else if unit.profile.doc {
"doc-"
} else {
""
};
let dep_filename = ["dep-", flavor, kind, "-", &context.file_stem(&unit)].concat();
let path = context.fingerprint_dir(&unit).join(&dep_filename);
let depfile = read_dep_file(&path)?;
add_deps(&depfile, deps);
Ok(())
}

pub fn output_depinfo(context: &mut Context, unit: &Unit) -> CargoResult<()> {
let mut deps = HashSet::new();
add_deps_for_unit(&mut deps, context, unit)?;
for dep_unit in &context.dep_targets(unit)? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this'll want to be recursive to pick up path dependencies of path dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

let source_id = dep_unit.pkg.package_id().source_id();
if source_id.is_path() {
add_deps_for_unit(&mut deps, context, dep_unit)?;
}
}
let filename = target_filename(context, unit)?;
let mut output_path = filename.clone().into_os_string();
output_path.push(".d");
let basedir = None; // TODO
let target_fn = render_filename(filename, basedir)?;
let mut outfile = File::create(output_path)?;
write!(outfile, "{}:", target_fn)?;
for dep in &deps {
write!(outfile, " {}", render_filename(dep, basedir)?)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here we'll want to escape spaces in paths, right? (that's done elsewhere I believe)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right we parse escapes, we don't personally escape, nvmd

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I do need to re-escape spaces. Thanks for the heads-up!

}
writeln!(outfile, "")?;
Ok(())
}