Skip to content

Commit

Permalink
Auto merge of #14360 - dpaoliello:basepath1, r=epage
Browse files Browse the repository at this point in the history
Implement base paths (RFC 3529) 1/n: path dep and patch support

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

This PR add support in path dependencies and `patch` table and the `workspace` built-in path base.
  • Loading branch information
bors committed Aug 14, 2024
2 parents 3a20ea7 + 502c74e commit ff6df29
Show file tree
Hide file tree
Showing 7 changed files with 840 additions and 9 deletions.
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
4 changes: 4 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,10 @@ pub(crate) fn validate_feature_name(name: &str) -> Result<()> {
Ok(())
}

pub(crate) fn validate_path_base_name(name: &str) -> Result<()> {
validate_name(name, "path base name")
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
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
117 changes: 108 additions & 9 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 @@ -310,6 +312,7 @@ fn normalize_toml(
inherit_cell
.try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config))
};
let workspace_root = || inherit().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 +393,7 @@ fn normalize_toml(
&activated_opt_deps,
None,
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -410,6 +414,7 @@ fn normalize_toml(
&activated_opt_deps,
Some(DepKind::Development),
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -430,6 +435,7 @@ fn normalize_toml(
&activated_opt_deps,
Some(DepKind::Build),
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -443,6 +449,7 @@ fn normalize_toml(
&activated_opt_deps,
None,
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -463,6 +470,7 @@ fn normalize_toml(
&activated_opt_deps,
Some(DepKind::Development),
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -483,6 +491,7 @@ fn normalize_toml(
&activated_opt_deps,
Some(DepKind::Build),
&inherit,
&workspace_root,
package_root,
warnings,
)?;
Expand All @@ -499,6 +508,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 +535,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<&'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 +757,7 @@ fn normalize_dependencies<'a>(
activated_opt_deps: &HashSet<&str>,
kind: Option<DepKind>,
inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>,
workspace_root: &dyn Fn() -> CargoResult<&'a PathBuf>,
package_root: &Path,
warnings: &mut Vec<String>,
) -> CargoResult<Option<BTreeMap<manifest::PackageName, manifest::InheritableDependency>>> {
Expand Down Expand Up @@ -768,6 +816,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,6 +836,23 @@ fn normalize_dependencies<'a>(
Ok(Some(deps))
}

fn normalize_path_dependency<'a>(
gctx: &GlobalContext,
detailed_dep: &mut TomlDetailedDependency,
workspace_root: &dyn Fn() -> CargoResult<&'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,
Expand Down Expand Up @@ -901,13 +968,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 +2222,33 @@ fn to_dependency_source_id<P: ResolveToPath + Clone>(
}
}

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

// HACK: 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 to exclude `.`
let base_key = format!("path-bases.{base}");

// Look up the relevant base in the Config and use that as the root.
if let Some(path_bases) = gctx.get::<Option<ConfigRelativePath>>(&base_key)? {
Ok(path_bases.resolve_path(gctx))
} else {
// Otherwise, check the built-in bases.
match base.as_str() {
"workspace" => Ok(workspace_root()?.clone()),
_ => 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 +2963,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
55 changes: 55 additions & 0 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ Each new feature described below should explain how to use it.
* [Edition 2024](#edition-2024) — Adds support for the 2024 Edition.
* [Profile `trim-paths` option](#profile-trim-paths-option) --- Control the sanitization of file paths in build outputs.
* [`[lints.cargo]`](#lintscargo) --- Allows configuring lints for Cargo.
* [path bases](#path-bases) --- Named base directories for path dependencies.
* Information and metadata
* [Build-plan](#build-plan) --- Emits JSON information on which commands will be run.
* [unit-graph](#unit-graph) --- Emits JSON for Cargo's internal graph structure.
Expand Down Expand Up @@ -1570,6 +1571,60 @@ implicit-features = "warn"
workspace = true
```

## Path Bases

* Tracking Issue: [#14355](https://github.com/rust-lang/cargo/issues/14355)

A `path` dependency may optionally specify a base by setting the `base` key to
the name of a path base from the `[path-bases]` table in either the
[configuration](config.md) or one of the [built-in path bases](#built-in-path-bases).
The value of that path base is prepended to the `path` value (along with a path
separator if necessary) to produce the actual location where Cargo will look for
the dependency.

For example, if the `Cargo.toml` contains:

```toml
cargo-features = ["path-bases"]

[dependencies]
foo = { base = "dev", path = "foo" }
```

Given a `[path-bases]` table in the configuration that contains:

```toml
[path-bases]
dev = "/home/user/dev/rust/libraries/"
```

This will produce a `path` dependency `foo` located at
`/home/user/dev/rust/libraries/foo`.

Path bases can be either absolute or relative. Relative path bases are relative
to the parent directory of the configuration file that declared that path base.

The name of a path base must use only [alphanumeric](https://doc.rust-lang.org/std/primitive.char.html#method.is_alphanumeric)
characters or `-` or `_`, must start with an [alphabetic](https://doc.rust-lang.org/std/primitive.char.html#method.is_alphabetic)
character, and must not be empty.

If the name of path base used in a dependency is neither in the configuration
nor one of the built-in path base, then Cargo will raise an error.

#### Built-in path bases

Cargo provides implicit path bases that can be used without the need to specify
them in a `[path-bases]` table.

* `workspace` - If a project is [a workspace or workspace member](workspaces.md)
then this path base is defined as the parent directory of the root `Cargo.toml`
of the workspace.

If a built-in path base name is also declared in the configuration, then Cargo
will prefer the value in the configuration. The allows Cargo to add new built-in
path bases without compatibility issues (as existing uses will shadow the
built-in name).

# Stabilized and removed features

## Compile progress
Expand Down
Loading

0 comments on commit ff6df29

Please sign in to comment.