Skip to content

Commit 167dc14

Browse files
committed
Start implementing a helper for ALTERNATIVE_LOCATIONS
1 parent e9eabeb commit 167dc14

File tree

1 file changed

+86
-0
lines changed

1 file changed

+86
-0
lines changed

gix-path/src/env/git.rs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,92 @@ 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))] // So this can always be tested.
19+
fn alternative_locations_from_env<F>(var_os_func: F) -> Vec<PathBuf>
20+
where
21+
F: Fn(&str) -> Option<std::ffi::OsString>,
22+
{
23+
// FIXME: Define pairs of the environment variable name and the path suffix to apply, or
24+
// something, because right now this new function is totally wrong because it returns the
25+
// program files directory paths instead of the Git for Windows bin directory paths under them.
26+
//
27+
// But I am not really sure what this should do to handle the ProgramFiles environment variable
28+
// and figure out if it is 64-bit or 32-bit -- which we need in order to know whether to append
29+
// `Git/mingw64/bin` or `Git/mingw32/bin`. We do need to look at the ProgramFiles environment
30+
// variable in addition to the other two preferable architecture-specific ones (for which the
31+
// mingw64 vs. mingw32 decision is easy), at least some of the time, for two reasons.
32+
//
33+
// First, we may be on a 32-bit system. Then we do not have either of the other two variables.
34+
//
35+
// Second, the others may also absent on a 64-bit system, if a parent process whittles down the
36+
// variables to pass to this child process, out of an erroneous belief about which ones are
37+
// needed to provide access to the program files directory.
38+
//
39+
// On a 64-bit system, the way a child process inherits its ProgramFiles variable to be for its
40+
// own architecture -- since after all its parent's architecture could be different -- is:
41+
//
42+
// - The value of a 64-bit child's ProgramFiles variable comes from whatever the parent passed
43+
// down as ProgramW6432, if that variable was passed down.
44+
//
45+
// - The value of a 32-bit child's ProgramFiles variable comes from whatever the parent passed
46+
// down as ProgramFiles(x86), if that variable was passed down.
47+
//
48+
// - If the variable corresponding to the child's architecture was not passed down by the
49+
// parent, but ProgramFiles was passed down, then the child receives that as the value of
50+
// its ProgramFiles variable. Only in this case does ProgramFiles come from ProgramFiles.
51+
//
52+
// The problem -- besides that those rules are not well known, and parent processes that seek
53+
// to pass down minimal environments often do not take heed of them, such that a child process
54+
// will get the wrong architecture's value for its ProgramFiles environment variable -- is that
55+
// the point of this function is to make use of environment variables in order to avoid:
56+
//
57+
// - Calling Windows API functions explicitly, even via the higher level `windows` crate.
58+
// - Accessing the Windows registry, even through the very widely used `winreg` crate.
59+
// - Adding any significant new production dependencies, such as the `known-folders` crate.
60+
//
61+
// Possible solutions:
62+
//
63+
// 1. Abandon at least one of those goals.
64+
// 2. Check both `mingw*` subdirectories regardless of which program files Git is in.
65+
// 3. Inspect the names for substrings like ` (x86)` in an expected position.
66+
// 4. Assume the value of `ProgramFiles` is correct, i.e., is for the process's architecture.
67+
//
68+
// I think (4) is the way to go, at least until (1) is assessed. With (2), we would be checking
69+
// paths that there is no specific reason to think have *working* Git executables...though this
70+
// does have the advantage that its logic would be the same as would be needed in the local
71+
// program files directory (usually `%LocalAppData$/Programs`) if we ever add that. With (3),
72+
// the risk of getting it wrong is low, but the logic is more complex, and we lose the
73+
// simplicity of getting the paths from outside rather than applying assumptions about them.
74+
//
75+
// With (4), we take the ProgramFiles environment variable at its word. This is good not just
76+
// for abstract correctness, but also if the parent modifies these variables intentionally on a
77+
// 64-bit system. A parent process can't reasonably expect this to be followed, because a child
78+
// process may use another mechanism such as known folders. However, following it, when we are
79+
// using environment variables already, satisfies a weaker expectation that the environment
80+
// value *or* actual value (obtainable via known folders or the registry), rather than some
81+
// third value, is used. (4) is also a simple way to do the right thing on a 32-bit system.
82+
let suffix_x86 =
83+
let rules = [
84+
85+
];
86+
87+
let names = [
88+
"ProgramW6432", // 64-bit path from a 32-bit or 64-bit process on a 64-bit system.
89+
"ProgramFiles(x86)", // 32-bit path from a 32-bit or 64-bit process on a 64-bit system.
90+
"ProgramFiles", // 32-bit path on 32-bit system. Or if the parent cleared the others.
91+
];
92+
93+
let mut locations = vec![];
94+
95+
for path in names.into_iter().filter_map(var_os_func).map(PathBuf::from) {
96+
if !locations.contains(&path) {
97+
locations.push(path);
98+
}
99+
}
100+
101+
locations
102+
}
103+
18104
#[cfg(windows)]
19105
pub(super) static EXE_NAME: &str = "git.exe";
20106
#[cfg(not(windows))]

0 commit comments

Comments
 (0)