Skip to content

Add a workspace.exclude key #3837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 23, 2017
Merged

Add a workspace.exclude key #3837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 51 additions & 10 deletions src/cargo/core/workspace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Copy link
Member

@matklad matklad Mar 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should check that exclude is a relative path, to be conservative? Absolute paths probably do not make sense in this context.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could yeah, but we don't have checks like that for members and other such keys. I could imagine that in debugging situations it's useful to have absolute paths (although certainly never committed)

},

/// Indicates that `[workspace]` was present and the `root` field is the
/// optional value of `package.workspace`, if present.
Expand Down Expand Up @@ -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 { .. } => {}
Expand Down Expand Up @@ -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()),
}
Expand All @@ -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(())
Expand All @@ -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());

Expand All @@ -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(())
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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()]
Expand Down
11 changes: 9 additions & 2 deletions src/cargo/util/toml.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ pub struct TomlProject {
#[derive(Deserialize)]
pub struct TomlWorkspace {
members: Option<Vec<String>>,
exclude: Option<Vec<String>>,
}

pub struct TomlVersion {
Expand Down Expand Up @@ -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() }
Expand Down Expand Up @@ -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]");
Expand Down
7 changes: 6 additions & 1 deletion src/doc/manifest.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
114 changes: 114 additions & 0 deletions tests/workspaces.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}