Hi Ben
On 20/05/2025 20:34, D. Ben Knoble wrote:
This has caused trouble in the past [1] [2];
Another way of looking at this is that the trouble is caused by a script
that makes incorrect assumptions about git.
The assumption that the scripts from contrib as installed at a fixed
location relative to the git binary is false. Where they are installed
and whether they are installed at all is down to the discretion of the
distribution that you're using. Looking for "git jump" at a fixed offset
from the git binary is no more portable than looking for it in a fixed
location.
I think that the assumption that git should not change the environment
when it runs the editor is unrealistic. "git commit file" will use a
temporary index to create the commit and sets GIT_INDEX_FILE when
running the editor. This means that if the editor wants to display the
staged changes by running "git diff --cached HEAD" the diff will
accurately represent the changes being committed. Adding GIT_EXEC_PATH
to the beginning of PATH ensures that the diff will be created by the
same version of git the the user ran which avoids subtle bugs where a
sub-process of git runs a git command using an incompatible version of
git. There are several other environment variables that may be set when
running the editor such as GIT_DIR if the command is run from a linked
wortree.
To create a clean environment when opening a terminal from your editor
you can add
PATH="${PATH#$GIT_EXEC_PATH:}"
unset $(git rev-parse --local-env-vars)
to your shell setup script.
I think the first two patches are very welcome cleanups but I'm not
convinced by the rationale for patches 3 & 4.
Best Wishes
Phillip
after attempting to help
Git-for-Windows avoid that problem in Vim [3] by recommendation [4], it
was suggested that upstreaming the change instead would be a better
solution. Indeed, this should work for more uses/editors/etc.
[1]: https://public-inbox.org/git/CALnO6CDtGRRav8zK2GKi1oHTZWrHFTxZNmnOWu64-ab+oY3_Lw@xxxxxxxxxxxxxx/
[2]: https://benknoble.github.io/blog/2020/05/22/libexec-git-core-on-path/
[3]: https://github.com/git-for-windows/build-extra/pull/616
[4]: https://github.com/benknoble/Dotfiles/issues/143#issuecomment-2869525481
I haven't managed to test this on Windows, so any extra eyeballs there
are greatly appreciated. I'd also appreciate suggestions to fix the
memory leak.
Structure: patches 1 & 2 are cleanups. In particular, patch 1 is essential to
the tests in patch 4. I don't think patch 2 is strictly needed. Patch 3
refactors a little to make patch 4 take effect on all platforms. Patch 4 does
the real work.
D. Ben Knoble (4):
t7005: sanitize test environment for subsequent tests
editor: use standard strvec API to receive environment for external
editors
run-command: prep_childenv on all platforms
drop git_exec_path() from non-Git commands' PATH
builtin/commit.c | 2 +-
editor.c | 10 ++++-----
editor.h | 7 +++---
run-command.c | 55 ++++++++++++++++++++++++++++++++++++++++++-----
t/t7005-editor.sh | 18 +++++++++++++---
5 files changed, 75 insertions(+), 17 deletions(-)
base-commit: 7a1d2bd0a596f42a8a7a68d55577967bb454fec0
