Add new packages to [workspace.members] automatically

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 exclude list, or if the members list doesn't exist already.

Signed-off-by: David Calavera <[email protected]>

src/cargo/ops/cargo_compile/packages.rs

@@ -4,10 +4,10 @@ use std::collections::BTreeSet;
 
 use crate::core::Package;
 use crate::core::{PackageIdSpec, Workspace};
-use crate::util::restricted_names::is_glob_pattern;
+use crate::util::restricted_names::{build_glob, is_glob_pattern};
 use crate::util::CargoResult;
 
-use anyhow::{bail, Context as _};
+use anyhow::bail;
 
 /// Represents the selected packages that will be built.
 ///
@@ -213,8 +213,3 @@ fn match_patterns(pkg: &Package, patterns: &mut Vec<(glob::Pattern, bool)>) -> b
         is_matched
     })
 }
-
-/// Build [`glob::Pattern`] with informative context.
-pub fn build_glob(pat: &str) -> CargoResult<glob::Pattern> {
-    glob::Pattern::new(pat).with_context(|| format!("cannot build glob pattern from `{}`", pat))
-}

src/cargo/ops/cargo_compile/unit_generator.rs

@@ -12,11 +12,10 @@ use crate::core::resolver::features::{self, FeaturesFor};
 use crate::core::resolver::{HasDevUnits, Resolve};
 use crate::core::{FeatureValue, Package, PackageSet, Summary, Target};
 use crate::core::{TargetKind, Workspace};
-use crate::util::restricted_names::is_glob_pattern;
+use crate::util::restricted_names::{build_glob, is_glob_pattern};
 use crate::util::{closest_msg, CargoResult};
 
 use super::compile_filter::{CompileFilter, FilterRule, LibRule};
-use super::packages::build_glob;
 
 /// A proposed target.
 ///

src/cargo/ops/cargo_new.rs

@@ -1,6 +1,7 @@
 use crate::core::{Edition, Shell, Workspace};
 use crate::util::errors::CargoResult;
 use crate::util::important_paths::find_root_manifest_for_wd;
+use crate::util::restricted_names::build_glob;
 use crate::util::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo};
 use crate::util::{restricted_names, Config};
 use anyhow::{anyhow, Context as _};
@@ -55,6 +56,7 @@ pub struct NewOptions {
     pub auto_detect_kind: bool,
     /// Absolute path to the directory for the new package
     pub path: PathBuf,
+    pub relative_path: Option<String>,
     pub name: Option<String>,
     pub edition: Option<String>,
     pub registry: Option<String>,
@@ -92,6 +94,7 @@ struct MkOptions<'a> {
     version_control: Option<VersionControl>,
     path: &'a Path,
     name: &'a str,
+    relative_path: &'a str,
     source_files: Vec<SourceFileInformation>,
     edition: Option<&'a str>,
     registry: Option<&'a str>,
@@ -103,6 +106,7 @@ impl NewOptions {
         bin: bool,
         lib: bool,
         path: PathBuf,
+        relative_path: Option<String>,
         name: Option<String>,
         edition: Option<String>,
         registry: Option<String>,
@@ -120,6 +124,7 @@ impl NewOptions {
             kind,
             auto_detect_kind,
             path,
+            relative_path,
             name,
             edition,
             registry,
@@ -441,11 +446,13 @@ pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> {
 
     let name = get_name(path, opts)?;
     check_name(name, opts.name.is_none(), is_bin, &mut config.shell())?;
+    let relative_path = opts.relative_path.as_deref().unwrap_or(name);
 
     let mkopts = MkOptions {
         version_control: opts.version_control,
         path,
         name,
+        relative_path,
         source_files: vec![plan_new_source_file(opts.kind.is_bin(), name.to_string())],
         edition: opts.edition.as_deref(),
         registry: opts.registry.as_deref(),
@@ -510,6 +517,7 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<NewProjectKind> {
     }
 
     check_name(name, opts.name.is_none(), has_bin, &mut config.shell())?;
+    let relative_path = opts.relative_path.as_deref().unwrap_or(name);
 
     let mut version_control = opts.version_control;
 
@@ -551,6 +559,7 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<NewProjectKind> {
         version_control,
         path,
         name,
+        relative_path,
         source_files: src_paths_types,
         edition: opts.edition.as_deref(),
         registry: opts.registry.as_deref(),
@@ -803,7 +812,7 @@ fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
         // Sometimes the root manifest is not a valid manifest, so we only try to parse it if it is.
         // This should not block the creation of the new project. It is only a best effort to
         // inherit the workspace package keys.
-        if let Ok(workspace_document) = root_manifest.parse::<toml_edit::Document>() {
+        if let Ok(mut workspace_document) = root_manifest.parse::<toml_edit::Document>() {
             if let Some(workspace_package_keys) = workspace_document
                 .get("workspace")
                 .and_then(|workspace| workspace.get("package"))
@@ -826,6 +835,13 @@ fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
                 table["workspace"] = toml_edit::value(true);
                 manifest["lints"] = toml_edit::Item::Table(table);
             }
+
+            // Try to add the new package to the workspace members.
+            update_manifest_with_new_member(
+                &root_manifest_path,
+                &mut workspace_document,
+                opts.relative_path,
+            )?;
         }
     }
 
@@ -925,3 +941,79 @@ fn update_manifest_with_inherited_workspace_package_keys(
         try_remove_and_inherit_package_key(key, manifest);
     }
 }
+
+/// Adds the new package member to the [workspace.members] array.
+/// - It first checks if the name matches any element in [workspace.exclude],
+///  and it ignores the name if there is a match.
+/// - Then it check if the name matches any element already in [workspace.members],
+/// and it ignores the name if there is a match.
+/// - If [workspace.members] doesn't exist in the manifest, the new package
+/// is not added to the members array. This prevents manifest files from breaking
+/// if they're only used for package templating.
+fn update_manifest_with_new_member(
+    manifest_path: &Path,
+    workspace_document: &mut toml_edit::Document,
+    relative_path: &str,
+) -> CargoResult<()> {
+    // Don't add the new package to the workspace's members
+    // if there is an exclusion match for it.
+    if let Some(members) = workspace_document
+        .get("workspace")
+        .and_then(|workspace| workspace.get("exclude"))
+        .and_then(|members| members.as_array())
+    {
+        let members = members.iter().map(|s| s.as_str().unwrap());
+        if matches_in_package_list(members, relative_path)? {
+            return Ok(());
+        }
+    }
+
+    if let Some(members) = workspace_document
+        .get("workspace")
+        .and_then(|workspace| workspace.get("members"))
+        .and_then(|members| members.as_array())
+    {
+        let members = members.iter().map(|s| s.as_str().unwrap());
+        if !matches_in_package_list(members, relative_path)? {
+            let Some(toml_members) = workspace_document["workspace"]["members"].as_array_mut()
+            else {
+                anyhow::bail!("[workspace.members] is not an array");
+            };
+            toml_members.push(relative_path);
+            paths::write(&manifest_path, workspace_document.to_string())?;
+        }
+    }
+
+    Ok(())
+}
+
+fn matches_in_package_list<'a, P: Iterator<Item = &'a str>>(
+    members: P,
+    package_path: &str,
+) -> CargoResult<bool> {
+    for name in members {
+        let pattern = build_glob(name)?;
+        if pattern.matches(package_path) {
+            return Ok(true);
+        }
+    }
+
+    Ok(false)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::matches_in_package_list;
+
+    #[test]
+    fn test_member_matching() {
+        let ret = matches_in_package_list(vec!["crates/*"].into_iter(), "crates/foo").unwrap();
+        assert!(ret, "`crates/*` doesn't match `crates/foo`");
+
+        let ret = matches_in_package_list(vec!["crates/foo"].into_iter(), "crates/foo").unwrap();
+        assert!(ret, "`crates/foo` doesn't match `crates/foo`");
+
+        let ret = matches_in_package_list(vec!["foo"].into_iter(), "crates/foo").unwrap();
+        assert!(!ret, "`foo` matches `crates/foo`, but it shouldn't");
+    }
+}

src/cargo/util/command_prelude.rs

@@ -788,6 +788,7 @@ Run `{cmd}` to see possible targets."
             self.flag("bin"),
             self.flag("lib"),
             self.value_of_path("path", config).unwrap(),
+            self._value_of("path").map(|s| s.to_string()),
             self._value_of("name").map(|s| s.to_string()),
             self._value_of("edition").map(|s| s.to_string()),
             self.registry(config)?,

src/cargo/util/restricted_names.rs

@@ -4,6 +4,8 @@ use crate::util::CargoResult;
 use anyhow::bail;
 use std::path::Path;
 
+use anyhow::Context as _;
+
 /// Returns `true` if the name contains non-ASCII characters.
 pub fn is_non_ascii_name(name: &str) -> bool {
     name.chars().any(|ch| ch > '\x7f')
@@ -120,3 +122,8 @@ pub fn is_windows_reserved_path(path: &Path) -> bool {
 pub fn is_glob_pattern<T: AsRef<str>>(name: T) -> bool {
     name.as_ref().contains(&['*', '?', '[', ']'][..])
 }
+
+/// Build [`glob::Pattern`] with informative context.
+pub fn build_glob(pat: &str) -> CargoResult<glob::Pattern> {
+    glob::Pattern::new(pat).with_context(|| format!("cannot build glob pattern from `{}`", pat))
+}

tests/testsuite/cargo_new/add_member_to_workspace/Cargo.toml

@@ -0,0 +1,6 @@
+[workspace]
+resolver = "2"
+members = ["crates/*"]
+
+[workspace.lints.rust]
+unsafe_code = "forbid"

tests/testsuite/cargo_new/add_member_to_workspace/empty_members/in/Cargo.toml

@@ -0,0 +1,3 @@
+[workspace]
+resolver = "2"
+members = []

tests/testsuite/cargo_new/add_member_to_workspace/empty_members/in/src/lib.rs

@@ -0,0 +1,14 @@
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {
+        let result = add(2, 2);
+        assert_eq!(result, 4);
+    }
+}

tests/testsuite/cargo_new/add_member_to_workspace/empty_members/out/Cargo.toml

@@ -0,0 +1,3 @@
+[workspace]
+resolver = "2"
+members = ["crates/foo"]

tests/testsuite/cargo_new/add_member_to_workspace/empty_members/out/crates/foo/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "foo"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

tests/testsuite/cargo_new/add_member_to_workspace/empty_members/out/crates/foo/src/main.rs

@@ -0,0 +1,3 @@
+fn main() {
+    println!("Hello, world!");
+}

tests/testsuite/cargo_new/add_member_to_workspace/empty_members/out/src/lib.rs

@@ -0,0 +1,14 @@
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {
+        let result = add(2, 2);
+        assert_eq!(result, 4);
+    }
+}

tests/testsuite/cargo_new/add_member_to_workspace/exclude_list/in/Cargo.toml

@@ -0,0 +1,4 @@
+[workspace]
+resolver = "2"
+members = []
+exclude = ["crates/foo"]

tests/testsuite/cargo_new/add_member_to_workspace/exclude_list/in/src/lib.rs

@@ -0,0 +1,14 @@
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {
+        let result = add(2, 2);
+        assert_eq!(result, 4);
+    }
+}

tests/testsuite/cargo_new/add_member_to_workspace/exclude_list/out/Cargo.toml

@@ -0,0 +1,4 @@
+[workspace]
+resolver = "2"
+members = []
+exclude = ["crates/foo"]

tests/testsuite/cargo_new/add_member_to_workspace/exclude_list/out/crates/foo/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "foo"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

tests/testsuite/cargo_new/add_member_to_workspace/exclude_list/out/crates/foo/src/main.rs

@@ -0,0 +1,3 @@
+fn main() {
+    println!("Hello, world!");
+}

tests/testsuite/cargo_new/add_member_to_workspace/exclude_list/out/src/lib.rs

@@ -0,0 +1,14 @@
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {
+        let result = add(2, 2);
+        assert_eq!(result, 4);
+    }
+}

tests/testsuite/cargo_new/add_member_to_workspace/members_glob/in/Cargo.toml

@@ -0,0 +1,3 @@
+[workspace]
+resolver = "2"
+members = ["crates/*"]

tests/testsuite/cargo_new/add_member_to_workspace/members_glob/in/src/lib.rs

@@ -0,0 +1,14 @@
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {
+        let result = add(2, 2);
+        assert_eq!(result, 4);
+    }
+}

tests/testsuite/cargo_new/add_member_to_workspace/members_glob/out/Cargo.toml

@@ -0,0 +1,3 @@
+[workspace]
+resolver = "2"
+members = ["crates/*"]

tests/testsuite/cargo_new/add_member_to_workspace/members_glob/out/crates/foo/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "foo"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

tests/testsuite/cargo_new/add_member_to_workspace/members_glob/out/crates/foo/src/main.rs

@@ -0,0 +1,3 @@
+fn main() {
+    println!("Hello, world!");
+}

tests/testsuite/cargo_new/add_member_to_workspace/members_glob/out/src/lib.rs

@@ -0,0 +1,14 @@
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {
+        let result = add(2, 2);
+        assert_eq!(result, 4);
+    }
+}