[core/workspace] Create WorkspaceRootConfig

src/cargo/core/mod.rs

@@ -10,7 +10,7 @@ pub use self::resolver::Resolve;
 pub use self::shell::{Shell, Verbosity};
 pub use self::source::{Source, SourceId, SourceMap, GitReference};
 pub use self::summary::Summary;
-pub use self::workspace::{Members, Workspace, WorkspaceConfig};
+pub use self::workspace::{Members, Workspace, WorkspaceConfig, WorkspaceRootConfig};
 
 pub mod source;
 pub mod package;

src/cargo/core/workspace.rs

@@ -75,16 +75,24 @@ enum MaybePackage {
 pub enum WorkspaceConfig {
     /// Indicates that `[workspace]` was present and the members were
     /// optionally specified as well.
-    Root {
-        members: Option<Vec<String>>,
-        exclude: Vec<String>,
-    },
+    Root(WorkspaceRootConfig),
 
     /// Indicates that `[workspace]` was present and the `root` field is the
     /// optional value of `package.workspace`, if present.
     Member { root: Option<String> },
 }
 
+/// Intermediate configuration of a workspace root in a manifest.
+///
+/// Knows the Workspace Root path, as well as `members` and `exclude` lists of path patterns, which
+/// together tell if some path is recognized as a member by this root or not.
+#[derive(Debug, Clone)]
+pub struct WorkspaceRootConfig {
+    root_dir: PathBuf,
+    members: Option<Vec<String>>,
+    exclude: Vec<String>,
+}
+
 /// An iterator over the member packages of a workspace, returned by
 /// `Workspace::members`
 pub struct Members<'a, 'cfg: 'a> {
@@ -202,7 +210,7 @@ impl<'cfg> Workspace<'cfg> {
         let root = self.root_manifest.as_ref().unwrap_or(&self.current_manifest);
         match *self.packages.get(root) {
             MaybePackage::Package(ref p) => p.manifest().profiles(),
-            MaybePackage::Virtual(ref m) => m.profiles(),
+            MaybePackage::Virtual(ref vm) => vm.profiles(),
         }
     }
 
@@ -233,7 +241,7 @@ impl<'cfg> Workspace<'cfg> {
         };
         match *self.packages.get(path) {
             MaybePackage::Package(ref p) => p.manifest().replace(),
-            MaybePackage::Virtual(ref v) => v.replace(),
+            MaybePackage::Virtual(ref vm) => vm.replace(),
         }
     }
 
@@ -247,7 +255,7 @@ impl<'cfg> Workspace<'cfg> {
         };
         match *self.packages.get(path) {
             MaybePackage::Package(ref p) => p.manifest().patch(),
-            MaybePackage::Virtual(ref v) => v.patch(),
+            MaybePackage::Virtual(ref vm) => vm.patch(),
         }
     }
 
@@ -289,7 +297,7 @@ impl<'cfg> Workspace<'cfg> {
         {
             let current = self.packages.load(manifest_path)?;
             match *current.workspace_config() {
-                WorkspaceConfig::Root { .. } => {
+                WorkspaceConfig::Root(_) => {
                     debug!("find_root - is root {}", manifest_path.display());
                     return Ok(Some(manifest_path.to_path_buf()))
                 }
@@ -301,20 +309,20 @@ impl<'cfg> Workspace<'cfg> {
         }
 
         for path in paths::ancestors(manifest_path).skip(2) {
-            let manifest = path.join("Cargo.toml");
-            debug!("find_root - trying {}", manifest.display());
-            if manifest.exists() {
-                match *self.packages.load(&manifest)?.workspace_config() {
-                    WorkspaceConfig::Root { ref exclude, ref members } => {
+            let ances_manifest_path = path.join("Cargo.toml");
+            debug!("find_root - trying {}", ances_manifest_path.display());
+            if ances_manifest_path.exists() {
+                match *self.packages.load(&ances_manifest_path)?.workspace_config() {
+                    WorkspaceConfig::Root(ref ances_root_config) => {
                         debug!("find_root - found a root checking exclusion");
-                        if !is_excluded(members, exclude, path, manifest_path) {
+                        if !ances_root_config.is_excluded(&manifest_path) {
                             debug!("find_root - found!");
-                            return Ok(Some(manifest))
+                            return Ok(Some(ances_manifest_path))
                         }
                     }
                     WorkspaceConfig::Member { root: Some(ref path_to_root) } => {
                         debug!("find_root - found pointer");
-                        return Ok(Some(read_root_pointer(&manifest, path_to_root)?))
+                        return Ok(Some(read_root_pointer(&ances_manifest_path, path_to_root)?))
                     }
                     WorkspaceConfig::Member { .. } => {}
                 }
@@ -332,47 +340,29 @@ impl<'cfg> Workspace<'cfg> {
     /// will transitively follow all `path` dependencies looking for members of
     /// the workspace.
     fn find_members(&mut self) -> CargoResult<()> {
-        let root_manifest = match self.root_manifest {
+        let root_manifest_path = match self.root_manifest {
             Some(ref path) => path.clone(),
             None => {
                 debug!("find_members - only me as a member");
                 self.members.push(self.current_manifest.clone());
                 return Ok(())
             }
         };
-        let members = {
-            let root = self.packages.load(&root_manifest)?;
-            match *root.workspace_config() {
-                WorkspaceConfig::Root { ref members, .. } => members.clone(),
+
+        let members_paths = {
+            let root_package = self.packages.load(&root_manifest_path)?;
+            match *root_package.workspace_config() {
+                WorkspaceConfig::Root(ref root_config) => root_config.members_paths()?,
                 _ => bail!("root of a workspace inferred but wasn't a root: {}",
-                           root_manifest.display()),
+                           root_manifest_path.display()),
             }
         };
 
-        if let Some(list) = members {
-            let root = root_manifest.parent().unwrap();
-
-            let mut expanded_list = Vec::new();
-            for path in list {
-                let pathbuf = root.join(path);
-                let expanded_paths = expand_member_path(&pathbuf)?;
-
-                // If glob does not find any valid paths, then put the original
-                // path in the expanded list to maintain backwards compatibility.
-                if expanded_paths.is_empty() {
-                    expanded_list.push(pathbuf);
-                } else {
-                    expanded_list.extend(expanded_paths);
-                }
-            }
-
-            for path in expanded_list {
-                let manifest_path = path.join("Cargo.toml");
-                self.find_path_deps(&manifest_path, &root_manifest, false)?;
-            }
+        for path in members_paths {
+            self.find_path_deps(&path.join("Cargo.toml"), &root_manifest_path, false)?;
         }
 
-        self.find_path_deps(&root_manifest, &root_manifest, false)
+        self.find_path_deps(&root_manifest_path, &root_manifest_path, false)
     }
 
     fn find_path_deps(&mut self,
@@ -391,10 +381,9 @@ 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) {
+            WorkspaceConfig::Root(ref root_config) => {
+                if root_config.is_excluded(&manifest_path) {
                     return Ok(())
                 }
             }
@@ -439,7 +428,7 @@ impl<'cfg> Workspace<'cfg> {
             for member in self.members.iter() {
                 let package = self.packages.get(member);
                 match *package.workspace_config() {
-                    WorkspaceConfig::Root { .. } => {
+                    WorkspaceConfig::Root(_) => {
                         roots.push(member.parent().unwrap().to_path_buf());
                     }
                     WorkspaceConfig::Member { .. } => {}
@@ -505,6 +494,8 @@ impl<'cfg> Workspace<'cfg> {
             let current_dir = self.current_manifest.parent().unwrap();
             let root_pkg = self.packages.get(root);
 
+            // FIXME: Make this more generic by using a relative path resolver between member and
+            // root.
             let members_msg = match current_dir.strip_prefix(root_dir) {
                 Ok(rel) => {
                     format!("this may be fixable by adding `{}` to the \
@@ -522,11 +513,11 @@ impl<'cfg> Workspace<'cfg> {
             let extra = match *root_pkg {
                 MaybePackage::Virtual(_) => members_msg,
                 MaybePackage::Package(ref p) => {
-                    let members = match *p.manifest().workspace_config() {
-                        WorkspaceConfig::Root { ref members, .. } => members,
+                    let has_members_list = match *p.manifest().workspace_config() {
+                        WorkspaceConfig::Root(ref root_config) => root_config.has_members_list(),
                         WorkspaceConfig::Member { .. } => unreachable!(),
                     };
-                    if members.is_none() {
+                    if !has_members_list {
                         format!("this may be fixable by ensuring that this \
                                  crate is depended on by the workspace \
                                  root: {}", root.display())
@@ -576,41 +567,6 @@ impl<'cfg> Workspace<'cfg> {
     }
 }
 
-fn expand_member_path(path: &Path) -> CargoResult<Vec<PathBuf>> {
-    let path = match path.to_str() {
-        Some(p) => p,
-        None => return Ok(Vec::new()),
-    };
-    let res = glob(path).chain_err(|| {
-        format!("could not parse pattern `{}`", &path)
-    })?;
-    res.map(|p| {
-        p.chain_err(|| {
-            format!("unable to match path to pattern `{}`", &path)
-        })
-    }).collect()
-}
-
-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 {
@@ -627,11 +583,10 @@ impl<'cfg> Packages<'cfg> {
                     read_manifest(manifest_path, &source_id, self.config)?;
                 Ok(v.insert(match manifest {
                     EitherManifest::Real(manifest) => {
-                        MaybePackage::Package(Package::new(manifest,
-                                                           manifest_path))
+                        MaybePackage::Package(Package::new(manifest, manifest_path))
                     }
-                    EitherManifest::Virtual(v) => {
-                        MaybePackage::Virtual(v)
+                    EitherManifest::Virtual(vm) => {
+                        MaybePackage::Virtual(vm)
                     }
                 }))
             }
@@ -665,8 +620,83 @@ impl<'a, 'cfg> Iterator for Members<'a, 'cfg> {
 impl MaybePackage {
     fn workspace_config(&self) -> &WorkspaceConfig {
         match *self {
-            MaybePackage::Virtual(ref v) => v.workspace_config(),
-            MaybePackage::Package(ref v) => v.manifest().workspace_config(),
+            MaybePackage::Package(ref p) => p.manifest().workspace_config(),
+            MaybePackage::Virtual(ref vm) => vm.workspace_config(),
         }
     }
 }
+
+impl WorkspaceRootConfig {
+    /// Create a new Intermediate Workspace Root configuration.
+    pub fn new(
+        root_dir: &Path,
+        members: &Option<Vec<String>>,
+        exclude: &Option<Vec<String>>,
+    ) -> WorkspaceRootConfig {
+        WorkspaceRootConfig {
+            root_dir: root_dir.to_path_buf(),
+            members: members.clone(),
+            exclude: exclude.clone().unwrap_or_default(),
+        }
+    }
+
+    /// Checks the path against the `excluded` list.
+    ///
+    /// This method does NOT consider the `members` list.
+    fn is_excluded(&self, manifest_path: &Path) -> bool {
+        let excluded = self.exclude.iter().any(|ex| {
+            manifest_path.starts_with(self.root_dir.join(ex))
+        });
+
+        let explicit_member = match self.members {
+            Some(ref members) => {
+                members.iter().any(|mem| {
+                    manifest_path.starts_with(self.root_dir.join(mem))
+                })
+            }
+            None => false,
+        };
+
+        !explicit_member && excluded
+    }
+
+    fn has_members_list(&self) -> bool {
+        self.members.is_some()
+    }
+
+    fn members_paths(&self) -> CargoResult<Vec<PathBuf>> {
+        let mut expanded_list = Vec::new();
+
+        if let Some(globs) = self.members.clone() {
+            for glob in globs {
+                let pathbuf = self.root_dir.join(glob);
+                let expanded_paths = Self::expand_member_path(&pathbuf)?;
+
+                // If glob does not find any valid paths, then put the original
+                // path in the expanded list to maintain backwards compatibility.
+                if expanded_paths.is_empty() {
+                    expanded_list.push(pathbuf);
+                } else {
+                    expanded_list.extend(expanded_paths);
+                }
+            }
+        }
+
+        Ok(expanded_list)
+    }
+
+    fn expand_member_path(path: &Path) -> CargoResult<Vec<PathBuf>> {
+        let path = match path.to_str() {
+            Some(p) => p,
+            None => return Ok(Vec::new()),
+        };
+        let res = glob(path).chain_err(|| {
+            format!("could not parse pattern `{}`", &path)
+        })?;
+        res.map(|p| {
+            p.chain_err(|| {
+                format!("unable to match path to pattern `{}`", &path)
+            })
+        }).collect()
+    }
+}

src/cargo/util/toml/mod.rs

@@ -12,7 +12,7 @@ use serde_ignored;
 use toml;
 use url::Url;
 
-use core::{SourceId, Profiles, PackageIdSpec, GitReference, WorkspaceConfig};
+use core::{SourceId, Profiles, PackageIdSpec, GitReference, WorkspaceConfig, WorkspaceRootConfig};
 use core::{Summary, Manifest, Target, Dependency, PackageId};
 use core::{EitherManifest, VirtualManifest, Features};
 use core::dependency::{Kind, Platform};
@@ -634,10 +634,9 @@ impl TomlManifest {
         let workspace_config = match (me.workspace.as_ref(),
                                       project.workspace.as_ref()) {
             (Some(config), None) => {
-                WorkspaceConfig::Root {
-                    members: config.members.clone(),
-                    exclude: config.exclude.clone().unwrap_or_default(),
-                }
+                WorkspaceConfig::Root(
+                    WorkspaceRootConfig::new(&package_root, &config.members, &config.exclude)
+                )
             }
             (None, root) => {
                 WorkspaceConfig::Member { root: root.cloned() }
@@ -728,10 +727,9 @@ impl TomlManifest {
         let profiles = build_profiles(&me.profile);
         let workspace_config = match me.workspace {
             Some(ref config) => {
-                WorkspaceConfig::Root {
-                    members: config.members.clone(),
-                    exclude: config.exclude.clone().unwrap_or_default(),
-                }
+                WorkspaceConfig::Root(
+                    WorkspaceRootConfig::new(&root, &config.members, &config.exclude)
+                )
             }
             None => {
                 bail!("virtual manifests must be configured with [workspace]");