Skip to content

Commit

Permalink
Implement base paths (RFC 3529) 1/n: path dep and patch support
Browse files Browse the repository at this point in the history
  • Loading branch information
dpaoliello committed Aug 12, 2024
1 parent 403bc5b commit a591ec4
Show file tree
Hide file tree
Showing 8 changed files with 728 additions and 20 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
[a2b58c3d...HEAD](https://github.com/rust-lang/cargo/compare/a2b58c3d...HEAD)

### Added
- Added the `path-bases` feature to support paths that resolve relatively to a
base specified in the config.
[docs](https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#path-bases)
[#14360](https://github.com/rust-lang/cargo/pull/14360)

### Changed

Expand Down
12 changes: 12 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<PathBaseName>,
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 Expand Up @@ -1413,6 +1415,16 @@ impl<T: AsRef<str>> FeatureName<T> {
}
}

str_newtype!(PathBaseName);

impl<T: AsRef<str>> PathBaseName<T> {
/// Validated path base name
pub fn new(name: T) -> Result<Self, NameValidationError> {
restricted_names::validate_path_base_name(name.as_ref())?;
Ok(Self(name))
}
}

/// Corresponds to a `target` entry, but `TomlTarget` is already used.
#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(rename_all = "kebab-case")]
Expand Down
42 changes: 42 additions & 0 deletions crates/cargo-util-schemas/src/restricted_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,37 @@ pub(crate) fn validate_feature_name(name: &str) -> Result<()> {
Ok(())
}

pub(crate) fn validate_path_base_name(name: &str) -> Result<()> {
let what = "path base name";
let mut chars = name.chars();

let Some(first_char) = chars.next() else {
return Err(ErrorKind::Empty(what).into());
};

if !first_char.is_alphabetic() {
return Err(ErrorKind::InvalidCharacter {
ch: first_char,
what,
name: name.into(),
reason: "first character must be a letter",
}
.into());
}

if let Some(ch) = chars.find(|c| !c.is_alphanumeric() && *c != '_' && *c != '-') {
return Err(ErrorKind::InvalidCharacter {
ch,
what,
name: name.into(),
reason: "allowed characters are letters, numbers, underscore, and hyphen",
}
.into());
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -264,4 +295,15 @@ mod tests {
assert!(validate_feature_name("a¼").is_err());
assert!(validate_feature_name("").is_err());
}

#[test]
fn validate_path_base_names() {
assert!(validate_path_base_name("foo").is_ok());
assert!(validate_path_base_name("foo12342_42-hello").is_ok());

assert!(validate_path_base_name("").is_err());
assert!(validate_path_base_name("42foo").is_err());
assert!(validate_path_base_name("_foo").is_err());
assert!(validate_path_base_name("foo+bar").is_err());
}
}
3 changes: 3 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,9 @@ features! {

/// Allow multiple packages to participate in the same API namespace
(unstable, open_namespaces, "", "reference/unstable.html#open-namespaces"),

/// Allow paths that resolve relatively to a base specified in the config.
(unstable, path_bases, "", "reference/unstable.html#path-bases"),
}

/// Status and metadata for a single unstable feature.
Expand Down
150 changes: 130 additions & 20 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use crate::AlreadyPrintedError;
use anyhow::{anyhow, bail, Context as _};
use cargo_platform::Platform;
use cargo_util::paths::{self, normalize_path};
use cargo_util_schemas::manifest::{self, TomlManifest};
use cargo_util_schemas::manifest::{
self, PackageName, PathBaseName, TomlDependency, TomlDetailedDependency, TomlManifest,
};
use cargo_util_schemas::manifest::{RustVersion, StringOrBool};
use itertools::Itertools;
use lazycell::LazyCell;
Expand Down Expand Up @@ -296,7 +298,7 @@ fn normalize_toml(
features: None,
target: None,
replace: original_toml.replace.clone(),
patch: original_toml.patch.clone(),
patch: None,
workspace: original_toml.workspace.clone(),
badges: None,
lints: None,
Expand All @@ -305,11 +307,18 @@ fn normalize_toml(

let package_root = manifest_file.parent().unwrap();

let inherit_cell: LazyCell<InheritableFields> = LazyCell::new();
let inherit = || {
let inherit_cell: LazyCell<Option<InheritableFields>> = LazyCell::new();
let load_inherit = || {
inherit_cell
.try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config))
};
let inherit = || {
load_inherit()?
.as_ref()
.ok_or_else(|| anyhow!("failed to find a workspace root"))
};
let workspace_root =
|| load_inherit().map(|inherit| inherit.as_ref().map(|fields| fields.ws_root()));

if let Some(original_package) = original_toml.package() {
let package_name = &original_package.name;
Expand Down Expand Up @@ -390,6 +399,7 @@ fn normalize_toml(
&activated_opt_deps,
None,
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -410,6 +420,7 @@ fn normalize_toml(
&activated_opt_deps,
Some(DepKind::Development),
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -430,6 +441,7 @@ fn normalize_toml(
&activated_opt_deps,
Some(DepKind::Build),
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -443,6 +455,7 @@ fn normalize_toml(
&activated_opt_deps,
None,
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -463,6 +476,7 @@ fn normalize_toml(
&activated_opt_deps,
Some(DepKind::Development),
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -483,6 +497,7 @@ fn normalize_toml(
&activated_opt_deps,
Some(DepKind::Build),
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -499,6 +514,13 @@ fn normalize_toml(
}
normalized_toml.target = (!normalized_target.is_empty()).then_some(normalized_target);

normalized_toml.patch = normalize_patch(
gctx,
original_toml.patch.as_ref(),
&workspace_root,
features,
)?;

let normalized_lints = original_toml
.lints
.clone()
Expand All @@ -519,6 +541,37 @@ fn normalize_toml(
Ok(normalized_toml)
}

fn normalize_patch<'a>(
gctx: &GlobalContext,
original_patch: Option<&BTreeMap<String, BTreeMap<PackageName, TomlDependency>>>,
workspace_root: &dyn Fn() -> CargoResult<Option<&'a PathBuf>>,
features: &Features,
) -> CargoResult<Option<BTreeMap<String, BTreeMap<PackageName, TomlDependency>>>> {
if let Some(patch) = original_patch {
let mut normalized_patch = BTreeMap::new();
for (name, packages) in patch {
let mut normalized_packages = BTreeMap::new();
for (pkg, dep) in packages {
let dep = if let TomlDependency::Detailed(dep) = dep {
let mut dep = dep.clone();
normalize_path_dependency(gctx, &mut dep, workspace_root, features)
.with_context(|| {
format!("resolving path for patch of ({pkg}) for source ({name})")
})?;
TomlDependency::Detailed(dep)
} else {
dep.clone()
};
normalized_packages.insert(pkg.clone(), dep);
}
normalized_patch.insert(name.clone(), normalized_packages);
}
Ok(Some(normalized_patch))
} else {
Ok(None)
}
}

#[tracing::instrument(skip_all)]
fn normalize_package_toml<'a>(
original_package: &manifest::TomlPackage,
Expand Down Expand Up @@ -710,6 +763,7 @@ fn normalize_dependencies<'a>(
activated_opt_deps: &HashSet<&str>,
kind: Option<DepKind>,
inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>,
workspace_root: &dyn Fn() -> CargoResult<Option<&'a PathBuf>>,
package_root: &Path,
warnings: &mut Vec<String>,
) -> CargoResult<Option<BTreeMap<manifest::PackageName, manifest::InheritableDependency>>> {
Expand Down Expand Up @@ -768,6 +822,8 @@ fn normalize_dependencies<'a>(
}
}
}
normalize_path_dependency(gctx, d, workspace_root, features)
.with_context(|| format!("resolving path dependency ({name_in_toml})"))?;
}

// if the dependency is not optional, it is always used
Expand All @@ -786,13 +842,30 @@ fn normalize_dependencies<'a>(
Ok(Some(deps))
}

fn normalize_path_dependency<'a>(
gctx: &GlobalContext,
detailed_dep: &mut TomlDetailedDependency,
workspace_root: &dyn Fn() -> CargoResult<Option<&'a PathBuf>>,
features: &Features,
) -> CargoResult<()> {
if let Some(base) = detailed_dep.base.take() {
if let Some(path) = detailed_dep.path.as_mut() {
let new_path = lookup_path_base(&base, gctx, workspace_root, features)?.join(&path);
*path = new_path.to_str().unwrap().to_string();
} else {
bail!("`base` can only be used with path dependencies");
}
}
Ok(())
}

fn load_inheritable_fields(
gctx: &GlobalContext,
normalized_path: &Path,
workspace_config: &WorkspaceConfig,
) -> CargoResult<InheritableFields> {
) -> CargoResult<Option<InheritableFields>> {
match workspace_config {
WorkspaceConfig::Root(root) => Ok(root.inheritable().clone()),
WorkspaceConfig::Root(root) => Ok(Some(root.inheritable().clone())),
WorkspaceConfig::Member {
root: Some(ref path_to_root),
} => {
Expand All @@ -802,14 +875,11 @@ fn load_inheritable_fields(
.join(path_to_root)
.join("Cargo.toml");
let root_path = paths::normalize_path(&path);
inheritable_from_path(gctx, root_path)
}
WorkspaceConfig::Member { root: None } => {
match find_workspace_root(&normalized_path, gctx)? {
Some(path_to_root) => inheritable_from_path(gctx, path_to_root),
None => Err(anyhow!("failed to find a workspace root")),
}
inheritable_from_path(gctx, root_path).map(Some)
}
WorkspaceConfig::Member { root: None } => find_workspace_root(&normalized_path, gctx)?
.map(|path_to_root| inheritable_from_path(gctx, path_to_root))
.transpose(),
}
}

Expand Down Expand Up @@ -901,13 +971,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 @@ -2151,6 +2225,41 @@ fn to_dependency_source_id<P: ResolveToPath + Clone>(
}
}

pub(crate) fn lookup_path_base<'a>(
base: &PathBaseName,
gctx: &GlobalContext,
workspace_root: &dyn Fn() -> CargoResult<Option<&'a PathBuf>>,
features: &Features,
) -> CargoResult<PathBuf> {
features.require(Feature::path_bases())?;

// Look up the relevant base in the Config and use that as the root.
// NOTE: The `base` string is user controlled, but building the path is safe from injection
// attacks since the `PathBaseName` type restricts the characters that can be used.
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.as_str() {
"workspace" => {
if let Some(workspace_root) = workspace_root()? {
Ok(workspace_root.clone())
} 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 +2974,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
Loading

0 comments on commit a591ec4

Please sign in to comment.