Skip to content

Commit

Permalink
rustpkg: Build dependencies into the correct workspace when using --r…
Browse files Browse the repository at this point in the history
…ust-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.
  • Loading branch information
catamorphism committed Oct 27, 2013
1 parent 16b8a41 commit 0e6a575
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 12 deletions.
6 changes: 4 additions & 2 deletions src/librustpkg/package_source.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions src/librustpkg/rustpkg.rs
Expand Up @@ -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(),
Expand Down
24 changes: 24 additions & 0 deletions src/librustpkg/tests.rs
Expand Up @@ -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(
Expand Down
25 changes: 17 additions & 8 deletions src/librustpkg/util.rs
Expand Up @@ -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
Expand Down

8 comments on commit 0e6a575

@bors
Copy link
Contributor

@bors bors commented on 0e6a575 Oct 28, 2013

Choose a reason for hiding this comment

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

saw approval from metajack
at catamorphism@0e6a575

@bors
Copy link
Contributor

@bors bors commented on 0e6a575 Oct 28, 2013

Choose a reason for hiding this comment

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

merging catamorphism/rust/rustpkg-dependency-build-dir = 0e6a575 into auto

@bors
Copy link
Contributor

@bors bors commented on 0e6a575 Oct 28, 2013

Choose a reason for hiding this comment

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

catamorphism/rust/rustpkg-dependency-build-dir = 0e6a575 merged ok, testing candidate = 35349a74

@bors
Copy link
Contributor

@bors bors commented on 0e6a575 Oct 28, 2013

Choose a reason for hiding this comment

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

saw approval from metajack
at catamorphism@0e6a575

@bors
Copy link
Contributor

@bors bors commented on 0e6a575 Oct 28, 2013

Choose a reason for hiding this comment

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

merging catamorphism/rust/rustpkg-dependency-build-dir = 0e6a575 into auto

@bors
Copy link
Contributor

@bors bors commented on 0e6a575 Oct 28, 2013

Choose a reason for hiding this comment

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

catamorphism/rust/rustpkg-dependency-build-dir = 0e6a575 merged ok, testing candidate = cd6e9f4

@bors
Copy link
Contributor

@bors bors commented on 0e6a575 Oct 28, 2013

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 0e6a575 Oct 28, 2013

Choose a reason for hiding this comment

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

fast-forwarding master to auto = cd6e9f4

Please sign in to comment.