fix(config): [env] relative paths definition (#16957)

### What does this PR try to resolve?

`EnvConfigValue` was wrapped in `Value<T>`,
which captured the table-level `Definition` for path resolution.

1. In `merge_helper`,
we merge all keys in a table but not the definition of the table itself:

<https://github.com/rust-lang/cargo/blob/60960cbe4148295890ccd0f2890c2fb5dd9a0914/src/cargo/util/context/config_value.rs?plain=1#L177-L199>

It is probably hard to defined the source definition of a merged struct
though, perhaps people shouldn't access it to do anything meaningful.

2. When deserializing a `[env]` struct with `Value<T>`,
   `Value<T>` delegate to `ValueDeserializer` to deserialize the value,
   which only takes the struct definition into account.
   And that turns out to be the stale table definition:

<https://github.com/rust-lang/cargo/blob/60960cbe4148295890ccd0f2890c2fb5dd9a0914/src/cargo/util/context/de.rs?plain=1#L119-L137>

3. Unlike file config discovery,
   `include` is loaded before the including config,
   hence its definition is stale and is used.

This caused in #16954 that `[env]` struct got a stale old
config source definition.

### How to test and review this PR?

New regression tests should have covered cases in
<#16954>, and also captured the
non-canonicalized case which is a separate issue.

This fix doesn't touch the core problem that a merged `Value<T>` table
holds a stale source definition. Holding either new or old doesn't seem
correct to me, as it is a merged one. This is really a footgun.

Fixes #16954

Author: weihanglo

src/cargo/util/context/schema.rs

@@ -422,7 +422,7 @@ impl<'de> Deserialize<'de> for ProgressConfig {
 enum EnvConfigValueInner {
     Simple(String),
     WithOptions {
-        value: String,
+        value: ConfigRelativePath,
         force: bool,
         relative: bool,
     },
@@ -435,7 +435,7 @@ impl<'de> Deserialize<'de> for EnvConfigValueInner {
     {
         #[derive(Deserialize)]
         struct WithOptions {
-            value: String,
+            value: ConfigRelativePath,
             #[serde(default)]
             force: bool,
             #[serde(default)]
@@ -473,13 +473,13 @@ impl<'de> Deserialize<'de> for EnvConfigValueInner {
 #[derive(Debug, Deserialize)]
 #[serde(transparent)]
 pub struct EnvConfigValue {
-    inner: Value<EnvConfigValueInner>,
+    inner: EnvConfigValueInner,
 }
 
 impl EnvConfigValue {
     /// Whether this value should override existing environment variables.
     pub fn is_force(&self) -> bool {
-        match self.inner.val {
+        match self.inner {
             EnvConfigValueInner::Simple(_) => false,
             EnvConfigValueInner::WithOptions { force, .. } => force,
         }
@@ -490,18 +490,18 @@ impl EnvConfigValue {
     /// If `relative = true`,
     /// the value is interpreted as a [`ConfigRelativePath`]-like path.
     pub fn resolve<'a>(&'a self, cwd: &Path) -> Cow<'a, OsStr> {
-        match self.inner.val {
+        match self.inner {
             EnvConfigValueInner::Simple(ref s) => Cow::Borrowed(OsStr::new(s.as_str())),
             EnvConfigValueInner::WithOptions {
                 ref value,
                 relative,
                 ..
             } => {
                 if relative {
-                    let p = self.inner.definition.root(cwd).join(&value);
+                    let p = value.value().definition.root(cwd).join(value.raw_value());
                     Cow::Owned(p.into_os_string())
                 } else {
-                    Cow::Borrowed(OsStr::new(value.as_str()))
+                    Cow::Borrowed(OsStr::new(value.raw_value()))
                 }
             }
         }

tests/testsuite/cargo_env_config.rs

@@ -62,9 +62,6 @@ fn env_invalid() {
         .with_stderr_data(str![[r#"
 [ERROR] error in [ROOT]/foo/.cargo/config.toml: could not load config key `env.ENV_TEST_BOOL`
 
-Caused by:
-  error in [ROOT]/foo/.cargo/config.toml: could not load config key `env.ENV_TEST_BOOL`
-
 Caused by:
   invalid type: boolean `false`, expected a string or map
 

tests/testsuite/config_include.rs

@@ -645,3 +645,64 @@ fn env_relative_path_included_from_upper_level() {
         str!["[ROOT]/val"],
     );
 }
+
+#[cargo_test]
+fn env_relative_path_included_override() {
+    // See https://github.com/rust-lang/cargo/issues/16954
+    write_config_at(
+        "foo/.cargo/config.toml",
+        "
+        include = ['../../inc/inc.toml']
+
+        [env]
+        MY_ENV = { value = 'inner', relative = true }
+        ",
+    );
+    write_config_at(
+        "inc/inc.toml",
+        "
+        [env]
+        MY_ENV = { value = 'outer', relative = true }
+        ",
+    );
+    let gctx = GlobalContextBuilder::new().cwd("foo").build();
+    let env = gctx.env_config().unwrap();
+    let my_env = env.get("MY_ENV").unwrap();
+
+    assert_e2e().eq(my_env.to_str().unwrap(), str!["[ROOT]/foo/inner"]);
+}
+
+#[cargo_test]
+fn env_relative_path_nested_included_override() {
+    // See https://github.com/rust-lang/cargo/issues/16954
+    write_config_at(
+        "foo/.cargo/config.toml",
+        "
+        include = ['../../mid/mid.toml']
+
+        [env]
+        MY_ENV = { value = 'inner', relative = true }
+        ",
+    );
+    write_config_at(
+        "mid/mid.toml",
+        "
+        include = ['../deep/deep.toml']
+
+        [env]
+        MY_ENV = { value = 'mid', relative = true }
+        ",
+    );
+    write_config_at(
+        "deep/deep.toml",
+        "
+        [env]
+        MY_ENV = { value = 'deep', relative = true }
+        ",
+    );
+    let gctx = GlobalContextBuilder::new().cwd("foo").build();
+    let env = gctx.env_config().unwrap();
+    let my_env = env.get("MY_ENV").unwrap();
+
+    assert_e2e().eq(my_env.to_str().unwrap(), str!["[ROOT]/foo/inner"]);
+}