{"thread":{"id":"14757","subject":"[PATCH] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack.","startedAt":"2008-07-30T11:27:18Z","lastAt":"2008-07-30T18:53:32Z","messageCount":5,"participants":["Steve Haslam","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"85598","messageId":"1217417238-26731-1-git-send-email-shaslam@lastminute.com","threadId":"14757","inReplyTo":null,"subject":"[PATCH] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack.","fromName":"Steve Haslam","fromEmail":"shaslam@lastminute.com","sentAt":"2008-07-30T11:27:18Z","receivedAt":"2008-07-30T11:27:18Z","isPatch":true,"sender":{"key":"shaslam@lastminute.com","avatar":"https://gravatar.com/avatar/df44c553ba2fa56f01414f0c0873212d6f8506f3426586238271bac83fde2edf?d=mp&s=160"},"body":"If the user specified the full path to git-upload-pack as the -u option to\n\"git clone\" when cloning a remote repository, and git was not on the default\nPATH on the remote machine, git-upload-pack was failing to exec\ngit-pack-objects.\n\nBy making the argv[0] path (if any) available to setup_path(), this will\nallow finding the \"git\" executable in the same directory as\n\"git-upload-pack\". The default built in to exec_cmd.c is to look for \"git\"\nin the \".../libexec/git-core\" directory, but it is not installed there (any\nlonger).\n\nMuch the same applies to invoking git-receive-pack from a non-PATH location\nusing the \"--exec\" argument to \"git push\". Since both commands require the\nsame treatment, a single function has been added in exec_cmd.c to serve both.\n\nSigned-off-by: Steve Haslam <shaslam@lastminute.com>\n---\n exec_cmd.c     |   15 +++++++++++++--\n exec_cmd.h     |    2 +-\n git.c          |   20 +++++---------------\n receive-pack.c |    3 +++\n upload-pack.c  |    3 +++\n 5 files changed, 25 insertions(+), 18 deletions(-)\n\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex ce6741e..a2a6a0d 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -17,9 +17,20 @@ const char *system_path(const char *path)\n \treturn path;\n }\n \n-void git_set_argv0_path(const char *path)\n+const char *git_extract_argv0_path(const char *argv0)\n {\n-\targv0_path = path;\n+\tconst char *slash = argv0 + strlen(argv0);\n+\n+\tdo\n+\t\t--slash;\n+\twhile (slash >= argv0 && !is_dir_sep(*slash));\n+\n+\tif (slash >= argv0) {\n+\t\targv0_path = xstrndup(argv0, slash - argv0);\n+\t\treturn slash + 1;\n+\t}\n+\n+\treturn argv0;\n }\n \n void git_set_argv_exec_path(const char *exec_path)\ndiff --git a/exec_cmd.h b/exec_cmd.h\nindex 594f961..392e903 100644\n--- a/exec_cmd.h\n+++ b/exec_cmd.h\n@@ -2,7 +2,7 @@\n #define GIT_EXEC_CMD_H\n \n extern void git_set_argv_exec_path(const char *exec_path);\n-extern void git_set_argv0_path(const char *path);\n+extern const char* git_extract_argv0_path(const char *path);\n extern const char* git_exec_path(void);\n extern void setup_path(void);\n extern const char **prepare_git_cmd(const char **argv);\ndiff --git a/git.c b/git.c\nindex 37b1d76..b1eff4a 100644\n--- a/git.c\n+++ b/git.c\n@@ -416,23 +416,13 @@ static void execv_dashed_external(const char **argv)\n \n int main(int argc, const char **argv)\n {\n-\tconst char *cmd = argv[0] && *argv[0] ? argv[0] : \"git-help\";\n-\tchar *slash = (char *)cmd + strlen(cmd);\n+\tconst char *cmd;\n \tint done_alias = 0;\n \n-\t/*\n-\t * Take the basename of argv[0] as the command\n-\t * name, and the dirname as the default exec_path\n-\t * if we don't have anything better.\n-\t */\n-\tdo\n-\t\t--slash;\n-\twhile (cmd <= slash && !is_dir_sep(*slash));\n-\tif (cmd <= slash) {\n-\t\t*slash++ = 0;\n-\t\tgit_set_argv0_path(cmd);\n-\t\tcmd = slash;\n-\t}\n+\tif (argv[0] && *argv[0])\n+\t\tcmd = git_extract_argv0_path(argv[0]);\n+\telse\n+\t\tcmd = \"git-help\";\n \n \t/*\n \t * \"git-xxxx\" is the same as \"git xxxx\", but we obviously:\ndiff --git a/receive-pack.c b/receive-pack.c\nindex d44c19e..3699b16 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -467,6 +467,9 @@ int main(int argc, char **argv)\n \tint i;\n \tchar *dir = NULL;\n \n+\tif (argv[0] && *argv[0])\n+\t\tgit_extract_argv0_path(argv[0]);\n+\n \targv++;\n \tfor (i = 1; i < argc; i++) {\n \t\tchar *arg = *argv++;\ndiff --git a/upload-pack.c b/upload-pack.c\nindex c911e70..086eff6 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -616,6 +616,9 @@ int main(int argc, char **argv)\n \tint i;\n \tint strict = 0;\n \n+\tif (argv[0] && *argv[0])\n+\t\tgit_extract_argv0_path(argv[0]);\n+\n \tfor (i = 1; i < argc; i++) {\n \t\tchar *arg = argv[i];\n \n-- \n1.5.6.3\n"},{"id":"85632","messageId":"alpine.LSU.1.00.0807301650060.3486@wbgn129.biozentrum.uni-wuerzburg.de","threadId":"14757","inReplyTo":"1217417238-26731-1-git-send-email-shaslam@lastminute.com","subject":"Re: [PATCH] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-30T14:56:49Z","receivedAt":"2008-07-30T14:56:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\n> diff --git a/upload-pack.c b/upload-pack.c\n> index c911e70..086eff6 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -616,6 +616,9 @@ int main(int argc, char **argv)\n>  \tint i;\n>  \tint strict = 0;\n>  \n> +\tif (argv[0] && *argv[0])\n> +\t\tgit_extract_argv0_path(argv[0]);\n> +\n\nThis is ugly.  The called function should already do it itself.\n\nFurther, why not go the full nine yards and avoid the calculation \naltogether, until it is necessary?  Then the change to add \nlookup_program_in_path() would be nice and non-intrusive.\n\nIOW why not leave the function name as-is, and just enhance system_path() \nto have a static variable \"initialized\", which does the whole calculation? \nI.e. move the calculation from git.c to exec_cmd.c, but at the same time \ndo it only when needed.\n\nAnd your change to set argv0_path from receive-pack and upload-pack would \nbe a second patch.\n\nAnd then the patch to add support to \"glean\" (did not know that word) the \npath from the PATH (lookup_program_in_path()) could come as a third patch.\n\nCiao,\nDscho\n"},{"id":"85647","messageId":"48909396.3080500@lastminute.com","threadId":"14757","inReplyTo":"alpine.LSU.1.00.0807301650060.3486@wbgn129.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack.","fromName":"Steve Haslam","fromEmail":"shaslam@lastminute.com","sentAt":"2008-07-30T16:15:18Z","receivedAt":"2008-07-30T16:15:18Z","isPatch":true,"sender":{"key":"shaslam@lastminute.com","avatar":"https://gravatar.com/avatar/df44c553ba2fa56f01414f0c0873212d6f8506f3426586238271bac83fde2edf?d=mp&s=160"},"body":"[sorry, was intended to cc Johannes Sixt when I mailed the patch to the \nlist... not doing well with the recipient etiquette]\n\n[and then Icedove decided it was going to send mail as HTML. This is so embarassing.]\n\nJohannes Schindelin wrote:\n>> diff --git a/upload-pack.c b/upload-pack.c\n>> index c911e70..086eff6 100644\n>> --- a/upload-pack.c\n>> +++ b/upload-pack.c\n>> @@ -616,6 +616,9 @@ int main(int argc, char **argv)\n>>  \tint i;\n>>  \tint strict = 0;\n>>  \n>> +\tif (argv[0] && *argv[0])\n>> +\t\tgit_extract_argv0_path(argv[0]);\n>> +\n>>     \n>\n> This is ugly.  The called function should already do it itself.\n>   \n\nFair enough.\n\n> Further, why not go the full nine yards and avoid the calculation \n> altogether, until it is necessary?  Then the change to add \n> lookup_program_in_path() would be nice and non-intrusive.\n\ngit.c will always need to do the calculation, to determine which command \nit is being invoked as, so is there much value in delaying until necessary?\n\nIf the code in git.c is left alone, then it needs to be eventually \nduplicated in upload-pack.c and receive-pack.c, or in exec_cmd.c. [I \nbotched when sending out the patch originally and only sent it to \nJohannes and not the list, so it's my fault that the history of how this \nhas evolved is unclear I'm afraid]\n\n> IOW why not leave the function name as-is, and just enhance system_path() \n> to have a static variable \"initialized\", which does the whole calculation? \n> I.e. move the calculation from git.c to exec_cmd.c, but at the same time \n> do it only when needed.\n\nHmm, system_path and setup_path both use argv0_path; git.c would need to \ncall an additional function in exec_cmd.c to get the \"leafname\" result \nof the calculation, though.\n\n> And your change to set argv0_path from receive-pack and upload-pack would \n> be a second patch.\n\nOK.\n\n> And then the patch to add support to \"glean\" (did not know that word) the \n> path from the PATH (lookup_program_in_path()) could come as a third patch.\n\nI think that once git-upload-pack.c et al get the argv[0] path over to \nsetup_path() then there's nothing more to do; setup_path() already uses \nargv0_path in its list of paths to try. I'm confused to the reference to \nPATH, though: we're avoiding the PATH environment variable completely.\n\nSRH\n"},{"id":"85658","messageId":"alpine.LSU.1.00.0807302020300.3486@wbgn129.biozentrum.uni-wuerzburg.de","threadId":"14757","inReplyTo":"48909396.3080500@lastminute.com","subject":"Re: [PATCH] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-30T18:21:47Z","receivedAt":"2008-07-30T18:21:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 30 Jul 2008, Steve Haslam wrote:\n\n> I think that once git-upload-pack.c et al get the argv[0] path over to \n> setup_path() then there's nothing more to do; setup_path() already uses \n> argv0_path in its list of paths to try. I'm confused to the reference to \n> PATH, though: we're avoiding the PATH environment variable completely.\n\nThere is an issue in setting up the argv0_path when Git is called as \"git\" \nand resolved via the PATH: argv[0] then is literally \"git\".\n\nAnd we need argv0_path for other things than just calling programs: \nrelative etc/gitconfig and libexecdir.\n\nCiao,\nDscho\n"},{"id":"85665","messageId":"4890B8AC.4010106@lastminute.com","threadId":"14757","inReplyTo":"alpine.LSU.1.00.0807302020300.3486@wbgn129.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack.","fromName":"Steve Haslam","fromEmail":"shaslam@lastminute.com","sentAt":"2008-07-30T18:53:32Z","receivedAt":"2008-07-30T18:53:32Z","isPatch":true,"sender":{"key":"shaslam@lastminute.com","avatar":"https://gravatar.com/avatar/df44c553ba2fa56f01414f0c0873212d6f8506f3426586238271bac83fde2edf?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi,\n>\n> On Wed, 30 Jul 2008, Steve Haslam wrote:\n>   \n>> I think that once git-upload-pack.c et al get the argv[0] path over to \n>> setup_path() then there's nothing more to do; setup_path() already uses \n>> argv0_path in its list of paths to try. I'm confused to the reference to \n>> PATH, though: we're avoiding the PATH environment variable completely.\n>>     \n>\n> There is an issue in setting up the argv0_path when Git is called as \"git\" \n> and resolved via the PATH: argv[0] then is literally \"git\".\n>\n> And we need argv0_path for other things than just calling programs: \n> relative etc/gitconfig and libexecdir.\n>   \nAh, I see what you meant. And all setup_path() actually does is add to \nthe end of PATH. I should have understood before.\n\nOK, I will split the patch up and tidy it.\n\nSRH\n"}]}