Skip to content

Commit

Permalink
Auto merge of #14427 - dpaoliello:add, r=epage
Browse files Browse the repository at this point in the history 
Implement path-bases (RFC 3529) 2/n: `cargo [add|remove|update]` support

RFC: rust-lang/rfcs#3529
Tracking Issue: #14355

This PR adds the `--base` option to `cargo add` to allow adding a path dependency with a path base.
  • Loading branch information
@bors
bors committed Sep 6, 2024
2 parents be1bbda + 8de5554 commit 64c4b6d
Show file tree
Hide file tree
Showing 70 changed files with 1,014 additions and 116 deletions.
8 changes: 8 additions & 0 deletions src/bin/cargo/commands/add.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ Example uses:
.help("Filesystem path to local crate to add")
.group("selected")
.conflicts_with("git"),
clap::Arg::new("base")
.long("base")
.action(ArgAction::Set)
.value_name("BASE")
.help("The path base to use when adding from a local crate (unstable).")
.requires("path"),
clap::Arg::new("git")
.long("git")
.action(ArgAction::Set)
Expand Down Expand Up @@ -224,6 +230,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {

fn parse_dependencies(gctx: &GlobalContext, matches: &ArgMatches) -> CargoResult<Vec<DepOp>> {
let path = matches.get_one::<String>("path");
let base = matches.get_one::<String>("base");
let git = matches.get_one::<String>("git");
let branch = matches.get_one::<String>("branch");
let rev = matches.get_one::<String>("rev");
Expand Down Expand Up @@ -329,6 +336,7 @@ fn parse_dependencies(gctx: &GlobalContext, matches: &ArgMatches) -> CargoResult
public,
registry: registry.clone(),
path: path.map(String::from),
base: base.map(String::from),
git: git.map(String::from),
branch: branch.map(String::from),
rev: rev.map(String::from),
Expand Down
57 changes: 44 additions & 13 deletions src/bin/cargo/commands/remove.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,20 +167,37 @@ fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {

let members = workspace
.members()
.map(|p| LocalManifest::try_new(p.manifest_path()))
.map(|p| {
Ok((
LocalManifest::try_new(p.manifest_path())?,
p.manifest().unstable_features(),
))
})
.collect::<CargoResult<Vec<_>>>()?;

let mut dependencies = members
.iter()
.flat_map(|manifest| {
manifest.get_sections().into_iter().flat_map(|(_, table)| {
table
.as_table_like()
.unwrap()
.iter()
.map(|(key, item)| Dependency::from_toml(&manifest.path, key, item))
.collect::<Vec<_>>()
})
.into_iter()
.flat_map(|(manifest, unstable_features)| {
manifest
.get_sections()
.into_iter()
.flat_map(move |(_, table)| {
table
.as_table_like()
.unwrap()
.iter()
.map(|(key, item)| {
Dependency::from_toml(
workspace.gctx(),
workspace.root(),
&manifest.path,
&unstable_features,
key,
item,
)
})
.collect::<Vec<_>>()
})
})
.collect::<CargoResult<Vec<_>>>()?;

Expand All @@ -192,7 +209,14 @@ fn gc_workspace(workspace: &Workspace<'_>) -> CargoResult<()> {
{
deps_table.set_implicit(true);
for (key, item) in deps_table.iter_mut() {
let ws_dep = Dependency::from_toml(&workspace.root(), key.get(), item)?;
let ws_dep = Dependency::from_toml(
workspace.gctx(),
workspace.root(),
&workspace.root(),
workspace.unstable_features(),
key.get(),
item,
)?;

// search for uses of this workspace dependency
let mut is_used = false;
Expand Down Expand Up @@ -329,7 +353,14 @@ fn gc_unused_patches(workspace: &Workspace<'_>, resolve: &Resolve) -> CargoResul
patch_table.set_implicit(true);

for (key, item) in patch_table.iter_mut() {
let dep = Dependency::from_toml(&workspace.root_manifest(), key.get(), item)?;
let dep = Dependency::from_toml(
workspace.gctx(),
workspace.root(),
&workspace.root_manifest(),
workspace.unstable_features(),
key.get(),
item,
)?;

// Generate a PackageIdSpec url for querying
let url = if let MaybeWorkspace::Other(source_id) =
Expand Down
85 changes: 73 additions & 12 deletions src/cargo/ops/cargo_add/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::str::FromStr;
use anyhow::Context as _;
use cargo_util::paths;
use cargo_util_schemas::core::PartialVersion;
use cargo_util_schemas::manifest::PathBaseName;
use cargo_util_schemas::manifest::RustVersion;
use indexmap::IndexSet;
use itertools::Itertools;
Expand All @@ -20,6 +21,7 @@ use toml_edit::Item as TomlItem;
use crate::core::dependency::DepKind;
use crate::core::registry::PackageRegistry;
use crate::core::FeatureValue;
use crate::core::Features;
use crate::core::Package;
use crate::core::Registry;
use crate::core::Shell;
Expand All @@ -28,6 +30,7 @@ use crate::core::Workspace;
use crate::sources::source::QueryKind;
use crate::util::cache_lock::CacheLockMode;
use crate::util::style;
use crate::util::toml::lookup_path_base;
use crate::util::toml_mut::dependency::Dependency;
use crate::util::toml_mut::dependency::GitSource;
use crate::util::toml_mut::dependency::MaybeWorkspace;
Expand Down Expand Up @@ -197,7 +200,13 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(

print_dep_table_msg(&mut options.gctx.shell(), &dep)?;

manifest.insert_into_table(&dep_table, &dep)?;
manifest.insert_into_table(
&dep_table,
&dep,
workspace.gctx(),
workspace.root(),
options.spec.manifest().unstable_features(),
)?;
if dep.optional == Some(true) {
let is_namespaced_features_supported =
check_rust_version_for_optional_dependency(options.spec.rust_version())?;
Expand Down Expand Up @@ -270,8 +279,11 @@ pub struct DepOp {
/// Registry for looking up dependency version
pub registry: Option<String>,

/// Git repo for dependency
/// File system path for dependency
pub path: Option<String>,
/// Specify a named base for a path dependency
pub base: Option<String>,

/// Git repo for dependency
pub git: Option<String>,
/// Specify an alternative git branch
Expand Down Expand Up @@ -332,7 +344,19 @@ fn resolve_dependency(
selected
} else if let Some(raw_path) = &arg.path {
let path = paths::normalize_path(&std::env::current_dir()?.join(raw_path));
let src = PathSource::new(&path);
let mut src = PathSource::new(path);
src.base = arg.base.clone();

if let Some(base) = &arg.base {
// Validate that the base is valid.
let workspace_root = || Ok(ws.root_manifest().parent().unwrap());
lookup_path_base(
&PathBaseName::new(base.clone())?,
&gctx,
&workspace_root,
spec.manifest().unstable_features(),
)?;
}

let selected = if let Some(crate_spec) = &crate_spec {
if let Some(v) = crate_spec.version_req() {
Expand All @@ -349,9 +373,13 @@ fn resolve_dependency(
}
selected
} else {
let mut source = crate::sources::PathSource::new(&path, src.source_id()?, gctx);
let mut source = crate::sources::PathSource::new(&src.path, src.source_id()?, gctx);
let package = source.root_package()?;
Dependency::from(package.summary())
let mut selected = Dependency::from(package.summary());
if let Some(Source::Path(selected_src)) = &mut selected.source {
selected_src.base = src.base;
}
selected
};
selected
} else if let Some(crate_spec) = &crate_spec {
Expand All @@ -361,9 +389,16 @@ fn resolve_dependency(
};
selected_dep = populate_dependency(selected_dep, arg);

let lookup = |dep_key: &_| get_existing_dependency(manifest, dep_key, section);
let lookup = |dep_key: &_| {
get_existing_dependency(
ws,
spec.manifest().unstable_features(),
manifest,
dep_key,
section,
)
};
let old_dep = fuzzy_lookup(&mut selected_dep, lookup, gctx)?;

let mut dependency = if let Some(mut old_dep) = old_dep.clone() {
if old_dep.name != selected_dep.name {
// Assuming most existing keys are not relevant when the package changes
Expand All @@ -385,7 +420,9 @@ fn resolve_dependency(
if dependency.source().is_none() {
// Checking for a workspace dependency happens first since a member could be specified
// in the workspace dependencies table as a dependency
let lookup = |toml_key: &_| Ok(find_workspace_dep(toml_key, ws.root_manifest()).ok());
let lookup = |toml_key: &_| {
Ok(find_workspace_dep(toml_key, ws, ws.root_manifest(), ws.unstable_features()).ok())
};
if let Some(_dep) = fuzzy_lookup(&mut dependency, lookup, gctx)? {
dependency = dependency.set_source(WorkspaceSource::new());
} else if let Some(package) = ws.members().find(|p| p.name().as_str() == dependency.name) {
Expand Down Expand Up @@ -432,7 +469,12 @@ fn resolve_dependency(
let query = dependency.query(gctx)?;
let query = match query {
MaybeWorkspace::Workspace(_workspace) => {
let dep = find_workspace_dep(dependency.toml_key(), ws.root_manifest())?;
let dep = find_workspace_dep(
dependency.toml_key(),
ws,
ws.root_manifest(),
ws.unstable_features(),
)?;
if let Some(features) = dep.features.clone() {
dependency = dependency.set_inherited_features(features);
}
Expand Down Expand Up @@ -547,6 +589,8 @@ fn check_rust_version_for_optional_dependency(
/// If it doesn't exist but exists in another table, let's use that as most likely users
/// want to use the same version across all tables unless they are renaming.
fn get_existing_dependency(
ws: &Workspace<'_>,
unstable_features: &Features,
manifest: &LocalManifest,
dep_key: &str,
section: &DepTable,
Expand All @@ -561,7 +605,7 @@ fn get_existing_dependency(
}

let mut possible: Vec<_> = manifest
.get_dependency_versions(dep_key)
.get_dependency_versions(dep_key, ws, unstable_features)
.map(|(path, dep)| {
let key = if path == *section {
(Key::Existing, true)
Expand Down Expand Up @@ -776,6 +820,11 @@ fn select_package(
if let Some(reg_name) = dependency.registry.as_deref() {
dep = dep.set_registry(reg_name);
}
if let Some(Source::Path(PathSource { base, .. })) = dependency.source() {
if let Some(Source::Path(dep_src)) = &mut dep.source {
dep_src.base = base.clone();
}
}
Ok(dep)
}
_ => {
Expand Down Expand Up @@ -1107,7 +1156,12 @@ fn format_features_version_suffix(dep: &DependencyUI) -> String {
}
}

fn find_workspace_dep(toml_key: &str, root_manifest: &Path) -> CargoResult<Dependency> {
fn find_workspace_dep(
toml_key: &str,
ws: &Workspace<'_>,
root_manifest: &Path,
unstable_features: &Features,
) -> CargoResult<Dependency> {
let manifest = LocalManifest::try_new(root_manifest)?;
let manifest = manifest
.data
Expand All @@ -1127,7 +1181,14 @@ fn find_workspace_dep(toml_key: &str, root_manifest: &Path) -> CargoResult<Depen
let dep_item = dependencies
.get(toml_key)
.with_context(|| format!("could not find {toml_key} in `workspace.dependencies`"))?;
Dependency::from_toml(root_manifest.parent().unwrap(), toml_key, dep_item)
Dependency::from_toml(
ws.gctx(),
ws.root(),
root_manifest.parent().unwrap(),
unstable_features,
toml_key,
dep_item,
)
}

/// Convert a `semver::VersionReq` into a rendered `semver::Version` if all fields are fully
Expand Down
23 changes: 19 additions & 4 deletions src/cargo/ops/cargo_update.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,16 @@ pub fn write_manifest_upgrades(

let mut any_file_has_changed = false;

let manifest_paths = std::iter::once(ws.root_manifest())
.chain(ws.members().map(|member| member.manifest_path()))
let items = std::iter::once((ws.root_manifest(), ws.unstable_features()))
.chain(ws.members().map(|member| {
(
member.manifest_path(),
member.manifest().unstable_features(),
)
}))
.collect::<Vec<_>>();

for manifest_path in manifest_paths {
for (manifest_path, unstable_features) in items {
trace!("updating TOML manifest at `{manifest_path:?}` with upgraded dependencies");

let crate_root = manifest_path
Expand All @@ -417,7 +422,10 @@ pub fn write_manifest_upgrades(
for (mut dep_key, dep_item) in dep_table.iter_mut() {
let dep_key_str = dep_key.get();
let dependency = crate::util::toml_mut::dependency::Dependency::from_toml(
ws.gctx(),
ws.root(),
&manifest_path,
unstable_features,
dep_key_str,
dep_item,
)?;
Expand Down Expand Up @@ -472,7 +480,14 @@ pub fn write_manifest_upgrades(
dep.source = Some(Source::Registry(source));

trace!("upgrading dependency {name}");
dep.update_toml(&crate_root, &mut dep_key, dep_item);
dep.update_toml(
ws.gctx(),
ws.root(),
&crate_root,
unstable_features,
&mut dep_key,
dep_item,
)?;
manifest_has_changed = true;
any_file_has_changed = true;
}
Expand Down
Loading

0 comments on commit 64c4b6d

Please sign in to comment.