Skip to content

Commit

Permalink
Auto merge of #10606 - Muscraft:cargo-add-support, r=epage
Browse files Browse the repository at this point in the history
Cargo add support for workspace inheritance

Tracking issue: #8415
RFC: rust-lang/rfcs#2906

This PR adds all the required support for workspace inheritance within `cargo-add`. It was split up across a few different PRs as it required `snapbox` support from #10581 and a new `toml_edit` version from #10603. `@epage` and I decided to go ahead with this PR and add in some of the changes those PRs made. `@epage's` name on the commits is from helping to rewrite commits and some very minor additions.

Changes:
  - #10585
  - Muscraft#1
  - Muscraft#3
  - Muscraft#2
  - Muscraft#4

r? `@epage`
  • Loading branch information
bors committed Apr 28, 2022
2 parents 5600004 + aa7d116 commit caca7f2
Show file tree
Hide file tree
Showing 137 changed files with 1,149 additions and 79 deletions.
133 changes: 122 additions & 11 deletions src/cargo/ops/cargo_add/dependency.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use std::collections::BTreeMap;
use std::fmt::{Display, Formatter};
use std::path::{Path, PathBuf};

use indexmap::IndexSet;
use toml_edit::KeyMut;

use super::manifest::str_or_1_len_table;
use crate::core::FeatureMap;
Expand All @@ -27,6 +29,8 @@ pub struct Dependency {
pub features: Option<IndexSet<String>>,
/// Whether default features are enabled
pub default_features: Option<bool>,
/// List of features inherited from a workspace dependency
pub inherited_features: Option<IndexSet<String>>,

/// Where the dependency comes from
pub source: Option<Source>,
Expand All @@ -49,6 +53,7 @@ impl Dependency {
optional: None,
features: None,
default_features: None,
inherited_features: None,
source: None,
registry: None,
rename: None,
Expand All @@ -74,6 +79,7 @@ impl Dependency {
Some(Source::Git(git)) => {
git.version = None;
}
Some(Source::Workspace(_workspace)) => {}
None => {}
}
self
Expand Down Expand Up @@ -150,6 +156,12 @@ impl Dependency {
self
}

/// Set features as an array of string (does some basic parsing)
pub fn set_inherited_features(mut self, features: IndexSet<String>) -> Self {
self.inherited_features = Some(features);
self
}

/// Get the dependency source
pub fn source(&self) -> Option<&Source> {
self.source.as_ref()
Expand All @@ -161,6 +173,7 @@ impl Dependency {
Source::Registry(src) => Some(src.version.as_str()),
Source::Path(src) => src.version.as_deref(),
Source::Git(src) => src.version.as_deref(),
Source::Workspace(_) => None,
}
}

Expand All @@ -185,29 +198,47 @@ impl Dependency {
}

/// Get the SourceID for this dependency
pub fn source_id(&self, config: &Config) -> CargoResult<SourceId> {
pub fn source_id(&self, config: &Config) -> CargoResult<MaybeWorkspace<SourceId>> {
match &self.source.as_ref() {
Some(Source::Registry(_)) | None => {
if let Some(r) = self.registry() {
let source_id = SourceId::alt_registry(config, r)?;
Ok(source_id)
Ok(MaybeWorkspace::Other(source_id))
} else {
let source_id = SourceId::crates_io(config)?;
Ok(source_id)
Ok(MaybeWorkspace::Other(source_id))
}
}
Some(Source::Path(source)) => source.source_id(),
Some(Source::Git(source)) => source.source_id(),
Some(Source::Path(source)) => Ok(MaybeWorkspace::Other(source.source_id()?)),
Some(Source::Git(source)) => Ok(MaybeWorkspace::Other(source.source_id()?)),
Some(Source::Workspace(workspace)) => Ok(MaybeWorkspace::Workspace(workspace.clone())),
}
}

/// Query to find this dependency
pub fn query(&self, config: &Config) -> CargoResult<crate::core::dependency::Dependency> {
pub fn query(
&self,
config: &Config,
) -> CargoResult<MaybeWorkspace<crate::core::dependency::Dependency>> {
let source_id = self.source_id(config)?;
crate::core::dependency::Dependency::parse(self.name.as_str(), self.version(), source_id)
match source_id {
MaybeWorkspace::Workspace(workspace) => Ok(MaybeWorkspace::Workspace(workspace)),
MaybeWorkspace::Other(source_id) => Ok(MaybeWorkspace::Other(
crate::core::dependency::Dependency::parse(
self.name.as_str(),
self.version(),
source_id,
)?,
)),
}
}
}

pub enum MaybeWorkspace<T> {
Workspace(WorkspaceSource),
Other(T),
}

impl Dependency {
/// Create a dependency from a TOML table entry
pub fn from_toml(crate_root: &Path, key: &str, item: &toml_edit::Item) -> CargoResult<Self> {
Expand Down Expand Up @@ -271,6 +302,15 @@ impl Dependency {
invalid_type(key, "version", version.type_name(), "string")
})?);
src.into()
} else if let Some(workspace) = table.get("workspace") {
let workspace_bool = workspace.as_bool().ok_or_else(|| {
invalid_type(key, "workspace", workspace.type_name(), "bool")
})?;
if !workspace_bool {
anyhow::bail!("`{key}.workspace = false` is unsupported")
}
let src = WorkspaceSource::new();
src.into()
} else {
anyhow::bail!("Unrecognized dependency source for `{key}`");
};
Expand Down Expand Up @@ -320,6 +360,7 @@ impl Dependency {
features,
available_features,
optional,
inherited_features: None,
};
Ok(dep)
} else {
Expand Down Expand Up @@ -367,6 +408,12 @@ impl Dependency {
None,
None,
) => toml_edit::value(v),
(false, None, true, Some(Source::Workspace(WorkspaceSource {})), None, None) => {
let mut table = toml_edit::InlineTable::default();
table.set_dotted(true);
table.insert("workspace", true.into());
toml_edit::value(toml_edit::Value::InlineTable(table))
}
// Other cases are represented as an inline table
(_, _, _, _, _, _) => {
let mut table = toml_edit::InlineTable::default();
Expand Down Expand Up @@ -397,6 +444,9 @@ impl Dependency {
table.insert("version", r.into());
}
}
Some(Source::Workspace(_)) => {
table.insert("workspace", true.into());
}
None => {}
}
if table.contains_key("version") {
Expand Down Expand Up @@ -427,16 +477,24 @@ impl Dependency {
}

/// Modify existing entry to match this dependency
pub fn update_toml(&self, crate_root: &Path, item: &mut toml_edit::Item) {
pub fn update_toml<'k>(
&self,
crate_root: &Path,
key: &mut KeyMut<'k>,
item: &mut toml_edit::Item,
) {
if str_or_1_len_table(item) {
// Nothing to preserve
*item = self.to_toml(crate_root);
if self.source != Some(Source::Workspace(WorkspaceSource)) {
key.fmt();
}
} else if let Some(table) = item.as_table_like_mut() {
match &self.source {
Some(Source::Registry(src)) => {
table.insert("version", toml_edit::value(src.version.as_str()));

for key in ["path", "git", "branch", "tag", "rev"] {
for key in ["path", "git", "branch", "tag", "rev", "workspace"] {
table.remove(key);
}
}
Expand All @@ -449,7 +507,7 @@ impl Dependency {
table.remove("version");
}

for key in ["git", "branch", "tag", "rev"] {
for key in ["git", "branch", "tag", "rev", "workspace"] {
table.remove(key);
}
}
Expand All @@ -476,7 +534,23 @@ impl Dependency {
table.remove("version");
}

for key in ["path"] {
for key in ["path", "workspace"] {
table.remove(key);
}
}
Some(Source::Workspace(_)) => {
table.set_dotted(true);
for key in [
"version",
"registry",
"registry-index",
"path",
"git",
"branch",
"tag",
"rev",
"package",
] {
table.remove(key);
}
}
Expand Down Expand Up @@ -516,12 +590,14 @@ impl Dependency {
.unwrap_or_default();
features.extend(new_features.iter().map(|s| s.as_str()));
let features = toml_edit::value(features.into_iter().collect::<toml_edit::Value>());
table.set_dotted(false);
table.insert("features", features);
} else {
table.remove("features");
}
match self.optional {
Some(v) => {
table.set_dotted(false);
table.insert("optional", toml_edit::value(v));
}
None => {
Expand Down Expand Up @@ -596,6 +672,8 @@ pub enum Source {
Path(PathSource),
/// Dependency from a git repo
Git(GitSource),
/// Dependency from a workspace
Workspace(WorkspaceSource),
}

impl Source {
Expand Down Expand Up @@ -624,6 +702,15 @@ impl Source {
_ => None,
}
}

/// Access the workspace source, if present
#[allow(dead_code)]
pub fn as_workspace(&self) -> Option<&WorkspaceSource> {
match self {
Self::Workspace(src) => Some(src),
_ => None,
}
}
}

impl std::fmt::Display for Source {
Expand All @@ -632,6 +719,7 @@ impl std::fmt::Display for Source {
Self::Registry(src) => src.fmt(f),
Self::Path(src) => src.fmt(f),
Self::Git(src) => src.fmt(f),
Self::Workspace(src) => src.fmt(f),
}
}
}
Expand Down Expand Up @@ -660,6 +748,12 @@ impl From<GitSource> for Source {
}
}

impl From<WorkspaceSource> for Source {
fn from(inner: WorkspaceSource) -> Self {
Self::Workspace(inner)
}
}

/// Dependency from a registry
#[derive(Debug, Hash, PartialEq, Eq, Clone)]
#[non_exhaustive]
Expand Down Expand Up @@ -822,6 +916,23 @@ impl std::fmt::Display for GitSource {
}
}

/// Dependency from a workspace
#[derive(Debug, Hash, PartialEq, Eq, Clone)]
#[non_exhaustive]
pub struct WorkspaceSource;

impl WorkspaceSource {
pub fn new() -> Self {
Self
}
}

impl Display for WorkspaceSource {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
"workspace".fmt(f)
}
}

#[cfg(test)]
mod tests {
use std::path::Path;
Expand Down
8 changes: 6 additions & 2 deletions src/cargo/ops/cargo_add/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,12 @@ impl LocalManifest {
let dep_key = dep.toml_key();

let table = self.get_table_mut(table_path)?;
if let Some(dep_item) = table.as_table_like_mut().unwrap().get_mut(dep_key) {
dep.update_toml(&crate_root, dep_item);
if let Some((mut dep_key, dep_item)) = table
.as_table_like_mut()
.unwrap()
.get_key_value_mut(dep_key)
{
dep.update_toml(&crate_root, &mut dep_key, dep_item);
} else {
let new_dependency = dep.to_toml(&crate_root);
table[dep_key] = new_dependency;
Expand Down
Loading

0 comments on commit caca7f2

Please sign in to comment.