Skip to content

Commit c8b2eb3

Browse files
committed
Simplify path construction and config; improve comments
This stops attempting to make the Windows-specific helper function semi-portable. I had done that with the idea tha it could be tested on all platforms so as to gain better coverage in practice. But it doesn't really make sense to do this because it keeps the native path separator `\` from being used even where it is now clearer to use it, and because such tests would not be able to use common values such as `C:\Program Files` because a path that starts with `C:\`, while absolute on Windows, is relative on Unix-like systems.
1 parent 1f0c3bf commit c8b2eb3

File tree

1 file changed

+7
-8
lines changed

1 file changed

+7
-8
lines changed

gix-path/src/env/git.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ pub(super) static ALTERNATIVE_LOCATIONS: Lazy<Vec<PathBuf>> = Lazy::new(|| {
1515
#[cfg(not(windows))]
1616
pub(super) static ALTERNATIVE_LOCATIONS: Lazy<Vec<PathBuf>> = Lazy::new(|| vec![]);
1717

18-
#[cfg(any(windows, test))]
18+
#[cfg(windows)]
1919
fn alternative_windows_locations_from_environment<F>(var_os_func: F) -> Vec<PathBuf>
2020
where
2121
F: Fn(&str) -> Option<std::ffi::OsString>,
@@ -38,10 +38,10 @@ where
3838
let varname_current = "ProgramFiles";
3939

4040
// 64-bit relative bin dir. So far, this is always mingw64, not ucrt64, clang64, or clangarm64.
41-
let suffix_64 = Path::new("Git/bin/mingw64");
41+
let suffix_64 = Path::new(r"Git\bin\mingw64");
4242

4343
// 32-bit relative bin dir. So far, this is always mingw32, not clang32.
44-
let suffix_32 = Path::new("Git/bin/mingw32");
44+
let suffix_32 = Path::new(r"Git\bin\mingw32");
4545

4646
// Whichever of the 64-bit or 32-bit relative bin better matches this process's architecture.
4747
// Unlike the system architecture, the process architecture is always known at compile time.
@@ -62,12 +62,11 @@ where
6262
if let Some(value) = var_os_func(name) {
6363
let pf = Path::new(&value);
6464
if pf.is_relative() {
65-
// This should never happen, but if it does then we should not use the path.
65+
// This shouldn't happen, but if it does then don't use the path. This is mainly in
66+
// case we are accidentally invoked with the environment variable set but empty.
6667
continue;
6768
};
68-
// Chain components to weakly normalize the path, mostly just for its separators.
69-
let components = pf.iter().chain(suffix.iter());
70-
let location = PathBuf::from_iter(components);
69+
let location = pf.join(suffix);
7170
if !locations.contains(&location) {
7271
locations.push(location);
7372
}
@@ -289,7 +288,7 @@ mod tests {
289288
.expect("The x86 program files folder will in practice always be available.");
290289

291290
let maybe_pf_64bit = RegKey::predef(HKEY_LOCAL_MACHINE)
292-
.open_subkey_with_flags(r#"SOFTWARE\Microsoft\Windows\CurrentVersion"#, KEY_QUERY_VALUE)
291+
.open_subkey_with_flags(r"SOFTWARE\Microsoft\Windows\CurrentVersion", KEY_QUERY_VALUE)
293292
.expect("The `CurrentVersion` key exists and allows reading.")
294293
.get_value::<OsString, _>("ProgramW6432Dir")
295294
.map(PathBuf::from)

0 commit comments

Comments
 (0)