From 07c667fb2659e3f56bca5ecc28dc07cf70e93e16 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 22 Dec 2016 20:56:40 +0300 Subject: [PATCH 1/3] Path deps outside workspace are not members closes #3192 --- src/cargo/core/workspace.rs | 17 ++++++++++++----- tests/workspaces.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 13898323010..fdd14ee23e3 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -293,21 +293,28 @@ impl<'cfg> Workspace<'cfg> { }; if let Some(list) = members { - let root = root_manifest.parent().unwrap(); for path in list { + let root = root_manifest.parent().unwrap(); let manifest_path = root.join(path).join("Cargo.toml"); - self.find_path_deps(&manifest_path)?; + self.find_path_deps(&manifest_path, false)?; } } - self.find_path_deps(&root_manifest) + self.find_path_deps(&root_manifest, false) } - fn find_path_deps(&mut self, manifest_path: &Path) -> CargoResult<()> { + fn find_path_deps(&mut self, manifest_path: &Path, is_path_dep: bool) -> CargoResult<()> { let manifest_path = paths::normalize_path(manifest_path); if self.members.iter().any(|p| p == &manifest_path) { return Ok(()) } + if is_path_dep + && !manifest_path.parent().unwrap().starts_with(self.root()) + && self.find_root(&manifest_path)? != self.root_manifest { + // If `manifest_path` is a path dependency outside of the workspace, + // don't add it, or any of its dependencies, as a members. + return Ok(()) + } debug!("find_members - {}", manifest_path.display()); self.members.push(manifest_path.clone()); @@ -326,7 +333,7 @@ impl<'cfg> Workspace<'cfg> { .collect::>() }; for candidate in candidates { - self.find_path_deps(&candidate)?; + self.find_path_deps(&candidate, true)?; } Ok(()) } diff --git a/tests/workspaces.rs b/tests/workspaces.rs index 86775d81587..51cb366c3ed 100644 --- a/tests/workspaces.rs +++ b/tests/workspaces.rs @@ -1100,4 +1100,32 @@ fn relative_path_for_member_works() { assert_that(p.cargo("build").cwd(p.root().join("foo")), execs().with_status(0)); assert_that(p.cargo("build").cwd(p.root().join("bar")), execs().with_status(0)); +} + +#[test] +fn path_dep_outside_workspace_is_not_member() { + let p = project("foo") + .file("ws/Cargo.toml", r#" + [project] + name = "ws" + version = "0.1.0" + authors = [] + + [dependencies] + foo = { path = "../foo" } + + [workspace] + "#) + .file("ws/src/lib.rs", r"extern crate foo;") + .file("foo/Cargo.toml", r#" + [project] + name = "foo" + version = "0.1.0" + authors = [] + "#) + .file("foo/src/lib.rs", ""); + p.build(); + + assert_that(p.cargo("build").cwd(p.root().join("ws")), + execs().with_status(0)); } \ No newline at end of file From c8e861c421d6cc456c96a3b8ad033936de14f645 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 2 Jan 2017 21:00:05 +0300 Subject: [PATCH 2/3] Document new rules about path deps in workspace --- src/doc/manifest.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/doc/manifest.md b/src/doc/manifest.md index ccdc6c3cc67..27fd6807bf1 100644 --- a/src/doc/manifest.md +++ b/src/doc/manifest.md @@ -384,11 +384,11 @@ properties: [RFC 1525]: https://github.com/rust-lang/rfcs/blob/master/text/1525-cargo-workspace.md -The root crate of a workspace, indicated by the presence of `[workspace]` in -its manifest, is responsible for defining the entire workspace (listing all -members). This can be done through the `members` key, and if it is omitted then -members are implicitly included through all `path` dependencies. Note that -members of the workspaces listed explicitly will also have their path +The root crate of a workspace, indicated by the presence of `[workspace]` in its +manifest, is responsible for defining the entire workspace. All `path` +dependencies residing in the workspace directory become members. You can add +additional packages to the workspace by listing them in the `members` key. Note +that members of the workspaces listed explicitly will also have their path dependencies included in the workspace. The `package.workspace` manifest key (described above) is used in member crates From 7429347eb377666ae8a9f0583bb1d683b21c8d76 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 15 Jan 2017 01:14:47 +0300 Subject: [PATCH 3/3] More test for workspaces \w path dependencies --- tests/workspaces.rs | 104 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 1 deletion(-) diff --git a/tests/workspaces.rs b/tests/workspaces.rs index 51cb366c3ed..6d6be8a2d29 100644 --- a/tests/workspaces.rs +++ b/tests/workspaces.rs @@ -1128,4 +1128,106 @@ fn path_dep_outside_workspace_is_not_member() { assert_that(p.cargo("build").cwd(p.root().join("ws")), execs().with_status(0)); -} \ No newline at end of file +} + +#[test] +fn test_in_and_out_of_workspace() { + let p = project("foo") + .file("ws/Cargo.toml", r#" + [project] + name = "ws" + version = "0.1.0" + authors = [] + + [dependencies] + foo = { path = "../foo" } + + [workspace] + members = [ "../bar" ] + "#) + .file("ws/src/lib.rs", r"extern crate foo; pub fn f() { foo::f() }") + .file("foo/Cargo.toml", r#" + [project] + name = "foo" + version = "0.1.0" + authors = [] + + [dependencies] + bar = { path = "../bar" } + "#) + .file("foo/src/lib.rs", "extern crate bar; pub fn f() { bar::f() }") + .file("bar/Cargo.toml", r#" + [project] + workspace = "../ws" + name = "bar" + version = "0.1.0" + authors = [] + "#) + .file("bar/src/lib.rs", "pub fn f() { }"); + p.build(); + + assert_that(p.cargo("build").cwd(p.root().join("ws")), + execs().with_status(0)); + + assert_that(&p.root().join("ws/Cargo.lock"), existing_file()); + assert_that(&p.root().join("ws/target"), existing_dir()); + assert_that(&p.root().join("foo/Cargo.lock"), is_not(existing_file())); + assert_that(&p.root().join("foo/target"), is_not(existing_dir())); + assert_that(&p.root().join("bar/Cargo.lock"), is_not(existing_file())); + assert_that(&p.root().join("bar/target"), is_not(existing_dir())); + + assert_that(p.cargo("build").cwd(p.root().join("foo")), + execs().with_status(0)); + assert_that(&p.root().join("foo/Cargo.lock"), existing_file()); + assert_that(&p.root().join("foo/target"), existing_dir()); + assert_that(&p.root().join("bar/Cargo.lock"), is_not(existing_file())); + assert_that(&p.root().join("bar/target"), is_not(existing_dir())); +} + +#[test] +fn test_path_dependency_under_member() { + let p = project("foo") + .file("ws/Cargo.toml", r#" + [project] + name = "ws" + version = "0.1.0" + authors = [] + + [dependencies] + foo = { path = "../foo" } + + "#) + .file("ws/src/lib.rs", r"extern crate foo; pub fn f() { foo::f() }") + .file("foo/Cargo.toml", r#" + [project] + workspace = "../ws" + name = "foo" + version = "0.1.0" + authors = [] + + [dependencies] + bar = { path = "./bar" } + "#) + .file("foo/src/lib.rs", "extern crate bar; pub fn f() { bar::f() }") + .file("foo/bar/Cargo.toml", r#" + [project] + name = "bar" + version = "0.1.0" + authors = [] + "#) + .file("foo/bar/src/lib.rs", "pub fn f() { }"); + p.build(); + + assert_that(p.cargo("build").cwd(p.root().join("ws")), + execs().with_status(0)); + + assert_that(&p.root().join("foo/bar/Cargo.lock"), is_not(existing_file())); + assert_that(&p.root().join("foo/bar/target"), is_not(existing_dir())); + + assert_that(p.cargo("build").cwd(p.root().join("foo/bar")), + execs().with_status(0)); + // Ideally, `foo/bar` should be a member of the workspace, + // because it is hierarchically under the workspace member. + assert_that(&p.root().join("foo/bar/Cargo.lock"), existing_file()); + assert_that(&p.root().join("foo/bar/target"), existing_dir()); +}