auto merge of #10110 : catamorphism/rust/rustpkg-dependency-build-dir, r=metajack

r? @metajack When invoked with the --rust-path-hack flag, rustpkg was correctly building
the package into the default workspace (and not into the build/ subdirectory of the
parent directory of the source directory), but not correctly putting the output
for any dependencies into the default workspace as well.

Spotted by Jack.

4 files changed, 45 insertions, 12 deletions

diff --git a/src/librustpkg/package_source.rs b/src/librustpkg/package_source.rs
index 17ba79862e0..797ea3372cc 100644
--- a/src/librustpkg/package_source.rs
+++ b/src/librustpkg/package_source.rs
@@ -422,6 +422,8 @@ impl PkgSrc {
                                 fail!("Bad kind in build_crates")
                             });
                     }
+                    debug!("Compiling crate {}; its output will be in {}",
+                           subpath.display(), sub_dir.display());
                     let result = compile_crate(&subcx,
                                                exec,
                                                &id,
@@ -473,8 +475,8 @@ impl PkgSrc {
         let tests = self.tests.clone();
         let benchs = self.benchs.clone();
         debug!("Building libs in {}, destination = {}",
-               self.destination_workspace.display(),
-               self.destination_workspace.display());
+               self.source_workspace.display(),
+               self.build_workspace().display());
         self.build_crates(build_context,
                           &mut deps,
                           libs,
diff --git a/src/librustpkg/rustpkg.rs b/src/librustpkg/rustpkg.rs
index 1711dc99235..517d43432ec 100644
--- a/src/librustpkg/rustpkg.rs
+++ b/src/librustpkg/rustpkg.rs
@@ -587,8 +587,6 @@ impl CtxMethods for BuildContext {
                                            build_inputs,
                                            &pkg_src.destination_workspace,
                                            &id).map(|s| Path::new(s.as_slice()));
-        debug!("install: id = {}, about to call discover_outputs, {:?}",
-               id.to_str(), result.map(|p| p.display().to_str()));
         installed_files = installed_files + result;
         note(format!("Installed package {} to {}",
                      id.to_str(),
diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs
index f5973bff4fc..8f1b269f1ca 100644
--- a/src/librustpkg/tests.rs
+++ b/src/librustpkg/tests.rs
@@ -1512,6 +1512,30 @@ fn rust_path_hack_build_no_arg() {
 }
 
 #[test]
+fn rust_path_hack_build_with_dependency() {
+    let foo_id = PkgId::new("foo");
+    let dep_id = PkgId::new("dep");
+    // Tests that when --rust-path-hack is in effect, dependencies get built
+    // into the destination workspace and not the source directory
+    let work_dir = create_local_package(&foo_id);
+    let work_dir = work_dir.path();
+    let dep_workspace = create_local_package(&dep_id);
+    let dep_workspace = dep_workspace.path();
+    let dest_workspace = mk_emptier_workspace("dep");
+    let dest_workspace = dest_workspace.path();
+    let source_dir = work_dir.join_many(["src", "foo-0.1"]);
+    writeFile(&source_dir.join("lib.rs"), "extern mod dep; pub fn f() { }");
+    let dep_dir = dep_workspace.join_many(["src", "dep-0.1"]);
+    let rust_path = Some(~[(~"RUST_PATH",
+                          format!("{}:{}",
+                                  dest_workspace.display(),
+                                  dep_dir.display()))]);
+    command_line_test_with_env([~"build", ~"--rust-path-hack", ~"foo"], work_dir, rust_path);
+    assert_built_library_exists(dest_workspace, "dep");
+    assert!(!built_library_exists(dep_workspace, "dep"));
+}
+
+#[test]
 fn rust_path_install_target() {
     let dir_for_path = TempDir::new(
         "source_workspace").expect("rust_path_install_target failed");
diff --git a/src/librustpkg/util.rs b/src/librustpkg/util.rs
index 026f443ec79..9d835dcc20b 100644
--- a/src/librustpkg/util.rs
+++ b/src/librustpkg/util.rs
@@ -465,22 +465,31 @@ impl<'self> Visitor<()> for ViewItemVisitor<'self> {
                         // Find all the workspaces in the RUST_PATH that contain this package.
                         let workspaces = pkg_parent_workspaces(&self.context.context,
                                                                &pkg_id);
-                        // Two cases:
+                        // Three cases:
                         // (a) `workspaces` is empty. That means there's no local source
                         // for this package. In that case, we pass the default workspace
                         // into `PkgSrc::new`, so that if it exists as a remote repository,
-                        // its sources will be fetched into it.
-                        // (b) `workspaces` is non-empty -- we found a local source for this
-                        // package.
-                        let dest_workspace = if workspaces.is_empty() {
-                            default_workspace()
-                        } else { workspaces[0] };
+                        // its sources will be fetched into it. We also put the output in the
+                        // same workspace.
+                        // (b) We're using the Rust path hack. In that case, the output goes
+                        // in the destination workspace.
+                        // (c) `workspaces` is non-empty -- we found a local source for this
+                        // package and will build in that workspace.
+                        let (source_workspace, dest_workspace) = if workspaces.is_empty() {
+                            (default_workspace(), default_workspace())
+                        } else {
+                            if self.context.context.use_rust_path_hack {
+                                (workspaces[0], default_workspace())
+                            } else {
+                                 (workspaces[0].clone(), workspaces[0])
+                            }
+                        };
                         // In this case, the source and destination workspaces are the same:
                         // Either it's a remote package, so the local sources don't exist
                         // and the `PkgSrc` constructor will detect that;
                         // or else it's already in a workspace and we'll build into that
                         // workspace
-                        let pkg_src = PkgSrc::new(dest_workspace.clone(),
+                        let pkg_src = PkgSrc::new(source_workspace,
                                                   dest_workspace,
                         // Use the rust_path_hack to search for dependencies iff
                         // we were already using it