Skip to content

Commit 2bce0d2

Browse files
committed
Don't set/change ceiling directories
The tiny chance of a performance or robustness benefit is not worth the complexity, because: - Git, at least versions I've looked at, will avoid doing the particular traversals that this stands to prevent, just from `GIT_DIR`. - This shouldn't be needed for correctness or security, since the effect of `GIT_DIR` follows from the documentation (as well as being tested on various versions). - This amount of redundancy is hard to justify. Even if `GIT_DIR` failed to have the desired effect, the protection in any recent or otherwise properly patched versions of Git should prevent a malicious repository at a location like `C:\` from affecting the configuration gleaned (or any `git` behavior). - Besides the complexity of the code, there is also the complexity of satisfying oneself that it really is acceptable to *clobber* existing configuration of ceiling directories, in the particular situation we are in. Of course, this could be avoided by prepending it and a `;` (which is the separator on Windows). But that would potentially worsen a situation where, if used, the entries take time for Git to canonicalize due to a slow fileystem. This commit is mostly just a revert of the previous commit, but it does also adjust some comments in light of further insights, to avoid suggesting benefits that are not known to pertain, and to mention the case of a nonexistent current directory.
1 parent 073e277 commit 2bce0d2

File tree

1 file changed

+18
-24
lines changed

1 file changed

+18
-24
lines changed

gix-path/src/env/git/mod.rs

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,19 @@ fn git_cmd(executable: PathBuf) -> Command {
115115
const CREATE_NO_WINDOW: u32 = 0x08000000;
116116
cmd.creation_flags(CREATE_NO_WINDOW);
117117
}
118+
// We will try to run `git` from a location fairly high in the filesystem, in the hope that it
119+
// may help if we are deeply nested, on network store, or in a directory that has been deleted.
120+
let cwd = if cfg!(windows) {
121+
// We try the Windows directory (usually `C:\Windows`) first. It is given by `SystemRoot`,
122+
// except in rare cases where our own parent has not passed down that environment variable.
123+
env::var_os("SystemRoot")
124+
.or_else(|| env::var_os("windir"))
125+
.map(PathBuf::from)
126+
.filter(|p| p.is_absolute())
127+
.unwrap_or_else(env::temp_dir)
128+
} else {
129+
"/".into()
130+
};
118131
// Git 2.8.0 and higher support --show-origin. The -l, -z, and --name-only options were
119132
// supported even before that. In contrast, --show-scope was introduced later, in Git 2.26.0.
120133
// Low versions of Git are still sometimes used, and this is sometimes reasonable because
@@ -126,32 +139,13 @@ fn git_cmd(executable: PathBuf) -> Command {
126139
// file under `/Library` is shown as an "unknown" scope but takes precedence over the system
127140
// scope. Although `GIT_CONFIG_NOSYSTEM` will suppress this as well, passing --system omits it.
128141
cmd.args(["config", "-lz", "--show-origin", "--name-only"])
129-
.stdin(Stdio::null())
130-
.stderr(Stdio::null())
131-
.env_remove("GIT_COMMON_DIR") // Because we are setting `GIT_DIR`.
142+
.current_dir(cwd)
143+
.env_remove("GIT_COMMON_DIR") // We are setting `GIT_DIR`.
132144
.env_remove("GIT_DISCOVERY_ACROSS_FILESYSTEM")
133145
.env("GIT_DIR", NULL_DEVICE) // Avoid getting local-scope config.
134-
.env("GIT_WORK_TREE", NULL_DEVICE); // Just to avoid confusion when debugging.
135-
136-
// We will run `git` from a location fairly high in the filesystem if we can. This can be
137-
// faster than running it from our own CWD, if we are deeply nested or on network storage.
138-
if cfg!(windows) {
139-
// We try the Windows directory (usually `C:\Windows`) first. It is given by `SystemRoot`,
140-
// except in rare cases where our own parent has not passed down that environment variable.
141-
let cwd = env::var_os("SystemRoot")
142-
.or_else(|| env::var_os("windir"))
143-
.map(PathBuf::from)
144-
.filter(|p| p.is_absolute())
145-
.unwrap_or_else(env::temp_dir);
146-
if let Some(parent) = cwd.parent().filter(|p| p.is_absolute() && *p != cwd) {
147-
cmd.env("GIT_CEILING_DIRECTORIES", parent);
148-
}
149-
cmd.current_dir(cwd);
150-
} else {
151-
// The root should always be available, with nothing above it to worry about traversing to.
152-
cmd.current_dir("/");
153-
}
154-
146+
.env("GIT_WORK_TREE", NULL_DEVICE) // Avoid confusion when debugging.
147+
.stdin(Stdio::null())
148+
.stderr(Stdio::null());
155149
cmd
156150
}
157151

0 commit comments

Comments
 (0)