Skip to content

Commit

Permalink
Implement support for base paths
Browse files Browse the repository at this point in the history
  • Loading branch information
dpaoliello committed Aug 6, 2024
1 parent fa64658 commit 8a352b6
Show file tree
Hide file tree
Showing 62 changed files with 1,374 additions and 76 deletions.
2 changes: 2 additions & 0 deletions crates/cargo-util-schemas/src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ pub struct TomlDetailedDependency<P: Clone = String> {
// `path` is relative to the file it appears in. If that's a `Cargo.toml`, it'll be relative to
// that TOML file, and if it's a `.cargo/config` file, it'll be relative to that file.
pub path: Option<P>,
pub base: Option<String>,
pub git: Option<String>,
pub branch: Option<String>,
pub tag: Option<String>,
Expand Down Expand Up @@ -815,6 +816,7 @@ impl<P: Clone> Default for TomlDetailedDependency<P> {
registry: Default::default(),
registry_index: Default::default(),
path: Default::default(),
base: Default::default(),
git: Default::default(),
branch: Default::default(),
tag: Default::default(),
Expand Down
8 changes: 8 additions & 0 deletions src/bin/cargo/commands/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,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.")
.requires("path"),
clap::Arg::new("git")
.long("git")
.action(ArgAction::Set)
Expand Down Expand Up @@ -223,6 +229,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 @@ -328,6 +335,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
2 changes: 2 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ unstable_cli_options!(
no_index_update: bool = ("Do not update the registry index even if the cache is outdated"),
package_workspace: bool = ("Handle intra-workspace dependencies when packaging"),
panic_abort_tests: bool = ("Enable support to run tests with -Cpanic=abort"),
path_bases: bool = ("Allow paths that resolve relatively to a base specified in the config"),
profile_rustflags: bool = ("Enable the `rustflags` option in profiles in .cargo/config.toml file"),
public_dependency: bool = ("Respect a dependency's `public` field in Cargo.toml to control public/private dependencies"),
publish_timeout: bool = ("Enable the `publish.timeout` key in .cargo/config.toml file"),
Expand Down Expand Up @@ -1280,6 +1281,7 @@ impl CliUnstable {
"package-workspace" => self.package_workspace= parse_empty(k, v)?,
"panic-abort-tests" => self.panic_abort_tests = parse_empty(k, v)?,
"public-dependency" => self.public_dependency = parse_empty(k, v)?,
"path-bases" => self.path_bases = parse_empty(k, v)?,
"profile-rustflags" => self.profile_rustflags = parse_empty(k, v)?,
"trim-paths" => self.trim_paths = parse_empty(k, v)?,
"publish-timeout" => self.publish_timeout = parse_empty(k, v)?,
Expand Down
28 changes: 23 additions & 5 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,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 @@ -270,8 +271,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 @@ -331,10 +335,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 (path, base_name_and_value) = if let Some(base_name) = &arg.base {
let base_value = lookup_path_base(base_name, gctx, manifest.path.parent().unwrap())?;
(
base_value.join(raw_path),
Some((base_name.clone(), base_value)),
)
} else {
(std::env::current_dir()?.join(raw_path), None)
};
let path = paths::normalize_path(&path);
let src = PathSource::new(path);

let selected = if let Some(crate_spec) = &crate_spec {
let mut selected = if let Some(crate_spec) = &crate_spec {
if let Some(v) = crate_spec.version_req() {
// crate specifier includes a version (e.g. `[email protected]`)
anyhow::bail!("cannot specify a path (`{raw_path}`) with a version (`{v}`).");
Expand All @@ -349,10 +362,15 @@ 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())
};
if let Some(selected_source) = selected.source.as_mut() {
if let Source::Path(selected_source) = selected_source {
selected_source.base_name_and_value = base_name_and_value;
}
}
selected
} else if let Some(crate_spec) = &crate_spec {
crate_spec.to_dependency()?
Expand Down
40 changes: 35 additions & 5 deletions src/cargo/sources/path.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::collections::{HashMap, HashSet};
use std::fmt::{self, Debug, Formatter};
use std::fs;
Expand All @@ -14,7 +15,7 @@ use crate::sources::IndexSummary;
use crate::util::errors::CargoResult;
use crate::util::important_paths::find_project_manifest_exact;
use crate::util::internal;
use crate::util::toml::read_manifest;
use crate::util::toml::{lookup_path_base, read_manifest};
use crate::util::GlobalContext;
use anyhow::Context as _;
use cargo_util::paths;
Expand Down Expand Up @@ -878,7 +879,7 @@ fn read_packages(
}
}

fn nested_paths(manifest: &Manifest) -> Vec<PathBuf> {
fn nested_paths(manifest: &Manifest) -> Vec<(PathBuf, Option<String>)> {
let mut nested_paths = Vec::new();
let normalized = manifest.normalized_toml();
let dependencies = normalized
Expand Down Expand Up @@ -910,7 +911,7 @@ fn nested_paths(manifest: &Manifest) -> Vec<PathBuf> {
let Some(path) = dep.path.as_ref() else {
continue;
};
nested_paths.push(PathBuf::from(path.as_str()));
nested_paths.push((PathBuf::from(path.as_str()), dep.base.clone()));
}
}
nested_paths
Expand Down Expand Up @@ -1000,8 +1001,36 @@ fn read_nested_packages(
//
// TODO: filesystem/symlink implications?
if !source_id.is_registry() {
for p in nested.iter() {
let path = paths::normalize_path(&path.join(p));
let mut manifest_gctx = None;

for (p, base) in nested.iter() {
let p = if let Some(base) = base {
// If the dependency has a path base, then load the global context for the current
// manifest and use it to resolve the path base.
let manifest_gctx = match manifest_gctx {
Some(ref gctx) => gctx,
None => {
let mut new_manifest_gctx = match GlobalContext::default() {
Ok(gctx) => gctx,
Err(err) => return Err(err),
};
if let Err(err) = new_manifest_gctx.reload_rooted_at(&manifest_path) {
return Err(err);
}
manifest_gctx.insert(new_manifest_gctx)
}
};
match lookup_path_base(base, manifest_gctx, manifest_path.parent().unwrap()) {
Ok(base) => Cow::Owned(base.join(p)),
Err(err) => {
errors.push(err);
continue;
}
}
} else {
Cow::Borrowed(p)
};
let path = paths::normalize_path(&path.join(p.as_path()));
let result =
read_nested_packages(&path, all_packages, source_id, gctx, visited, errors);
// Ignore broken manifests found on git repositories.
Expand All @@ -1019,6 +1048,7 @@ fn read_nested_packages(
);
errors.push(err);
} else {
trace!("Failed to manifest: {:?}", err);
return Err(err);
}
}
Expand Down
78 changes: 70 additions & 8 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -901,13 +901,17 @@ impl InheritableFields {
};
let mut dep = dep.clone();
if let manifest::TomlDependency::Detailed(detailed) = &mut dep {
if let Some(rel_path) = &detailed.path {
detailed.path = Some(resolve_relative_path(
name,
self.ws_root(),
package_root,
rel_path,
)?);
if detailed.base.is_none() {
// If this is a path dependency without a base, then update the path to be relative
// to the workspace root instead.
if let Some(rel_path) = &detailed.path {
detailed.path = Some(resolve_relative_path(
name,
self.ws_root(),
package_root,
rel_path,
)?);
}
}
}
Ok(dep)
Expand Down Expand Up @@ -2135,7 +2139,20 @@ fn to_dependency_source_id<P: ResolveToPath + Clone>(
// always end up hashing to the same value no matter where it's
// built from.
if manifest_ctx.source_id.is_path() {
let path = manifest_ctx.root.join(path);
let path = if let Some(base) = orig.base.as_ref() {
if !manifest_ctx.gctx.cli_unstable().path_bases {
bail!("usage of path bases requires `-Z path-bases`");
}

lookup_path_base(&base, manifest_ctx.gctx, manifest_ctx.root)
.with_context(|| {
format!("resolving path base for dependency ({name_in_toml})")
})?
.join(path)
} else {
// This is a standard path with no prefix.
manifest_ctx.root.join(path)
};
let path = paths::normalize_path(&path);
SourceId::for_path(&path)
} else {
Expand All @@ -2151,6 +2168,50 @@ fn to_dependency_source_id<P: ResolveToPath + Clone>(
}
}

pub(crate) fn lookup_path_base(
base: &str,
gctx: &GlobalContext,
manifest_root: &Path,
) -> CargoResult<PathBuf> {
// Validate the path base name.
if base.is_empty()
|| !base.chars().next().unwrap().is_alphabetic()
|| base
.chars()
.skip(1)
.any(|c| !c.is_alphanumeric() && c != '_' && c != '-')
{
bail!(
"invalid path base name `{base}`. \
Path base names must start with a letter and contain only letters, numbers, hyphens, and underscores."
);
}

// Look up the relevant base in the Config and use that as the root.
if let Some(path_bases) =
gctx.get::<Option<ConfigRelativePath>>(&format!("path-bases.{base}"))?
{
Ok(path_bases.resolve_path(gctx))
} else {
// Otherwise, check the built-in bases.
match base {
"workspace" => {
if let Some(workspace_root) = find_workspace_root(manifest_root, gctx)? {
Ok(workspace_root.parent().unwrap().to_path_buf())
} else {
bail!(
"the `workspace` built-in path base cannot be used outside of a workspace."
)
}
}
_ => bail!(
"path base `{base}` is undefined. \
You must add an entry for `{base}` in the Cargo configuration [path-bases] table."
),
}
}
}

pub trait ResolveToPath {
fn resolve(&self, gctx: &GlobalContext) -> PathBuf;
}
Expand Down Expand Up @@ -2865,6 +2926,7 @@ fn prepare_toml_for_publish(
let mut d = d.clone();
// Path dependencies become crates.io deps.
d.path.take();
d.base.take();
// Same with git dependencies.
d.git.take();
d.branch.take();
Expand Down
34 changes: 29 additions & 5 deletions src/cargo/util/toml_mut/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,10 +412,18 @@ impl Dependency {
table.insert("version", src.version.as_str().into());
}
Some(Source::Path(src)) => {
let relpath = path_field(crate_root, &src.path);
if let Some(r) = src.version.as_deref() {
table.insert("version", r.into());
}
let relative_to = if let Some((base_name, base_value)) =
src.base_name_and_value.as_ref()
{
table.insert("base", base_name.into());
base_value
} else {
crate_root
};
let relpath = path_field(relative_to, &src.path);
table.insert("path", relpath.into());
}
Some(Source::Git(src)) => {
Expand Down Expand Up @@ -493,12 +501,20 @@ impl Dependency {
Some(Source::Registry(src)) => {
overwrite_value(table, "version", src.version.as_str());

for key in ["path", "git", "branch", "tag", "rev", "workspace"] {
for key in ["path", "git", "branch", "tag", "rev", "workspace", "base"] {
table.remove(key);
}
}
Some(Source::Path(src)) => {
let relpath = path_field(crate_root, &src.path);
let relative_to =
if let Some((base_name, base_value)) = src.base_name_and_value.as_ref() {
overwrite_value(table, "base", base_name);
base_value
} else {
table.remove("base");
crate_root
};
let relpath = path_field(relative_to, &src.path);
overwrite_value(table, "path", relpath);
if let Some(r) = src.version.as_deref() {
overwrite_value(table, "version", r);
Expand Down Expand Up @@ -533,7 +549,7 @@ impl Dependency {
table.remove("version");
}

for key in ["path", "workspace"] {
for key in ["path", "workspace", "base"] {
table.remove(key);
}
}
Expand All @@ -552,6 +568,7 @@ impl Dependency {
"rev",
"package",
"default-features",
"base",
] {
table.remove(key);
}
Expand Down Expand Up @@ -812,6 +829,8 @@ impl std::fmt::Display for RegistrySource {
pub struct PathSource {
/// Local, absolute path.
pub path: PathBuf,
/// If using a named base, the base and relative path.
pub base_name_and_value: Option<(String, PathBuf)>,
/// Version requirement for when published.
pub version: Option<String>,
}
Expand All @@ -821,6 +840,7 @@ impl PathSource {
pub fn new(path: impl Into<PathBuf>) -> Self {
Self {
path: path.into(),
base_name_and_value: None,
version: None,
}
}
Expand All @@ -843,7 +863,11 @@ impl PathSource {

impl std::fmt::Display for PathSource {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.path.display().fmt(f)
if let Some((base_name, _)) = &self.base_name_and_value {
write!(f, "{} (@{base_name})", self.path.display())
} else {
self.path.display().fmt(f)
}
}
}

Expand Down
Loading

0 comments on commit 8a352b6

Please sign in to comment.