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

Implement support for base paths (RFC 3529) #12974

Closed
wants to merge 1 commit into from
Closed
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
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. `docopt@0.8`)
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