Re: bug: git-diff silently fails when run outside of a repository (v1.5.4.2)

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Tue, 29 Apr 2008, Junio C Hamano wrote:

> "Mike Coleman" <tutufan@gmail.com> writes:
> 
> > On Tue, Apr 29, 2008 at 5:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> "Mike Coleman" <tutufan@gmail.com> writes:
> >>
> >>  > At least in version 1.5.4.2, git-diff silently fails when not run
> >>  > inside a repository.  It should give an error diagnostic, especially
> >>  > since "no output" would otherwise be a meaningful response.
> >>
> >>  Unfortunately this does not have enough information to go by, as unlike
> >>  many other programs, "git diff" contains a hack to be usable as a better
> >>  (for certain definition of "better" I may not necessarily agree with) GNU
> >>  diff replacement when run outside a repository.
> >>
> >>  i.e.
> >>
> >>         mkdir -p /var/tmp/junk
> >>         cd /var/tmp/junk
> >>         rm -fr .git ;# make sure it is not a repository
> >>         echo >a hello
> >>         echo >b world
> >>         git diff --color a b
> >>
> >>  is supposed to work.
> 
> > Oh, I didn't realize that.  It doesn't seem to be mentioned on the man
> > page, though I can't necessarily claim that I would have seen it if it
> > had.
> >
> > Even so, this seems like a bug.  If I do this:
> >
> >     $ cd /
> >     $ git-diff
> >
> > there is no error message and no error status.  A diagnostic would be
> > very helpful.
> 
> Ah, that indeed is not very helpful.
> 
> Unfortunately, every time I look at this hack, I seem to find an unrelated
> bug in it.  Here is today's.
> 
> 	$ for i in 1 2 3; do >/var/tmp/$i; done
>         $ cd /
>         $ git diff /var/tmp/1
>         Segmentation Fault
> 
> When nongit is true, we know the user has to be asking --no-index diff, so
> perhaps we can fix it by doing something like this?
> 
> diff --git a/diff-lib.c b/diff-lib.c
> index 069e450..cfd629d 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -264,6 +264,9 @@ int setup_diff_no_index(struct rev_info *revs,
>  			DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
>  			break;
>  		}
> +	if (nongit && argc != i + 2)
> +		die("git diff [--no-index] takes two paths");
> +
>  	if (argc != i + 2 || (!is_outside_repo(argv[i + 1], nongit, prefix) &&
>  				!is_outside_repo(argv[i], nongit, prefix)))
>  		return -1;

That looks to me as if the second if() should have triggered, and the 
caller of setup_diff_no_index() should have errored out.

Ciao,
Dscho "who has too many issues with git-submodule right now"