Rework config walk_tree in preparation for safe_directories

This reworks the walk_tree so that the home config can be loaded first
in order to check safe_directories.

Author: ehuss

src/cargo/util/config/mod.rs

@@ -1004,18 +1004,19 @@ impl Config {
     }
 
     pub(crate) fn load_values_unmerged(&self) -> CargoResult<Vec<ConfigValue>> {
+        self._load_values_unmerged()
+            .with_context(|| "could not load Cargo configuration")
+    }
+
+    fn _load_values_unmerged(&self) -> CargoResult<Vec<ConfigValue>> {
+        let (cvs, mut seen) = self.walk_tree(&self.cwd, false)?;
         let mut result = Vec::new();
-        let mut seen = HashSet::new();
-        let home = self.home_path.clone().into_path_unlocked();
-        self.walk_tree(&self.cwd, &home, |path| {
-            let mut cv = self._load_file(path, &mut seen, false)?;
+        for mut cv in cvs {
             if self.cli_unstable().config_include {
                 self.load_unmerged_include(&mut cv, &mut seen, &mut result)?;
             }
             result.push(cv);
-            Ok(())
-        })
-        .with_context(|| "could not load Cargo configuration")?;
+        }
         Ok(result)
     }
 
@@ -1037,20 +1038,19 @@ impl Config {
     }
 
     fn load_values_from(&self, path: &Path) -> CargoResult<HashMap<String, ConfigValue>> {
+        let (cvs, _) = self
+            .walk_tree(path, true)
+            .with_context(|| "could not load Cargo configuration")?;
+
+        // Merge all the files together.
         // This definition path is ignored, this is just a temporary container
         // representing the entire file.
         let mut cfg = CV::Table(HashMap::new(), Definition::Path(PathBuf::from(".")));
-        let home = self.home_path.clone().into_path_unlocked();
-
-        self.walk_tree(path, &home, |path| {
-            let value = self.load_file(path, true)?;
-            cfg.merge(value, false).with_context(|| {
+        for cv in cvs {
+            cfg.merge(cv, false).with_context(|| {
                 format!("failed to merge configuration at `{}`", path.display())
             })?;
-            Ok(())
-        })
-        .with_context(|| "could not load Cargo configuration")?;
-
+        }
         match cfg {
             CV::Table(map, _) => Ok(map),
             _ => unreachable!(),
@@ -1355,29 +1355,39 @@ impl Config {
         }
     }
 
-    fn walk_tree<F>(&self, pwd: &Path, home: &Path, mut walk: F) -> CargoResult<()>
-    where
-        F: FnMut(&Path) -> CargoResult<()>,
-    {
-        let mut stash: HashSet<PathBuf> = HashSet::new();
+    /// Walks from the given path upwards, loading `config.toml` files along the way.
+    ///
+    /// `includes` indicates whether or not `includes` directives should be loaded.
+    ///
+    /// Returns a `Vec` of each config file in the order they were loaded (home directory is last).
+    fn walk_tree(&self, pwd: &Path, includes: bool) -> CargoResult<(Vec<CV>, HashSet<PathBuf>)> {
+        let mut seen: HashSet<PathBuf> = HashSet::new();
+        // Load "home" first so that safe.directories can be loaded. However,
+        // "home" will be last in the result.
+        let home = self.home_path.clone().into_path_unlocked();
+        let home_cv = if let Some(path) = self.get_file_path(&home, "config", true)? {
+            Some(self._load_file(&path, &mut seen, includes)?)
+        } else {
+            None
+        };
+
+        let mut result = Vec::new();
 
         for current in paths::ancestors(pwd, self.search_stop_path.as_deref()) {
-            if let Some(path) = self.get_file_path(&current.join(".cargo"), "config", true)? {
-                walk(&path)?;
-                stash.insert(path);
+            let dot_cargo = current.join(".cargo");
+            if dot_cargo == home {
+                // home is already loaded, don't load again.
+                continue;
             }
-        }
-
-        // Once we're done, also be sure to walk the home directory even if it's not
-        // in our history to be sure we pick up that standard location for
-        // information.
-        if let Some(path) = self.get_file_path(home, "config", true)? {
-            if !stash.contains(&path) {
-                walk(&path)?;
+            if let Some(path) = self.get_file_path(&dot_cargo, "config", true)? {
+                let cv = self._load_file(&path, &mut seen, includes)?;
+                result.push(cv);
             }
         }
-
-        Ok(())
+        if let Some(home_cv) = home_cv {
+            result.push(home_cv);
+        }
+        Ok((result, seen))
     }
 
     /// Gets the index for a registry.

tests/testsuite/config.rs

@@ -1181,10 +1181,7 @@ fn table_merge_failure() {
     assert_error(
         config.get::<Table>("table").unwrap_err(),
         "\
-could not load Cargo configuration
-
-Caused by:
-  failed to merge configuration at `[..]/.cargo/config`
+failed to merge configuration at `[ROOT]/foo`
 
 Caused by:
   failed to merge key `table` between [..]/foo/.cargo/config and [..]/.cargo/config