{"thread":{"id":"4674","subject":"[PATCH] git.c: Re-introduce sane error messages on missing commands.","startedAt":"2006-06-27T08:28:22Z","lastAt":"2006-06-28T14:59:44Z","messageCount":9,"participants":["Andreas Ericsson","Junio C Hamano","Johannes Schindelin","Marco Roeland","Christopher Faylor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"22646","messageId":"20060627083508.E912A5BBAB@nox.op5.se","threadId":"4674","inReplyTo":null,"subject":"[PATCH] git.c: Re-introduce sane error messages on missing commands.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-06-27T08:28:22Z","receivedAt":"2006-06-27T08:28:22Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Somewhere in the alias handling git turned hostile on fat fingers:\n\n\t$ git showbranch\n\tFailed to run command '': Is a directory\n\nThis patch fixes that by doing 2 things:\n * The variable git_command[MAX_PATH + 1] is now retired from\n git.c:main() where it was never set. Instead the variable \"cmd\"\n is used for all error messages.\n * The introduction of the \"exec_errno\" variable which preserves the\n errno number from the execve() attempt. Later \"why did it fail\"\n tests evaluate exec_errno, which gives the correct error message\n along with the brief command-list (telling me I missed the dash in\n \"show-branch\").\n\nIt's possible that alias handling can fail and set errno to something\nproper, making this change less sane than I think, but handle_alias()\nseems to take care of its own error-handling so it shouldn't matter.\n\nSigned-off-by: Andreas Ericsson \n---\n git.c | 10 ++++++----\n 1 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex 94e9a4a..b04424f 100644\n--- a/git.c\n+++ b/git.c\n@@ -206,9 +206,8 @@ int main(int argc, const char **argv, ch\n {\n \tconst char *cmd = argv[0];\n \tchar *slash = strrchr(cmd, '/');\n-\tchar git_command[PATH_MAX + 1];\n \tconst char *exec_path = NULL;\n-\tint done_alias = 0;\n+\tint exec_errno = 0, done_alias = 0;\n \n \t/*\n \t * Take the basename of argv[0] as the command\n@@ -300,6 +299,9 @@ int main(int argc, const char **argv, ch\n \t\t/* .. then try the external ones */\n \t\texecv_git_cmd(argv);\n \n+\t\t/* if it's not an alias, the execve() errno is what we want */\n+\t\texec_errno = errno;\n+\n \t\t/* It could be an alias -- this works around the insanity\n \t\t * of overriding \"git log\" with \"git show\" by having\n \t\t * alias.log = show\n@@ -309,11 +311,11 @@ int main(int argc, const char **argv, ch\n \t\tdone_alias = 1;\n \t}\n \n-\tif (errno == ENOENT)\n+\tif (exec_errno == ENOENT)\n \t\tcmd_usage(0, exec_path, \"'%s' is not a git-command\", cmd);\n \n \tfprintf(stderr, \"Failed to run command '%s': %s\\n\",\n-\t\tgit_command, strerror(errno));\n+\t\tcmd, strerror(exec_errno));\n \n \treturn 1;\n }\n-- \n1.4.1.rc1.g1ef9\n"},{"id":"22694","messageId":"7vpsgu6wba.fsf@assigned-by-dhcp.cox.net","threadId":"4674","inReplyTo":"20060627083508.E912A5BBAB@nox.op5.se","subject":"Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-27T22:57:45Z","receivedAt":"2006-06-27T22:57:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Ericsson writes:\n\n> Somewhere in the alias handling git turned hostile on fat fingers:\n>\n> \t$ git showbranch\n> \tFailed to run command '': Is a directory\n\nDoes not happen here (nor on Cygwin 1.4.1.rc1). Care to help\nreproducing it?\n"},{"id":"22712","messageId":"44A23A38.3090206@op5.se","threadId":"4674","inReplyTo":"7vpsgu6wba.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-06-28T08:13:44Z","receivedAt":"2006-06-28T08:13:44Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Andreas Ericsson writes:\n> \n> \n>>Somewhere in the alias handling git turned hostile on fat fingers:\n>>\n>>\t$ git showbranch\n>>\tFailed to run command '': Is a directory\n> \n> \n> Does not happen here (nor on Cygwin 1.4.1.rc1). Care to help\n> reproducing it?\n> \n\nHere's the complete procedure I used:\n\n\t$ git pull && make; # works ok\n\t$ make strip install; # works ok\n\t$ git showbranch\n\tFailed to run command '': Is a directory\n\t(confusion and puzzlement...)\n\t$ git checkout master; git reset --hard master; # works ok\n\t$ git pull; # gets nothing (I try stupid things first ;p)\n\t$ make clean install; # works ok\n\t$ git showbranch\n\tFailed to run command '': Is a directory\n\t$ git --version\n\tgit version 1.4.1.rc1.g1ef9\n\n\nIt's reliably reproducible here. Notable though is that I have no \n.git/config in my git.git clone and no ~/.gitconfig (or ~/.gitrc or \nwhatever), so the handle_alias() function never finds a config file to \nlook for aliases in.\n\nEither way, removing the variable \"char git_command[MAX_PATH + 1];\" from \ngit.c:main() is correct since it's never used for anything but printing \nthe (erroneous) error message above.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"22721","messageId":"Pine.LNX.4.63.0606281118330.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4674","inReplyTo":"44A23A38.3090206@op5.se","subject":"Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-28T09:21:21Z","receivedAt":"2006-06-28T09:21:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 28 Jun 2006, Andreas Ericsson wrote:\n\n> Junio C Hamano wrote:\n> > Andreas Ericsson writes:\n> > \n> > \n> > > Somewhere in the alias handling git turned hostile on fat fingers:\n> > > \n> > > \t$ git showbranch\n> > > \tFailed to run command '': Is a directory\n> > \n> > \n> > Does not happen here (nor on Cygwin 1.4.1.rc1). Care to help\n> > reproducing it?\n\nTry this:\n\n$ mkdir 5\n$ cd 5\n$ git-init-db\n$ rm .git/config # yes, really.\n$ git abc\n\nCiao,\nDscho\n"},{"id":"22725","messageId":"7vr71938t4.fsf@assigned-by-dhcp.cox.net","threadId":"4674","inReplyTo":"Pine.LNX.4.63.0606281118330.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-28T09:53:43Z","receivedAt":"2006-06-28T09:53:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Try this:\n>\n> $ mkdir 5\n> $ cd 5\n> $ git-init-db\n> $ rm .git/config # yes, really.\n> $ git abc\n\nThanks for trying to help, but not really.\n\n: gitster; mkdir 5\n: gitster; cd 5\n: gitster; git-init-db\ndefaulting to local storage area\n: gitster; rm .git/config\n: gitster; ~/git-master/bin/git abc\ngit: 'abc' is not a git-command\n\nThe most commonly used git commands are:\n add Add files to the index file\n apply Apply patch on a git index file and a work\n...\n tag Create a tag object signed with GPG\n verify-tag Check the GPG signature of tag\n(use 'git help -a' to get a list of all installed git commands)\n: gitster; ~/git-master/bin/git --version\ngit version 1.4.1.rc1.g8096\n: gitster; ls -ld ~/.gitrc ~/.gitconfig ~/.git-config\n: gitster; ls -ld ~/.gitrc ~/.gitconfig ~/.git-config\nls: /home/junio/.gitrc: No such file or directory\nls: /home/junio/.gitconfig: No such file or directory\nls: /home/junio/.git-config: No such file or directory\n: gitster; : confused...\n"},{"id":"22735","messageId":"Pine.LNX.4.63.0606281240480.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4674","inReplyTo":"7vr71938t4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-28T10:45:27Z","receivedAt":"2006-06-28T10:45:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 28 Jun 2006, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Try this:\n> >\n> > $ mkdir 5\n> > $ cd 5\n> > $ git-init-db\n> > $ rm .git/config # yes, really.\n> > $ git abc\n> \n> Thanks for trying to help, but not really.\n\nOkay. Does not happen with 'next' here, too. I have some changes in my \nprivate repo (which eventually should culminate in the big mmap()ed sooper \nconfig parsing / writing thingie), which make it break. The following \npatch fixes this (and potentially Andreas' problem, too).\n\n-- cut here --\n\n[PATCH] save errno in handle_alias()\n\ngit.c:main() relies on the value of errno being set by the last attempt to \nexecute the command. However, if something goes awry in handle_alias(), \nthat assumption is wrong. So restore errno before returning from \nhandle_alias().\n\nSigned-off-by: Johannes Schindelin \n\n---\n\ndiff --git a/git.c b/git.c\nindex 94e9a4a..7c7106e 100644\n--- a/git.c\n+++ b/git.c\n@@ -99,7 +99,7 @@ static int split_cmdline(char *cmdline, \n \n static int handle_alias(int *argcp, const char ***argv)\n {\n-\tint nongit = 0, ret = 0;\n+\tint nongit = 0, ret = 0, saved_errno = errno;\n \tconst char *subdir;\n \n \tsubdir = setup_git_directory_gently(&nongit);\n@@ -137,6 +137,8 @@ static int handle_alias(int *argcp, cons\n \tif (subdir)\n \t\tchdir(subdir);\n \n+\terrno = saved_errno;\n+\n \treturn ret;\n }\n \n"},{"id":"22738","messageId":"44A26DB6.1080408@op5.se","threadId":"4674","inReplyTo":"Pine.LNX.4.63.0606281240480.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-06-28T11:53:26Z","receivedAt":"2006-06-28T11:53:26Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n> On Wed, 28 Jun 2006, Junio C Hamano wrote:\n> \n> \n>>Johannes Schindelin writes:\n>>\n>>\n>>>Try this:\n>>>\n>>>$ mkdir 5\n>>>$ cd 5\n>>>$ git-init-db\n>>>$ rm .git/config # yes, really.\n>>>$ git abc\n>>\n>>Thanks for trying to help, but not really.\n> \n> \n> Okay. Does not happen with 'next' here, too. I have some changes in my \n> private repo (which eventually should culminate in the big mmap()ed sooper \n> config parsing / writing thingie), which make it break. The following \n> patch fixes this (and potentially Andreas' problem, too).\n> \n\nIt should, although the command it tried to execute will still be empty \nif it fails for some other reason (file not executable / permission \ndenied), since it only does the right thing on ENOENT.\n\nThis is also, imo, a bit worse than preserving the errno from the \nexecve() call in the caller, since errno is sometimes a macro (yes, only \nin threaded apps atm, but still...), and it will be easy to forget to \nlook in handle_alias() if other things change in main() that makes this \nbug resurface.\n\nOh, and the part of my patch removing the git_command variable from \ngit.c:main() still has to be applied for arbitrary error-messages to \nlook sane.\n\n$ grep -B1 git_command git.c\n char *slash = strrchr(cmd, '/');\n char git_command[PATH_MAX + 1];\n--\n fprintf(stderr, \"Failed to run command '%s': %s\\n\",\n git_command, strerror(errno));\n\n\nBtw, Junio, did you try this with 'master' as of yesterday morning (git \nversion 1.4.1.rc1.g1ef9)? It's reproducible on every machine I've tried \nso far (well, only five, but still), so it seems odd that you don't see it.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"22739","messageId":"20060628120044.GA3228@fiberbit.xs4all.nl","threadId":"4674","inReplyTo":"Pine.LNX.4.63.0606281240480.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.","fromName":"Marco Roeland","fromEmail":"marco.roeland@xs4all.nl","sentAt":"2006-06-28T12:00:44Z","receivedAt":"2006-06-28T12:00:44Z","isPatch":true,"sender":{"key":"marco.roeland@xs4all.nl","avatar":null},"body":"On Wednesday June 28th 2006 Johannes Schindelin wrote:\n\n> [PATCH] save errno in handle_alias()\n> \n> git.c:main() relies on the value of errno being set by the last attempt to \n> execute the command. However, if something goes awry in handle_alias(), \n> that assumption is wrong. So restore errno before returning from \n> handle_alias().\n\nIf we rely on the value of errno we should always immediately store it's\nvalue anyway. On some neolithic systems like the \"MSVCRT.DLL\" C runtime\nlibrary on Windows (used by e.g. the Mingw compiler, don't know about\nCygwin) a lot of runtime functions actually even reset the value of\nerrno to 0 on success!\n-- \nMarco Roeland\n"},{"id":"22742","messageId":"20060628145944.GB5567@trixie.casa.cgf.cx","threadId":"4674","inReplyTo":"20060628120044.GA3228@fiberbit.xs4all.nl","subject":"Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.","fromName":"Christopher Faylor","fromEmail":"me@cgf.cx","sentAt":"2006-06-28T14:59:44Z","receivedAt":"2006-06-28T14:59:44Z","isPatch":true,"sender":{"key":"me@cgf.cx","avatar":null},"body":"On Wed, Jun 28, 2006 at 02:00:44PM +0200, Marco Roeland wrote:\n>On Wednesday June 28th 2006 Johannes Schindelin wrote:\n>\n>> [PATCH] save errno in handle_alias()\n>> \n>> git.c:main() relies on the value of errno being set by the last attempt to \n>> execute the command. However, if something goes awry in handle_alias(), \n>> that assumption is wrong. So restore errno before returning from \n>> handle_alias().\n>\n>If we rely on the value of errno we should always immediately store it's\n>value anyway. On some neolithic systems like the \"MSVCRT.DLL\" C runtime\n>library on Windows (used by e.g. the Mingw compiler, don't know about\n>Cygwin) a lot of runtime functions actually even reset the value of\n>errno to 0 on success!\n\nCygwin does not use MSVCRT.DLL and tries to be careful about spurious\nresetting of errno.\n\ncgf\n"}]}