From: Jon Gjengset Date: Fri, 26 Feb 2021 17:29:33 +0000 (-0800) Subject: Only apply config patches on resolve X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=140a7707adbfb2085e8d24e5900d79bfe49cdd6d;p=cargo.git Only apply config patches on resolve --- diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index f72f0915c..63a7a3e06 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 9cab4434f..f03473697 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 a0dc0963c..eb04e9a66 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 d31c7114d..10eb2556f 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 12f230506..1c94f7de9 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();