Skip to content

Commit 04621e2

Browse files
borsbors
committed
Auto merge of #12779 - calavera:workspace_adds_implicit_members, r=epage
Add new packages to [workspace.members] automatically ### What does this PR try to resolve? If a new package is created in a workspace, this change adds the package's path to the workspace's members list automatically. It doesn't add the package to the list if the path is in the workspace's exclude list, or if the members list doesn't exist already. I noticed that a `cargo_new` test broke if I create the members list when it doesn't exist. This is because the workspace's manifest can be used for package templating. I think it's better to not break that use case. Fixes #6378 ### How should we test and review this PR? I've included tests in the `cargo_new` suite.
2 parents d1830f6 + 1a8bfdf commit 04621e2

File tree

65 files changed

+462
-49
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+462
-49
lines changed

‎Cargo.lock

+6-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ tempfile = "3.8.0"
9292
thiserror = "1.0.49"
9393
time = { version = "0.3", features = ["parsing", "formatting", "serde"] }
9494
toml = "0.8.2"
95-
toml_edit = { version = "0.20.2", features = ["serde"] }
95+
toml_edit = { version = "0.20.7", features = ["serde"] }
9696
tracing = "0.1.37"
9797
tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
9898
unicase = "2.7.0"

‎src/cargo/ops/cargo_add/mod.rs

+1-16
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use crate::util::toml_mut::dependency::MaybeWorkspace;
3131
use crate::util::toml_mut::dependency::PathSource;
3232
use crate::util::toml_mut::dependency::Source;
3333
use crate::util::toml_mut::dependency::WorkspaceSource;
34+
use crate::util::toml_mut::is_sorted;
3435
use crate::util::toml_mut::manifest::DepTable;
3536
use crate::util::toml_mut::manifest::LocalManifest;
3637
use crate::util::RustVersion;
@@ -1004,22 +1005,6 @@ fn format_features_version_suffix(dep: &DependencyUI) -> String {
10041005
}
10051006
}
10061007

1007-
// Based on Iterator::is_sorted from nightly std; remove in favor of that when stabilized.
1008-
fn is_sorted(mut it: impl Iterator<Item = impl PartialOrd>) -> bool {
1009-
let Some(mut last) = it.next() else {
1010-
return true;
1011-
};
1012-
1013-
for curr in it {
1014-
if curr < last {
1015-
return false;
1016-
}
1017-
last = curr;
1018-
}
1019-
1020-
true
1021-
}
1022-
10231008
fn find_workspace_dep(toml_key: &str, root_manifest: &Path) -> CargoResult<Dependency> {
10241009
let manifest = LocalManifest::try_new(root_manifest)?;
10251010
let manifest = manifest

‎src/cargo/ops/cargo_new.rs

+104-3
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
use crate::core::{Edition, Shell, Workspace};
22
use crate::util::errors::CargoResult;
33
use crate::util::important_paths::find_root_manifest_for_wd;
4+
use crate::util::toml_mut::is_sorted;
45
use crate::util::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo};
56
use crate::util::{restricted_names, Config};
6-
use anyhow::{anyhow, Context as _};
7-
use cargo_util::paths;
7+
use anyhow::{anyhow, Context};
8+
use cargo_util::paths::{self, write_atomic};
89
use serde::de;
910
use serde::Deserialize;
1011
use std::collections::BTreeMap;
@@ -13,6 +14,7 @@ use std::io::{BufRead, BufReader, ErrorKind};
1314
use std::path::{Path, PathBuf};
1415
use std::str::FromStr;
1516
use std::{fmt, slice};
17+
use toml_edit::{Array, Value};
1618

1719
#[derive(Clone, Copy, Debug, PartialEq)]
1820
pub enum VersionControl {
@@ -809,7 +811,7 @@ fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
809811
// Sometimes the root manifest is not a valid manifest, so we only try to parse it if it is.
810812
// This should not block the creation of the new project. It is only a best effort to
811813
// inherit the workspace package keys.
812-
if let Ok(workspace_document) = root_manifest.parse::<toml_edit::Document>() {
814+
if let Ok(mut workspace_document) = root_manifest.parse::<toml_edit::Document>() {
813815
if let Some(workspace_package_keys) = workspace_document
814816
.get("workspace")
815817
.and_then(|workspace| workspace.get("package"))
@@ -832,6 +834,13 @@ fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
832834
table["workspace"] = toml_edit::value(true);
833835
manifest["lints"] = toml_edit::Item::Table(table);
834836
}
837+
838+
// Try to add the new package to the workspace members.
839+
update_manifest_with_new_member(
840+
&root_manifest_path,
841+
&mut workspace_document,
842+
opts.path,
843+
)?;
835844
}
836845
}
837846

@@ -931,3 +940,95 @@ fn update_manifest_with_inherited_workspace_package_keys(
931940
try_remove_and_inherit_package_key(key, manifest);
932941
}
933942
}
943+
944+
/// Adds the new package member to the [workspace.members] array.
945+
/// - It first checks if the name matches any element in [workspace.exclude],
946+
/// and it ignores the name if there is a match.
947+
/// - Then it check if the name matches any element already in [workspace.members],
948+
/// and it ignores the name if there is a match.
949+
/// - If [workspace.members] doesn't exist in the manifest, it will add a new section
950+
/// with the new package in it.
951+
fn update_manifest_with_new_member(
952+
root_manifest_path: &Path,
953+
workspace_document: &mut toml_edit::Document,
954+
package_path: &Path,
955+
) -> CargoResult<()> {
956+
// Find the relative path for the package from the workspace root directory.
957+
let workspace_root = root_manifest_path.parent().with_context(|| {
958+
format!(
959+
"workspace root manifest doesn't have a parent directory `{}`",
960+
root_manifest_path.display()
961+
)
962+
})?;
963+
let relpath = pathdiff::diff_paths(package_path, workspace_root).with_context(|| {
964+
format!(
965+
"path comparison requires two absolute paths; package_path: `{}`, workspace_path: `{}`",
966+
package_path.display(),
967+
workspace_root.display()
968+
)
969+
})?;
970+
971+
let mut components = Vec::new();
972+
for comp in relpath.iter() {
973+
let comp = comp.to_str().with_context(|| {
974+
format!("invalid unicode component in path `{}`", relpath.display())
975+
})?;
976+
components.push(comp);
977+
}
978+
let display_path = components.join("/");
979+
980+
// Don't add the new package to the workspace's members
981+
// if there is an exclusion match for it.
982+
if let Some(exclude) = workspace_document
983+
.get("workspace")
984+
.and_then(|workspace| workspace.get("exclude"))
985+
.and_then(|exclude| exclude.as_array())
986+
{
987+
for member in exclude {
988+
let pat = member
989+
.as_str()
990+
.with_context(|| format!("invalid non-string exclude path `{}`", member))?;
991+
if pat == display_path {
992+
return Ok(());
993+
}
994+
}
995+
}
996+
997+
// If the members element already exist, check if one of the patterns
998+
// in the array already includes the new package's relative path.
999+
// - Add the relative path if the members don't match the new package's path.
1000+
// - Create a new members array if there are no members element in the workspace yet.
1001+
if let Some(members) = workspace_document
1002+
.get_mut("workspace")
1003+
.and_then(|workspace| workspace.get_mut("members"))
1004+
.and_then(|members| members.as_array_mut())
1005+
{
1006+
for member in members.iter() {
1007+
let pat = member
1008+
.as_str()
1009+
.with_context(|| format!("invalid non-string member `{}`", member))?;
1010+
let pattern = glob::Pattern::new(pat)
1011+
.with_context(|| format!("cannot build glob pattern from `{}`", pat))?;
1012+
1013+
if pattern.matches(&display_path) {
1014+
return Ok(());
1015+
}
1016+
}
1017+
1018+
let was_sorted = is_sorted(members.iter().map(Value::as_str));
1019+
members.push(&display_path);
1020+
if was_sorted {
1021+
members.sort_by(|lhs, rhs| lhs.as_str().cmp(&rhs.as_str()));
1022+
}
1023+
} else {
1024+
let mut array = Array::new();
1025+
array.push(&display_path);
1026+
1027+
workspace_document["workspace"]["members"] = toml_edit::value(array);
1028+
}
1029+
1030+
write_atomic(
1031+
&root_manifest_path,
1032+
workspace_document.to_string().to_string().as_bytes(),
1033+
)
1034+
}

‎src/cargo/util/toml_mut/mod.rs

+16
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,19 @@
1111
1212
pub mod dependency;
1313
pub mod manifest;
14+
15+
// Based on Iterator::is_sorted from nightly std; remove in favor of that when stabilized.
16+
pub fn is_sorted(mut it: impl Iterator<Item = impl PartialOrd>) -> bool {
17+
let Some(mut last) = it.next() else {
18+
return true;
19+
};
20+
21+
for curr in it {
22+
if curr < last {
23+
return false;
24+
}
25+
last = curr;
26+
}
27+
28+
true
29+
}

‎tests/testsuite/cargo_init/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,4 @@ mod simple_hg_ignore_exists;
4242
mod simple_lib;
4343
mod unknown_flags;
4444
mod with_argument;
45+
mod workspace_add_member;

‎tests/testsuite/cargo_init/workspace_add_member/in/Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[workspace]
2+
resolver = "2"

‎tests/testsuite/cargo_init/empty_dir/.keep ‎tests/testsuite/cargo_init/workspace_add_member/in/crates/foo/.keep

File renamed without changes.

‎tests/testsuite/cargo_init/workspace_add_member/mod.rs

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
use cargo_test_support::compare::assert_ui;
2+
use cargo_test_support::prelude::*;
3+
use cargo_test_support::Project;
4+
5+
use cargo_test_support::curr_dir;
6+
7+
#[cargo_test]
8+
fn case() {
9+
let project = Project::from_template(curr_dir!().join("in"));
10+
let project_root = &project.root();
11+
12+
snapbox::cmd::Command::cargo_ui()
13+
.arg_line("init --bin --vcs none")
14+
.current_dir(project_root.join("crates").join("foo"))
15+
.assert()
16+
.success()
17+
.stdout_matches_path(curr_dir!().join("stdout.log"))
18+
.stderr_matches_path(curr_dir!().join("stderr.log"));
19+
20+
assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
21+
}

‎tests/testsuite/cargo_init/workspace_add_member/out/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[workspace]
2+
resolver = "2"
3+
members = ["crates/foo"]

‎tests/testsuite/cargo_init/workspace_add_member/out/crates/foo/Cargo.toml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[package]
2+
name = "foo"
3+
version = "0.1.0"
4+
edition = "2021"
5+
6+
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
7+
8+
[dependencies]

‎tests/testsuite/cargo_init/workspace_add_member/out/crates/foo/src/main.rs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
fn main() {
2+
println!("Hello, world!");
3+
}

‎tests/testsuite/cargo_init/workspace_add_member/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Created binary (application) package

‎tests/testsuite/cargo_init/workspace_add_member/stdout.log

Whitespace-only changes.

‎tests/testsuite/cargo_new/add_members_to_workspace_format_previous_items/in/Cargo.toml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
[workspace]
2+
resolver = "2"
3+
4+
[workspace.package]
5+
authors = ["Rustaceans"]

‎tests/testsuite/cargo_new/add_members_to_workspace_format_previous_items/mod.rs

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
use cargo_test_support::compare::assert_ui;
2+
use cargo_test_support::curr_dir;
3+
use cargo_test_support::CargoCommand;
4+
use cargo_test_support::Project;
5+
6+
#[cargo_test]
7+
fn case() {
8+
let project = Project::from_template(curr_dir!().join("in"));
9+
let project_root = project.root();
10+
let cwd = &project_root;
11+
12+
snapbox::cmd::Command::cargo_ui()
13+
.arg("new")
14+
.args(["crates/foo"])
15+
.current_dir(cwd)
16+
.assert()
17+
.success()
18+
.stdout_matches_path(curr_dir!().join("stdout.log"))
19+
.stderr_matches_path(curr_dir!().join("stderr.log"));
20+
21+
assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
22+
}

‎tests/testsuite/cargo_new/add_members_to_workspace_format_previous_items/out/Cargo.toml

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[workspace]
2+
resolver = "2"
3+
members = ["crates/foo"]
4+
5+
[workspace.package]
6+
authors = ["Rustaceans"]

‎tests/testsuite/cargo_new/add_members_to_workspace_format_previous_items/out/crates/foo/Cargo.toml

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[package]
2+
name = "foo"
3+
version = "0.1.0"
4+
edition = "2021"
5+
authors.workspace = true
6+
7+
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
8+
9+
[dependencies]

‎tests/testsuite/cargo_new/add_members_to_workspace_format_previous_items/out/crates/foo/src/main.rs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
fn main() {
2+
println!("Hello, world!");
3+
}

‎tests/testsuite/cargo_new/add_members_to_workspace_format_previous_items/stderr.log

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Created binary (application) `crates/foo` package

‎tests/testsuite/cargo_new/add_members_to_workspace_format_previous_items/stdout.log

Whitespace-only changes.

‎tests/testsuite/cargo_new/add_members_to_workspace_format_sorted/in/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[workspace]
2+
resolver = "2"
3+
members = ["crates/bar", "crates/qux"]

‎tests/testsuite/cargo_new/add_members_to_workspace_format_sorted/in/crates/bar/Cargo.toml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[package]
2+
name = "bar"
3+
version = "0.1.0"
4+
edition = "2021"
5+
6+
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
7+
8+
[dependencies]

‎tests/testsuite/cargo_new/add_members_to_workspace_format_sorted/in/crates/bar/src/main.rs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
fn main() {
2+
println!("Hello, world!");
3+
}

‎tests/testsuite/cargo_new/add_members_to_workspace_format_sorted/in/crates/qux/Cargo.toml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[package]
2+
name = "qux"
3+
version = "0.1.0"
4+
edition = "2021"
5+
6+
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
7+
8+
[dependencies]

‎tests/testsuite/cargo_new/add_members_to_workspace_format_sorted/in/crates/qux/src/main.rs

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
fn main() {
2+
println!("Hello, world!");
3+
}

‎tests/testsuite/cargo_new/add_members_to_workspace_format_sorted/mod.rs

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
use cargo_test_support::compare::assert_ui;
2+
use cargo_test_support::curr_dir;
3+
use cargo_test_support::CargoCommand;
4+
use cargo_test_support::Project;
5+
6+
#[cargo_test]
7+
fn case() {
8+
let project = Project::from_template(curr_dir!().join("in"));
9+
let project_root = project.root();
10+
let cwd = &project_root;
11+
12+
snapbox::cmd::Command::cargo_ui()
13+
.arg("new")
14+
.args(["crates/foo"])
15+
.current_dir(cwd)
16+
.assert()
17+
.success()
18+
.stdout_matches_path(curr_dir!().join("stdout.log"))
19+
.stderr_matches_path(curr_dir!().join("stderr.log"));
20+
21+
assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
22+
}

‎tests/testsuite/cargo_new/add_members_to_workspace_format_sorted/out/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[workspace]
2+
resolver = "2"
3+
members = ["crates/bar", "crates/foo", "crates/qux"]

0 commit comments

Comments
 (0)