* [PATCH] git.c: Re-introduce sane error messages on missing commands.
@ 2006-06-27 8:28 Andreas Ericsson
2006-06-27 22:57 ` Junio C Hamano
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Ericsson @ 2006-06-27 8:28 UTC (permalink / raw)
To: git
Somewhere in the alias handling git turned hostile on fat fingers:
$ git showbranch
Failed to run command '': Is a directory
This patch fixes that by doing 2 things:
* The variable git_command[MAX_PATH + 1] is now retired from
git.c:main() where it was never set. Instead the variable "cmd"
is used for all error messages.
* The introduction of the "exec_errno" variable which preserves the
errno number from the execve() attempt. Later "why did it fail"
tests evaluate exec_errno, which gives the correct error message
along with the brief command-list (telling me I missed the dash in
"show-branch").
It's possible that alias handling can fail and set errno to something
proper, making this change less sane than I think, but handle_alias()
seems to take care of its own error-handling so it shouldn't matter.
Signed-off-by: Andreas Ericsson <ae@op5•se>
---
git.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/git.c b/git.c
index 94e9a4a..b04424f 100644
--- a/git.c
+++ b/git.c
@@ -206,9 +206,8 @@ int main(int argc, const char **argv, ch
{
const char *cmd = argv[0];
char *slash = strrchr(cmd, '/');
- char git_command[PATH_MAX + 1];
const char *exec_path = NULL;
- int done_alias = 0;
+ int exec_errno = 0, done_alias = 0;
/*
* Take the basename of argv[0] as the command
@@ -300,6 +299,9 @@ int main(int argc, const char **argv, ch
/* .. then try the external ones */
execv_git_cmd(argv);
+ /* if it's not an alias, the execve() errno is what we want */
+ exec_errno = errno;
+
/* It could be an alias -- this works around the insanity
* of overriding "git log" with "git show" by having
* alias.log = show
@@ -309,11 +311,11 @@ int main(int argc, const char **argv, ch
done_alias = 1;
}
- if (errno == ENOENT)
+ if (exec_errno == ENOENT)
cmd_usage(0, exec_path, "'%s' is not a git-command", cmd);
fprintf(stderr, "Failed to run command '%s': %s\n",
- git_command, strerror(errno));
+ cmd, strerror(exec_errno));
return 1;
}
--
1.4.1.rc1.g1ef9
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
2006-06-27 8:28 [PATCH] git.c: Re-introduce sane error messages on missing commands Andreas Ericsson
@ 2006-06-27 22:57 ` Junio C Hamano
2006-06-28 8:13 ` Andreas Ericsson
0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2006-06-27 22:57 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: git
Andreas Ericsson <ae@op5•se> writes:
> Somewhere in the alias handling git turned hostile on fat fingers:
>
> $ git showbranch
> Failed to run command '': Is a directory
Does not happen here (nor on Cygwin 1.4.1.rc1). Care to help
reproducing it?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
2006-06-27 22:57 ` Junio C Hamano
@ 2006-06-28 8:13 ` Andreas Ericsson
2006-06-28 9:21 ` Johannes Schindelin
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Ericsson @ 2006-06-28 8:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano wrote:
> Andreas Ericsson <ae@op5•se> writes:
>
>
>>Somewhere in the alias handling git turned hostile on fat fingers:
>>
>> $ git showbranch
>> Failed to run command '': Is a directory
>
>
> Does not happen here (nor on Cygwin 1.4.1.rc1). Care to help
> reproducing it?
>
Here's the complete procedure I used:
$ git pull && make; # works ok
$ make strip install; # works ok
$ git showbranch
Failed to run command '': Is a directory
(confusion and puzzlement...)
$ git checkout master; git reset --hard master; # works ok
$ git pull; # gets nothing (I try stupid things first ;p)
$ make clean install; # works ok
$ git showbranch
Failed to run command '': Is a directory
$ git --version
git version 1.4.1.rc1.g1ef9
It's reliably reproducible here. Notable though is that I have no
.git/config in my git.git clone and no ~/.gitconfig (or ~/.gitrc or
whatever), so the handle_alias() function never finds a config file to
look for aliases in.
Either way, removing the variable "char git_command[MAX_PATH + 1];" from
git.c:main() is correct since it's never used for anything but printing
the (erroneous) error message above.
--
Andreas Ericsson andreas.ericsson@op5•se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
2006-06-28 8:13 ` Andreas Ericsson
@ 2006-06-28 9:21 ` Johannes Schindelin
2006-06-28 9:53 ` Junio C Hamano
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Schindelin @ 2006-06-28 9:21 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Junio C Hamano, git
Hi,
On Wed, 28 Jun 2006, Andreas Ericsson wrote:
> Junio C Hamano wrote:
> > Andreas Ericsson <ae@op5•se> writes:
> >
> >
> > > Somewhere in the alias handling git turned hostile on fat fingers:
> > >
> > > $ git showbranch
> > > Failed to run command '': Is a directory
> >
> >
> > Does not happen here (nor on Cygwin 1.4.1.rc1). Care to help
> > reproducing it?
Try this:
$ mkdir 5
$ cd 5
$ git-init-db
$ rm .git/config # yes, really.
$ git abc
Ciao,
Dscho
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
2006-06-28 9:21 ` Johannes Schindelin
@ 2006-06-28 9:53 ` Junio C Hamano
2006-06-28 10:45 ` Johannes Schindelin
0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2006-06-28 9:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Andreas Ericsson, git
Johannes Schindelin <Johannes.Schindelin@gmx•de> writes:
> Try this:
>
> $ mkdir 5
> $ cd 5
> $ git-init-db
> $ rm .git/config # yes, really.
> $ git abc
Thanks for trying to help, but not really.
: gitster; mkdir 5
: gitster; cd 5
: gitster; git-init-db
defaulting to local storage area
: gitster; rm .git/config
: gitster; ~/git-master/bin/git abc
git: 'abc' is not a git-command
The most commonly used git commands are:
add Add files to the index file
apply Apply patch on a git index file and a work
...
tag Create a tag object signed with GPG
verify-tag Check the GPG signature of tag
(use 'git help -a' to get a list of all installed git commands)
: gitster; ~/git-master/bin/git --version
git version 1.4.1.rc1.g8096
: gitster; ls -ld ~/.gitrc ~/.gitconfig ~/.git-config
: gitster; ls -ld ~/.gitrc ~/.gitconfig ~/.git-config
ls: /home/junio/.gitrc: No such file or directory
ls: /home/junio/.gitconfig: No such file or directory
ls: /home/junio/.git-config: No such file or directory
: gitster; : confused...
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
2006-06-28 9:53 ` Junio C Hamano
@ 2006-06-28 10:45 ` Johannes Schindelin
2006-06-28 11:53 ` Andreas Ericsson
2006-06-28 12:00 ` Marco Roeland
0 siblings, 2 replies; 9+ messages in thread
From: Johannes Schindelin @ 2006-06-28 10:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andreas Ericsson, git
Hi,
On Wed, 28 Jun 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx•de> writes:
>
> > Try this:
> >
> > $ mkdir 5
> > $ cd 5
> > $ git-init-db
> > $ rm .git/config # yes, really.
> > $ git abc
>
> Thanks for trying to help, but not really.
Okay. Does not happen with 'next' here, too. I have some changes in my
private repo (which eventually should culminate in the big mmap()ed sooper
config parsing / writing thingie), which make it break. The following
patch fixes this (and potentially Andreas' problem, too).
-- cut here --
[PATCH] save errno in handle_alias()
git.c:main() relies on the value of errno being set by the last attempt to
execute the command. However, if something goes awry in handle_alias(),
that assumption is wrong. So restore errno before returning from
handle_alias().
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx•de>
---
diff --git a/git.c b/git.c
index 94e9a4a..7c7106e 100644
--- a/git.c
+++ b/git.c
@@ -99,7 +99,7 @@ static int split_cmdline(char *cmdline,
static int handle_alias(int *argcp, const char ***argv)
{
- int nongit = 0, ret = 0;
+ int nongit = 0, ret = 0, saved_errno = errno;
const char *subdir;
subdir = setup_git_directory_gently(&nongit);
@@ -137,6 +137,8 @@ static int handle_alias(int *argcp, cons
if (subdir)
chdir(subdir);
+ errno = saved_errno;
+
return ret;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
2006-06-28 10:45 ` Johannes Schindelin
@ 2006-06-28 11:53 ` Andreas Ericsson
2006-06-28 12:00 ` Marco Roeland
1 sibling, 0 replies; 9+ messages in thread
From: Andreas Ericsson @ 2006-06-28 11:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
Johannes Schindelin wrote:
> Hi,
>
> On Wed, 28 Jun 2006, Junio C Hamano wrote:
>
>
>>Johannes Schindelin <Johannes.Schindelin@gmx•de> writes:
>>
>>
>>>Try this:
>>>
>>>$ mkdir 5
>>>$ cd 5
>>>$ git-init-db
>>>$ rm .git/config # yes, really.
>>>$ git abc
>>
>>Thanks for trying to help, but not really.
>
>
> Okay. Does not happen with 'next' here, too. I have some changes in my
> private repo (which eventually should culminate in the big mmap()ed sooper
> config parsing / writing thingie), which make it break. The following
> patch fixes this (and potentially Andreas' problem, too).
>
It should, although the command it tried to execute will still be empty
if it fails for some other reason (file not executable / permission
denied), since it only does the right thing on ENOENT.
This is also, imo, a bit worse than preserving the errno from the
execve() call in the caller, since errno is sometimes a macro (yes, only
in threaded apps atm, but still...), and it will be easy to forget to
look in handle_alias() if other things change in main() that makes this
bug resurface.
Oh, and the part of my patch removing the git_command variable from
git.c:main() still has to be applied for arbitrary error-messages to
look sane.
$ grep -B1 git_command git.c
char *slash = strrchr(cmd, '/');
char git_command[PATH_MAX + 1];
--
fprintf(stderr, "Failed to run command '%s': %s\n",
git_command, strerror(errno));
Btw, Junio, did you try this with 'master' as of yesterday morning (git
version 1.4.1.rc1.g1ef9)? It's reproducible on every machine I've tried
so far (well, only five, but still), so it seems odd that you don't see it.
--
Andreas Ericsson andreas.ericsson@op5•se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
2006-06-28 10:45 ` Johannes Schindelin
2006-06-28 11:53 ` Andreas Ericsson
@ 2006-06-28 12:00 ` Marco Roeland
2006-06-28 14:59 ` Christopher Faylor
1 sibling, 1 reply; 9+ messages in thread
From: Marco Roeland @ 2006-06-28 12:00 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Andreas Ericsson, git
On Wednesday June 28th 2006 Johannes Schindelin wrote:
> [PATCH] save errno in handle_alias()
>
> git.c:main() relies on the value of errno being set by the last attempt to
> execute the command. However, if something goes awry in handle_alias(),
> that assumption is wrong. So restore errno before returning from
> handle_alias().
If we rely on the value of errno we should always immediately store it's
value anyway. On some neolithic systems like the "MSVCRT.DLL" C runtime
library on Windows (used by e.g. the Mingw compiler, don't know about
Cygwin) a lot of runtime functions actually even reset the value of
errno to 0 on success!
--
Marco Roeland
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.
2006-06-28 12:00 ` Marco Roeland
@ 2006-06-28 14:59 ` Christopher Faylor
0 siblings, 0 replies; 9+ messages in thread
From: Christopher Faylor @ 2006-06-28 14:59 UTC (permalink / raw)
To: Andreas Ericsson, Junio C Hamano, Johannes Schindelin,
Marco Roeland, git
On Wed, Jun 28, 2006 at 02:00:44PM +0200, Marco Roeland wrote:
>On Wednesday June 28th 2006 Johannes Schindelin wrote:
>
>> [PATCH] save errno in handle_alias()
>>
>> git.c:main() relies on the value of errno being set by the last attempt to
>> execute the command. However, if something goes awry in handle_alias(),
>> that assumption is wrong. So restore errno before returning from
>> handle_alias().
>
>If we rely on the value of errno we should always immediately store it's
>value anyway. On some neolithic systems like the "MSVCRT.DLL" C runtime
>library on Windows (used by e.g. the Mingw compiler, don't know about
>Cygwin) a lot of runtime functions actually even reset the value of
>errno to 0 on success!
Cygwin does not use MSVCRT.DLL and tries to be careful about spurious
resetting of errno.
cgf
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2006-06-28 14:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-27 8:28 [PATCH] git.c: Re-introduce sane error messages on missing commands Andreas Ericsson
2006-06-27 22:57 ` Junio C Hamano
2006-06-28 8:13 ` Andreas Ericsson
2006-06-28 9:21 ` Johannes Schindelin
2006-06-28 9:53 ` Junio C Hamano
2006-06-28 10:45 ` Johannes Schindelin
2006-06-28 11:53 ` Andreas Ericsson
2006-06-28 12:00 ` Marco Roeland
2006-06-28 14:59 ` Christopher Faylor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox