Auto merge of #9204 - jonhoo:patch-in-config, r=alexcrichton

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.

Author: bors

src/cargo/core/features.rs

@@ -556,6 +556,7 @@ pub struct CliUnstable {
     pub namespaced_features: bool,
     pub weak_dep_features: bool,
     pub extra_link_arg: bool,
+    pub patch_in_config: bool,
     pub credential_process: bool,
     pub configurable_env: bool,
     pub enable_future_incompat_feature: bool,
@@ -719,6 +720,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

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<Resolve> {
-        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<String, SourceId> {
+fn build_path_deps(ws: &Workspace<'_>) -> CargoResult<HashMap<String, SourceId>> {
     // 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<String, SourceId> {
     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<String, SourceId> {
         build_dep(dep, ws, &mut ret, &mut visited);
     }
 
-    return ret;
+    return Ok(ret);
 
     fn build_pkg(
         pkg: &Package,

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,14 +362,105 @@ impl<'cfg> Workspace<'cfg> {
         }
     }
 
+    fn config_patch(&self) -> CargoResult<HashMap<Url, Vec<Dependency>>> {
+        let config_patch: Option<
+            BTreeMap<String, BTreeMap<String, TomlDependency<ConfigRelativePath>>>,
+        > = 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.
+                            &Path::new("unused-relative-path"),
+                            self.unstable_features(),
+                            /* kind */ None,
+                        )
+                    })
+                    .collect::<CargoResult<Vec<_>>>()?,
+            );
+        }
+
+        for message in warnings {
+            self.config
+                .shell()
+                .warn(format!("[patch] in cargo config: {}", message))?
+        }
+
+        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<Url, Vec<Dependency>> {
-        match self.root_maybe() {
+    pub fn root_patch(&self) -> CargoResult<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()?;
+        if from_config.is_empty() {
+            return Ok(from_manifest.clone());
+        }
+        if from_manifest.is_empty() {
+            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) {
+                // 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());
+            }
         }
+        Ok(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/toml/mod.rs

@@ -1,5 +1,6 @@
 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;
@@ -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<P = String> {
     /// In the simple format, only a version is specified, eg.
     /// `package = "<version>"`
     Simple(String),
     /// The simple format is equivalent to a detailed dependency
     /// specifying only a version, eg.
     /// `package = { version = "<version>" }`
-    Detailed(DetailedTomlDependency),
+    Detailed(DetailedTomlDependency<P>),
 }
 
-impl<'de> de::Deserialize<'de> for TomlDependency {
+impl<'de, P: Deserialize<'de>> de::Deserialize<'de> for TomlDependency<P> {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: de::Deserializer<'de>,
     {
-        struct TomlDependencyVisitor;
+        struct TomlDependencyVisitor<P>(PhantomData<P>);
 
-        impl<'de> de::Visitor<'de> for TomlDependencyVisitor {
-            type Value = TomlDependency;
+        impl<'de, P: Deserialize<'de>> de::Visitor<'de> for TomlDependencyVisitor<P> {
+            type Value = TomlDependency<P>;
 
             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<P = String> {
     version: Option<String>,
     registry: Option<String>,
     /// 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<String>,
-    path: Option<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.
+    path: Option<P>,
     git: Option<String>,
     branch: Option<String>,
     tag: Option<String>,
@@ -272,6 +293,28 @@ pub struct DetailedTomlDependency {
     public: Option<bool>,
 }
 
+// Explicit implementation so we avoid pulling in P: Default
+impl<P> Default for DetailedTomlDependency<P> {
+    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")]
@@ -1627,15 +1670,45 @@ fn unique_build_targets(targets: &[Target], package_root: &Path) -> Result<(), S
     Ok(())
 }
 
-impl TomlDependency {
+impl<P: ResolveToPath> TomlDependency<P> {
+    pub(crate) fn to_dependency_split(
+        &self,
+        name: &str,
+        pkgid: Option<PackageId>,
+        source_id: SourceId,
+        nested_paths: &mut Vec<PathBuf>,
+        config: &Config,
+        warnings: &mut Vec<String>,
+        platform: Option<Platform>,
+        root: &Path,
+        features: &Features,
+        kind: Option<DepKind>,
+    ) -> CargoResult<Dependency> {
+        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,
         cx: &mut Context<'_, '_>,
         kind: Option<DepKind>,
     ) -> CargoResult<Dependency> {
         match *self {
-            TomlDependency::Simple(ref version) => DetailedTomlDependency {
+            TomlDependency::Simple(ref version) => DetailedTomlDependency::<P> {
                 version: Some(version.clone()),
                 ..Default::default()
             }
@@ -1652,7 +1725,7 @@ impl TomlDependency {
     }
 }
 
-impl DetailedTomlDependency {
+impl<P: ResolveToPath> DetailedTomlDependency<P> {
     fn to_dependency(
         &self,
         name_in_toml: &str,
@@ -1762,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 `..`.