refactor: Define cargo script's target-dir using build-dir templating (#16073)

### What does this PR try to resolve?

`build-dir`s templating was modeled off of the `target-dir` for cargo
scripts but the root dir and hashing are currently duplicated. This
consolidates the logic by defining the cargo script `target-dir` as
`{cargo-cache-home}/target/{workspace-path-hash}`.

This also serves as preparation for splitting cargo scripts `build-dir`
and `target-dir`.

### How to test and review this PR?

Author: weihanglo

src/cargo/core/workspace.rs

@@ -215,7 +215,6 @@ impl<'gctx> Workspace<'gctx> {
     /// before returning it, so `Ok` is only returned for valid workspaces.
     pub fn new(manifest_path: &Path, gctx: &'gctx GlobalContext) -> CargoResult<Workspace<'gctx>> {
         let mut ws = Workspace::new_default(manifest_path.to_path_buf(), gctx);
-        ws.target_dir = gctx.target_dir()?;
 
         if manifest_path.is_relative() {
             bail!(
@@ -226,11 +225,8 @@ impl<'gctx> Workspace<'gctx> {
             ws.root_manifest = ws.find_root(manifest_path)?;
         }
 
-        ws.build_dir = gctx.build_dir(
-            ws.root_manifest
-                .as_ref()
-                .unwrap_or(&manifest_path.to_path_buf()),
-        )?;
+        ws.target_dir = gctx.target_dir()?;
+        ws.build_dir = gctx.build_dir(ws.root_manifest())?;
 
         ws.custom_metadata = ws
             .load_workspace_config()?
@@ -448,13 +444,14 @@ impl<'gctx> Workspace<'gctx> {
 
     fn default_target_dir(&self) -> Filesystem {
         if self.root_maybe().is_embedded() {
-            let hash = crate::util::hex::short_hash(&self.root_manifest().to_string_lossy());
-            let mut rel_path = PathBuf::new();
-            rel_path.push("target");
-            rel_path.push(&hash[0..2]);
-            rel_path.push(&hash[2..]);
-
-            self.gctx().home().join(rel_path)
+            let default = ConfigRelativePath::new(
+                "{cargo-cache-home}/target/{workspace-path-hash}"
+                    .to_owned()
+                    .into(),
+            );
+            self.gctx()
+                .custom_build_dir(&default, self.root_manifest())
+                .expect("template is correct")
         } else {
             Filesystem::new(self.root().join("target"))
         }

src/cargo/util/context/de.rs

@@ -564,18 +564,19 @@ impl<'de, 'gctx> de::MapAccess<'de> for ValueDeserializer<'gctx> {
         // ... otherwise we're deserializing the `definition` field, so we need
         // to figure out where the field we just deserialized was defined at.
         match self.definition() {
+            Definition::BuiltIn => seed.deserialize(0.into_deserializer()),
             Definition::Path(path) => {
-                seed.deserialize(Tuple2Deserializer(0i32, path.to_string_lossy()))
+                seed.deserialize(Tuple2Deserializer(1i32, path.to_string_lossy()))
             }
             Definition::Environment(env) => {
-                seed.deserialize(Tuple2Deserializer(1i32, env.as_str()))
+                seed.deserialize(Tuple2Deserializer(2i32, env.as_str()))
             }
             Definition::Cli(path) => {
                 let s = path
                     .as_ref()
                     .map(|p| p.to_string_lossy())
                     .unwrap_or_default();
-                seed.deserialize(Tuple2Deserializer(2i32, s))
+                seed.deserialize(Tuple2Deserializer(3i32, s))
             }
         }
     }

src/cargo/util/context/mod.rs

@@ -653,42 +653,54 @@ impl GlobalContext {
 
     /// The directory to use for intermediate build artifacts.
     ///
-    /// Falls back to the target directory if not specified.
+    /// Callers should prefer [`Workspace::build_dir`] instead.
+    pub fn build_dir(&self, workspace_manifest_path: &Path) -> CargoResult<Option<Filesystem>> {
+        let Some(val) = &self.build_config()?.build_dir else {
+            return Ok(None);
+        };
+        self.custom_build_dir(val, workspace_manifest_path)
+            .map(Some)
+    }
+
+    /// The directory to use for intermediate build artifacts.
     ///
     /// Callers should prefer [`Workspace::build_dir`] instead.
-    pub fn build_dir(&self, workspace_manifest_path: &PathBuf) -> CargoResult<Option<Filesystem>> {
-        if let Some(val) = &self.build_config()?.build_dir {
-            let replacements = vec![
-                (
-                    "{workspace-root}",
-                    workspace_manifest_path
-                        .parent()
-                        .unwrap()
-                        .to_str()
-                        .context("workspace root was not valid utf-8")?
-                        .to_string(),
-                ),
-                (
-                    "{cargo-cache-home}",
-                    self.home()
-                        .as_path_unlocked()
-                        .to_str()
-                        .context("cargo home was not valid utf-8")?
-                        .to_string(),
-                ),
-                ("{workspace-path-hash}", {
-                    let real_path = std::fs::canonicalize(workspace_manifest_path)?;
-                    let hash = crate::util::hex::short_hash(&real_path);
-                    format!("{}{}{}", &hash[0..2], std::path::MAIN_SEPARATOR, &hash[2..])
-                }),
-            ];
-
-            let template_variables = replacements
-                .iter()
-                .map(|(key, _)| key[1..key.len() - 1].to_string())
-                .collect_vec();
+    pub fn custom_build_dir(
+        &self,
+        val: &ConfigRelativePath,
+        workspace_manifest_path: &Path,
+    ) -> CargoResult<Filesystem> {
+        let replacements = [
+            (
+                "{workspace-root}",
+                workspace_manifest_path
+                    .parent()
+                    .unwrap()
+                    .to_str()
+                    .context("workspace root was not valid utf-8")?
+                    .to_string(),
+            ),
+            (
+                "{cargo-cache-home}",
+                self.home()
+                    .as_path_unlocked()
+                    .to_str()
+                    .context("cargo home was not valid utf-8")?
+                    .to_string(),
+            ),
+            ("{workspace-path-hash}", {
+                let real_path = std::fs::canonicalize(workspace_manifest_path)?;
+                let hash = crate::util::hex::short_hash(&real_path);
+                format!("{}{}{}", &hash[0..2], std::path::MAIN_SEPARATOR, &hash[2..])
+            }),
+        ];
+
+        let template_variables = replacements
+            .iter()
+            .map(|(key, _)| key[1..key.len() - 1].to_string())
+            .collect_vec();
 
-            let path = val
+        let path = val
                 .resolve_templated_path(self, replacements)
                 .map_err(|e| match e {
                     path::ResolveTemplateError::UnexpectedVariable {
@@ -716,20 +728,15 @@ impl GlobalContext {
                     }
                 })?;
 
-            // Check if the target directory is set to an empty string in the config.toml file.
-            if val.raw_value().is_empty() {
-                bail!(
-                    "the build directory is set to an empty string in {}",
-                    val.value().definition
-                )
-            }
-
-            Ok(Some(Filesystem::new(path)))
-        } else {
-            // For now, fallback to the previous implementation.
-            // This will change in the future.
-            return self.target_dir();
+        // Check if the target directory is set to an empty string in the config.toml file.
+        if val.raw_value().is_empty() {
+            bail!(
+                "the build directory is set to an empty string in {}",
+                val.value().definition
+            )
         }
+
+        Ok(Filesystem::new(path))
     }
 
     /// Get a configuration value by key.
@@ -1364,7 +1371,9 @@ impl GlobalContext {
         let abs = |path: &str, def: &Definition| -> (String, PathBuf, Definition) {
             let abs_path = match def {
                 Definition::Path(p) | Definition::Cli(Some(p)) => p.parent().unwrap().join(&path),
-                Definition::Environment(_) | Definition::Cli(None) => self.cwd().join(&path),
+                Definition::Environment(_) | Definition::Cli(None) | Definition::BuiltIn => {
+                    self.cwd().join(&path)
+                }
             };
             (path.to_string(), abs_path, def.clone())
         };

src/cargo/util/context/value.rs

@@ -55,6 +55,7 @@ pub(crate) static FIELDS: [&str; 2] = [VALUE_FIELD, DEFINITION_FIELD];
 /// Location where a config value is defined.
 #[derive(Clone, Debug, Eq)]
 pub enum Definition {
+    BuiltIn,
     /// Defined in a `.cargo/config`, includes the path to the file.
     Path(PathBuf),
     /// Defined in an environment variable, includes the environment key.
@@ -90,7 +91,7 @@ impl Definition {
     pub fn root<'a>(&'a self, gctx: &'a GlobalContext) -> &'a Path {
         match self {
             Definition::Path(p) | Definition::Cli(Some(p)) => p.parent().unwrap().parent().unwrap(),
-            Definition::Environment(_) | Definition::Cli(None) => gctx.cwd(),
+            Definition::Environment(_) | Definition::Cli(None) | Definition::BuiltIn => gctx.cwd(),
         }
     }
 
@@ -102,7 +103,10 @@ impl Definition {
             (self, other),
             (Definition::Cli(_), Definition::Environment(_))
                 | (Definition::Cli(_), Definition::Path(_))
+                | (Definition::Cli(_), Definition::BuiltIn)
                 | (Definition::Environment(_), Definition::Path(_))
+                | (Definition::Environment(_), Definition::BuiltIn)
+                | (Definition::Path(_), Definition::BuiltIn)
         )
     }
 }
@@ -123,6 +127,16 @@ impl fmt::Display for Definition {
             Definition::Path(p) | Definition::Cli(Some(p)) => p.display().fmt(f),
             Definition::Environment(key) => write!(f, "environment variable `{}`", key),
             Definition::Cli(None) => write!(f, "--config cli option"),
+            Definition::BuiltIn => write!(f, "default"),
+        }
+    }
+}
+
+impl<T> From<T> for Value<T> {
+    fn from(val: T) -> Self {
+        Self {
+            val,
+            definition: Definition::BuiltIn,
         }
     }
 }
@@ -236,9 +250,10 @@ impl<'de> de::Deserialize<'de> for Definition {
     {
         let (discr, value) = <(u32, String)>::deserialize(deserializer)?;
         match discr {
-            0 => Ok(Definition::Path(value.into())),
-            1 => Ok(Definition::Environment(value)),
-            2 => {
+            0 => Ok(Definition::BuiltIn),
+            1 => Ok(Definition::Path(value.into())),
+            2 => Ok(Definition::Environment(value)),
+            3 => {
                 let path = (value.len() > 0).then_some(value.into());
                 Ok(Definition::Cli(path))
             }

tests/testsuite/bad_config.rs

@@ -159,7 +159,10 @@ fn invalid_global_config() {
     p.cargo("check -v")
         .with_status(101)
         .with_stderr_data(str![[r#"
-[ERROR] could not load Cargo configuration
+[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
+
+Caused by:
+  could not load Cargo configuration
 
 Caused by:
   could not parse TOML configuration in `[ROOT]/foo/.cargo/config.toml`

tests/testsuite/config.rs

@@ -1649,7 +1649,19 @@ target-dir = ''
 
 #[cargo_test]
 fn cargo_target_empty_env() {
-    let project = project().build();
+    let project = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                authors = []
+                version = "0.0.0"
+                build = "build.rs"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
 
     project.cargo("check")
         .env("CARGO_TARGET_DIR", "")