Auto merge of #12101 - ehuss:disallow-rustup-home, r=weihanglo

Disallow RUSTUP_HOME in the [env] table.

This adds a check to prevent RUSTUP_HOME from being set in the `[env]` config table under the same reasoning as was done in #11590. Cargo will likely behave incorrectly if this key is set in the config since it will not change the home used by the outer cargo itself.

This is a breaking change, though I think it is unlikely to be used in practice. When cargo is executed via a rustup proxy, the proxy sets RUSTUP_HOME which overrides the `[env]` table entry. It may be feasible that someone is invoking cargo directly without the rustup wrappers, and then using this to steer the rustc invocations to a different rustup location, but I'm not sure that's a use case we need to be supporting.

This is being added as a further assurance for #11590 to make sure the environment is configured as expected.

We could potentially add other Rustup env vars to reject, but I'm not sure I want to delve into analyzing all the possible reasons or interactions for each one.

Author: bors

src/cargo/util/config/mod.rs

@@ -1745,8 +1745,27 @@ impl Config {
             .env_config
             .try_borrow_with(|| self.get::<EnvConfig>("env"))?;
 
-        if env_config.get("CARGO_HOME").is_some() {
-            bail!("setting the `CARGO_HOME` environment variable is not supported in the `[env]` configuration table")
+        // Reasons for disallowing these values:
+        //
+        // - CARGO_HOME: The initial call to cargo does not honor this value
+        //   from the [env] table. Recursive calls to cargo would use the new
+        //   value, possibly behaving differently from the outer cargo.
+        //
+        // - RUSTUP_HOME: Under normal usage with rustup, this will have no
+        //   effect because the rustup proxy sets RUSTUP_HOME, and that would
+        //   override the [env] table. If the outer cargo is executed directly
+        //   circumventing the rustup proxy, then this would affect calls to
+        //   rustc (assuming that is a proxy), which could potentially cause
+        //   problems with cargo and rustc being from different toolchains. We
+        //   consider this to be not a use case we would like to support,
+        //   since it will likely cause problems or lead to confusion.
+        for disallowed in &["CARGO_HOME", "RUSTUP_HOME"] {
+            if env_config.contains_key(*disallowed) {
+                bail!(
+                    "setting the `{disallowed}` environment variable is not supported \
+                    in the `[env]` configuration table"
+                );
+            }
         }
 
         Ok(env_config)

tests/testsuite/cargo_env_config.rs

@@ -59,29 +59,31 @@ fn env_invalid() {
 }
 
 #[cargo_test]
-fn env_no_cargo_home() {
+fn env_no_disallowed() {
+    // Checks for keys that are not allowed in the [env] table.
     let p = project()
-        .file("Cargo.toml", &basic_bin_manifest("foo"))
-        .file(
-            "src/main.rs",
-            r#"
-        fn main() {
-        }
-        "#,
-        )
-        .file(
-            ".cargo/config",
-            r#"
-                [env]
-                CARGO_HOME = "/"
-            "#,
-        )
+        .file("Cargo.toml", &basic_manifest("foo", "1.0.0"))
+        .file("src/lib.rs", "")
         .build();
 
-    p.cargo("check")
-        .with_status(101)
-        .with_stderr_contains("[..]setting the `CARGO_HOME` environment variable is not supported in the `[env]` configuration table")
-        .run();
+    for disallowed in &["CARGO_HOME", "RUSTUP_HOME"] {
+        p.change_file(
+            ".cargo/config",
+            &format!(
+                r#"
+                    [env]
+                    {disallowed} = "foo"
+                "#
+            ),
+        );
+        p.cargo("check")
+            .with_status(101)
+            .with_stderr(&format!(
+                "[ERROR] setting the `{disallowed}` environment variable \
+                is not supported in the `[env]` configuration table"
+            ))
+            .run();
+    }
 }
 
 #[cargo_test]