Skip to content

Commit

Permalink
Don't set/change ceiling directories
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EliahKagan committed Aug 30, 2024
1 parent 073e277 commit 2bce0d2
Showing 1 changed file with 18 additions and 24 deletions.
42 changes: 18 additions & 24 deletions gix-path/src/env/git/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,19 @@ fn git_cmd(executable: PathBuf) -> Command {
const CREATE_NO_WINDOW: u32 = 0x08000000;
cmd.creation_flags(CREATE_NO_WINDOW);
}
// We will try to run `git` from a location fairly high in the filesystem, in the hope that it
// may help if we are deeply nested, on network store, or in a directory that has been deleted.
let cwd = if cfg!(windows) {
// We try the Windows directory (usually `C:\Windows`) first. It is given by `SystemRoot`,
// except in rare cases where our own parent has not passed down that environment variable.
env::var_os("SystemRoot")
.or_else(|| env::var_os("windir"))
.map(PathBuf::from)
.filter(|p| p.is_absolute())
.unwrap_or_else(env::temp_dir)
} else {
"/".into()
};
// Git 2.8.0 and higher support --show-origin. The -l, -z, and --name-only options were
// supported even before that. In contrast, --show-scope was introduced later, in Git 2.26.0.
// Low versions of Git are still sometimes used, and this is sometimes reasonable because
Expand All @@ -126,32 +139,13 @@ fn git_cmd(executable: PathBuf) -> Command {
// file under `/Library` is shown as an "unknown" scope but takes precedence over the system
// scope. Although `GIT_CONFIG_NOSYSTEM` will suppress this as well, passing --system omits it.
cmd.args(["config", "-lz", "--show-origin", "--name-only"])
.stdin(Stdio::null())
.stderr(Stdio::null())
.env_remove("GIT_COMMON_DIR") // Because we are setting `GIT_DIR`.
.current_dir(cwd)
.env_remove("GIT_COMMON_DIR") // We are setting `GIT_DIR`.
.env_remove("GIT_DISCOVERY_ACROSS_FILESYSTEM")
.env("GIT_DIR", NULL_DEVICE) // Avoid getting local-scope config.
.env("GIT_WORK_TREE", NULL_DEVICE); // Just to avoid confusion when debugging.

// We will run `git` from a location fairly high in the filesystem if we can. This can be
// faster than running it from our own CWD, if we are deeply nested or on network storage.
if cfg!(windows) {
// We try the Windows directory (usually `C:\Windows`) first. It is given by `SystemRoot`,
// except in rare cases where our own parent has not passed down that environment variable.
let cwd = env::var_os("SystemRoot")
.or_else(|| env::var_os("windir"))
.map(PathBuf::from)
.filter(|p| p.is_absolute())
.unwrap_or_else(env::temp_dir);
if let Some(parent) = cwd.parent().filter(|p| p.is_absolute() && *p != cwd) {
cmd.env("GIT_CEILING_DIRECTORIES", parent);
}
cmd.current_dir(cwd);
} else {
// The root should always be available, with nothing above it to worry about traversing to.
cmd.current_dir("/");
}

.env("GIT_WORK_TREE", NULL_DEVICE) // Avoid confusion when debugging.
.stdin(Stdio::null())
.stderr(Stdio::null());
cmd
}

Expand Down

0 comments on commit 2bce0d2

Please sign in to comment.