{"thread":{"id":"10390","subject":"Re: [PATCH] When exec'ing sub-commands, fall back on execvp (thePATH)","startedAt":"2007-10-20T08:12:17Z","lastAt":"2007-10-23T15:29:49Z","messageCount":17,"participants":["Scott R Parish","Johannes Schindelin","Scott Parish","Shawn O. Pearce","Andreas Ericsson","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"56679","messageId":"1192867937.v2.fusewebmail-240137@f","threadId":"10390","inReplyTo":null,"subject":"Re: [PATCH] When exec'ing sub-commands, fall back on execvp (thePATH)","fromName":"Scott R Parish","fromEmail":"srp@srparish.net","sentAt":"2007-10-20T08:12:17Z","receivedAt":"2007-10-20T08:12:17Z","isPatch":true,"sender":{"key":"srp@srparish.net","avatar":"https://gravatar.com/avatar/870e5b6fc4f710cf4db5684bd9af7f2cee5734b3dab3209b13e00cf64f6c9f0e?d=mp&s=160"},"body":"Yeah, that seems to work fine. The theoretical drawback to this approach\nis that it could possibly effect the order in which the paths are tried.\nFor instance, if a user did \"export GIT_EXEC_PATH=\", then the\nbuiltin_exec_path wouldn't be tried before the PATH. (i doubt that it\nwould be a problem, but thought i should note it)\n\nsRp\n\n\n----- Original Message -----\nSubject: Re: [PATCH] When exec'ing sub-commands, fall back on execvp\n(thePATH)\nDate: Sat, October 20, 2007 0:30\nFrom: \"Johannes Schindelin\" \n\n> Hi,\n>\n> On Fri, 19 Oct 2007, Scott Parish wrote:\n>\n> > diff --git a/exec_cmd.c b/exec_cmd.c\n> > index 9b74ed2..674c9f3 100644\n> > --- a/exec_cmd.c\n> > +++ b/exec_cmd.c\n> > @@ -34,15 +34,15 @@ int execv_git_cmd(const char **argv)\n> > {\n> > char git_command[PATH_MAX + 1];\n> > int i;\n> > + int rc;\n> > const char *paths[] = { current_exec_path,\n> > getenv(EXEC_PATH_ENVIRONMENT),\n> > builtin_exec_path };\n> > + const char *tmp;\n> > + size_t len;\n> >\n> > for (i = 0; i < ARRAY_SIZE(paths); ++i) {\n> > - size_t len;\n> > - int rc;\n> > const char *exec_dir = paths[i];\n> > - const char *tmp;\n> >\n> > if (!exec_dir || !*exec_dir) continue;\n> >\n> > @@ -106,8 +106,26 @@ int execv_git_cmd(const char **argv)\n> >\n> > argv[0] = tmp;\n> > }\n> > - return -1;\n> >\n> > + rc = snprintf(git_command, sizeof(git_command), \"git-%s\",\nargv[0]);\n> > + if (rc < 0 || rc >= sizeof(git_command) - len) {\n> > + fprintf(stderr, \"git: command name given is too\nlong.\\n\");\n> > + return -1;\n> > + }\n> > +\n> > + tmp = argv[0];\n> > + argv[0] = git_command;\n> > +\n> > + trace_argv_printf(argv, -1, \"trace: exec:\");\n> > +\n> > + /* execve() can only ever return if it fails */\n> > + execvp(git_command, (char **)argv);\n> > +\n> > + trace_printf(\"trace: exec failed: %s\\n\", strerror(errno));\n> > +\n> > + argv[0] = tmp;\n> > +\n> > + return -1;\n> > }\n>\n> I am not sure that this is elegant enough: Something like this (completely\n> untested) might be better:\n>\n> diff --git a/exec_cmd.c b/exec_cmd.c\n> index 9b74ed2..c928f37 100644\n> --- a/exec_cmd.c\n> +++ b/exec_cmd.c\n> @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n> int i;\n> const char *paths[] = { current_exec_path,\n> getenv(EXEC_PATH_ENVIRONMENT),\n> - builtin_exec_path };\n> + builtin_exec_path,\n> + \"\" };\n>\n> for (i = 0; i < ARRAY_SIZE(paths); ++i) {\n> size_t len;\n> @@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)\n> const char *exec_dir = paths[i];\n> const char *tmp;\n>\n> - if (!exec_dir || !*exec_dir) continue;\n> + if (!exec_dir) continue;\n>\n> - if (*exec_dir != '/') {\n> + if (!*exec_dir)\n> + /* try PATH */\n> + *git_command = '\\0';\n> + else if (*exec_dir != '/') {\n> if (!getcwd(git_command, sizeof(git_command))) {\n> fprintf(stderr, \"git: cannot determine \"\n> \"current directory: %s\\n\",\n> @@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)\n>\n> len = strlen(git_command);\n> rc = snprintf(git_command + len, sizeof(git_command) -\nlen,\n> - \"/git-%s\", argv[0]);\n> + \"%sgit-%s\", *exec_dir ? \"/\" : \"\", argv[0]);\n> if (rc < 0 || rc >= sizeof(git_command) - len) {\n> fprintf(stderr,\n> \"git: command name given is too long.\\n\");\n>\n> Ciao,\n> Dscho\n>\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n>\n>\n"},{"id":"56727","messageId":"Pine.LNX.4.64.0710202124050.25221@racer.site","threadId":"10390","inReplyTo":"1192867937.v2.fusewebmail-240137@f","subject":"Re: [PATCH] When exec'ing sub-commands, fall back on execvp (thePATH)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-20T20:25:20Z","receivedAt":"2007-10-20T20:25:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\n[please do not top post. Just delete everything you do not reply to, and \nput your answers below the text you are replying to. This spares others \nso much time.]\n\nOn Sat, 20 Oct 2007, Scott R Parish wrote:\n\n> The theoretical drawback to this approach is that it could possibly \n> effect the order in which the paths are tried. For instance, if a user \n> did \"export GIT_EXEC_PATH=\", then the builtin_exec_path wouldn't be \n> tried before the PATH. (i doubt that it would be a problem, but thought \n> i should note it)\n\nIn that respect, my code does not change anything from your code.\n\nCiao,\nDscho\n"},{"id":"56728","messageId":"20071020205721.GA16291@srparish.net","threadId":"10390","inReplyTo":"1192867937.v2.fusewebmail-240137@f","subject":"Re: [PATCH] When exec'ing sub-commands, fall back on execvp (thePATH)","fromName":"Scott Parish","fromEmail":"srp@srparish.net","sentAt":"2007-10-20T20:57:26Z","receivedAt":"2007-10-20T20:57:26Z","isPatch":true,"sender":{"key":"srp@srparish.net","avatar":"https://gravatar.com/avatar/870e5b6fc4f710cf4db5684bd9af7f2cee5734b3dab3209b13e00cf64f6c9f0e?d=mp&s=160"},"body":"Actually, i didn't test it right, execve() were using the files in\nmy cwd. In addition to you patch, you'd need to use execvp() instead\nof execve().\n\nsRp\n\nOn Sat, Oct 20, 2007 at 03:12:17AM -0500, Scott R Parish wrote:\n\n> Yeah, that seems to work fine. The theoretical drawback to this approach\n> is that it could possibly effect the order in which the paths are tried.\n> For instance, if a user did \"export GIT_EXEC_PATH=\", then the\n> builtin_exec_path wouldn't be tried before the PATH. (i doubt that it\n> would be a problem, but thought i should note it)\n> \n> sRp\n> \n> \n> ----- Original Message -----\n> Subject: Re: [PATCH] When exec'ing sub-commands, fall back on execvp\n> (thePATH)\n> Date: Sat, October 20, 2007 0:30\n> From: \"Johannes Schindelin\" \n> \n> > Hi,\n> >\n> > On Fri, 19 Oct 2007, Scott Parish wrote:\n> >\n> > > diff --git a/exec_cmd.c b/exec_cmd.c\n> > > index 9b74ed2..674c9f3 100644\n> > > --- a/exec_cmd.c\n> > > +++ b/exec_cmd.c\n> > > @@ -34,15 +34,15 @@ int execv_git_cmd(const char **argv)\n> > > {\n> > > char git_command[PATH_MAX + 1];\n> > > int i;\n> > > + int rc;\n> > > const char *paths[] = { current_exec_path,\n> > > getenv(EXEC_PATH_ENVIRONMENT),\n> > > builtin_exec_path };\n> > > + const char *tmp;\n> > > + size_t len;\n> > >\n> > > for (i = 0; i < ARRAY_SIZE(paths); ++i) {\n> > > - size_t len;\n> > > - int rc;\n> > > const char *exec_dir = paths[i];\n> > > - const char *tmp;\n> > >\n> > > if (!exec_dir || !*exec_dir) continue;\n> > >\n> > > @@ -106,8 +106,26 @@ int execv_git_cmd(const char **argv)\n> > >\n> > > argv[0] = tmp;\n> > > }\n> > > - return -1;\n> > >\n> > > + rc = snprintf(git_command, sizeof(git_command), \"git-%s\",\n> argv[0]);\n> > > + if (rc < 0 || rc >= sizeof(git_command) - len) {\n> > > + fprintf(stderr, \"git: command name given is too\n> long.\\n\");\n> > > + return -1;\n> > > + }\n> > > +\n> > > + tmp = argv[0];\n> > > + argv[0] = git_command;\n> > > +\n> > > + trace_argv_printf(argv, -1, \"trace: exec:\");\n> > > +\n> > > + /* execve() can only ever return if it fails */\n> > > + execvp(git_command, (char **)argv);\n> > > +\n> > > + trace_printf(\"trace: exec failed: %s\\n\", strerror(errno));\n> > > +\n> > > + argv[0] = tmp;\n> > > +\n> > > + return -1;\n> > > }\n> >\n> > I am not sure that this is elegant enough: Something like this (completely\n> > untested) might be better:\n> >\n> > diff --git a/exec_cmd.c b/exec_cmd.c\n> > index 9b74ed2..c928f37 100644\n> > --- a/exec_cmd.c\n> > +++ b/exec_cmd.c\n> > @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n> > int i;\n> > const char *paths[] = { current_exec_path,\n> > getenv(EXEC_PATH_ENVIRONMENT),\n> > - builtin_exec_path };\n> > + builtin_exec_path,\n> > + \"\" };\n> >\n> > for (i = 0; i < ARRAY_SIZE(paths); ++i) {\n> > size_t len;\n> > @@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)\n> > const char *exec_dir = paths[i];\n> > const char *tmp;\n> >\n> > - if (!exec_dir || !*exec_dir) continue;\n> > + if (!exec_dir) continue;\n> >\n> > - if (*exec_dir != '/') {\n> > + if (!*exec_dir)\n> > + /* try PATH */\n> > + *git_command = '\\0';\n> > + else if (*exec_dir != '/') {\n> > if (!getcwd(git_command, sizeof(git_command))) {\n> > fprintf(stderr, \"git: cannot determine \"\n> > \"current directory: %s\\n\",\n> > @@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)\n> >\n> > len = strlen(git_command);\n> > rc = snprintf(git_command + len, sizeof(git_command) -\n> len,\n> > - \"/git-%s\", argv[0]);\n> > + \"%sgit-%s\", *exec_dir ? \"/\" : \"\", argv[0]);\n> > if (rc < 0 || rc >= sizeof(git_command) - len) {\n> > fprintf(stderr,\n> > \"git: command name given is too long.\\n\");\n> >\n> > Ciao,\n> > Dscho\n> >\n> > -\n> > To unsubscribe from this list: send the line \"unsubscribe git\" in\n> > the body of a message to majordomo@vger.kernel.org\n> > More majordomo info at http://vger.kernel.org/majordomo-info.html\n> >\n> >\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n-- \nScott Parish\nhttp://srparish.net/\n"},{"id":"56731","messageId":"Pine.LNX.4.64.0710202258440.25221@racer.site","threadId":"10390","inReplyTo":"20071020205721.GA16291@srparish.net","subject":"[PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-20T22:00:25Z","receivedAt":"2007-10-20T22:00:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nEarlier, we tried to find the git commands in several possible exec\ndirs. Now, if all of these failed, try to find the git command in\nPATH.\n\nThis allows you to install the git programs somewhere else than\noriginally specified when building git, as long as you add that location\nto the PATH.\n\nInitial implementation by Scott R Parish.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Sat, 20 Oct 2007, Scott Parish wrote:\n\n\t> Actually, i didn't test it right, execve() were using the files \n\t> in my cwd. In addition to you patch, you'd need to use execvp() \n\t> instead of execve().\n\n\tAh, right. I missed that one ;-)\n\n\tHow about this instead?\n\n exec_cmd.c | 12 ++++++++----\n 1 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex 9b74ed2..c928f37 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n \tint i;\n \tconst char *paths[] = { current_exec_path,\n \t\t\t\tgetenv(EXEC_PATH_ENVIRONMENT),\n-\t\t\t\tbuiltin_exec_path };\n+\t\t\t\tbuiltin_exec_path,\n+\t\t\t\t\"\" };\n \n \tfor (i = 0; i < ARRAY_SIZE(paths); ++i) {\n \t\tsize_t len;\n@@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)\n \t\tconst char *exec_dir = paths[i];\n \t\tconst char *tmp;\n \n-\t\tif (!exec_dir || !*exec_dir) continue;\n+\t\tif (!exec_dir) continue;\n \n-\t\tif (*exec_dir != '/') {\n+\t\tif (!*exec_dir)\n+\t\t\t/* try PATH */\n+\t\t\t*git_command = '\\0';\n+\t\telse if (*exec_dir != '/') {\n \t\t\tif (!getcwd(git_command, sizeof(git_command))) {\n \t\t\t\tfprintf(stderr, \"git: cannot determine \"\n \t\t\t\t\t\"current directory: %s\\n\",\n@@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)\n \n \t\tlen = strlen(git_command);\n \t\trc = snprintf(git_command + len, sizeof(git_command) - len,\n-\t\t\t \"/git-%s\", argv[0]);\n+\t\t\t \"%sgit-%s\", *exec_dir ? \"/\" : \"\", argv[0]);\n \t\tif (rc < 0 || rc >= sizeof(git_command) - len) {\n \t\t\tfprintf(stderr,\n \t\t\t\t\"git: command name given is too long.\\n\");\n-- \n1.5.3.4.1287.g8b31e\n"},{"id":"56735","messageId":"20071021023614.GB14735@spearce.org","threadId":"10390","inReplyTo":"Pine.LNX.4.64.0710202258440.25221@racer.site","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-21T02:36:14Z","receivedAt":"2007-10-21T02:36:14Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> \tOn Sat, 20 Oct 2007, Scott Parish wrote:\n> \n> \t> Actually, i didn't test it right, execve() were using the files \n> \t> in my cwd. In addition to you patch, you'd need to use execvp() \n> \t> instead of execve().\n> \n> \tAh, right. I missed that one ;-)\n> \n> \tHow about this instead?\n\nUhhh. Its the same, isn't it? Still using execve() which means\nwe will not look at PATH in the final attempt.\n \n> exec_cmd.c | 12 ++++++++----\n> 1 files changed, 8 insertions(+), 4 deletions(-)\n> \n> diff --git a/exec_cmd.c b/exec_cmd.c\n> index 9b74ed2..c928f37 100644\n> --- a/exec_cmd.c\n> +++ b/exec_cmd.c\n> @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n> \tint i;\n> \tconst char *paths[] = { current_exec_path,\n> \t\t\t\tgetenv(EXEC_PATH_ENVIRONMENT),\n> -\t\t\t\tbuiltin_exec_path };\n> +\t\t\t\tbuiltin_exec_path,\n> +\t\t\t\t\"\" };\n> \n> \tfor (i = 0; i < ARRAY_SIZE(paths); ++i) {\n> \t\tsize_t len;\n> @@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)\n> \t\tconst char *exec_dir = paths[i];\n> \t\tconst char *tmp;\n> \n> -\t\tif (!exec_dir || !*exec_dir) continue;\n> +\t\tif (!exec_dir) continue;\n> \n> -\t\tif (*exec_dir != '/') {\n> +\t\tif (!*exec_dir)\n> +\t\t\t/* try PATH */\n> +\t\t\t*git_command = '\\0';\n> +\t\telse if (*exec_dir != '/') {\n> \t\t\tif (!getcwd(git_command, sizeof(git_command))) {\n> \t\t\t\tfprintf(stderr, \"git: cannot determine \"\n> \t\t\t\t\t\"current directory: %s\\n\",\n> @@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)\n> \n> \t\tlen = strlen(git_command);\n> \t\trc = snprintf(git_command + len, sizeof(git_command) - len,\n> -\t\t\t \"/git-%s\", argv[0]);\n> +\t\t\t \"%sgit-%s\", *exec_dir ? \"/\" : \"\", argv[0]);\n> \t\tif (rc < 0 || rc >= sizeof(git_command) - len) {\n> \t\t\tfprintf(stderr,\n> \t\t\t\t\"git: command name given is too long.\\n\");\n> -- \n> 1.5.3.4.1287.g8b31e\n\n-- \nShawn.\n"},{"id":"56776","messageId":"20071021182150.GG16291@srparish.net","threadId":"10390","inReplyTo":"Pine.LNX.4.64.0710202258440.25221@racer.site","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Scott Parish","fromEmail":"srp@srparish.net","sentAt":"2007-10-21T18:21:50Z","receivedAt":"2007-10-21T18:21:50Z","isPatch":true,"sender":{"key":"srp@srparish.net","avatar":"https://gravatar.com/avatar/870e5b6fc4f710cf4db5684bd9af7f2cee5734b3dab3209b13e00cf64f6c9f0e?d=mp&s=160"},"body":"Earlier, we tried to find the git commands in several possible exec\ndirs. Now, if all of these failed, try to find the git command in\nPATH.\n\nThis allows you to install the git programs somewhere else than\noriginally specified when building git, as long as you add that location\nto the PATH.\n\nImplementation by Johannes Schindelin\n\nSigned-off-by: Scott R Parish \n---\n exec_cmd.c | 16 ++++++++++------\n 1 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex 9b74ed2..374ffc9 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n \tint i;\n \tconst char *paths[] = { current_exec_path,\n \t\t\t\tgetenv(EXEC_PATH_ENVIRONMENT),\n-\t\t\t\tbuiltin_exec_path };\n+\t\t\t\tbuiltin_exec_path,\n+\t\t\t\t\"\" };\n \n \tfor (i = 0; i < ARRAY_SIZE(paths); ++i) {\n \t\tsize_t len;\n@@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)\n \t\tconst char *exec_dir = paths[i];\n \t\tconst char *tmp;\n \n-\t\tif (!exec_dir || !*exec_dir) continue;\n+\t\tif (!exec_dir) continue;\n \n-\t\tif (*exec_dir != '/') {\n+\t\tif (!*exec_dir)\n+\t\t\t/* try PATH */\n+\t\t\t*git_command = '\\0';\n+\t\telse if (*exec_dir != '/') {\n \t\t\tif (!getcwd(git_command, sizeof(git_command))) {\n \t\t\t\tfprintf(stderr, \"git: cannot determine \"\n \t\t\t\t\t\"current directory: %s\\n\",\n@@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)\n \n \t\tlen = strlen(git_command);\n \t\trc = snprintf(git_command + len, sizeof(git_command) - len,\n-\t\t\t \"/git-%s\", argv[0]);\n+\t\t\t \"%sgit-%s\", *exec_dir ? \"/\" : \"\", argv[0]);\n \t\tif (rc < 0 || rc >= sizeof(git_command) - len) {\n \t\t\tfprintf(stderr,\n \t\t\t\t\"git: command name given is too long.\\n\");\n@@ -99,8 +103,8 @@ int execv_git_cmd(const char **argv)\n \n \t\ttrace_argv_printf(argv, -1, \"trace: exec:\");\n \n-\t\t/* execve() can only ever return if it fails */\n-\t\texecve(git_command, (char **)argv, environ);\n+\t\t/* execvp() can only ever return if it fails */\n+\t\texecvp(git_command, (char **)argv);\n \n \t\ttrace_printf(\"trace: exec failed: %s\\n\", strerror(errno));\n \n-- \n1.5.3.4.206.g58ba4-dirty\n"},{"id":"56782","messageId":"Pine.LNX.4.64.0710212256270.25221@racer.site","threadId":"10390","inReplyTo":"20071021023614.GB14735@spearce.org","subject":"[PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-21T21:59:01Z","receivedAt":"2007-10-21T21:59:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nEarlier, we tried to find the git commands in several possible exec\ndirs. Now, if all of these failed, try to find the git command in\nPATH.\n\nThis allows you to install the git programs somewhere else than\noriginally specified when building git, as long as you add that location\nto the PATH.\n\nInitial implementation by Scott R Parish.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Sat, 20 Oct 2007, Shawn O. Pearce wrote:\n\n\t> Johannes Schindelin wrote:\n\t> > On Sat, 20 Oct 2007, Scott Parish wrote:\n\t> > \n\t> > > Actually, i didn't test it right, execve() were using the \n\t> > > files in my cwd. In addition to you patch, you'd need to use \n\t> > > execvp() instead of execve().\n\t> > \n\t> > Ah, right. I missed that one ;-)\n\t> > \n\t> > How about this instead?\n\t> \n\t> Uhhh. Its the same, isn't it? Still using execve() which means \n\t> we will not look at PATH in the final attempt.\n\n\tSorry. I forgot to --amend.\n\n exec_cmd.c | 17 ++++++++++++-----\n 1 files changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex 9b74ed2..70b84b0 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n \tint i;\n \tconst char *paths[] = { current_exec_path,\n \t\t\t\tgetenv(EXEC_PATH_ENVIRONMENT),\n-\t\t\t\tbuiltin_exec_path };\n+\t\t\t\tbuiltin_exec_path,\n+\t\t\t\t\"\" };\n \n \tfor (i = 0; i < ARRAY_SIZE(paths); ++i) {\n \t\tsize_t len;\n@@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)\n \t\tconst char *exec_dir = paths[i];\n \t\tconst char *tmp;\n \n-\t\tif (!exec_dir || !*exec_dir) continue;\n+\t\tif (!exec_dir) continue;\n \n-\t\tif (*exec_dir != '/') {\n+\t\tif (!*exec_dir)\n+\t\t\t/* try PATH */\n+\t\t\t*git_command = '\\0';\n+\t\telse if (*exec_dir != '/') {\n \t\t\tif (!getcwd(git_command, sizeof(git_command))) {\n \t\t\t\tfprintf(stderr, \"git: cannot determine \"\n \t\t\t\t\t\"current directory: %s\\n\",\n@@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)\n \n \t\tlen = strlen(git_command);\n \t\trc = snprintf(git_command + len, sizeof(git_command) - len,\n-\t\t\t \"/git-%s\", argv[0]);\n+\t\t\t \"%sgit-%s\", *exec_dir ? \"/\" : \"\", argv[0]);\n \t\tif (rc < 0 || rc >= sizeof(git_command) - len) {\n \t\t\tfprintf(stderr,\n \t\t\t\t\"git: command name given is too long.\\n\");\n@@ -100,7 +104,10 @@ int execv_git_cmd(const char **argv)\n \t\ttrace_argv_printf(argv, -1, \"trace: exec:\");\n \n \t\t/* execve() can only ever return if it fails */\n-\t\texecve(git_command, (char **)argv, environ);\n+\t\tif (*exec_dir)\n+\t\t\texecve(git_command, (char **)argv, environ);\n+\t\telse\n+\t\t\texecvp(git_command, (char **)argv);\n \n \t\ttrace_printf(\"trace: exec failed: %s\\n\", strerror(errno));\n \n-- \n1.5.3.4.1289.ga1432\n"},{"id":"56783","messageId":"Pine.LNX.4.64.0710212301490.25221@racer.site","threadId":"10390","inReplyTo":"20071021182150.GG16291@srparish.net","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-21T22:02:46Z","receivedAt":"2007-10-21T22:02:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 21 Oct 2007, Scott Parish wrote:\n\n> diff --git a/exec_cmd.c b/exec_cmd.c\n> index 9b74ed2..374ffc9 100644\n> --- a/exec_cmd.c\n> +++ b/exec_cmd.c\n> @@ -99,8 +103,8 @@ int execv_git_cmd(const char **argv)\n> \n> \t\ttrace_argv_printf(argv, -1, \"trace: exec:\");\n> \n> -\t\t/* execve() can only ever return if it fails */\n> -\t\texecve(git_command, (char **)argv, environ);\n> +\t\t/* execvp() can only ever return if it fails */\n> +\t\texecvp(git_command, (char **)argv);\n\nI'd rather have it conditional upon *exec_dir (see the brown paper bag \npatch I just sent).\n\nCiao,\nDscho\n"},{"id":"56798","messageId":"20071022042110.GJ14735@spearce.org","threadId":"10390","inReplyTo":"Pine.LNX.4.64.0710212256270.25221@racer.site","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-22T04:21:10Z","receivedAt":"2007-10-22T04:21:10Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> Earlier, we tried to find the git commands in several possible exec\n> dirs. Now, if all of these failed, try to find the git command in\n> PATH.\n...\n> diff --git a/exec_cmd.c b/exec_cmd.c\n> index 9b74ed2..70b84b0 100644\n> --- a/exec_cmd.c\n> +++ b/exec_cmd.c\n> @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n> \tint i;\n> \tconst char *paths[] = { current_exec_path,\n> \t\t\t\tgetenv(EXEC_PATH_ENVIRONMENT),\n> -\t\t\t\tbuiltin_exec_path };\n> +\t\t\t\tbuiltin_exec_path,\n> +\t\t\t\t\"\" };\n\nSo if the user sets GIT_EXEC_PATH=\"\" and exports it we'll search\n$PATH before the builtin exec path that Git was compiled with?\nAre we sure we want to do that?\n\nI'm going to throw this into pu tonight just so I don't lose it,\nbut I have a feeling we want to amend it before merging.\n\n-- \nShawn.\n"},{"id":"56852","messageId":"Pine.LNX.4.64.0710221135100.25221@racer.site","threadId":"10390","inReplyTo":"20071022042110.GJ14735@spearce.org","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-22T10:35:48Z","receivedAt":"2007-10-22T10:35:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 22 Oct 2007, Shawn O. Pearce wrote:\n\n> Johannes Schindelin wrote:\n> > Earlier, we tried to find the git commands in several possible exec\n> > dirs. Now, if all of these failed, try to find the git command in\n> > PATH.\n> ...\n> > diff --git a/exec_cmd.c b/exec_cmd.c\n> > index 9b74ed2..70b84b0 100644\n> > --- a/exec_cmd.c\n> > +++ b/exec_cmd.c\n> > @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n> > \tint i;\n> > \tconst char *paths[] = { current_exec_path,\n> > \t\t\t\tgetenv(EXEC_PATH_ENVIRONMENT),\n> > -\t\t\t\tbuiltin_exec_path };\n> > +\t\t\t\tbuiltin_exec_path,\n> > +\t\t\t\t\"\" };\n> \n> So if the user sets GIT_EXEC_PATH=\"\" and exports it we'll search $PATH \n> before the builtin exec path that Git was compiled with? Are we sure we \n> want to do that?\n\nI thought the proper way to unset EXEC_PATH was to \"unset GIT_EXEC_PATH\". \nIn that case, getenv(EXEC_PATH_ENVIRONMENT) returns NULL and we're fine, \nno?\n\nCiao,\nDscho\n"},{"id":"56884","messageId":"20071022143637.GP16291@srparish.net","threadId":"10390","inReplyTo":"Pine.LNX.4.64.0710212256270.25221@racer.site","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Scott Parish","fromEmail":"srp@srparish.net","sentAt":"2007-10-22T14:36:37Z","receivedAt":"2007-10-22T14:36:37Z","isPatch":true,"sender":{"key":"srp@srparish.net","avatar":"https://gravatar.com/avatar/870e5b6fc4f710cf4db5684bd9af7f2cee5734b3dab3209b13e00cf64f6c9f0e?d=mp&s=160"},"body":"On Sun, Oct 21, 2007 at 10:59:01PM +0100, Johannes Schindelin wrote:\n\n> Earlier, we tried to find the git commands in several possible exec\n> dirs. Now, if all of these failed, try to find the git command in\n> PATH.\n\nI'm tempted to try a different approach. What if instead of looping\nand building up strings of all the different absolute paths we want\nto try we just prepend to PATH with the correct extra precedence,\nand then call execvp on the command we want?\n\nsRp\n\n-- \nScott Parish\nhttp://srparish.net/\n"},{"id":"56889","messageId":"471CBF88.6020300@op5.se","threadId":"10390","inReplyTo":"20071022143637.GP16291@srparish.net","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-10-22T15:19:36Z","receivedAt":"2007-10-22T15:19:36Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Scott Parish wrote:\n> On Sun, Oct 21, 2007 at 10:59:01PM +0100, Johannes Schindelin wrote:\n> \n>> Earlier, we tried to find the git commands in several possible exec\n>> dirs. Now, if all of these failed, try to find the git command in\n>> PATH.\n> \n> I'm tempted to try a different approach. What if instead of looping\n> and building up strings of all the different absolute paths we want\n> to try we just prepend to PATH with the correct extra precedence,\n> and then call execvp on the command we want?\n> \n\nThat's how the original git --exec-dir feature got implemented. There's \neven a nifty function for it in git.c; prepend_to_path(). It's a \nprovably workable solution.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"56892","messageId":"471CC380.5030603@viscovery.net","threadId":"10390","inReplyTo":"471CBF88.6020300@op5.se","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-10-22T15:36:32Z","receivedAt":"2007-10-22T15:36:32Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andreas Ericsson schrieb:\n> Scott Parish wrote:\n>> I'm tempted to try a different approach. What if instead of looping\n>> and building up strings of all the different absolute paths we want\n>> to try we just prepend to PATH with the correct extra precedence,\n>> and then call execvp on the command we want?\n>>\n> \n> That's how the original git --exec-dir feature got implemented. There's \n> even a nifty function for it in git.c; prepend_to_path(). It's a \n> provably workable solution.\n\nThe reason that this was done is for the sake of shell scripts: They need to \nhave the path that was finally decided as exec-path in $PATH.\n\nBut I can't think of any negative side effect if *all* exec-path candidates \nare in $PATH. It's important, though, that all paths are absolute because \nthe tools chdir every now and then.\n\n-- Hannes\n"},{"id":"56956","messageId":"20071023043405.GA14735@spearce.org","threadId":"10390","inReplyTo":"Pine.LNX.4.64.0710221135100.25221@racer.site","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-23T04:34:05Z","receivedAt":"2007-10-23T04:34:05Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> On Mon, 22 Oct 2007, Shawn O. Pearce wrote:\n> > Johannes Schindelin wrote:\n> > > Earlier, we tried to find the git commands in several possible exec\n> > > dirs. Now, if all of these failed, try to find the git command in\n> > > PATH.\n> > ...\n> > > diff --git a/exec_cmd.c b/exec_cmd.c\n> > > index 9b74ed2..70b84b0 100644\n> > > --- a/exec_cmd.c\n> > > +++ b/exec_cmd.c\n> > > @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n> > > \tint i;\n> > > \tconst char *paths[] = { current_exec_path,\n> > > \t\t\t\tgetenv(EXEC_PATH_ENVIRONMENT),\n> > > -\t\t\t\tbuiltin_exec_path };\n> > > +\t\t\t\tbuiltin_exec_path,\n> > > +\t\t\t\t\"\" };\n> > \n> > So if the user sets GIT_EXEC_PATH=\"\" and exports it we'll search $PATH \n> > before the builtin exec path that Git was compiled with? Are we sure we \n> > want to do that?\n> \n> I thought the proper way to unset EXEC_PATH was to \"unset GIT_EXEC_PATH\". \n> In that case, getenv(EXEC_PATH_ENVIRONMENT) returns NULL and we're fine, \n> no?\n\nSure. But can't you also export an environment variable that is\nset to the empty string? At least on UNIX. Windows thinks unset\nand empty string are the same thing.\n\n-- \nShawn.\n"},{"id":"56998","messageId":"471DD703.70608@op5.se","threadId":"10390","inReplyTo":"471CC380.5030603@viscovery.net","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-10-23T11:12:03Z","receivedAt":"2007-10-23T11:12:03Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Johannes Sixt wrote:\n> Andreas Ericsson schrieb:\n>> Scott Parish wrote:\n>>> I'm tempted to try a different approach. What if instead of looping\n>>> and building up strings of all the different absolute paths we want\n>>> to try we just prepend to PATH with the correct extra precedence,\n>>> and then call execvp on the command we want?\n>>>\n>>\n>> That's how the original git --exec-dir feature got implemented. \n>> There's even a nifty function for it in git.c; prepend_to_path(). It's \n>> a provably workable solution.\n> \n> The reason that this was done is for the sake of shell scripts: They \n> need to have the path that was finally decided as exec-path in $PATH.\n> \n> But I can't think of any negative side effect if *all* exec-path \n> candidates are in $PATH. It's important, though, that all paths are \n> absolute because the tools chdir every now and then.\n> \n\nSo long as they're added in \"success:failed:failed\" order, I don't see\nany issues either. Assuming we stop prepending once we find something\nthat works, that should be a non-issue.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"56995","messageId":"Pine.LNX.4.64.0710231209240.25221@racer.site","threadId":"10390","inReplyTo":"20071023043405.GA14735@spearce.org","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-23T11:12:25Z","receivedAt":"2007-10-23T11:12:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 23 Oct 2007, Shawn O. Pearce wrote:\n\n> Johannes Schindelin wrote:\n> > On Mon, 22 Oct 2007, Shawn O. Pearce wrote:\n> > > Johannes Schindelin wrote:\n> > > > Earlier, we tried to find the git commands in several possible exec\n> > > > dirs. Now, if all of these failed, try to find the git command in\n> > > > PATH.\n> > > ...\n> > > > diff --git a/exec_cmd.c b/exec_cmd.c\n> > > > index 9b74ed2..70b84b0 100644\n> > > > --- a/exec_cmd.c\n> > > > +++ b/exec_cmd.c\n> > > > @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)\n> > > > \tint i;\n> > > > \tconst char *paths[] = { current_exec_path,\n> > > > \t\t\t\tgetenv(EXEC_PATH_ENVIRONMENT),\n> > > > -\t\t\t\tbuiltin_exec_path };\n> > > > +\t\t\t\tbuiltin_exec_path,\n> > > > +\t\t\t\t\"\" };\n> > > \n> > > So if the user sets GIT_EXEC_PATH=\"\" and exports it we'll search \n> > > $PATH before the builtin exec path that Git was compiled with? Are \n> > > we sure we want to do that?\n> > \n> > I thought the proper way to unset EXEC_PATH was to \"unset \n> > GIT_EXEC_PATH\". In that case, getenv(EXEC_PATH_ENVIRONMENT) returns \n> > NULL and we're fine, no?\n> \n> Sure. But can't you also export an environment variable that is set to \n> the empty string? At least on UNIX. Windows thinks unset and empty \n> string are the same thing.\n\nNot here. I just tried (with msysGit, of course).\n\nAnyway, I like the other patch Scott sent much more than mine, which \noffloads the work to execvp().\n\nCiao,\nDscho\n"},{"id":"56999","messageId":"471E136D.4010905@viscovery.net","threadId":"10390","inReplyTo":"471DD703.70608@op5.se","subject":"Re: [PATCH] execv_git_cmd(): also try PATH if everything else fails.","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-10-23T15:29:49Z","receivedAt":"2007-10-23T15:29:49Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andreas Ericsson schrieb:\n> Johannes Sixt wrote:\n>> But I can't think of any negative side effect if *all* exec-path \n>> candidates are in $PATH. It's important, though, that all paths are \n>> absolute because the tools chdir every now and then.\n>>\n> \n> So long as they're added in \"success:failed:failed\" order, I don't see\n> any issues either. Assuming we stop prepending once we find something\n> that works, that should be a non-issue.\n\nNo, the point is exactly to let execvp() do all the work and we don't care \nwhich of the paths is the \"success\". And I don't think that this has any \nnegative side effects.\n\n-- Hannes\n"}]}