Change env section parsing

`untagged` enum variants are not recognised by serde if the variant contains a `Value<T>` type.
This change uses an transparent "inner" enum `Value<T>` member, which is handled correctly by serde.

Author: wickerwaka

src/cargo/util/config/mod.rs

@@ -1248,37 +1248,9 @@ impl Config {
         &self.progress_config
     }
 
-    /// Create an EnvConfigValue hashmap from the "env" table
-    fn get_env_config(&self) -> CargoResult<EnvConfig> {
-        // We cannot use pure serde handling for this. The Value<_> type does not
-        // work when parsing the env table to a hashmap. So iterator through each
-        // entry in the "env" table, determine it's type and then use `get` method
-        // to deserialize it.
-        let env_table = &self.get_table(&ConfigKey::from_str("env"))?;
-        let mut vars = EnvConfig::new();
-
-        if env_table.is_none() {
-            return Ok(vars);
-        }
-
-        let env_table = &env_table.as_ref().unwrap().val;
-
-        for (key, value) in env_table.iter() {
-            let full_key = format!("env.{}", key);
-            let e = match value {
-                ConfigValue::Table(..) => self.get::<EnvConfigValue>(&full_key)?,
-                _ => {
-                    let v = self.get::<Value<String>>(&full_key)?;
-                    EnvConfigValue::from_value(v)
-                }
-            };
-            vars.insert(key.clone(), e);
-        }
-        Ok(vars)
-    }
-
     pub fn env_config(&self) -> CargoResult<&EnvConfig> {
-        self.env_config.try_borrow_with(|| self.get_env_config())
+        self.env_config
+            .try_borrow_with(|| self.get::<EnvConfig>("env"))
     }
 
     /// This is used to validate the `term` table has valid syntax.
@@ -1991,33 +1963,47 @@ where
 }
 
 #[derive(Debug, Deserialize)]
+#[serde(untagged)]
+enum EnvConfigValueInner {
+    Simple(String),
+    WithOptions {
+        value: String,
+        #[serde(default)]
+        force: bool,
+        #[serde(default)]
+        relative: bool,
+    },
+}
+
+#[derive(Debug, Deserialize)]
+#[serde(transparent)]
 pub struct EnvConfigValue {
-    value: Value<String>,
-    #[serde(default)]
-    force: bool,
-    #[serde(default)]
-    relative: bool,
+    inner: Value<EnvConfigValueInner>,
 }
 
 impl EnvConfigValue {
-    fn from_value(value: Value<String>) -> EnvConfigValue {
-        EnvConfigValue {
-            value,
-            force: false,
-            relative: false,
-        }
-    }
-
     pub fn is_force(&self) -> bool {
-        self.force
+        match self.inner.val {
+            EnvConfigValueInner::Simple(_) => false,
+            EnvConfigValueInner::WithOptions { force, .. } => force,
+        }
     }
 
     pub fn resolve<'a>(&'a self, config: &Config) -> Cow<'a, OsStr> {
-        if self.relative {
-            let p = self.value.definition.root(config).join(&self.value.val);
-            Cow::Owned(p.into_os_string())
-        } else {
-            Cow::Borrowed(OsStr::new(&self.value.val))
+        match self.inner.val {
+            EnvConfigValueInner::Simple(ref s) => Cow::Borrowed(OsStr::new(s.as_str())),
+            EnvConfigValueInner::WithOptions {
+                ref value,
+                relative,
+                ..
+            } => {
+                if relative {
+                    let p = self.inner.definition.root(config).join(&value);
+                    Cow::Owned(p.into_os_string())
+                } else {
+                    Cow::Borrowed(OsStr::new(value.as_str()))
+                }
+            }
         }
     }
 }

tests/testsuite/cargo_env_config.rs

@@ -55,7 +55,7 @@ fn env_invalid() {
     p.cargo("build -Zconfigurable-env")
         .masquerade_as_nightly_cargo()
         .with_status(101)
-        .with_stderr_contains("[..]`env.ENV_TEST_BOOL` expected a string, but found a boolean")
+        .with_stderr_contains("[..]could not load config key `env.ENV_TEST_BOOL`")
         .run();
 }
 
@@ -70,14 +70,16 @@ fn env_force() {
         fn main() {
             println!( "ENV_TEST_FORCED:{}", env!("ENV_TEST_FORCED") );
             println!( "ENV_TEST_UNFORCED:{}", env!("ENV_TEST_UNFORCED") );
+            println!( "ENV_TEST_UNFORCED_DEFAULT:{}", env!("ENV_TEST_UNFORCED_DEFAULT") );
         }
         "#,
         )
         .file(
             ".cargo/config",
             r#"
                 [env]
-                ENV_TEST_UNFORCED = "from-config"
+                ENV_TEST_UNFORCED_DEFAULT = "from-config"
+                ENV_TEST_UNFORCED = { value = "from-config", force = false }
                 ENV_TEST_FORCED = { value = "from-config", force = true }
             "#,
         )
@@ -87,8 +89,10 @@ fn env_force() {
         .masquerade_as_nightly_cargo()
         .env("ENV_TEST_FORCED", "from-env")
         .env("ENV_TEST_UNFORCED", "from-env")
+        .env("ENV_TEST_UNFORCED_DEFAULT", "from-env")
         .with_stdout_contains("ENV_TEST_FORCED:from-config")
         .with_stdout_contains("ENV_TEST_UNFORCED:from-env")
+        .with_stdout_contains("ENV_TEST_UNFORCED_DEFAULT:from-env")
         .run();
 }
 
@@ -102,20 +106,23 @@ fn env_relative() {
         use std::env;
         use std::path::Path;
         fn main() {
+            println!( "ENV_TEST_REGULAR:{}", env!("ENV_TEST_REGULAR") );
+            println!( "ENV_TEST_REGULAR_DEFAULT:{}", env!("ENV_TEST_REGULAR_DEFAULT") );
             println!( "ENV_TEST_RELATIVE:{}", env!("ENV_TEST_RELATIVE") );
-            println!( "ENV_TEST_ABSOLUTE:{}", env!("ENV_TEST_ABSOLUTE") );
 
-            assert!( Path::new(env!("ENV_TEST_ABSOLUTE")).is_absolute() );
-            assert!( !Path::new(env!("ENV_TEST_RELATIVE")).is_absolute() );
+            assert!( Path::new(env!("ENV_TEST_RELATIVE")).is_absolute() );
+            assert!( !Path::new(env!("ENV_TEST_REGULAR")).is_absolute() );
+            assert!( !Path::new(env!("ENV_TEST_REGULAR_DEFAULT")).is_absolute() );
         }
         "#,
         )
         .file(
             ".cargo/config",
             r#"
                 [env]
-                ENV_TEST_RELATIVE = "Cargo.toml"
-                ENV_TEST_ABSOLUTE = { value = "Cargo.toml", relative = true }
+                ENV_TEST_REGULAR = { value = "Cargo.toml", relative = false }
+                ENV_TEST_REGULAR_DEFAULT = "Cargo.toml"
+                ENV_TEST_RELATIVE = { value = "Cargo.toml", relative = true }
             "#,
         )
         .build();