t7005 and vi in GIT_EXEC_PATH

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

Hi,

On Sat, 10 Nov 2007, Brian Gernhardt wrote:

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.

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

On Nov 11, 2007, at 10:58 AM, Johannes Schindelin wrote:

quoted

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.

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.

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

Hi,

On Sun, 11 Nov 2007, Brian Gernhardt wrote:

On Nov 11, 2007, at 10:58 AM, Johannes Schindelin wrote:

quoted

quoted

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.

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.

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.

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

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 <redacted>
---
 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örn Steinbrink wrote:

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.

Hmm.  This will probably stop working when you do not have git installed, 
because you now tell git to search for git programs in ".", where they are 
not.  Probably git-commit executes your installed write-tree, commit-tree 
and friends, instead of the compiled ones.

Ciao,
Dscho

On Nov 11, 2007, at 12:44 PM, Johannes Schindelin wrote:

Hi,

On Sun, 11 Nov 2007, Björn Steinbrink wrote:

quoted

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.

Hmm.  This will probably stop working when you do not have git  
installed,
because you now tell git to search for git programs in ".", where  
they are
not.  Probably git-commit executes your installed write-tree, commit- 
tree
and friends, instead of the compiled ones.

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

On 2007.11.11 17:44:14 +0000, Johannes Schindelin wrote:

Hi,

On Sun, 11 Nov 2007, Björn Steinbrink wrote:

quoted

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.

Hmm.  This will probably stop working when you do not have git installed, 
because you now tell git to search for git programs in ".", where they are 
not.  Probably git-commit executes your installed write-tree, commit-tree 
and friends, instead of the compiled ones.

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

Hi Brian, hi Björn,

On Sun, 11 Nov 2007, Brian Gernhardt wrote:

On Nov 11, 2007, at 12:44 PM, Johannes Schindelin wrote:

quoted

Probably git-commit executes your installed write-tree, commit-tree 
and friends, instead of the compiled ones.

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);

Ah, I forgot that --exec-path=<path> did not override GIT_EXEC_PATH.  
Thanks for clarifying!  Your patch is obviously good, then.

Ciao,
Dscho