{"thread":{"id":"10791","subject":"t7005 and vi in GIT_EXEC_PATH","startedAt":"2007-11-10T22:03:44Z","lastAt":"2007-11-11T21:40:47Z","messageCount":18,"participants":["Brian Gernhardt","David Kastrup","Johannes Schindelin","Björn Steinbrink","Junio C Hamano","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"59244","messageId":"9A9986E7-E03D-458A-9A19-A3EF0E7B203D@silverinsanity.com","threadId":"10791","inReplyTo":null,"subject":"t7005 and vi in GIT_EXEC_PATH","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2007-11-10T22:03:44Z","receivedAt":"2007-11-10T22:03:44Z","isPatch":false,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the real \nvi is invoked instead of the test vi script. This is because the git \nwrapper puts GIT_EXEC_PATH ahead of \".\". I see no easy solution to \nthis problem, and thought I should bring it up with the list.\n\n~~ Brian\n"},{"id":"59245","messageId":"85abpl69ck.fsf@lola.goethe.zz","threadId":"10791","inReplyTo":"9A9986E7-E03D-458A-9A19-A3EF0E7B203D@silverinsanity.com","subject":"Re: t7005 and vi in GIT_EXEC_PATH","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-11-10T22:09:47Z","receivedAt":"2007-11-10T22:09:47Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Brian Gernhardt writes:\n\n> If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the real\n> vi is invoked instead of the test vi script. This is because the git\n> wrapper puts GIT_EXEC_PATH ahead of \".\". I see no easy solution to\n> this problem, and thought I should bring it up with the list.\n\nPutting \".\" at the front of GIT_EXEC_PATH instead of PATH would appear\nto do the trick then, wouldn't it?\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"59249","messageId":"443CE405-547F-4BBC-B32E-FFEE7E46700D@silverinsanity.com","threadId":"10791","inReplyTo":"85abpl69ck.fsf@lola.goethe.zz","subject":"Re: t7005 and vi in GIT_EXEC_PATH","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2007-11-10T22:45:46Z","receivedAt":"2007-11-10T22:45:46Z","isPatch":false,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Nov 10, 2007, at 5:09 PM, David Kastrup wrote:\n\n> Brian Gernhardt writes:\n>\n>> If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the \n>> real\n>> vi is invoked instead of the test vi script. This is because the git\n>> wrapper puts GIT_EXEC_PATH ahead of \".\". I see no easy solution to\n>> this problem, and thought I should bring it up with the list.\n>\n> Putting \".\" at the front of GIT_EXEC_PATH instead of PATH would appear\n> to do the trick then, wouldn't it?\n\nThe GIT_EXEC_PATH I was referring to is the one set in the Makefile \nand compiled into git. The GIT_EXEC_PATH environment variable is set \nto the git repository. PATH ends up looking like this (paraphrased): \n\"git.git:install directory:.:normal PATH\". And since the install \ndirectory contains vi, the test fails (actually appears to hang \nbecause vi is waiting for input while it's output is being sent to / \ndev/null).\n\n~~ Brian\n"},{"id":"59334","messageId":"Pine.LNX.4.64.0711111557370.4362@racer.site","threadId":"10791","inReplyTo":"9A9986E7-E03D-458A-9A19-A3EF0E7B203D@silverinsanity.com","subject":"Re: t7005 and vi in GIT_EXEC_PATH","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T15:58:18Z","receivedAt":"2007-11-11T15:58:18Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 Nov 2007, Brian Gernhardt wrote:\n\n> If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the real \n> vi is invoked instead of the test vi script. This is because the git \n> wrapper puts GIT_EXEC_PATH ahead of \".\". I see no easy solution to this \n> problem, and thought I should bring it up with the list.\n\nI don't understand. GIT_EXEC_PATH should be set to the build directory \nwhen you are running the tests. Unless you copy vi _there_, you should \nnot have any problem.\n\nCiao,\nDscho\n"},{"id":"59335","messageId":"FCFF59B3-D3F1-4BEB-B3C3-D07DD5D5D8EF@silverinsanity.com","threadId":"10791","inReplyTo":"Pine.LNX.4.64.0711111557370.4362@racer.site","subject":"Re: t7005 and vi in GIT_EXEC_PATH","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2007-11-11T16:10:12Z","receivedAt":"2007-11-11T16:10:12Z","isPatch":false,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Nov 11, 2007, at 10:58 AM, Johannes Schindelin wrote:\n\n>> If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the \n>> real\n>> vi is invoked instead of the test vi script. This is because the git\n>> wrapper puts GIT_EXEC_PATH ahead of \".\". I see no easy solution to \n>> this\n>> problem, and thought I should bring it up with the list.\n>\n> I don't understand. GIT_EXEC_PATH should be set to the build \n> directory\n> when you are running the tests. Unless you copy vi _there_, you \n> should\n> not have any problem.\n\nI'm sorry, I should have been more clear. I was referring to the \nGIT_EXEC_PATH build variable, not the environment variable. The git \nwrapper always adds the path determined during build to the front of \nPATH. When I was changing my build script, this got set to \"/usr/ \nlocal/bin\" (I usually use /usr/local/stow/git, instead). Since I have \na /usr/local/bin/vim, PATH for git-commit.sh during the test was:\n\n- my git build directory\n- /usr/local/bin (containing a symlink vi -> vim)\n- the t/trash directory, added by the test via `PATH=\".:$PATH\"` \n(containing the test vi script)\n- my normal path\n\nThe test appeared to hang when running it normally. When I ran it \nwith -v, I saw that vim was started.\n\n~~ Brian\n"},{"id":"59336","messageId":"Pine.LNX.4.64.0711111622350.4362@racer.site","threadId":"10791","inReplyTo":"FCFF59B3-D3F1-4BEB-B3C3-D07DD5D5D8EF@silverinsanity.com","subject":"Re: t7005 and vi in GIT_EXEC_PATH","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T16:28:39Z","receivedAt":"2007-11-11T16:28:39Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 11 Nov 2007, Brian Gernhardt wrote:\n\n> On Nov 11, 2007, at 10:58 AM, Johannes Schindelin wrote:\n> \n> > > If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the \n> > > real vi is invoked instead of the test vi script. This is because \n> > > the git wrapper puts GIT_EXEC_PATH ahead of \".\". I see no easy \n> > > solution to this problem, and thought I should bring it up with the \n> > > list.\n> > \n> > I don't understand. GIT_EXEC_PATH should be set to the build \n> > directory when you are running the tests. Unless you copy vi _there_, \n> > you should not have any problem.\n> \n> I'm sorry, I should have been more clear. I was referring to the \n> GIT_EXEC_PATH build variable, not the environment variable. The git \n> wrapper always adds the path determined during build to the front of \n> PATH. When I was changing my build script, this got set to \n> \"/usr/local/bin\" (I usually use /usr/local/stow/git, instead). Since I \n> have a /usr/local/bin/vim, PATH for git-commit.sh during the test was:\n> \n> - my git build directory\n> - /usr/local/bin (containing a symlink vi -> vim)\n> - the t/trash directory, added by the test via `PATH=\".:$PATH\"` (containing\n> the test vi script)\n> - my normal path\n> \n> The test appeared to hang when running it normally. When I ran it with \n> -v, I saw that vim was started.\n\nThe obvious solution would be to copy \"vi\" into the git build directory \nfor the test, or skip the test if that copy could not be performed.\n\nCiao,\nDscho\n"},{"id":"59349","messageId":"1194802691-27610-1-git-send-email-B.Steinbrink@gmx.de","threadId":"10791","inReplyTo":"9A9986E7-E03D-458A-9A19-A3EF0E7B203D@silverinsanity.com","subject":"[PATCH] t7005-editor.sh: Don't invoke real vi when it is in GIT_EXEC_PATH","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-11-11T17:38:11Z","receivedAt":"2007-11-11T17:38:11Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"The git wrapper executable always prepends the GIT_EXEC_PATH build\nvariable to the current PATH, so prepending \".\" to the PATH is not\nenough to give precedence to the fake vi executable.\n\nThe --exec-path option allows to prepend a directory to PATH even before\nGIT_EXEC_PATH (which is added anyway), so we can use that instead.\n\nSigned-off-by: Björn Steinbrink \n---\n t/t7005-editor.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7005-editor.sh b/t/t7005-editor.sh\nindex 01cc0c0..0b36ee1 100755\n--- a/t/t7005-editor.sh\n+++ b/t/t7005-editor.sh\n@@ -61,7 +61,7 @@ do\n \t\t;;\n \tesac\n \ttest_expect_success \"Using $i\" '\n-\t\tgit commit --amend &&\n+\t\tgit --exec-path=. commit --amend &&\n \t\tgit show -s --pretty=oneline |\n \t\tsed -e \"s/^[0-9a-f]* //\" >actual &&\n \t\tdiff actual expect\n@@ -83,7 +83,7 @@ do\n \t\t;;\n \tesac\n \ttest_expect_success \"Using $i (override)\" '\n-\t\tgit commit --amend &&\n+\t\tgit --exec-path=. commit --amend &&\n \t\tgit show -s --pretty=oneline |\n \t\tsed -e \"s/^[0-9a-f]* //\" >actual &&\n \t\tdiff actual expect\n-- \n1.5.3.5.622.g6fd7a\n"},{"id":"59350","messageId":"Pine.LNX.4.64.0711111742010.4362@racer.site","threadId":"10791","inReplyTo":"1194802691-27610-1-git-send-email-B.Steinbrink@gmx.de","subject":"Re: [PATCH] t7005-editor.sh: Don't invoke real vi when it is in GIT_EXEC_PATH","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T17:44:14Z","receivedAt":"2007-11-11T17:44:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 11 Nov 2007, Björn Steinbrink wrote:\n\n> The git wrapper executable always prepends the GIT_EXEC_PATH build\n> variable to the current PATH, so prepending \".\" to the PATH is not\n> enough to give precedence to the fake vi executable.\n> \n> The --exec-path option allows to prepend a directory to PATH even before\n> GIT_EXEC_PATH (which is added anyway), so we can use that instead.\n\nHmm. This will probably stop working when you do not have git installed, \nbecause you now tell git to search for git programs in \".\", where they are \nnot. Probably git-commit executes your installed write-tree, commit-tree \nand friends, instead of the compiled ones.\n\nCiao,\nDscho\n"},{"id":"59351","messageId":"2AE2E502-7942-449E-B847-75876A5DAF37@silverinsanity.com","threadId":"10791","inReplyTo":"Pine.LNX.4.64.0711111742010.4362@racer.site","subject":"Re: [PATCH] t7005-editor.sh: Don't invoke real vi when it is in GIT_EXEC_PATH","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2007-11-11T17:49:57Z","receivedAt":"2007-11-11T17:49:57Z","isPatch":true,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Nov 11, 2007, at 12:44 PM, Johannes Schindelin wrote:\n\n> Hi,\n>\n> On Sun, 11 Nov 2007, Björn Steinbrink wrote:\n>\n>> The git wrapper executable always prepends the GIT_EXEC_PATH build\n>> variable to the current PATH, so prepending \".\" to the PATH is not\n>> enough to give precedence to the fake vi executable.\n>>\n>> The --exec-path option allows to prepend a directory to PATH even \n>> before\n>> GIT_EXEC_PATH (which is added anyway), so we can use that instead.\n>\n> Hmm. This will probably stop working when you do not have git \n> installed,\n> because you now tell git to search for git programs in \".\", where \n> they are\n> not. Probably git-commit executes your installed write-tree, commit- \n> tree\n> and friends, instead of the compiled ones.\n\nYou are wrong there. From exec_cmd.c:setup_path() (lines 51-54):\n\n add_path(&new_path, argv_exec_path);\n add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));\n add_path(&new_path, builtin_exec_path);\n add_path(&new_path, cmd_path);\n\nSo the path with this patch will still include the build directory \nbefore the install location.\n\n~~ Brian"},{"id":"59353","messageId":"20071111180108.GA27759@atjola.homenet","threadId":"10791","inReplyTo":"Pine.LNX.4.64.0711111742010.4362@racer.site","subject":"Re: [PATCH] t7005-editor.sh: Don't invoke real vi when it is in GIT_EXEC_PATH","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-11-11T18:01:08Z","receivedAt":"2007-11-11T18:01:08Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2007.11.11 17:44:14 +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 11 Nov 2007, Björn Steinbrink wrote:\n> \n> > The git wrapper executable always prepends the GIT_EXEC_PATH build\n> > variable to the current PATH, so prepending \".\" to the PATH is not\n> > enough to give precedence to the fake vi executable.\n> > \n> > The --exec-path option allows to prepend a directory to PATH even before\n> > GIT_EXEC_PATH (which is added anyway), so we can use that instead.\n> \n> Hmm. This will probably stop working when you do not have git installed, \n> because you now tell git to search for git programs in \".\", where they are \n> not. Probably git-commit executes your installed write-tree, commit-tree \n> and friends, instead of the compiled ones.\n\nThe . is prepended to PATH in _addition_ to the usual paths (as I wrote,\nsee setup_path() in exec_cmd.c). It does not replace anything AFAICT.\n\n$ echo $PATH\n/home/doener/bin:/usr/local/bin:/usr/bin:/bin:/usr/games\n\n$ GIT_EXEC_PATH=.. GIT_EDITOR=\"env;\" ../git commit | grep ^PATH=\nPATH=/home/doener/src/git:/home/doener/bin:/home/doener/src/git:/home/doener/bin:/usr/local/bin:/usr/bin:/bin:/usr/games\n\n$ GIT_EXEC_PATH=.. GIT_EDITOR=\"env;\" ../git --exec-path=. commit | grep\n^PATH=\nPATH=/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\n\n\nLooks good to me...\n\nBjörn\n"},{"id":"59356","messageId":"Pine.LNX.4.64.0711111829250.4362@racer.site","threadId":"10791","inReplyTo":"2AE2E502-7942-449E-B847-75876A5DAF37@silverinsanity.com","subject":"Re: [PATCH] t7005-editor.sh: Don't invoke real vi when it is in GIT_EXEC_PATH","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T18:31:05Z","receivedAt":"2007-11-11T18:31:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Brian, hi Björn,\n\nOn Sun, 11 Nov 2007, Brian Gernhardt wrote:\n\n> On Nov 11, 2007, at 12:44 PM, Johannes Schindelin wrote:\n> \n> > Probably git-commit executes your installed write-tree, commit-tree \n> > and friends, instead of the compiled ones.\n> \n> You are wrong there. From exec_cmd.c:setup_path() (lines 51-54):\n> \n> add_path(&new_path, argv_exec_path);\n> add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));\n> add_path(&new_path, builtin_exec_path);\n> add_path(&new_path, cmd_path);\n\nAh, I forgot that --exec-path= did not override GIT_EXEC_PATH. \nThanks for clarifying! Your patch is obviously good, then.\n\nCiao,\nDscho\n"},{"id":"59363","messageId":"7v3avcefg9.fsf@gitster.siamese.dyndns.org","threadId":"10791","inReplyTo":"FCFF59B3-D3F1-4BEB-B3C3-D07DD5D5D8EF@silverinsanity.com","subject":"Re: t7005 and vi in GIT_EXEC_PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-11T19:43:02Z","receivedAt":"2007-11-11T19:43:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Gernhardt writes:\n\n> I'm sorry, I should have been more clear. I was referring to the\n> GIT_EXEC_PATH build variable, not the environment variable. The git\n> wrapper always adds the path determined during build to the front of\n> PATH. When I was changing my build script, this got set to \"/usr/\n> local/bin\" (I usually use /usr/local/stow/git, instead). Since I have\n> a /usr/local/bin/vim, PATH for git-commit.sh during the test was:\n>\n> - my git build directory\n> - /usr/local/bin (containing a symlink vi -> vim)\n> - the t/trash directory, added by the test via `PATH=\".:$PATH\"`\n> (containing the test vi script)\n> - my normal path\n\nMaybe that is what is broken. t/test-lib.sh makes the\nenvironment variable point at the build directory, and that\nshould override the path that is compiled in, shouldn't it?\n"},{"id":"59372","messageId":"20071111203318.GA32671@atjola.homenet","threadId":"10791","inReplyTo":"7v3avcefg9.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Use the best available exec path only","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-11-11T20:33:18Z","receivedAt":"2007-11-11T20:33:18Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2007.11.11 11:43:02 -0800, Junio C Hamano wrote:\n> Brian Gernhardt writes:\n> \n> > I'm sorry, I should have been more clear. I was referring to the\n> > GIT_EXEC_PATH build variable, not the environment variable. The git\n> > wrapper always adds the path determined during build to the front of\n> > PATH. When I was changing my build script, this got set to \"/usr/\n> > local/bin\" (I usually use /usr/local/stow/git, instead). Since I have\n> > a /usr/local/bin/vim, PATH for git-commit.sh during the test was:\n> >\n> > - my git build directory\n> > - /usr/local/bin (containing a symlink vi -> vim)\n> > - the t/trash directory, added by the test via `PATH=\".:$PATH\"`\n> > (containing the test vi script)\n> > - my normal path\n> \n> Maybe that is what is broken. t/test-lib.sh makes the\n> environment variable point at the build directory, and that\n> should override the path that is compiled in, shouldn't it?\n\nMaybe you prefer this patch then? \"make test\" survived up to 9101/25,\nbut that fails with the current master anyway and I didn't bother to run\nthe remaining tests manually, so it seems to be fine. Might break some\nweird setups that rely on being able to set multiple additional paths\nthough (not that I think that that is a good idea to begin with).\n\nBjörn\n---\nInstead of adding all possible exec paths to PATH, only add the best\none, following the same rules that --exec-path, without arguments, uses\nto figure out which path to display.\n\nSigned-off-by: Björn Steinbrink \n---\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex 2d0a758..9c376ad 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -48,9 +48,7 @@ void setup_path(const char *cmd_path)\n \n \tstrbuf_init(&new_path, 0);\n \n-\tadd_path(&new_path, argv_exec_path);\n-\tadd_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));\n-\tadd_path(&new_path, builtin_exec_path);\n+\tadd_path(&new_path, git_exec_path());\n \tadd_path(&new_path, cmd_path);\n \n \tif (old_path)\n"},{"id":"59375","messageId":"Pine.LNX.4.64.0711112047170.4362@racer.site","threadId":"10791","inReplyTo":"20071111203318.GA32671@atjola.homenet","subject":"Re: [PATCH] Use the best available exec path only","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T20:50:40Z","receivedAt":"2007-11-11T20:50:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 11 Nov 2007, Bj?rn Steinbrink wrote:\n\n> On 2007.11.11 11:43:02 -0800, Junio C Hamano wrote:\n> > Brian Gernhardt writes:\n> > \n> > > I'm sorry, I should have been more clear. I was referring to the \n> > > GIT_EXEC_PATH build variable, not the environment variable. The git \n> > > wrapper always adds the path determined during build to the front of \n> > > PATH. When I was changing my build script, this got set to \"/usr/ \n> > > local/bin\" (I usually use /usr/local/stow/git, instead). Since I \n> > > have a /usr/local/bin/vim, PATH for git-commit.sh during the test \n> > > was:\n> > >\n> > > - my git build directory\n> > > - /usr/local/bin (containing a symlink vi -> vim)\n> > > - the t/trash directory, added by the test via `PATH=\".:$PATH\"`\n> > > (containing the test vi script)\n> > > - my normal path\n> > \n> > Maybe that is what is broken. t/test-lib.sh makes the environment \n> > variable point at the build directory, and that should override the \n> > path that is compiled in, shouldn't it?\n> \n> Maybe you prefer this patch then? \"make test\" survived up to 9101/25, \n> but that fails with the current master anyway and I didn't bother to run \n> the remaining tests manually, so it seems to be fine. Might break some \n> weird setups that rely on being able to set multiple additional paths \n> though (not that I think that that is a good idea to begin with).\n> \n> Bj?rn\n> ---\n> Instead of adding all possible exec paths to PATH, only add the best\n> one, following the same rules that --exec-path, without arguments, uses\n> to figure out which path to display.\n> \n> Signed-off-by: Bj?rn Steinbrink \n> ---\n\nFor easy application by the maintainer, please make the commit message the \nfirst part, then have a single \"---\", and then the quoted mail.\n\n> diff --git a/exec_cmd.c b/exec_cmd.c\n> index 2d0a758..9c376ad 100644\n> --- a/exec_cmd.c\n> +++ b/exec_cmd.c\n> @@ -48,9 +48,7 @@ void setup_path(const char *cmd_path)\n> \n> \tstrbuf_init(&new_path, 0);\n> \n> -\tadd_path(&new_path, argv_exec_path);\n> -\tadd_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));\n> -\tadd_path(&new_path, builtin_exec_path);\n> +\tadd_path(&new_path, git_exec_path());\n> \tadd_path(&new_path, cmd_path);\n\nI wonder why cmd_path is still there, then. (I'd have expected something \nlike\n\n\tadd_path(&new_path, cmd_path ? cmd_path : git_exec_path());\n\nIn related news, IMO cmd_path should be made absolute if it is not already \nthe case.\n\nCiao,\nDscho\n"},{"id":"59377","messageId":"fh7r84$vj$1@ger.gmane.org","threadId":"10791","inReplyTo":"Pine.LNX.4.64.0711112047170.4362@racer.site","subject":"Re: [PATCH] Use the best available exec path only","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-11-11T21:12:39Z","receivedAt":"2007-11-11T21:12:39Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Johannes Schindelin wrote:\n> On Sun, 11 Nov 2007, Björn Steinbrink wrote:\n>>\n>> ---\n>> Instead of adding all possible exec paths to PATH, only add the best\n>> one, following the same rules that --exec-path, without arguments, uses\n>> to figure out which path to display.\n>> \n>> Signed-off-by: Bj?rn Steinbrink \n>> ---\n> \n> For easy application by the maintainer, please make the commit message the \n> first part, then have a single \"---\", and then the quoted mail.\n\nThere are two conventions, for slightly different cases:\n\n1. Commit message, then diffstat / single \"---\", then comments,\n then patch\n\n2. Email (for example long discussion), then separator BUT NOT \"---\";\n use for example \"-- >8 --\" (cut here scissors), then commit message,\n then diffstat and patch, beginning with \"---\".\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"59379","messageId":"20071111211732.GA11871@atjola.homenet","threadId":"10791","inReplyTo":"Pine.LNX.4.64.0711112047170.4362@racer.site","subject":"Re: [PATCH] Use the best available exec path only","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-11-11T21:17:32Z","receivedAt":"2007-11-11T21:17:32Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2007.11.11 20:50:40 +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 11 Nov 2007, Bj?rn Steinbrink wrote:\n> \n> > On 2007.11.11 11:43:02 -0800, Junio C Hamano wrote:\n> > > Brian Gernhardt writes:\n> > > \n> > > > I'm sorry, I should have been more clear. I was referring to the \n> > > > GIT_EXEC_PATH build variable, not the environment variable. The git \n> > > > wrapper always adds the path determined during build to the front of \n> > > > PATH. When I was changing my build script, this got set to \"/usr/ \n> > > > local/bin\" (I usually use /usr/local/stow/git, instead). Since I \n> > > > have a /usr/local/bin/vim, PATH for git-commit.sh during the test \n> > > > was:\n> > > >\n> > > > - my git build directory\n> > > > - /usr/local/bin (containing a symlink vi -> vim)\n> > > > - the t/trash directory, added by the test via `PATH=\".:$PATH\"`\n> > > > (containing the test vi script)\n> > > > - my normal path\n> > > \n> > > Maybe that is what is broken. t/test-lib.sh makes the environment \n> > > variable point at the build directory, and that should override the \n> > > path that is compiled in, shouldn't it?\n> > \n> > Maybe you prefer this patch then? \"make test\" survived up to 9101/25, \n> > but that fails with the current master anyway and I didn't bother to run \n> > the remaining tests manually, so it seems to be fine. Might break some \n> > weird setups that rely on being able to set multiple additional paths \n> > though (not that I think that that is a good idea to begin with).\n> > \n> > Bj?rn\n> > ---\n> > Instead of adding all possible exec paths to PATH, only add the best\n> > one, following the same rules that --exec-path, without arguments, uses\n> > to figure out which path to display.\n> > \n> > Signed-off-by: Bj?rn Steinbrink \n> > ---\n> \n> For easy application by the maintainer, please make the commit message the \n> first part, then have a single \"---\", and then the quoted mail.\n\nOK.\n\n> > diff --git a/exec_cmd.c b/exec_cmd.c\n> > index 2d0a758..9c376ad 100644\n> > --- a/exec_cmd.c\n> > +++ b/exec_cmd.c\n> > @@ -48,9 +48,7 @@ void setup_path(const char *cmd_path)\n> > \n> > \tstrbuf_init(&new_path, 0);\n> > \n> > -\tadd_path(&new_path, argv_exec_path);\n> > -\tadd_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));\n> > -\tadd_path(&new_path, builtin_exec_path);\n> > +\tadd_path(&new_path, git_exec_path());\n> > \tadd_path(&new_path, cmd_path);\n> \n> I wonder why cmd_path is still there, then. (I'd have expected something \n> like\n> \n> \tadd_path(&new_path, cmd_path ? cmd_path : git_exec_path());\n\nWouldn't that break setups where only the \"git\" wrapper is in $PATH?\ncmd_path is just the path to the called executable (argv[0]). As I read\nthe code, it seems that cmd_path is just a worst case fallback, when the\n\"real\" exec path is messed up somehow. I figured that I'd better keep\nit, instead of just dropping it without knowing its intended purpose.\nBut if you're going to change anything about it, I'd guess that the only\noption is to drop it, or change its relative position in PATH.\n\n> In related news, IMO cmd_path should be made absolute if it is not already \n> the case.\n\nadd_path() already takes care of that.\n\nBjörn\n"},{"id":"59383","messageId":"7v4pfscvz6.fsf@gitster.siamese.dyndns.org","threadId":"10791","inReplyTo":"7v3avcefg9.fsf@gitster.siamese.dyndns.org","subject":"Re: t7005 and vi in GIT_EXEC_PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-11T21:29:01Z","receivedAt":"2007-11-11T21:29:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Brian Gernhardt writes:\n>\n>> I'm sorry, I should have been more clear. I was referring to the\n>> GIT_EXEC_PATH build variable, not the environment variable. The git\n>> wrapper always adds the path determined during build to the front of\n>> PATH. When I was changing my build script, this got set to \"/usr/\n>> local/bin\" (I usually use /usr/local/stow/git, instead). Since I have\n>> a /usr/local/bin/vim, PATH for git-commit.sh during the test was:\n>>\n>> - my git build directory\n>> - /usr/local/bin (containing a symlink vi -> vim)\n>> - the t/trash directory, added by the test via `PATH=\".:$PATH\"`\n>> (containing the test vi script)\n>> - my normal path\n>\n> Maybe that is what is broken. t/test-lib.sh makes the\n> environment variable point at the build directory, and that\n> should override the path that is compiled in, shouldn't it?\n\nAh, nevermind. I did not see the other messages in the thread.\n"},{"id":"59384","messageId":"Pine.LNX.4.64.0711112140110.4362@racer.site","threadId":"10791","inReplyTo":"20071111211732.GA11871@atjola.homenet","subject":"Re: [PATCH] Use the best available exec path only","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T21:40:47Z","receivedAt":"2007-11-11T21:40:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 11 Nov 2007, Bj?rn Steinbrink wrote:\n\n> > In related news, IMO cmd_path should be made absolute if it is not \n> > already the case.\n> \n> add_path() already takes care of that.\n\nYeah, and cmd_path is not used otherwise. Sorry for the noise.\n\nCiao,\nDscho\n"}]}