From 8178f22ee901b6bca67eea2f044e5105fbe4af01 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Wed, 24 Feb 2021 14:14:43 -0800 Subject: [PATCH 1/5] Support [patch] in .cargo/config files This patch adds support for `[patch]` sections in `.cargo/config.toml` files. Patches from config files defer to `[patch]` in `Cargo.toml` if both provide a patch for the same crate. The current implementation merge config patches into the workspace manifest patches. It's unclear if that's the right long-term plan, or whether these patches should be stored separately (though likely still in the manifest). Regardless, they _should_ likely continue to be parsed when the manifest is parsed so that errors and such occur in the same place regardless of where a patch is specified. Fixes #5539. --- src/cargo/core/features.rs | 2 + src/cargo/util/toml/mod.rs | 26 ++++- tests/testsuite/patch.rs | 217 +++++++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 377cf25fb58..d50a28828e6 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -551,6 +551,7 @@ pub struct CliUnstable { pub extra_link_arg: bool, pub credential_process: bool, pub configurable_env: bool, + pub patch_in_config: bool, } const STABILIZED_COMPILE_PROGRESS: &str = "The progress bar is now always \ @@ -707,6 +708,7 @@ impl CliUnstable { "panic-abort-tests" => self.panic_abort_tests = parse_empty(k, v)?, "jobserver-per-rustc" => self.jobserver_per_rustc = parse_empty(k, v)?, "configurable-env" => self.configurable_env = parse_empty(k, v)?, + "patch-in-config" => self.patch_in_config = parse_empty(k, v)?, "features" => { // For now this is still allowed (there are still some // unstable options like "compare"). This should be removed at diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 683f5e7c825..d31c7114d18 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1531,9 +1531,12 @@ impl TomlManifest { Ok(replace) } - fn patch(&self, cx: &mut Context<'_, '_>) -> CargoResult>> { + fn patch_( + table: Option<&BTreeMap>>, + cx: &mut Context<'_, '_>, + ) -> CargoResult>> { let mut patch = HashMap::new(); - for (url, deps) in self.patch.iter().flatten() { + for (url, deps) in table.into_iter().flatten() { let url = match &url[..] { CRATES_IO_REGISTRY => CRATES_IO_INDEX.parse().unwrap(), _ => cx @@ -1554,6 +1557,25 @@ impl TomlManifest { Ok(patch) } + fn patch(&self, cx: &mut Context<'_, '_>) -> CargoResult>> { + let from_manifest = Self::patch_(self.patch.as_ref(), cx)?; + + let config_patch: Option>> = + cx.config.get("patch")?; + + if config_patch.is_some() && !cx.config.cli_unstable().patch_in_config { + cx.warnings.push("`[patch]` in .cargo/config.toml ignored, the -Zpatch-in-config command-line flag is required".to_owned()); + return Ok(from_manifest); + } + + let mut from_config = Self::patch_(config_patch.as_ref(), cx)?; + if from_config.is_empty() { + return Ok(from_manifest); + } + from_config.extend(from_manifest); + Ok(from_config) + } + /// Returns the path to the build script if one exists for this crate. fn maybe_custom_build( &self, diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 0890b1e8565..12f23050661 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -66,6 +66,91 @@ fn replace() { p.cargo("build").with_stderr("[FINISHED] [..]").run(); } +#[cargo_test] +fn from_config_without_z() { + Package::new("bar", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + "#, + ) + .file( + ".cargo/config.toml", + r#" + [patch.crates-io] + bar = { path = 'bar' } + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.1")) + .file("bar/src/lib.rs", r#""#) + .build(); + + p.cargo("build") + .with_stderr( + "\ +[WARNING] `[patch]` in .cargo/config.toml ignored, the -Zpatch-in-config command-line flag is required +[UPDATING] `[ROOT][..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.1.0 ([..]) +[COMPILING] bar v0.1.0 +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} +#[cargo_test] +fn from_config() { + Package::new("bar", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + "#, + ) + .file( + ".cargo/config.toml", + r#" + [patch.crates-io] + bar = { path = 'bar' } + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.1")) + .file("bar/src/lib.rs", r#""#) + .build(); + + p.cargo("build -Zpatch-in-config") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[ROOT][..]` index +[COMPILING] bar v0.1.1 ([..]) +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + #[cargo_test] fn nonexistent() { Package::new("baz", "0.1.0").publish(); @@ -268,6 +353,78 @@ fn unused() { ); } +#[cargo_test] +fn unused_from_config() { + Package::new("bar", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + "#, + ) + .file( + ".cargo/config.toml", + r#" + [patch.crates-io] + bar = { path = "bar" } + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.2.0")) + .file("bar/src/lib.rs", "not rust code") + .build(); + + p.cargo("build -Zpatch-in-config") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[ROOT][..]` index +[WARNING] Patch `bar v0.2.0 ([CWD]/bar)` was not used in the crate graph. +[..] +[..] +[..] +[..] +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.1.0 [..] +[COMPILING] bar v0.1.0 +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + p.cargo("build -Zpatch-in-config") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[WARNING] Patch `bar v0.2.0 ([CWD]/bar)` was not used in the crate graph. +[..] +[..] +[..] +[..] +[FINISHED] [..] +", + ) + .run(); + + // unused patch should be in the lock file + let lock = p.read_lockfile(); + let toml: toml::Value = toml::from_str(&lock).unwrap(); + assert_eq!(toml["patch"]["unused"].as_array().unwrap().len(), 1); + assert_eq!(toml["patch"]["unused"][0]["name"].as_str(), Some("bar")); + assert_eq!( + toml["patch"]["unused"][0]["version"].as_str(), + Some("0.2.0") + ); +} + #[cargo_test] fn unused_git() { Package::new("bar", "0.1.0").publish(); @@ -395,6 +552,66 @@ fn add_patch() { p.cargo("build").with_stderr("[FINISHED] [..]").run(); } +#[cargo_test] +fn add_patch_from_config() { + Package::new("bar", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0")) + .file("bar/src/lib.rs", r#""#) + .build(); + + p.cargo("build") + .with_stderr( + "\ +[UPDATING] `[ROOT][..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.1.0 [..] +[COMPILING] bar v0.1.0 +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + p.cargo("build").with_stderr("[FINISHED] [..]").run(); + + p.change_file( + ".cargo/config.toml", + r#" + [patch.crates-io] + bar = { path = 'bar' } + "#, + ); + + p.cargo("build -Zpatch-in-config") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[COMPILING] bar v0.1.0 ([CWD]/bar) +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + p.cargo("build -Zpatch-in-config") + .masquerade_as_nightly_cargo() + .with_stderr("[FINISHED] [..]") + .run(); +} + #[cargo_test] fn add_ignored_patch() { Package::new("bar", "0.1.0").publish(); From 140a7707adbfb2085e8d24e5900d79bfe49cdd6d Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 26 Feb 2021 09:29:33 -0800 Subject: [PATCH 2/5] Only apply config patches on resolve --- src/cargo/core/workspace.rs | 40 +++++++++++++++++++++++++++++-- src/cargo/ops/resolve.rs | 2 +- src/cargo/util/config/mod.rs | 16 ++++++++++++- src/cargo/util/toml/mod.rs | 28 ++++++++++------------ tests/testsuite/patch.rs | 46 ++++++++++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 19 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index f72f0915c9f..63a7a3e061d 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::cell::RefCell; use std::collections::hash_map::{Entry, HashMap}; use std::collections::{BTreeMap, BTreeSet, HashSet}; @@ -365,11 +366,46 @@ impl<'cfg> Workspace<'cfg> { /// Returns the root `[patch]` section of this workspace. /// /// This may be from a virtual crate or an actual crate. - pub fn root_patch(&self) -> &HashMap> { - match self.root_maybe() { + pub fn root_patch(&self) -> Cow<'_, HashMap>> { + let from_manifest = match self.root_maybe() { MaybePackage::Package(p) => p.manifest().patch(), MaybePackage::Virtual(vm) => vm.patch(), + }; + + let from_config = self + .config + .patch() + .expect("config [patch] was never parsed"); + if from_config.is_empty() { + return Cow::Borrowed(from_manifest); + } + if from_manifest.is_empty() { + return Cow::Borrowed(from_config); + } + + // We could just chain from_manifest and from_config, + // but that's not quite right as it won't deal with overlaps. + let mut combined = from_manifest.clone(); + for (url, cdeps) in from_config { + if let Some(deps) = combined.get_mut(url) { + // We want from_manifest to take precedence for each patched name. + // NOTE: This is inefficient if the number of patches is large! + let mut left = cdeps.clone(); + for dep in &mut *deps { + if let Some(i) = left.iter().position(|cdep| { + // XXX: should this also take into account version numbers? + dep.name_in_toml() == cdep.name_in_toml() + }) { + left.swap_remove(i); + } + } + // Whatever is left does not exist in manifest dependencies. + deps.extend(left); + } else { + combined.insert(url.clone(), cdeps.clone()); + } } + Cow::Owned(combined) } /// Returns an iterator over all packages in this workspace diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 9cab4434f8b..f03473697a1 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -240,7 +240,7 @@ pub fn resolve_with_previous<'cfg>( // locked. let mut avoid_patch_ids = HashSet::new(); if register_patches { - for (url, patches) in ws.root_patch() { + for (url, patches) in ws.root_patch().iter() { let previous = match previous { Some(r) => r, None => { diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index a0dc0963c17..eb04e9a6664 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -74,7 +74,7 @@ use url::Url; use self::ConfigValue as CV; use crate::core::compiler::rustdoc::RustdocExternMap; use crate::core::shell::Verbosity; -use crate::core::{nightly_features_allowed, CliUnstable, Shell, SourceId, Workspace}; +use crate::core::{nightly_features_allowed, CliUnstable, Dependency, Shell, SourceId, Workspace}; use crate::ops; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::toml as cargo_toml; @@ -176,6 +176,8 @@ pub struct Config { /// Lock, if held, of the global package cache along with the number of /// acquisitions so far. package_cache_lock: RefCell, usize)>>, + /// `[patch]` section parsed from configuration. + patch: LazyCell>>, /// Cached configuration parsed by Cargo http_config: LazyCell, net_config: LazyCell, @@ -261,6 +263,7 @@ impl Config { upper_case_env, updated_sources: LazyCell::new(), package_cache_lock: RefCell::new(None), + patch: LazyCell::new(), http_config: LazyCell::new(), net_config: LazyCell::new(), build_config: LazyCell::new(), @@ -1291,6 +1294,17 @@ impl Config { Ok(*(self.crates_io_source_id.try_borrow_with(f)?)) } + pub fn maybe_init_patch(&self, f: F) -> CargoResult<&HashMap>> + where + F: FnMut() -> CargoResult>>, + { + self.patch.try_borrow_with(f) + } + + pub fn patch(&self) -> Option<&HashMap>> { + self.patch.borrow() + } + pub fn creation_time(&self) -> Instant { self.creation_time } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index d31c7114d18..10eb2556f67 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf}; use std::rc::Rc; use std::str; -use anyhow::{anyhow, bail}; +use anyhow::{anyhow, bail, Context as _}; use cargo_platform::Platform; use log::{debug, trace}; use semver::{self, VersionReq}; @@ -1558,22 +1558,20 @@ impl TomlManifest { } fn patch(&self, cx: &mut Context<'_, '_>) -> CargoResult>> { - let from_manifest = Self::patch_(self.patch.as_ref(), cx)?; - - let config_patch: Option>> = - cx.config.get("patch")?; + let _ = cx.config.maybe_init_patch(|| { + // Parse out the patches from .config while we're at it. + let config_patch: Option>> = + cx.config.get("patch")?; + + if config_patch.is_some() && !cx.config.cli_unstable().patch_in_config { + cx.warnings.push("`[patch]` in .cargo/config.toml ignored, the -Zpatch-in-config command-line flag is required".to_owned()); + return Ok(HashMap::new()); + } - if config_patch.is_some() && !cx.config.cli_unstable().patch_in_config { - cx.warnings.push("`[patch]` in .cargo/config.toml ignored, the -Zpatch-in-config command-line flag is required".to_owned()); - return Ok(from_manifest); - } + Self::patch_(config_patch.as_ref(), cx) + }).context("parse `[patch]` from .cargo/config.toml")?; - let mut from_config = Self::patch_(config_patch.as_ref(), cx)?; - if from_config.is_empty() { - return Ok(from_manifest); - } - from_config.extend(from_manifest); - Ok(from_config) + Self::patch_(self.patch.as_ref(), cx) } /// Returns the path to the build script if one exists for this crate. diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 12f23050661..1c94f7de9f8 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -109,6 +109,7 @@ fn from_config_without_z() { ) .run(); } + #[cargo_test] fn from_config() { Package::new("bar", "0.1.0").publish(); @@ -151,6 +152,51 @@ fn from_config() { .run(); } +#[cargo_test] +fn from_config_precedence() { + Package::new("bar", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + + [patch.crates-io] + bar = { path = 'bar' } + "#, + ) + .file( + ".cargo/config.toml", + r#" + [patch.crates-io] + bar = { path = 'no-such-path' } + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.1")) + .file("bar/src/lib.rs", r#""#) + .build(); + + p.cargo("build -Zpatch-in-config") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[ROOT][..]` index +[COMPILING] bar v0.1.1 ([..]) +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + #[cargo_test] fn nonexistent() { Package::new("baz", "0.1.0").publish(); From 2c88a633b4308dc41ad14bf500a5d3a03a759157 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Thu, 4 Mar 2021 15:16:37 -0800 Subject: [PATCH 3/5] Remove Cow over-optimization --- src/cargo/core/workspace.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 63a7a3e061d..92c7724220a 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1,4 +1,3 @@ -use std::borrow::Cow; use std::cell::RefCell; use std::collections::hash_map::{Entry, HashMap}; use std::collections::{BTreeMap, BTreeSet, HashSet}; @@ -366,7 +365,7 @@ impl<'cfg> Workspace<'cfg> { /// Returns the root `[patch]` section of this workspace. /// /// This may be from a virtual crate or an actual crate. - pub fn root_patch(&self) -> Cow<'_, HashMap>> { + pub fn root_patch(&self) -> HashMap> { let from_manifest = match self.root_maybe() { MaybePackage::Package(p) => p.manifest().patch(), MaybePackage::Virtual(vm) => vm.patch(), @@ -377,10 +376,10 @@ impl<'cfg> Workspace<'cfg> { .patch() .expect("config [patch] was never parsed"); if from_config.is_empty() { - return Cow::Borrowed(from_manifest); + return from_manifest.clone(); } if from_manifest.is_empty() { - return Cow::Borrowed(from_config); + return from_config.clone(); } // We could just chain from_manifest and from_config, @@ -405,7 +404,7 @@ impl<'cfg> Workspace<'cfg> { combined.insert(url.clone(), cdeps.clone()); } } - Cow::Owned(combined) + combined } /// Returns an iterator over all packages in this workspace From 6995e1dd1b89d919aa0e2f8443c7cbaf4931bb32 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 8 Mar 2021 13:53:29 -0800 Subject: [PATCH 4/5] Extract config patch on use --- src/cargo/core/resolver/encode.rs | 8 +- src/cargo/core/workspace.rs | 82 ++++++++++++++++--- src/cargo/ops/resolve.rs | 2 +- src/cargo/util/config/mod.rs | 16 +--- src/cargo/util/toml/mod.rs | 130 +++++++++++++++++++++--------- tests/testsuite/patch.rs | 44 +++++++++- 6 files changed, 211 insertions(+), 71 deletions(-) diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 5c329124d9b..9b93a744980 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -154,7 +154,7 @@ impl EncodableResolve { /// primary uses is to be used with `resolve_with_previous` to guide the /// resolver to create a complete Resolve. pub fn into_resolve(self, original: &str, ws: &Workspace<'_>) -> CargoResult { - let path_deps = build_path_deps(ws); + let path_deps = build_path_deps(ws)?; let mut checksums = HashMap::new(); let mut version = match self.version { @@ -402,7 +402,7 @@ impl EncodableResolve { } } -fn build_path_deps(ws: &Workspace<'_>) -> HashMap { +fn build_path_deps(ws: &Workspace<'_>) -> CargoResult> { // If a crate is **not** a path source, then we're probably in a situation // such as `cargo install` with a lock file from a remote dependency. In // that case we don't need to fixup any path dependencies (as they're not @@ -424,7 +424,7 @@ fn build_path_deps(ws: &Workspace<'_>) -> HashMap { for member in members.iter() { build_pkg(member, ws, &mut ret, &mut visited); } - for deps in ws.root_patch().values() { + for deps in ws.root_patch()?.values() { for dep in deps { build_dep(dep, ws, &mut ret, &mut visited); } @@ -433,7 +433,7 @@ fn build_path_deps(ws: &Workspace<'_>) -> HashMap { build_dep(dep, ws, &mut ret, &mut visited); } - return ret; + return Ok(ret); fn build_pkg( pkg: &Package, diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 92c7724220a..57e91c575f5 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -16,12 +16,12 @@ use crate::core::resolver::ResolveBehavior; use crate::core::{Dependency, Edition, PackageId, PackageIdSpec}; use crate::core::{EitherManifest, Package, SourceId, VirtualManifest}; use crate::ops; -use crate::sources::PathSource; +use crate::sources::{PathSource, CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::util::errors::{CargoResult, CargoResultExt, ManifestError}; use crate::util::interning::InternedString; use crate::util::paths; -use crate::util::toml::{read_manifest, TomlProfiles}; -use crate::util::{Config, Filesystem}; +use crate::util::toml::{read_manifest, TomlDependency, TomlProfiles}; +use crate::util::{config::ConfigRelativePath, Config, Filesystem, IntoUrl}; /// The core abstraction in Cargo for working with a workspace of crates. /// @@ -362,31 +362,89 @@ impl<'cfg> Workspace<'cfg> { } } + fn config_patch(&self) -> CargoResult>> { + let config_patch: Option< + BTreeMap>>, + > = self.config.get("patch")?; + + if config_patch.is_some() && !self.config.cli_unstable().patch_in_config { + self.config.shell().warn("`[patch]` in cargo config was ignored, the -Zpatch-in-config command-line flag is required".to_owned())?; + return Ok(HashMap::new()); + } + + let source = SourceId::for_path(self.root())?; + + let mut warnings = Vec::new(); + let mut nested_paths = Vec::new(); + + let mut patch = HashMap::new(); + for (url, deps) in config_patch.into_iter().flatten() { + let url = match &url[..] { + CRATES_IO_REGISTRY => CRATES_IO_INDEX.parse().unwrap(), + url => self + .config + .get_registry_index(url) + .or_else(|_| url.into_url()) + .chain_err(|| { + format!("[patch] entry `{}` should be a URL or registry name", url) + })?, + }; + patch.insert( + url, + deps.iter() + .map(|(name, dep)| { + dep.to_dependency_split( + name, + /* pkg_id */ None, + source, + &mut nested_paths, + self.config, + &mut warnings, + /* platform */ None, + // NOTE: Since we use ConfigRelativePath, this root isn't used as + // any relative paths are resolved before they'd be joined with root. + self.root(), + self.unstable_features(), + None, + ) + }) + .collect::>>()?, + ); + } + + for message in warnings { + self.config + .shell() + .warn(format!("[patch] in cargo config: {}", message))? + } + + let _ = nested_paths; + + Ok(patch) + } + /// Returns the root `[patch]` section of this workspace. /// /// This may be from a virtual crate or an actual crate. - pub fn root_patch(&self) -> HashMap> { + pub fn root_patch(&self) -> CargoResult>> { let from_manifest = match self.root_maybe() { MaybePackage::Package(p) => p.manifest().patch(), MaybePackage::Virtual(vm) => vm.patch(), }; - let from_config = self - .config - .patch() - .expect("config [patch] was never parsed"); + let from_config = self.config_patch()?; if from_config.is_empty() { - return from_manifest.clone(); + return Ok(from_manifest.clone()); } if from_manifest.is_empty() { - return from_config.clone(); + return Ok(from_config.clone()); } // We could just chain from_manifest and from_config, // but that's not quite right as it won't deal with overlaps. let mut combined = from_manifest.clone(); for (url, cdeps) in from_config { - if let Some(deps) = combined.get_mut(url) { + if let Some(deps) = combined.get_mut(&url) { // We want from_manifest to take precedence for each patched name. // NOTE: This is inefficient if the number of patches is large! let mut left = cdeps.clone(); @@ -404,7 +462,7 @@ impl<'cfg> Workspace<'cfg> { combined.insert(url.clone(), cdeps.clone()); } } - combined + Ok(combined) } /// Returns an iterator over all packages in this workspace diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index f03473697a1..d28ac92c0de 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -240,7 +240,7 @@ pub fn resolve_with_previous<'cfg>( // locked. let mut avoid_patch_ids = HashSet::new(); if register_patches { - for (url, patches) in ws.root_patch().iter() { + for (url, patches) in ws.root_patch()?.iter() { let previous = match previous { Some(r) => r, None => { diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index c880cae0f5c..b7b4ed36712 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -74,7 +74,7 @@ use url::Url; use self::ConfigValue as CV; use crate::core::compiler::rustdoc::RustdocExternMap; use crate::core::shell::Verbosity; -use crate::core::{features, CliUnstable, Dependency, Shell, SourceId, Workspace}; +use crate::core::{features, CliUnstable, Shell, SourceId, Workspace}; use crate::ops; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::toml as cargo_toml; @@ -176,8 +176,6 @@ pub struct Config { /// Lock, if held, of the global package cache along with the number of /// acquisitions so far. package_cache_lock: RefCell, usize)>>, - /// `[patch]` section parsed from configuration. - patch: LazyCell>>, /// Cached configuration parsed by Cargo http_config: LazyCell, net_config: LazyCell, @@ -279,7 +277,6 @@ impl Config { upper_case_env, updated_sources: LazyCell::new(), package_cache_lock: RefCell::new(None), - patch: LazyCell::new(), http_config: LazyCell::new(), net_config: LazyCell::new(), build_config: LazyCell::new(), @@ -1327,17 +1324,6 @@ impl Config { Ok(*(self.crates_io_source_id.try_borrow_with(f)?)) } - pub fn maybe_init_patch(&self, f: F) -> CargoResult<&HashMap>> - where - F: FnMut() -> CargoResult>>, - { - self.patch.try_borrow_with(f) - } - - pub fn patch(&self) -> Option<&HashMap>> { - self.patch.borrow() - } - pub fn creation_time(&self) -> Instant { self.creation_time } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 9dba719d0e0..7f7aadb4ca5 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1,10 +1,11 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fmt; +use std::marker::PhantomData; use std::path::{Path, PathBuf}; use std::rc::Rc; use std::str; -use anyhow::{anyhow, bail, Context as _}; +use anyhow::{anyhow, bail}; use cargo_platform::Platform; use log::{debug, trace}; use semver::{self, VersionReq}; @@ -22,7 +23,9 @@ use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, Worksp use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::util::errors::{CargoResult, CargoResultExt, ManifestError}; use crate::util::interning::InternedString; -use crate::util::{self, paths, validate_package_name, Config, IntoUrl}; +use crate::util::{ + self, config::ConfigRelativePath, paths, validate_package_name, Config, IntoUrl, +}; mod targets; use self::targets::targets; @@ -199,25 +202,25 @@ type TomlBenchTarget = TomlTarget; #[derive(Clone, Debug, Serialize)] #[serde(untagged)] -pub enum TomlDependency { +pub enum TomlDependency

{ /// In the simple format, only a version is specified, eg. /// `package = ""` Simple(String), /// The simple format is equivalent to a detailed dependency /// specifying only a version, eg. /// `package = { version = "" }` - Detailed(DetailedTomlDependency), + Detailed(DetailedTomlDependency

), } -impl<'de> de::Deserialize<'de> for TomlDependency { +impl<'de, P: Deserialize<'de>> de::Deserialize<'de> for TomlDependency

{ fn deserialize(deserializer: D) -> Result where D: de::Deserializer<'de>, { - struct TomlDependencyVisitor; + struct TomlDependencyVisitor

(PhantomData

); - impl<'de> de::Visitor<'de> for TomlDependencyVisitor { - type Value = TomlDependency; + impl<'de, P: Deserialize<'de>> de::Visitor<'de> for TomlDependencyVisitor

{ + type Value = TomlDependency

; fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter.write_str( @@ -242,13 +245,29 @@ impl<'de> de::Deserialize<'de> for TomlDependency { } } - deserializer.deserialize_any(TomlDependencyVisitor) + deserializer.deserialize_any(TomlDependencyVisitor(PhantomData)) } } -#[derive(Deserialize, Serialize, Clone, Debug, Default)] +pub trait ResolveToPath { + fn resolve(&self, config: &Config) -> PathBuf; +} + +impl ResolveToPath for String { + fn resolve(&self, _: &Config) -> PathBuf { + self.into() + } +} + +impl ResolveToPath for ConfigRelativePath { + fn resolve(&self, c: &Config) -> PathBuf { + self.resolve_path(c) + } +} + +#[derive(Deserialize, Serialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] -pub struct DetailedTomlDependency { +pub struct DetailedTomlDependency

{ version: Option, registry: Option, /// The URL of the `registry` field. @@ -258,7 +277,9 @@ pub struct DetailedTomlDependency { /// registry names configured, so Cargo can't rely on just the name for /// crates published by other users. registry_index: Option, - path: Option, + // `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. + path: Option

, git: Option, branch: Option, tag: Option, @@ -272,6 +293,28 @@ pub struct DetailedTomlDependency { public: Option, } +// Explicit implementation so we avoid pulling in P: Default +impl

Default for DetailedTomlDependency

{ + fn default() -> Self { + Self { + version: Default::default(), + registry: Default::default(), + registry_index: Default::default(), + path: Default::default(), + git: Default::default(), + branch: Default::default(), + tag: Default::default(), + rev: Default::default(), + features: Default::default(), + optional: Default::default(), + default_features: Default::default(), + default_features2: Default::default(), + package: Default::default(), + public: Default::default(), + } + } +} + /// This type is used to deserialize `Cargo.toml` files. #[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] @@ -1530,12 +1573,9 @@ impl TomlManifest { Ok(replace) } - fn patch_( - table: Option<&BTreeMap>>, - cx: &mut Context<'_, '_>, - ) -> CargoResult>> { + fn patch(&self, cx: &mut Context<'_, '_>) -> CargoResult>> { let mut patch = HashMap::new(); - for (url, deps) in table.into_iter().flatten() { + for (url, deps) in self.patch.iter().flatten() { let url = match &url[..] { CRATES_IO_REGISTRY => CRATES_IO_INDEX.parse().unwrap(), _ => cx @@ -1556,23 +1596,6 @@ impl TomlManifest { Ok(patch) } - fn patch(&self, cx: &mut Context<'_, '_>) -> CargoResult>> { - let _ = cx.config.maybe_init_patch(|| { - // Parse out the patches from .config while we're at it. - let config_patch: Option>> = - cx.config.get("patch")?; - - if config_patch.is_some() && !cx.config.cli_unstable().patch_in_config { - cx.warnings.push("`[patch]` in .cargo/config.toml ignored, the -Zpatch-in-config command-line flag is required".to_owned()); - return Ok(HashMap::new()); - } - - Self::patch_(config_patch.as_ref(), cx) - }).context("parse `[patch]` from .cargo/config.toml")?; - - Self::patch_(self.patch.as_ref(), cx) - } - /// Returns the path to the build script if one exists for this crate. fn maybe_custom_build( &self, @@ -1647,7 +1670,37 @@ fn unique_build_targets(targets: &[Target], package_root: &Path) -> Result<(), S Ok(()) } -impl TomlDependency { +impl TomlDependency

{ + pub(crate) fn to_dependency_split( + &self, + name: &str, + pkgid: Option, + source_id: SourceId, + nested_paths: &mut Vec, + config: &Config, + warnings: &mut Vec, + platform: Option, + root: &Path, + features: &Features, + kind: Option, + ) -> CargoResult { + self.to_dependency( + name, + &mut Context { + pkgid, + deps: &mut Vec::new(), + source_id, + nested_paths, + config, + warnings, + platform, + root, + features, + }, + kind, + ) + } + fn to_dependency( &self, name: &str, @@ -1655,7 +1708,7 @@ impl TomlDependency { kind: Option, ) -> CargoResult { match *self { - TomlDependency::Simple(ref version) => DetailedTomlDependency { + TomlDependency::Simple(ref version) => DetailedTomlDependency::

{ version: Some(version.clone()), ..Default::default() } @@ -1672,7 +1725,7 @@ impl TomlDependency { } } -impl DetailedTomlDependency { +impl DetailedTomlDependency

{ fn to_dependency( &self, name_in_toml: &str, @@ -1782,7 +1835,8 @@ impl DetailedTomlDependency { SourceId::for_git(&loc, reference)? } (None, Some(path), _, _) => { - cx.nested_paths.push(PathBuf::from(path)); + let path = path.resolve(cx.config); + cx.nested_paths.push(path.clone()); // If the source ID for the package we're parsing is a path // source, then we normalize the path here to get rid of // components like `..`. diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index 1c94f7de9f8..aebb4762601 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -98,7 +98,7 @@ fn from_config_without_z() { p.cargo("build") .with_stderr( "\ -[WARNING] `[patch]` in .cargo/config.toml ignored, the -Zpatch-in-config command-line flag is required +[WARNING] `[patch]` in cargo config was ignored, the -Zpatch-in-config command-line flag is required [UPDATING] `[ROOT][..]` index [DOWNLOADING] crates ... [DOWNLOADED] bar v0.1.0 ([..]) @@ -152,6 +152,48 @@ fn from_config() { .run(); } +#[cargo_test] +fn from_config_relative() { + Package::new("bar", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "0.1.0" + "#, + ) + .file( + "../.cargo/config.toml", + r#" + [patch.crates-io] + bar = { path = 'foo/bar' } + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.1")) + .file("bar/src/lib.rs", r#""#) + .build(); + + p.cargo("build -Zpatch-in-config") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[ROOT][..]` index +[COMPILING] bar v0.1.1 ([..]) +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + #[cargo_test] fn from_config_precedence() { Package::new("bar", "0.1.0").publish(); From b50cde88b458774f0f1f8211ef450ff7872f3894 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Mon, 15 Mar 2021 09:54:48 -0700 Subject: [PATCH 5/5] Fixes from review --- src/cargo/core/workspace.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 57e91c575f5..b9b3d476748 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -403,9 +403,9 @@ impl<'cfg> Workspace<'cfg> { /* platform */ None, // NOTE: Since we use ConfigRelativePath, this root isn't used as // any relative paths are resolved before they'd be joined with root. - self.root(), + &Path::new("unused-relative-path"), self.unstable_features(), - None, + /* kind */ None, ) }) .collect::>>()?, @@ -418,8 +418,6 @@ impl<'cfg> Workspace<'cfg> { .warn(format!("[patch] in cargo config: {}", message))? } - let _ = nested_paths; - Ok(patch) }