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 path-bases (RFC 3529) 1/n: path dep and patch support #14360

Merged
merged 1 commit into from
Aug 14, 2024
Merged
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
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
dpaoliello marked this conversation as resolved.
Show resolved Hide resolved
epage marked this conversation as resolved.
Show resolved Hide resolved
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)
dpaoliello marked this conversation as resolved.
Show resolved Hide resolved

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