refactor

- move `env_var` to path as it's the smaller crate that is easier to pull in from other plumbing crates.
- use fully qualified paths to make clearer what is what.
- add at least 'some' test for `gix_path::home_dir()`

Author: Byron

gix-config/src/file/init/comfort.rs

@@ -1,7 +1,3 @@
-use std::path::PathBuf;
-
-use gix_path::home;
-
 use crate::{
     file::{init, Metadata},
     path, source, File, Source,
@@ -32,7 +28,7 @@ impl File<'static> {
             .flat_map(|kind| kind.sources())
             .filter_map(|source| {
                 let path = source
-                    .storage_location(&mut crate::env_var)
+                    .storage_location(&mut gix_path::env_var)
                     .and_then(|p| p.is_file().then_some(p))
                     .map(|p| p.into_owned());
 
@@ -45,7 +41,7 @@ impl File<'static> {
                 .into()
             });
 
-        let home = home();
+        let home = gix_path::home_dir();
         let options = init::Options {
             includes: init::includes::Options::follow_without_conditional(home.as_deref()),
             ..Default::default()
@@ -61,7 +57,7 @@ impl File<'static> {
     ///
     /// [`gix-config`'s documentation]: https://git-scm.com/docs/gix-config#Documentation/gix-config.txt-GITCONFIGCOUNT
     pub fn from_environment_overrides() -> Result<File<'static>, init::from_env::Error> {
-        let home = home();
+        let home = gix_path::home_dir();
         let options = init::Options {
             includes: init::includes::Options::follow_without_conditional(home.as_deref()),
             ..Default::default()
@@ -90,7 +86,7 @@ impl File<'static> {
             let mut path = dir.into();
             path.push(
                 source
-                    .storage_location(&mut crate::env_var)
+                    .storage_location(&mut gix_path::env_var)
                     .expect("location available for local"),
             );
             let local = Self::from_path_no_includes(&path, source)?;
@@ -103,7 +99,7 @@ impl File<'static> {
                 let source = Source::Worktree;
                 let path = git_dir.join(
                     source
-                        .storage_location(&mut crate::env_var)
+                        .storage_location(&mut gix_path::env_var)
                         .expect("location available for worktree"),
                 );
                 Self::from_path_no_includes(path, source)
@@ -112,7 +108,7 @@ impl File<'static> {
         }
         .transpose()?;
 
-        let home = home();
+        let home = gix_path::home_dir();
         let options = init::Options {
             includes: init::includes::Options::follow(
                 path::interpolate::Context {

gix-config/src/lib.rs

@@ -44,21 +44,9 @@ pub mod lookup;
 pub mod parse;
 ///
 pub mod value;
-use std::ffi::OsString;
-
 pub use gix_config_value::{color, integer, path, Boolean, Color, Integer, Path};
 
 mod types;
 pub use types::{File, Source};
 ///
 pub mod source;
-
-/// Returns the contents of an eviorment variable with some specical handeling
-/// for certain enviorment variables (like $HOME) for platform compatability
-pub fn env_var(var: &str) -> Option<OsString> {
-    if var == "HOME" {
-        gix_path::home().map(|home| home.into_os_string())
-    } else {
-        std::env::var_os(var)
-    }
-}

gix-path/src/lib.rs

@@ -57,6 +57,7 @@ pub struct Spec(bstr::BString);
 
 mod convert;
 use std::env::var_os;
+use std::ffi::OsString;
 use std::path::PathBuf;
 
 pub use convert::*;
@@ -70,31 +71,39 @@ mod spec;
 pub mod realpath;
 pub use realpath::function::{realpath, realpath_opts};
 
-/// Returns a platform independent home directory
+/// Returns a platform independent home directory.
+///
 /// On unix this simply returns $HOME on windows this uses %HOMEDRIVE%\%HOMEPATH% or %USERPROFILE%
-pub fn home() -> Option<PathBuf> {
+pub fn home_dir() -> Option<PathBuf> {
     if let Some(home) = var_os("HOME") {
         return Some(home.into());
     }
 
-    // TODO cache env access on windows? git does this by setting HOME but
-    // setting enviorment variables is a *really* bad idea in library
-
-    // FIXME: technically we should also check HOMESHARE in case HOME is a UNC path
-    // but git doesn't do this either so probably best to wait for an upstream fix
+    // NOTE: technically we should also check HOMESHARE in case HOME is a UNC path
+    //       but git doesn't do this either so probably best to wait for an upstream fix.
     #[cfg(windows)]
-    if let Some(homedrive) = var_os("HOMEDRIVE") {
-        if let Some(home_path) = var_os("HOMEPATH") {
-            let home = PathBuf::from(homedrive).join(home_path);
-            if home.metadata().map_or(false, |home| home.is_dir()) {
-                return Some(home);
+    {
+        if let Some(homedrive) = var_os("HOMEDRIVE") {
+            if let Some(home_path) = var_os("HOMEPATH") {
+                let home = PathBuf::from(homedrive).join(home_path);
+                if home.metadata().map_or(false, |home| home.is_dir()) {
+                    return Some(home);
+                }
             }
         }
+        if let Some(userprofile) = var_os("USERPROFILE") {
+            return Some(userprofile.into());
+        }
     }
-    #[cfg(windows)]
-    if let Some(userprofile) = var_os("USERPROFILE") {
-        return Some(userprofile.into());
-    }
-
     None
 }
+
+/// Returns the contents of an environment variable of `name` with some special handling
+/// for certain environment variables (like `HOME`) for platform compatibility.
+pub fn env_var(name: &str) -> Option<OsString> {
+    if name == "HOME" {
+        home_dir().map(PathBuf::into_os_string)
+    } else {
+        std::env::var_os(name)
+    }
+}

gix-path/tests/path.rs

@@ -2,4 +2,15 @@ pub type Result<T = ()> = std::result::Result<T, Box<dyn std::error::Error>>;
 
 mod convert;
 mod realpath;
+mod home_dir {
+    #[test]
+    fn returns_existing_directory() {
+        if let Some(home) = gix_path::home_dir() {
+            assert!(
+                home.is_dir(),
+                "the home directory would typically exist, even though on unix we don't test for that."
+            );
+        }
+    }
+}
 mod util;

gix/src/config/cache/access.rs

@@ -2,7 +2,6 @@
 use std::{borrow::Cow, path::PathBuf, time::Duration};
 
 use gix_lock::acquire::Fail;
-use gix_path::home;
 
 use crate::{
     bstr::BStr,
@@ -204,7 +203,7 @@ impl Cache {
         std::env::var_os("XDG_CONFIG_HOME")
             .map(|path| (PathBuf::from(path), &self.xdg_config_home_env))
             .or_else(|| {
-                home().map(|mut p| {
+                gix_path::home_dir().map(|mut p| {
                     (
                         {
                             p.push(".config");
@@ -226,6 +225,6 @@ impl Cache {
     /// We never fail for here even if the permission is set to deny as we `gix-config` will fail later
     /// if it actually wants to use the home directory - we don't want to fail prematurely.
     pub(crate) fn home_dir(&self) -> Option<PathBuf> {
-        home().and_then(|path| self.home_env.check_opt(path))
+        gix_path::home_dir().and_then(|path| self.home_env.check_opt(path))
     }
 }

gix/src/config/cache/init.rs

@@ -95,7 +95,7 @@ impl Cache {
                             "HOME" => Some(home_env),
                             _ => None,
                         }
-                        .and_then(|perm| perm.check_opt(name).and_then(gix_config::env_var))
+                        .and_then(|perm| perm.check_opt(name).and_then(gix_path::env_var))
                     })
                     .map(|p| (source, p.into_owned()))
             })

gix/src/open/repository.rs

@@ -2,7 +2,6 @@
 use std::{borrow::Cow, path::PathBuf};
 
 use gix_features::threading::OwnShared;
-use gix_path::home;
 
 use super::{Error, Options};
 use crate::{
@@ -181,7 +180,7 @@ impl ThreadSafeRepository {
         };
         let head = refs.find("HEAD").ok();
         let git_install_dir = crate::path::install_dir().ok();
-        let home = home().and_then(|home| env.home.check_opt(home));
+        let home = gix_path::home_dir().and_then(|home| env.home.check_opt(home));
 
         let mut filter_config_section = filter_config_section.unwrap_or(config::section::is_trusted);
         let config = config::Cache::from_stage_one(