Skip to content
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.

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

Add new packages to [workspace.members] automatically #12779

Merged
merged 3 commits into from
Oct 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ tempfile = "3.8.0"
thiserror = "1.0.49"
time = { version = "0.3", features = ["parsing", "formatting", "serde"] }
toml = "0.8.2"
toml_edit = { version = "0.20.2", features = ["serde"] }
toml_edit = { version = "0.20.7", features = ["serde"] }
tracing = "0.1.37"
tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
unicase = "2.7.0"
Expand Down
17 changes: 1 addition & 16 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use crate::util::toml_mut::dependency::MaybeWorkspace;
use crate::util::toml_mut::dependency::PathSource;
use crate::util::toml_mut::dependency::Source;
use crate::util::toml_mut::dependency::WorkspaceSource;
use crate::util::toml_mut::is_sorted;
use crate::util::toml_mut::manifest::DepTable;
use crate::util::toml_mut::manifest::LocalManifest;
use crate::util::RustVersion;
Expand Down Expand Up @@ -1004,22 +1005,6 @@ fn format_features_version_suffix(dep: &DependencyUI) -> String {
}
}

// Based on Iterator::is_sorted from nightly std; remove in favor of that when stabilized.
fn is_sorted(mut it: impl Iterator<Item = impl PartialOrd>) -> bool {
let Some(mut last) = it.next() else {
return true;
};

for curr in it {
if curr < last {
return false;
}
last = curr;
}

true
}

fn find_workspace_dep(toml_key: &str, root_manifest: &Path) -> CargoResult<Dependency> {
let manifest = LocalManifest::try_new(root_manifest)?;
let manifest = manifest
Expand Down
107 changes: 104 additions & 3 deletions src/cargo/ops/cargo_new.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use crate::core::{Edition, Shell, Workspace};
use crate::util::errors::CargoResult;
use crate::util::important_paths::find_root_manifest_for_wd;
use crate::util::toml_mut::is_sorted;
use crate::util::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo};
use crate::util::{restricted_names, Config};
use anyhow::{anyhow, Context as _};
use cargo_util::paths;
use anyhow::{anyhow, Context};
use cargo_util::paths::{self, write_atomic};
use serde::de;
use serde::Deserialize;
use std::collections::BTreeMap;
Expand All @@ -13,6 +14,7 @@ use std::io::{BufRead, BufReader, ErrorKind};
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::{fmt, slice};
use toml_edit::{Array, Value};

#[derive(Clone, Copy, Debug, PartialEq)]
pub enum VersionControl {
Expand Down Expand Up @@ -809,7 +811,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"))
Expand All @@ -832,6 +834,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.path,
)?;
}
}

Expand Down Expand Up @@ -931,3 +940,95 @@ 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, it will add a new section
/// with the new package in it.
fn update_manifest_with_new_member(
root_manifest_path: &Path,
workspace_document: &mut toml_edit::Document,
package_path: &Path,
) -> CargoResult<()> {
// Find the relative path for the package from the workspace root directory.
let workspace_root = root_manifest_path.parent().with_context(|| {
format!(
"workspace root manifest doesn't have a parent directory `{}`",
root_manifest_path.display()
)
})?;
let relpath = pathdiff::diff_paths(package_path, workspace_root).with_context(|| {
format!(
"path comparison requires two absolute paths; package_path: `{}`, workspace_path: `{}`",
package_path.display(),
workspace_root.display()
)
})?;

let mut components = Vec::new();
for comp in relpath.iter() {
let comp = comp.to_str().with_context(|| {
calavera marked this conversation as resolved.
Show resolved Hide resolved
format!("invalid unicode component in path `{}`", relpath.display())
})?;
components.push(comp);
}
let display_path = components.join("/");

// Don't add the new package to the workspace's members
// if there is an exclusion match for it.
if let Some(exclude) = workspace_document
.get("workspace")
.and_then(|workspace| workspace.get("exclude"))
.and_then(|exclude| exclude.as_array())
{
for member in exclude {
let pat = member
.as_str()
.with_context(|| format!("invalid non-string exclude path `{}`", member))?;
if pat == display_path {
return Ok(());
}
}
}

// If the members element already exist, check if one of the patterns
// in the array already includes the new package's relative path.
// - Add the relative path if the members don't match the new package's path.
// - Create a new members array if there are no members element in the workspace yet.
if let Some(members) = workspace_document
.get_mut("workspace")
.and_then(|workspace| workspace.get_mut("members"))
.and_then(|members| members.as_array_mut())
{
for member in members.iter() {
let pat = member
.as_str()
.with_context(|| format!("invalid non-string member `{}`", member))?;
let pattern = glob::Pattern::new(pat)
.with_context(|| format!("cannot build glob pattern from `{}`", pat))?;

if pattern.matches(&display_path) {
return Ok(());
}
}

let was_sorted = is_sorted(members.iter().map(Value::as_str));
members.push(&display_path);
if was_sorted {
members.sort_by(|lhs, rhs| lhs.as_str().cmp(&rhs.as_str()));
}
} else {
let mut array = Array::new();
array.push(&display_path);

workspace_document["workspace"]["members"] = toml_edit::value(array);
}

write_atomic(
&root_manifest_path,
workspace_document.to_string().to_string().as_bytes(),
)
}
16 changes: 16 additions & 0 deletions src/cargo/util/toml_mut/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,19 @@

pub mod dependency;
pub mod manifest;

// Based on Iterator::is_sorted from nightly std; remove in favor of that when stabilized.
pub fn is_sorted(mut it: impl Iterator<Item = impl PartialOrd>) -> bool {
let Some(mut last) = it.next() else {
return true;
};

for curr in it {
if curr < last {
return false;
}
last = curr;
}

true
}
1 change: 1 addition & 0 deletions tests/testsuite/cargo_init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ mod simple_hg_ignore_exists;
mod simple_lib;
mod unknown_flags;
mod with_argument;
mod workspace_add_member;
2 changes: 2 additions & 0 deletions tests/testsuite/cargo_init/workspace_add_member/in/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[workspace]
resolver = "2"
21 changes: 21 additions & 0 deletions tests/testsuite/cargo_init/workspace_add_member/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::prelude::*;
use cargo_test_support::Project;

use cargo_test_support::curr_dir;

#[cargo_test]
fn case() {
let project = Project::from_template(curr_dir!().join("in"));
let project_root = &project.root();

snapbox::cmd::Command::cargo_ui()
.arg_line("init --bin --vcs none")
.current_dir(project_root.join("crates").join("foo"))
.assert()
.success()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[workspace]
resolver = "2"
members = ["crates/foo"]
Original file line number Diff line number Diff line change
@@ -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]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
1 change: 1 addition & 0 deletions tests/testsuite/cargo_init/workspace_add_member/stderr.log
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Created binary (application) package
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[workspace]
resolver = "2"

[workspace.package]
authors = ["Rustaceans"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::curr_dir;
use cargo_test_support::CargoCommand;
use cargo_test_support::Project;

#[cargo_test]
fn case() {
let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("new")
.args(["crates/foo"])
.current_dir(cwd)
.assert()
.success()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[workspace]
resolver = "2"
members = ["crates/foo"]

[workspace.package]
authors = ["Rustaceans"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "foo"
version = "0.1.0"
edition = "2021"
authors.workspace = true

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Created binary (application) `crates/foo` package
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[workspace]
resolver = "2"
members = ["crates/bar", "crates/qux"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "bar"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "qux"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use cargo_test_support::compare::assert_ui;
use cargo_test_support::curr_dir;
use cargo_test_support::CargoCommand;
use cargo_test_support::Project;

#[cargo_test]
fn case() {
let project = Project::from_template(curr_dir!().join("in"));
let project_root = project.root();
let cwd = &project_root;

snapbox::cmd::Command::cargo_ui()
.arg("new")
.args(["crates/foo"])
.current_dir(cwd)
.assert()
.success()
.stdout_matches_path(curr_dir!().join("stdout.log"))
.stderr_matches_path(curr_dir!().join("stderr.log"));

assert_ui().subset_matches(curr_dir!().join("out"), &project_root);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[workspace]
resolver = "2"
members = ["crates/bar", "crates/foo", "crates/qux"]
Loading