Find workspace root via encompassing member

src/cargo/core/workspace.rs

@@ -230,6 +230,14 @@ impl<'cfg> Workspace<'cfg> {
     /// if some other transient error happens.
     fn find_root(&mut self, manifest_path: &Path)
                  -> CargoResult<Option<PathBuf>> {
+        fn read_root_pointer(member_manifest: &Path, root_link: &str) -> CargoResult<PathBuf> {
+            let path = member_manifest.parent().unwrap()
+                .join(root_link)
+                .join("Cargo.toml");
+            debug!("find_root - pointer {}", path.display());
+            return Ok(paths::normalize_path(&path))
+        };
+
         {
             let current = self.packages.load(&manifest_path)?;
             match *current.workspace_config() {
@@ -238,11 +246,7 @@ impl<'cfg> Workspace<'cfg> {
                     return Ok(Some(manifest_path.to_path_buf()))
                 }
                 WorkspaceConfig::Member { root: Some(ref path_to_root) } => {
-                    let path = manifest_path.parent().unwrap()
-                                            .join(path_to_root)
-                                            .join("Cargo.toml");
-                    debug!("find_root - pointer {}", path.display());
-                    return Ok(Some(paths::normalize_path(&path)))
+                    return Ok(Some(read_root_pointer(manifest_path, path_to_root)?))
                 }
                 WorkspaceConfig::Member { root: None } => {}
             }
@@ -258,6 +262,9 @@ impl<'cfg> Workspace<'cfg> {
                         debug!("find_root - found");
                         return Ok(Some(manifest))
                     }
+                    WorkspaceConfig::Member { root: Some(ref path_to_root) } => {
+                        return Ok(Some(read_root_pointer(&manifest, path_to_root)?))
+                    }
                     WorkspaceConfig::Member { .. } => {}
                 }
             }
@@ -293,21 +300,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 +340,7 @@ impl<'cfg> Workspace<'cfg> {
                .collect::<Vec<_>>()
         };
         for candidate in candidates {
-            self.find_path_deps(&candidate)?;
+            self.find_path_deps(&candidate, true)?;
         }
         Ok(())
     }

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

tests/workspaces.rs

@@ -1100,4 +1100,134 @@ 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));
+}
+
+#[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" }
+
+            [workspace]
+        "#)
+        .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));
+
+    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()));
+}