Auto merge of #3837 - alexcrichton:workspace-exlucde, r=brson

Add a workspace.exclude key

This commit adds a new key to the `Cargo.toml` manifest, `workspace.exclude`.
This new key is a list of strings which is an array of directories that are
excluded from the workspace explicitly. This is intended for use cases such as
vendoring where path dependencies into a vendored directory don't want to pull
in the workspace dependencies.

There's a number of use cases mentioned on #3192 which I believe should all be
covered with this strategy. At a bare minimum it should suffice to `exclude`
every directory and then just explicitly whitelist crates through `members`
through inclusion, and that should give precise control over the structure of a
workspace.

Closes #3192

src/cargo/core/workspace.rs

@@ -68,7 +68,10 @@ enum MaybePackage {
 pub enum WorkspaceConfig {
     /// Indicates that `[workspace]` was present and the members were
     /// optionally specified as well.
-    Root { members: Option<Vec<String>> },
+    Root {
+        members: Option<Vec<String>>,
+        exclude: Vec<String>,
+    },
 
     /// Indicates that `[workspace]` was present and the `root` field is the
     /// optional value of `package.workspace`, if present.
@@ -269,11 +272,15 @@ impl<'cfg> Workspace<'cfg> {
             debug!("find_root - trying {}", manifest.display());
             if manifest.exists() {
                 match *self.packages.load(&manifest)?.workspace_config() {
-                    WorkspaceConfig::Root { .. } => {
-                        debug!("find_root - found");
-                        return Ok(Some(manifest))
+                    WorkspaceConfig::Root { ref exclude, ref members } => {
+                        debug!("find_root - found a root checking exclusion");
+                        if !is_excluded(members, exclude, path, manifest_path) {
+                            debug!("find_root - found!");
+                            return Ok(Some(manifest))
+                        }
                     }
                     WorkspaceConfig::Member { root: Some(ref path_to_root) } => {
+                        debug!("find_root - found pointer");
                         return Ok(Some(read_root_pointer(&manifest, path_to_root)?))
                     }
                     WorkspaceConfig::Member { .. } => {}
@@ -304,7 +311,7 @@ impl<'cfg> Workspace<'cfg> {
         let members = {
             let root = self.packages.load(&root_manifest)?;
             match *root.workspace_config() {
-                WorkspaceConfig::Root { ref members } => members.clone(),
+                WorkspaceConfig::Root { ref members, .. } => members.clone(),
                 _ => bail!("root of a workspace inferred but wasn't a root: {}",
                            root_manifest.display()),
             }
@@ -314,14 +321,17 @@ impl<'cfg> Workspace<'cfg> {
             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, false)?;
+                self.find_path_deps(&manifest_path, &root_manifest, false)?;
             }
         }
 
-        self.find_path_deps(&root_manifest, false)
+        self.find_path_deps(&root_manifest, &root_manifest, false)
     }
 
-    fn find_path_deps(&mut self, manifest_path: &Path, is_path_dep: bool) -> CargoResult<()> {
+    fn find_path_deps(&mut self,
+                      manifest_path: &Path,
+                      root_manifest: &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(())
@@ -334,6 +344,16 @@ impl<'cfg> Workspace<'cfg> {
             return Ok(())
         }
 
+        let root = root_manifest.parent().unwrap();
+        match *self.packages.load(root_manifest)?.workspace_config() {
+            WorkspaceConfig::Root { ref members, ref exclude } => {
+                if is_excluded(members, exclude, root, &manifest_path) {
+                    return Ok(())
+                }
+            }
+            _ => {}
+        }
+
         debug!("find_members - {}", manifest_path.display());
         self.members.push(manifest_path.clone());
 
@@ -351,7 +371,7 @@ impl<'cfg> Workspace<'cfg> {
                .collect::<Vec<_>>()
         };
         for candidate in candidates {
-            self.find_path_deps(&candidate, true)?;
+            self.find_path_deps(&candidate, root_manifest, true)?;
         }
         Ok(())
     }
@@ -456,7 +476,7 @@ impl<'cfg> Workspace<'cfg> {
                 MaybePackage::Virtual(_) => members_msg,
                 MaybePackage::Package(ref p) => {
                     let members = match *p.manifest().workspace_config() {
-                        WorkspaceConfig::Root { ref members } => members,
+                        WorkspaceConfig::Root { ref members, .. } => members,
                         WorkspaceConfig::Member { .. } => unreachable!(),
                     };
                     if members.is_none() {
@@ -509,6 +529,27 @@ impl<'cfg> Workspace<'cfg> {
     }
 }
 
+fn is_excluded(members: &Option<Vec<String>>,
+               exclude: &[String],
+               root_path: &Path,
+               manifest_path: &Path) -> bool {
+    let excluded = exclude.iter().any(|ex| {
+        manifest_path.starts_with(root_path.join(ex))
+    });
+
+    let explicit_member = match *members {
+        Some(ref members) => {
+            members.iter().any(|mem| {
+                manifest_path.starts_with(root_path.join(mem))
+            })
+        }
+        None => false,
+    };
+
+    !explicit_member && excluded
+}
+
+
 impl<'cfg> Packages<'cfg> {
     fn get(&self, manifest_path: &Path) -> &MaybePackage {
         &self.packages[manifest_path.parent().unwrap()]

src/cargo/util/toml.rs

@@ -437,6 +437,7 @@ pub struct TomlProject {
 #[derive(Deserialize)]
 pub struct TomlWorkspace {
     members: Option<Vec<String>>,
+    exclude: Option<Vec<String>>,
 }
 
 pub struct TomlVersion {
@@ -784,7 +785,10 @@ impl TomlManifest {
         let workspace_config = match (self.workspace.as_ref(),
                                       project.workspace.as_ref()) {
             (Some(config), None) => {
-                WorkspaceConfig::Root { members: config.members.clone() }
+                WorkspaceConfig::Root {
+                    members: config.members.clone(),
+                    exclude: config.exclude.clone().unwrap_or(Vec::new()),
+                }
             }
             (None, root) => {
                 WorkspaceConfig::Member { root: root.cloned() }
@@ -860,7 +864,10 @@ impl TomlManifest {
         let profiles = build_profiles(&self.profile);
         let workspace_config = match self.workspace {
             Some(ref config) => {
-                WorkspaceConfig::Root { members: config.members.clone() }
+                WorkspaceConfig::Root {
+                    members: config.members.clone(),
+                    exclude: config.exclude.clone().unwrap_or(Vec::new()),
+                }
             }
             None => {
                 bail!("virtual manifests must be configured with [workspace]");

src/doc/manifest.md

@@ -388,6 +388,9 @@ as:
 
 # Optional key, inferred if not present
 members = ["path/to/member1", "path/to/member2"]
+
+# Optional key, empty if not present
+exclude = ["path1", "path/to/dir2"]
 ```
 
 Workspaces were added to Cargo as part [RFC 1525] and have a number of
@@ -410,7 +413,9 @@ 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.
+dependencies included in the workspace. Finally, the `exclude` key can be used
+to blacklist paths from being included in a workspace. This can be useful if
+some path dependencies aren't desired to be in the workspace at all.
 
 The `package.workspace` manifest key (described above) is used in member crates
 to point at a workspace's root crate. If this key is omitted then it is inferred

tests/workspaces.rs

@@ -1264,3 +1264,117 @@ fn test_path_dependency_under_member() {
     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()));
 }
+
+#[test]
+fn excluded_simple() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "ws"
+            version = "0.1.0"
+            authors = []
+
+            [workspace]
+            exclude = ["foo"]
+        "#)
+        .file("src/lib.rs", "")
+        .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"),
+                execs().with_status(0));
+    assert_that(&p.root().join("target"), existing_dir());
+    assert_that(p.cargo("build").cwd(p.root().join("foo")),
+                execs().with_status(0));
+    assert_that(&p.root().join("foo/target"), existing_dir());
+}
+
+#[test]
+fn exclude_members_preferred() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "ws"
+            version = "0.1.0"
+            authors = []
+
+            [workspace]
+            members = ["foo/bar"]
+            exclude = ["foo"]
+        "#)
+        .file("src/lib.rs", "")
+        .file("foo/Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("foo/src/lib.rs", "")
+        .file("foo/bar/Cargo.toml", r#"
+            [project]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("foo/bar/src/lib.rs", "");
+    p.build();
+
+    assert_that(p.cargo("build"),
+                execs().with_status(0));
+    assert_that(&p.root().join("target"), existing_dir());
+    assert_that(p.cargo("build").cwd(p.root().join("foo")),
+                execs().with_status(0));
+    assert_that(&p.root().join("foo/target"), existing_dir());
+    assert_that(p.cargo("build").cwd(p.root().join("foo/bar")),
+                execs().with_status(0));
+    assert_that(&p.root().join("foo/bar/target"), is_not(existing_dir()));
+}
+
+#[test]
+fn exclude_but_also_depend() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "ws"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            bar = { path = "foo/bar" }
+
+            [workspace]
+            exclude = ["foo"]
+        "#)
+        .file("src/lib.rs", "")
+        .file("foo/Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("foo/src/lib.rs", "")
+        .file("foo/bar/Cargo.toml", r#"
+            [project]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("foo/bar/src/lib.rs", "");
+    p.build();
+
+    assert_that(p.cargo("build"),
+                execs().with_status(0));
+    assert_that(&p.root().join("target"), existing_dir());
+    assert_that(p.cargo("build").cwd(p.root().join("foo")),
+                execs().with_status(0));
+    assert_that(&p.root().join("foo/target"), existing_dir());
+    assert_that(p.cargo("build").cwd(p.root().join("foo/bar")),
+                execs().with_status(0));
+    assert_that(&p.root().join("foo/bar/target"), existing_dir());
+}