]> git.proxmox.com Git - cargo.git/commitdiff
Extract config patch on use
authorJon Gjengset <jongje@amazon.com>
Mon, 8 Mar 2021 21:53:29 +0000 (13:53 -0800)
committerJon Gjengset <jongje@amazon.com>
Mon, 8 Mar 2021 21:53:29 +0000 (13:53 -0800)
src/cargo/core/resolver/encode.rs
src/cargo/core/workspace.rs
src/cargo/ops/resolve.rs
src/cargo/util/config/mod.rs
src/cargo/util/toml/mod.rs
tests/testsuite/patch.rs

index 5c329124d9b14dbf6d4a84b0ab24e15cc86a7736..9b93a7449809bb9965162f78a2104bc120487cd5 100644 (file)
@@ -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,
index 92c7724220af9b4ecb352298facaabacf630037d..57e91c575f5ea26702c6cd2b997dc0e3f3cd32c0 100644 (file)
@@ -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<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.
+                            self.root(),
+                            self.unstable_features(),
+                            None,
+                        )
+                    })
+                    .collect::<CargoResult<Vec<_>>>()?,
+            );
+        }
+
+        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<Url, Vec<Dependency>> {
+    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()
-            .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
index f03473697a1d18d3dfb31b27560abc0d0a9b5651..d28ac92c0de6aba7152d9484e37d25aae963c173 100644 (file)
@@ -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 => {
index c880cae0f5c9fb43794a05f739701e31c5f9eae0..b7b4ed36712d8860942c554160c3a288a4c5b7f2 100644 (file)
@@ -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<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>,
@@ -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<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
     }
index 9dba719d0e06038523dbac531e51f0b1e4a964cb..7f7aadb4ca5d755594401af1fe3b47c8a4a37fda 100644 (file)
@@ -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<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")]
@@ -1530,12 +1573,9 @@ impl TomlManifest {
         Ok(replace)
     }
 
-    fn patch_(
-        table: Option<&BTreeMap<String, BTreeMap<String, TomlDependency>>>,
-        cx: &mut Context<'_, '_>,
-    ) -> CargoResult<HashMap<Url, Vec<Dependency>>> {
+    fn patch(&self, cx: &mut Context<'_, '_>) -> CargoResult<HashMap<Url, Vec<Dependency>>> {
         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<HashMap<Url, Vec<Dependency>>> {
-        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());
-            }
-
-            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<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,
@@ -1655,7 +1708,7 @@ impl TomlDependency {
         kind: Option<DepKind>,
     ) -> CargoResult<Dependency> {
         match *self {
-            TomlDependency::Simple(ref version) => DetailedTomlDependency {
+            TomlDependency::Simple(ref version) => DetailedTomlDependency::<P> {
                 version: Some(version.clone()),
                 ..Default::default()
             }
@@ -1672,7 +1725,7 @@ impl TomlDependency {
     }
 }
 
-impl DetailedTomlDependency {
+impl<P: ResolveToPath> DetailedTomlDependency<P> {
     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 `..`.
index 1c94f7de9f8b4d83dc1e999f3c3ffc5dac0ce6d8..aebb4762601d0dbbbb412695f5ab9966febdb182 100644 (file)
@@ -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();