Only apply config patches on resolve

Author: Jon Gjengset

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<Url, Vec<Dependency>> {
-        match self.root_maybe() {
+    pub fn root_patch(&self) -> Cow<'_, HashMap<Url, Vec<Dependency>>> {
+        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

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 => {

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<Option<(Option<FileLock>, usize)>>,
+    /// `[patch]` section parsed from configuration.
+    patch: LazyCell<HashMap<Url, Vec<Dependency>>>,
     /// Cached configuration parsed by Cargo
     http_config: LazyCell<CargoHttpConfig>,
     net_config: LazyCell<CargoNetConfig>,
@@ -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<F>(&self, f: F) -> CargoResult<&HashMap<Url, Vec<Dependency>>>
+    where
+        F: FnMut() -> CargoResult<HashMap<Url, Vec<Dependency>>>,
+    {
+        self.patch.try_borrow_with(f)
+    }
+
+    pub fn patch(&self) -> Option<&HashMap<Url, Vec<Dependency>>> {
+        self.patch.borrow()
+    }
+
     pub fn creation_time(&self) -> Instant {
         self.creation_time
     }

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<HashMap<Url, Vec<Dependency>>> {
-        let from_manifest = Self::patch_(self.patch.as_ref(), cx)?;
-
-        let config_patch: Option<BTreeMap<String, BTreeMap<String, TomlDependency>>> =
-            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<BTreeMap<String, BTreeMap<String, TomlDependency>>> =
+                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.

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();