If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the real

vi is invoked instead of the test vi script.  This is because the git

wrapper puts GIT_EXEC_PATH ahead of ".".  I see no easy solution to

this problem, and thought I should bring it up with the list.
~~ Brian

-

The git wrapper executable always prepends the GIT_EXEC_PATH build

variable to the current PATH, so prepending "." to the PATH is not

enough to give precedence to the fake vi executable.
The --exec-path option allows to prepend a directory to PATH even before

GIT_EXEC_PATH (which is added anyway), so we can use that instead.
Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>

---

 t/t7005-editor.sh |    4 ++--

 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh

index 01cc0c0..0b36ee1 100755

--- a/t/t7005-editor.sh

+++ b/t/t7005-editor.sh

@@ -61,7 +61,7 @@ do

 		;;

 	esac

 	test_expect_success "Using $i" '

-		git commit --amend &&

+		git --exec-path=. commit --amend &&

 		git show -s --pretty=oneline |

 		sed -e "s/^[0-9a-f]* //" >actual &&

 		diff actual expect

@@ -83,7 +83,7 @@ do

 		;;

 	esac

 	test_expect_success "Using $i (override)" '

-		git commit --amend &&

+		git --exec-path=. commit --amend &&

 		git show -s --pretty=oneline |

 		sed -e "s/^[0-9a-f]* //" >actual &&

 		diff actual expect

--

1.5.3.5.622.g6fd7a
-

Hi,
On Sun, 11 Nov 2007, Bj

The . is prepended to PATH in _addition_ to the usual paths (as I wrote,

see setup_path() in exec_cmd.c). It does not replace anything AFAICT.
$ echo $PATH

/home/doener/bin:/usr/local/bin:/usr/bin:/bin:/usr/games
$ GIT_EXEC_PATH=.. GIT_EDITOR="env;" ../git  commit | grep ^PATH=

PATH=/home/doener/src/git:/home/doener/bin:/home/doener/src/git:/home/doener/bin:/usr/local/bin:/usr/bin:/bin:/usr/games
$ GIT_EXEC_PATH=.. GIT_EDITOR="env;" ../git --exec-path=. commit | grep

^PATH=

PATH=/home/doener/src/git/t:/home/doener/src/git:/home/doener/bin:/home/doener/src/git:/home/doener/bin:/usr/local/bin:/usr/bin:/bin:/usr/games
Looks good to me...
Björn

-

You are wrong there.  From exec_cmd.c:setup_path() (lines 51-54):
     add_path(&new_path, argv_exec_path);

     add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));

     add_path(&new_path, builtin_exec_path);

     add_path(&new_path, cmd_path);
So the path with this patch will still include the build directory

before the install location.
~~ Brian-

Hi Brian, hi Bj

Hi,
I don't understand.  GIT_EXEC_PATH should be set to the build directory

when you are running the tests.  Unless you copy vi _there_, you should

not have any problem.
Ciao,

Dscho
-

I'm sorry, I should have been more clear.  I was referring to the

GIT_EXEC_PATH build variable, not the environment variable.  The git

wrapper always adds the path determined during build to the front of

PATH.  When I was changing my build script, this got set to "/usr/

local/bin" (I usually use /usr/local/stow/git, instead).  Since I have

a /usr/local/bin/vim, PATH for git-commit.sh during the test was:
- my git build directory

- /usr/local/bin (containing a symlink vi -> vim)

- the t/trash directory, added by the test via `PATH=".:$PATH"`

(containing the test vi script)

- my normal path
The test appeared to hang when running it normally.  When I ran it

with -v, I saw that vim was started.
~~ Brian

-

Maybe that is what is broken.  t/test-lib.sh makes the

environment variable point at the build directory, and that

should override the path that is compiled in, shouldn't it?

-

Ah, nevermind.  I did not see the other messages in the thread.

-

Maybe you prefer this patch then? "make test" survived up to 9101/25,

but that fails with the current master anyway and I didn't bother to run

the remaining tests manually, so it seems to be fine. Might break some

weird setups that rely on being able to set multiple additional paths

though (not that I think that that is a good idea to begin with).
Björn

---

Instead of adding all possible exec paths to PATH, only add the best

one, following the same rules that --exec-path, without arguments, uses

to figure out which path to display.
Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>

---

diff --git a/exec_cmd.c b/exec_cmd.c

index 2d0a758..9c376ad 100644

--- a/exec_cmd.c

+++ b/exec_cmd.c

@@ -48,9 +48,7 @@ void setup_path(const char *cmd_path)
 	strbuf_init(&new_path, 0);
-	add_path(&new_path, argv_exec_path);

-	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));

-	add_path(&new_path, builtin_exec_path);

+	add_path(&new_path, git_exec_path());

 	add_path(&new_path, cmd_path);
 	if (old_path)

-

Hi,
For easy application by the maintainer, please make the commit message the 
I wonder why cmd_path is still there, then.  (I'd have expected something

like
	add_path(&new_path, cmd_path ? cmd_path : git_exec_path());
In related news, IMO cmd_path should be made absolute if it is not already

the case.
Ciao,

Dscho
-

Wouldn't that break setups where only the "git" wrapper is in $PATH?

cmd_path is just the path to the called executable (argv[0]). As I read

the code, it seems that cmd_path is just a worst case fallback, when the

"real" exec path is messed up somehow. I figured that I'd better keep

it, instead of just dropping it without knowing its intended purpose.

But if you're going to change anything about it, I'd guess that the only
add_path() already takes care of that.
Björn

-

Hi,
Yeah, and cmd_path is not used otherwise.  Sorry for the noise.
Ciao,

Dscho
-

Hi,
The obvious solution would be to copy "vi" into the git build directory

for the test, or skip the test if that copy could not be performed.
Ciao,

Dscho
-

Putting "." at the front of GIT_EXEC_PATH instead of PATH would appear

to do the trick then, wouldn't it?
--

David Kastrup, Kriemhildstr. 15, 44793 Bochum

-

The GIT_EXEC_PATH I was referring to is the one set in the Makefile

and compiled into git.  The GIT_EXEC_PATH environment variable is set

to the git repository.  PATH ends up looking like this (paraphrased):

"git.git:install directory:.:normal PATH".  And since the install

directory contains vi, the test fails (actually appears to hang

because vi is waiting for input while it's output is being sent to /

dev/null).
~~ Brian

-