diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 2eb0c608b02..6b4e3906f8e 100644 --- a/src/cargo/core/mod.rs +++ b/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; diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 9c012bf221a..58b1412696e 100644 --- a/src/cargo/core/workspace.rs +++ b/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>, - exclude: Vec, - }, + Root(WorkspaceRootConfig), /// Indicates that `[workspace]` was present and the `root` field is the /// optional value of `package.workspace`, if present. Member { root: Option }, } +/// 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>, + exclude: Vec, +} + /// 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,7 +340,7 @@ 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"); @@ -340,39 +348,21 @@ impl<'cfg> Workspace<'cfg> { 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> { - 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>, - 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>, + exclude: &Option>, + ) -> 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> { + 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> { + 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() + } +} diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index e9e7fe8aeee..32122444d2e 100644 --- a/src/cargo/util/toml/mod.rs +++ b/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]"); diff --git a/tests/workspaces.rs b/tests/workspaces.rs index 2e2dc1b32a1..975d61f39fe 100644 --- a/tests/workspaces.rs +++ b/tests/workspaces.rs @@ -1432,6 +1432,72 @@ fn glob_syntax() { assert_that(&p.root().join("crates/qux/Cargo.lock"), existing_file()); } +/*FIXME: This fails because of how workspace.exclude and workspace.members are working. +#[test] +fn glob_syntax_2() { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.1.0" + authors = [] + + [workspace] + members = ["crates/b*"] + exclude = ["crates/q*"] + "#) + .file("src/main.rs", "fn main() {}") + .file("crates/bar/Cargo.toml", r#" + [project] + name = "bar" + version = "0.1.0" + authors = [] + workspace = "../.." + "#) + .file("crates/bar/src/main.rs", "fn main() {}") + .file("crates/baz/Cargo.toml", r#" + [project] + name = "baz" + version = "0.1.0" + authors = [] + workspace = "../.." + "#) + .file("crates/baz/src/main.rs", "fn main() {}") + .file("crates/qux/Cargo.toml", r#" + [project] + name = "qux" + version = "0.1.0" + authors = [] + "#) + .file("crates/qux/src/main.rs", "fn main() {}"); + p.build(); + + assert_that(p.cargo("build"), execs().with_status(0)); + assert_that(&p.bin("foo"), existing_file()); + assert_that(&p.bin("bar"), is_not(existing_file())); + assert_that(&p.bin("baz"), is_not(existing_file())); + + assert_that(p.cargo("build").cwd(p.root().join("crates/bar")), + execs().with_status(0)); + assert_that(&p.bin("foo"), existing_file()); + assert_that(&p.bin("bar"), existing_file()); + + assert_that(p.cargo("build").cwd(p.root().join("crates/baz")), + execs().with_status(0)); + assert_that(&p.bin("foo"), existing_file()); + assert_that(&p.bin("baz"), existing_file()); + + assert_that(p.cargo("build").cwd(p.root().join("crates/qux")), + execs().with_status(0)); + assert_that(&p.bin("qux"), is_not(existing_file())); + + assert_that(&p.root().join("Cargo.lock"), existing_file()); + assert_that(&p.root().join("crates/bar/Cargo.lock"), is_not(existing_file())); + assert_that(&p.root().join("crates/baz/Cargo.lock"), is_not(existing_file())); + assert_that(&p.root().join("crates/qux/Cargo.lock"), existing_file()); +} +*/ + #[test] fn glob_syntax_invalid_members() { let p = project("foo") @@ -1551,3 +1617,38 @@ fn dep_used_with_separate_features() { [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ")); } + +/*FIXME: This fails because of how workspace.exclude and workspace.members are working. +#[test] +fn include_and_exclude() { + let p = project("foo") + .file("Cargo.toml", r#" + [workspace] + members = ["foo"] + exclude = ["foo/bar"] + "#) + .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").cwd(p.root().join("foo")), + execs().with_status(0)); + assert_that(&p.root().join("target"), existing_dir()); + assert_that(&p.root().join("foo/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/target"), existing_dir()); +} +*/