Skip to content

Commit 0863469

Browse files
committed
Always build libraries into the same location
Previously Cargo would compile a library into a different location depending on whether it was the "root crate" or not. In the ongoing saga of reducing Cargo's reliance on the idea of a "root crate" this PR is the next step. With workspaces the root crate of a compliation changes all the time, so the output needs to be the same whether a crate is at the root or not. Fixing this inconsistence in turn fixes bugs like #2855 and #2897 which arise due to this discrepancy. Additionally, Cargo will no longer recompile a library when it's used as a "root crate" or not. This is fixed by taking a few steps: * Everything is now compiled into the `deps` directory, regardless of whether it's a root output or not. * If a "root crate" is being compiled, then the relevant outputs are hard-linked up one level to where they are today. This means that your binaries, dylibs, staticlibs, etc, will all show up where they used to. * The `-C metadata` flag is always omitted for path dependencies now. These dependencies are always path dependencies and already all have unique crate names. Additionally, they're the only crates in the DAG without metadata, so there's no need to provide additional metadata. This in turn means that none of the file names of the generated crates are mangled. Closes #2855
1 parent 0c08bd1 commit 0863469

18 files changed

+156
-115
lines changed

src/cargo/ops/cargo_clean.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> {
7373
try!(cx.probe_target_info(&units));
7474

7575
for unit in units.iter() {
76-
let layout = cx.layout(&unit.pkg, unit.kind);
76+
let layout = cx.layout(unit);
7777
try!(rm_rf(&layout.proxy().fingerprint(&unit.pkg)));
7878
try!(rm_rf(&layout.build(&unit.pkg)));
7979

src/cargo/ops/cargo_rustc/context.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
9696

9797
/// Prepare this context, ensuring that all filesystem directories are in
9898
/// place.
99-
pub fn prepare(&mut self, root: &Package) -> CargoResult<()> {
99+
pub fn prepare(&mut self) -> CargoResult<()> {
100100
let _p = profile::start("preparing layout");
101101

102102
try!(self.host.prepare().chain_error(|| {
@@ -111,10 +111,9 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
111111
None => {}
112112
}
113113

114-
self.compilation.root_output =
115-
self.layout(root, Kind::Target).proxy().dest().to_path_buf();
116-
self.compilation.deps_output =
117-
self.layout(root, Kind::Target).proxy().deps().to_path_buf();
114+
let layout = self.target.as_ref().unwrap_or(&self.host);
115+
self.compilation.root_output = layout.dest().to_path_buf();
116+
self.compilation.deps_output = layout.deps().to_path_buf();
118117
Ok(())
119118
}
120119

@@ -237,23 +236,28 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
237236
}
238237

239238
/// Returns the appropriate directory layout for either a plugin or not.
240-
pub fn layout(&self, pkg: &Package, kind: Kind) -> LayoutProxy {
241-
let primary = pkg.package_id() == self.resolve.root();
242-
match kind {
239+
pub fn layout(&self, unit: &Unit) -> LayoutProxy {
240+
let primary = unit.pkg.package_id() == self.resolve.root();
241+
match unit.kind {
243242
Kind::Host => LayoutProxy::new(&self.host, primary),
244243
Kind::Target => LayoutProxy::new(self.target.as_ref()
245244
.unwrap_or(&self.host),
246245
primary),
247246
}
248247
}
249248

249+
/// Returns the path for plugin/dylib dependencies
250+
pub fn host_dylib_path(&self) -> &Path {
251+
self.host.deps()
252+
}
253+
250254
/// Returns the appropriate output directory for the specified package and
251255
/// target.
252256
pub fn out_dir(&self, unit: &Unit) -> PathBuf {
253257
if unit.profile.doc {
254-
self.layout(unit.pkg, unit.kind).doc_root()
258+
self.layout(unit).doc_root()
255259
} else {
256-
self.layout(unit.pkg, unit.kind).out_dir(unit.pkg, unit.target)
260+
self.layout(unit).out_dir(unit)
257261
}
258262
}
259263

@@ -289,7 +293,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
289293
let mut metadata = unit.pkg.generate_metadata();
290294
metadata.mix(&format!("bin-{}", unit.target.name()));
291295
Some(metadata)
292-
} else if unit.pkg.package_id() == self.resolve.root() &&
296+
} else if unit.pkg.package_id().source_id().is_path() &&
293297
!unit.profile.test {
294298
// If we're not building a unit test then the root package never
295299
// needs any metadata as it's guaranteed to not conflict with any

src/cargo/ops/cargo_rustc/custom_build.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,10 @@ pub fn prepare<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
8787

8888
fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
8989
-> CargoResult<(Work, Work)> {
90+
let host_unit = Unit { kind: Kind::Host, ..*unit };
9091
let (script_output, build_output) = {
91-
(cx.layout(unit.pkg, Kind::Host).build(unit.pkg),
92-
cx.layout(unit.pkg, unit.kind).build_out(unit.pkg))
92+
(cx.layout(&host_unit).build(unit.pkg),
93+
cx.layout(unit).build_out(unit.pkg))
9394
};
9495

9596
// Building the command to execute
@@ -161,8 +162,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
161162
};
162163
cx.build_explicit_deps.insert(*unit, (output_file.clone(), rerun_if_changed));
163164

164-
try!(fs::create_dir_all(&cx.layout(unit.pkg, Kind::Host).build(unit.pkg)));
165-
try!(fs::create_dir_all(&cx.layout(unit.pkg, unit.kind).build(unit.pkg)));
165+
try!(fs::create_dir_all(&cx.layout(&host_unit).build(unit.pkg)));
166+
try!(fs::create_dir_all(&cx.layout(unit).build(unit.pkg)));
166167

167168
// Prepare the unit of "dirty work" which will actually run the custom build
168169
// command.

src/cargo/ops/cargo_rustc/fingerprint.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ pub fn prepare_init(cx: &mut Context, unit: &Unit) -> CargoResult<()> {
497497

498498
/// Return the (old, new) location for fingerprints for a package
499499
pub fn dir(cx: &Context, unit: &Unit) -> PathBuf {
500-
cx.layout(unit.pkg, unit.kind).proxy().fingerprint(unit.pkg)
500+
cx.layout(unit).proxy().fingerprint(unit.pkg)
501501
}
502502

503503
/// Returns the (old, new) location for the dep info file of a target.

src/cargo/ops/cargo_rustc/layout.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@ use std::fs;
4949
use std::io;
5050
use std::path::{PathBuf, Path};
5151

52-
use core::{Package, Target, Workspace};
52+
use core::{Package, Workspace};
5353
use util::{Config, FileLock, CargoResult, Filesystem};
5454
use util::hex::short_hash;
55+
use super::Unit;
5556

5657
pub struct Layout {
5758
root: PathBuf,
@@ -165,11 +166,13 @@ impl<'a> LayoutProxy<'a> {
165166

166167
pub fn proxy(&self) -> &'a Layout { self.root }
167168

168-
pub fn out_dir(&self, pkg: &Package, target: &Target) -> PathBuf {
169-
if target.is_custom_build() {
170-
self.build(pkg)
171-
} else if target.is_example() {
169+
pub fn out_dir(&self, unit: &Unit) -> PathBuf {
170+
if unit.target.is_custom_build() {
171+
self.build(unit.pkg)
172+
} else if unit.target.is_example() {
172173
self.examples().to_path_buf()
174+
} else if unit.target.is_lib() {
175+
self.deps().to_path_buf()
173176
} else {
174177
self.root().to_path_buf()
175178
}

src/cargo/ops/cargo_rustc/mod.rs

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
8787

8888
let mut queue = JobQueue::new(&cx);
8989

90-
try!(cx.prepare(root));
90+
try!(cx.prepare());
9191
try!(cx.probe_target_info(&units));
9292
try!(custom_build::build_map(&mut cx, &units));
9393

@@ -104,7 +104,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
104104
try!(queue.execute(&mut cx));
105105

106106
for unit in units.iter() {
107-
let out_dir = cx.layout(unit.pkg, unit.kind).build_out(unit.pkg)
107+
let out_dir = cx.layout(unit).build_out(unit.pkg)
108108
.display().to_string();
109109
cx.compilation.extra_env.entry(unit.pkg.package_id().clone())
110110
.or_insert(Vec::new())
@@ -228,6 +228,7 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
228228
let do_rename = unit.target.allows_underscores() && !unit.profile.test;
229229
let real_name = unit.target.name().to_string();
230230
let crate_name = unit.target.crate_name();
231+
let move_outputs_up = unit.pkg.package_id() == cx.resolve.root();
231232

232233
let rustc_dep_info_loc = if do_rename {
233234
root.join(&crate_name)
@@ -271,7 +272,7 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
271272
let src = dst.with_file_name(dst.file_name().unwrap()
272273
.to_str().unwrap()
273274
.replace(&real_name, &crate_name));
274-
if !has_custom_args || fs::metadata(&src).is_ok() {
275+
if !has_custom_args || src.exists() {
275276
try!(fs::rename(&src, &dst).chain_error(|| {
276277
internal(format!("could not rename crate {:?}", src))
277278
}));
@@ -286,6 +287,40 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
286287
try!(fingerprint::append_current_dir(&dep_info_loc, &cwd));
287288
}
288289

290+
// If we're a "root crate", e.g. the target of this compilation, then we
291+
// hard link our outputs out of the `deps` directory into the directory
292+
// above. This means that `cargo build` will produce binaries in
293+
// `target/debug` which one probably expects.
294+
if move_outputs_up {
295+
for &(ref filename, _linkable) in filenames.iter() {
296+
let src = root.join(filename);
297+
// This may have been a `cargo rustc` command which changes the
298+
// output, so the source may not actually exist.
299+
if !src.exists() {
300+
continue
301+
}
302+
303+
// We currently only lift files up from the `deps` directory. If
304+
// it was compiled into something like `example/` or `doc/` then
305+
// we don't want to link it up.
306+
let src_dir = src.parent().unwrap();
307+
if !src_dir.ends_with("deps") {
308+
continue
309+
}
310+
let dst = src_dir.parent().unwrap()
311+
.join(src.file_name().unwrap());
312+
if dst.exists() {
313+
try!(fs::remove_file(&dst).chain_error(|| {
314+
human(format!("failed to remove: {}", dst.display()))
315+
}));
316+
}
317+
try!(fs::hard_link(&src, &dst).chain_error(|| {
318+
human(format!("failed to link `{}` to `{}`",
319+
src.display(), dst.display()))
320+
}));
321+
}
322+
}
323+
289324
Ok(())
290325
}));
291326

@@ -390,8 +425,7 @@ fn rustdoc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
390425
try!(build_deps_args(&mut rustdoc, cx, unit));
391426

392427
if unit.pkg.has_custom_build() {
393-
rustdoc.env("OUT_DIR", &cx.layout(unit.pkg, unit.kind)
394-
.build_out(unit.pkg));
428+
rustdoc.env("OUT_DIR", &cx.layout(unit).build_out(unit.pkg));
395429
}
396430

397431
rustdoc.args(&try!(cx.rustdocflags_args(unit)));
@@ -552,12 +586,7 @@ fn build_plugin_args(cmd: &mut CommandPrototype, cx: &Context, unit: &Unit) {
552586

553587
fn build_deps_args(cmd: &mut CommandPrototype, cx: &Context, unit: &Unit)
554588
-> CargoResult<()> {
555-
let layout = cx.layout(unit.pkg, unit.kind);
556-
cmd.arg("-L").arg(&{
557-
let mut root = OsString::from("dependency=");
558-
root.push(layout.root());
559-
root
560-
});
589+
let layout = cx.layout(unit);
561590
cmd.arg("-L").arg(&{
562591
let mut deps = OsString::from("dependency=");
563592
deps.push(layout.deps());
@@ -569,7 +598,7 @@ fn build_deps_args(cmd: &mut CommandPrototype, cx: &Context, unit: &Unit)
569598
}
570599

571600
for unit in try!(cx.dep_targets(unit)).iter() {
572-
if unit.target.linkable() {
601+
if unit.target.linkable() && !unit.profile.doc {
573602
try!(link_to(cmd, cx, unit));
574603
}
575604
}
@@ -578,16 +607,14 @@ fn build_deps_args(cmd: &mut CommandPrototype, cx: &Context, unit: &Unit)
578607

579608
fn link_to(cmd: &mut CommandPrototype, cx: &Context, unit: &Unit)
580609
-> CargoResult<()> {
581-
let layout = cx.layout(unit.pkg, unit.kind);
582-
583610
for (filename, linkable) in try!(cx.target_filenames(unit)) {
584611
if !linkable {
585612
continue
586613
}
587614
let mut v = OsString::new();
588615
v.push(&unit.target.crate_name());
589616
v.push("=");
590-
v.push(layout.root());
617+
v.push(cx.out_dir(unit));
591618
v.push(&path::MAIN_SEPARATOR.to_string());
592619
v.push(&filename);
593620
cmd.arg("--extern").arg(&v);
@@ -600,9 +627,8 @@ pub fn process(cmd: CommandType, pkg: &Package,
600627
cx: &Context) -> CargoResult<CommandPrototype> {
601628
// When invoking a tool, we need the *host* deps directory in the dynamic
602629
// library search path for plugins and such which have dynamic dependencies.
603-
let layout = cx.layout(pkg, Kind::Host);
604630
let mut search_path = util::dylib_path();
605-
search_path.push(layout.deps().to_path_buf());
631+
search_path.push(cx.host_dylib_path().to_path_buf());
606632

607633
// We want to use the same environment and such as normal processes, but we
608634
// want to override the dylib search path with the one we just calculated.

src/cargo/ops/cargo_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ fn run_doc_tests(options: &TestOptions,
134134
p.arg("--test").arg(lib)
135135
.arg("--crate-name").arg(&crate_name);
136136

137-
for &rust_dep in &[&compilation.deps_output, &compilation.root_output] {
137+
for &rust_dep in &[&compilation.deps_output] {
138138
let mut arg = OsString::from("dependency=");
139139
arg.push(rust_dep);
140140
p.arg("-L").arg(arg);

tests/build-lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,8 @@ fn verbose_output_for_lib(p: &ProjectBuilder) -> String {
1010
format!("\
1111
[COMPILING] {name} v{version} ({url})
1212
[RUNNING] `rustc src{sep}lib.rs --crate-name {name} --crate-type lib -g \
13-
--out-dir {dir}{sep}target{sep}debug \
13+
--out-dir [..] \
1414
--emit=dep-info,link \
15-
-L dependency={dir}{sep}target{sep}debug \
1615
-L dependency={dir}{sep}target{sep}debug{sep}deps`
1716
[FINISHED] debug [unoptimized + debuginfo] target(s) in [..]
1817
", sep = SEP,

tests/build-script.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -766,19 +766,17 @@ fn build_cmd_with_a_build_cmd() {
766766
[RUNNING] `rustc a[..]build.rs [..] --extern b=[..]`
767767
[RUNNING] `[..]a-[..]build-script-build[..]`
768768
[RUNNING] `rustc [..]lib.rs --crate-name a --crate-type lib -g \
769-
-C metadata=[..] -C extra-filename=-[..] \
770769
--out-dir [..]target[..]deps --emit=dep-info,link \
771-
-L [..]target[..]deps -L [..]target[..]deps`
770+
-L [..]target[..]deps`
772771
[COMPILING] foo v0.5.0 (file://[..])
773772
[RUNNING] `rustc build.rs --crate-name build_script_build --crate-type bin \
774-
-g \
775-
--out-dir [..]build[..]foo-[..] --emit=dep-info,link \
776-
-L [..]target[..]debug -L [..]target[..]deps \
777-
--extern a=[..]liba-[..].rlib`
773+
-g --out-dir [..] --emit=dep-info,link \
774+
-L [..]target[..]deps \
775+
--extern a=[..]liba[..].rlib`
778776
[RUNNING] `[..]foo-[..]build-script-build[..]`
779777
[RUNNING] `rustc [..]lib.rs --crate-name foo --crate-type lib -g \
780-
--out-dir [..]target[..]debug --emit=dep-info,link \
781-
-L [..]target[..]debug -L [..]target[..]deps`
778+
--out-dir [..] --emit=dep-info,link \
779+
-L [..]target[..]deps`
782780
[FINISHED] debug [unoptimized + debuginfo] target(s) in [..]
783781
"));
784782
}
@@ -1175,7 +1173,7 @@ fn build_script_with_dynamic_native_dependency() {
11751173
11761174
fn main() {
11771175
let src = PathBuf::from(env::var("SRC").unwrap());
1178-
println!("cargo:rustc-link-search={}/target/debug",
1176+
println!("cargo:rustc-link-search={}/target/debug/deps",
11791177
src.display());
11801178
}
11811179
"#)

0 commit comments

Comments
 (0)