From 0e6a575635862deba5c280d38b32bb43093980c0 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Sun, 27 Oct 2013 16:30:29 -0700 Subject: [PATCH] rustpkg: Build dependencies into the correct workspace when using --rust-path-hack 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. --- src/librustpkg/package_source.rs | 6 ++++-- src/librustpkg/rustpkg.rs | 2 -- src/librustpkg/tests.rs | 24 ++++++++++++++++++++++++ src/librustpkg/util.rs | 25 +++++++++++++++++-------- 4 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/librustpkg/package_source.rs b/src/librustpkg/package_source.rs index 17ba79862e0ca..797ea3372ccfd 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 bd3a1b2f67282..bed7d9bc5dd21 100644 --- a/src/librustpkg/rustpkg.rs +++ b/src/librustpkg/rustpkg.rs @@ -585,8 +585,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 ce5e81d410923..a54bcd6f79f4c 100644 --- a/src/librustpkg/tests.rs +++ b/src/librustpkg/tests.rs @@ -1512,6 +1512,30 @@ fn rust_path_hack_build_no_arg() { assert!(!built_library_exists(&source_dir, "foo")); } +#[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( diff --git a/src/librustpkg/util.rs b/src/librustpkg/util.rs index 3824f6d38def9..10d53b1dfcc01 100644 --- a/src/librustpkg/util.rs +++ b/src/librustpkg/util.rs @@ -469,22 +469,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