Auto merge of #9839 - jonhoo:prefer-cargo-toml-patch, r=ehuss

bors · bors · commit 72aee9e81522 · 2021-09-01T04:55:13.000Z
Stabilize patch-in-config (and prefer config over manifest) Tracking issue: #9269 --- This stabilizes the `patch-in-config` feature ([unstable entry](https://doc.rust-lang.org/cargo/reference/unstable.html#patch-in-config)) following the discussion in #9269 (comment). As requested, this PR _also_ changes the precedence behavior such that a `[patch]` for the same dependency in both `.cargo/config.toml` and `Cargo.toml` prefers the patch from the configuration file over the one from the manifest, which matches the behavior of other overlapping configuration options. The corresponding test has also been updated to reflect this change in behavior.
diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs
@@ -648,7 +648,6 @@ unstable_cli_options!(
     panic_abort_tests: bool = ("Enable support to run tests with -Cpanic=abort"),
     host_config: bool = ("Enable the [host] section in the .cargo/config.toml file"),
     target_applies_to_host: bool = ("Enable the `target-applies-to-host` key in the .cargo/config.toml file"),
-    patch_in_config: bool = ("Allow `[patch]` sections in .cargo/config.toml files"),
     rustdoc_map: bool = ("Allow passing external documentation mappings to rustdoc"),
     separate_nightlies: bool = (HIDDEN),
     terminal_width: Option<Option<usize>>  = ("Provide a terminal width to rustc for error truncation"),
@@ -697,6 +696,8 @@ const STABILIZED_EXTRA_LINK_ARG: &str = "Additional linker arguments are now \
 
 const STABILIZED_CONFIGURABLE_ENV: &str = "The [env] section is now always enabled.";
 
+const STABILIZED_PATCH_IN_CONFIG: &str = "The patch-in-config feature is now always enabled.";
+
 fn deserialize_build_std<'de, D>(deserializer: D) -> Result<Option<Vec<String>>, D::Error>
 where
     D: serde::Deserializer<'de>,
@@ -841,7 +842,6 @@ impl CliUnstable {
             "jobserver-per-rustc" => self.jobserver_per_rustc = parse_empty(k, v)?,
             "host-config" => self.host_config = parse_empty(k, v)?,
             "target-applies-to-host" => self.target_applies_to_host = 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
@@ -877,6 +877,7 @@ impl CliUnstable {
             "package-features" => stabilized_warn(k, "1.51", STABILIZED_PACKAGE_FEATURES),
             "extra-link-arg" => stabilized_warn(k, "1.56", STABILIZED_EXTRA_LINK_ARG),
             "configurable-env" => stabilized_warn(k, "1.56", STABILIZED_CONFIGURABLE_ENV),
+            "patch-in-config" => stabilized_warn(k, "1.56", STABILIZED_PATCH_IN_CONFIG),
             "future-incompat-report" => self.future_incompat_report = parse_empty(k, v)?,
             _ => bail!("unknown `-Z` flag specified: {}", k),
         }
diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs
@@ -362,11 +362,6 @@ impl<'cfg> Workspace<'cfg> {
             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();
@@ -434,24 +429,24 @@ impl<'cfg> Workspace<'cfg> {
 
         // 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.
+        let mut combined = from_config;
+        for (url, deps_from_manifest) in from_manifest {
+            if let Some(deps_from_config) = combined.get_mut(&url) {
+                // We want from_config 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| {
+                let mut from_manifest_pruned = deps_from_manifest.clone();
+                for dep_from_config in &mut *deps_from_config {
+                    if let Some(i) = from_manifest_pruned.iter().position(|dep_from_manifest| {
                         // XXX: should this also take into account version numbers?
-                        dep.name_in_toml() == cdep.name_in_toml()
+                        dep_from_config.name_in_toml() == dep_from_manifest.name_in_toml()
                     }) {
-                        left.swap_remove(i);
+                        from_manifest_pruned.swap_remove(i);
                     }
                 }
                 // Whatever is left does not exist in manifest dependencies.
-                deps.extend(left);
+                deps_from_config.extend(from_manifest_pruned);
             } else {
-                combined.insert(url.clone(), cdeps.clone());
+                combined.insert(url.clone(), deps_from_manifest.clone());
             }
         }
         Ok(combined)
diff --git a/src/doc/src/reference/config.md b/src/doc/src/reference/config.md
@@ -111,6 +111,9 @@ retry = 2                   # network retries
 git-fetch-with-cli = true   # use the `git` executable for git operations
 offline = false             # do not access the network
 
+[patch.<registry>]
+# Same keys as for [patch] in Cargo.toml
+
 [profile.<name>]         # Modify profile settings via config.
 opt-level = 0            # Optimization level.
 debug = true             # Include debug info.
@@ -650,6 +653,31 @@ needed, and generate an error if it encounters a network error.
 
 Can be overridden with the `--offline` command-line option.
 
+#### `[patch]`
+
+Just as you can override dependencies using [`[patch]` in
+`Cargo.toml`](overriding-dependencies.md#the-patch-section), you can
+override them in the cargo configuration file to apply those patches to
+any affected build. The format is identical to the one used in
+`Cargo.toml`.
+
+Since `.cargo/config.toml` files are not usually checked into source
+control, you should prefer patching using `Cargo.toml` where possible to
+ensure that other developers can compile your crate in their own
+environments. Patching through cargo configuration files is generally
+only appropriate when the patch section is automatically generated by an
+external build tool.
+
+If a given dependency is patched both in a cargo configuration file and
+a `Cargo.toml` file, the patch in the configuration file is used. If
+multiple configuration files patch the same dependency, standard cargo
+configuration merging is used, which prefers the value defined closest
+to the current directory, with `$HOME/.cargo/config.toml` taking the
+lowest precedence.
+
+Relative `path` dependencies in such a `[patch]` section are resolved
+relative to the configuration file they appear in.
+
 #### `[profile]`
 
 The `[profile]` table can be used to globally change profile settings, and
diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md
@@ -100,7 +100,6 @@ Each new feature described below should explain how to use it.
 * Configuration
     * [config-cli](#config-cli) — Adds the ability to pass configuration options on the command-line.
     * [config-include](#config-include) — Adds the ability for config files to include other files.
-    * [patch-in-config](#patch-in-config) — Adds support for specifying the `[patch]` table in config files.
     * [`cargo config`](#cargo-config) — Adds a new subcommand for viewing config files.
 * Registries
     * [credential-process](#credential-process) — Adds support for fetching registry tokens from an external authentication program.
@@ -1227,31 +1226,6 @@ The supported values for `FREQUENCY` are 'always` and 'never', which control
 whether or not a message is printed out at the end of `cargo build` / `cargo check`.
 
 
-### patch-in-config
-* Original Pull Request: [#9204](https://github.com/rust-lang/cargo/pull/9204)
-* Tracking Issue: [#9269](https://github.com/rust-lang/cargo/issues/9269)
-
-The `-Z patch-in-config` flag enables the use of `[patch]` sections in
-cargo configuration files (`.cargo/config.toml`). The format of such
-`[patch]` sections is identical to the one used in `Cargo.toml`.
-
-Since `.cargo/config.toml` files are not usually checked into source
-control, you should prefer patching using `Cargo.toml` where possible to
-ensure that other developers can compile your crate in their own
-environments. Patching through cargo configuration files is generally
-only appropriate when the patch section is automatically generated by an
-external build tool.
-
-If a given dependency is patched both in a cargo configuration file and
-a `Cargo.toml` file, the patch in `Cargo.toml` is used. If multiple
-configuration files patch the same dependency, standard cargo
-configuration merging is used, which prefers the value defined closest
-to the current directory, with `$HOME/.cargo/config.toml` taking the
-lowest precedence.
-
-Relative `path` dependencies in such a `[patch]` section are resolved
-relative to the configuration file they appear in.
-
 ### `cargo config`
 
 * Original Issue: [#2362](https://github.com/rust-lang/cargo/issues/2362)
@@ -1463,3 +1437,10 @@ serde = "1.0.117"
 [profile.dev.package.foo]
 codegen-backend = "cranelift"
 ```
+
+### patch-in-config
+
+The `-Z patch-in-config` flag, and the corresponding support for
+`[patch]` section in Cargo configuration files has been stabilized in
+the 1.56 release. See the [patch field](config.html#patch) for more
+information.
diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs
@@ -66,50 +66,6 @@ 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 was ignored, the -Zpatch-in-config command-line flag is required
-[UPDATING] `dummy-registry` 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();
@@ -139,8 +95,7 @@ fn from_config() {
         .file("bar/src/lib.rs", r#""#)
         .build();
 
-    p.cargo("build -Zpatch-in-config")
-        .masquerade_as_nightly_cargo()
+    p.cargo("build")
         .with_stderr(
             "\
 [UPDATING] `dummy-registry` index
@@ -181,8 +136,7 @@ fn from_config_relative() {
         .file("bar/src/lib.rs", r#""#)
         .build();
 
-    p.cargo("build -Zpatch-in-config")
-        .masquerade_as_nightly_cargo()
+    p.cargo("build")
         .with_stderr(
             "\
 [UPDATING] `dummy-registry` index
@@ -211,23 +165,22 @@ fn from_config_precedence() {
                 bar = "0.1.0"
 
                 [patch.crates-io]
-                bar = { path = 'bar' }
+                bar = { path = 'no-such-path' }
             "#,
         )
         .file(
             ".cargo/config.toml",
             r#"
                 [patch.crates-io]
-                bar = { path = 'no-such-path' }
+                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()
+    p.cargo("build")
         .with_stderr(
             "\
 [UPDATING] `dummy-registry` index
@@ -519,8 +472,7 @@ fn unused_from_config() {
         .file("bar/src/lib.rs", "not rust code")
         .build();
 
-    p.cargo("build -Zpatch-in-config")
-        .masquerade_as_nightly_cargo()
+    p.cargo("build")
         .with_stderr(
             "\
 [UPDATING] `dummy-registry` index
@@ -537,8 +489,7 @@ fn unused_from_config() {
 ",
         )
         .run();
-    p.cargo("build -Zpatch-in-config")
-        .masquerade_as_nightly_cargo()
+    p.cargo("build")
         .with_stderr(
             "\
 [WARNING] Patch `bar v0.2.0 ([CWD]/bar)` was not used in the crate graph.
@@ -733,8 +684,7 @@ fn add_patch_from_config() {
         "#,
     );
 
-    p.cargo("build -Zpatch-in-config")
-        .masquerade_as_nightly_cargo()
+    p.cargo("build")
         .with_stderr(
             "\
 [COMPILING] bar v0.1.0 ([CWD]/bar)
@@ -743,10 +693,7 @@ fn add_patch_from_config() {
 ",
         )
         .run();
-    p.cargo("build -Zpatch-in-config")
-        .masquerade_as_nightly_cargo()
-        .with_stderr("[FINISHED] [..]")
-        .run();
+    p.cargo("build").with_stderr("[FINISHED] [..]").run();
 }
 
 #[cargo_test]
