Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.

5 messages, 5 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:31

Johannes Schindelin [off-list ref] 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...

Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:42:31

Hi,

On Wed, 28 Jun 2006, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
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 <redacted>

---
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;
 }
 

Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.

From: Andreas Ericsson <hidden>
Date: 2016-06-15 22:42:31

Johannes Schindelin wrote:
Hi,

On Wed, 28 Jun 2006, Junio C Hamano wrote:

quoted
Johannes Schindelin [off-list ref] writes:

quoted
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

Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.

From: Marco Roeland <hidden>
Date: 2016-06-15 22:42:31

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

Re: [PATCH] git.c: Re-introduce sane error messages on missing commands.

From: Christopher Faylor <hidden>
Date: 2016-06-15 22:42:31

On Wed, Jun 28, 2006 at 02:00:44PM +0200, Marco Roeland wrote:
On Wednesday June 28th 2006 Johannes Schindelin wrote:
quoted
[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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help