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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 29, 2008, 23:37 UTC
Message-ID: <7vskx444h5.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <3c6c07c20804291603q4fbe957eq3e3da39d4a2e29c0@mail.gmail.com>

"Mike Coleman" <tutufan@gmail.com> writes:

Show 22 quoted lines

> 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.

Show 11 quoted lines

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