Re: [PATCH] Allow git-diff exit with codes similar to diff(1)

From: Junio C Hamano <junkio@cox.net>
Date: Mar 14, 2007, 08:58 UTC
Message-ID: <7vfy88dwvw.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <81b0412b0703140119s4691211bw8a5ef3ba25c5cbdf@mail.gmail.com>

"Alex Riesen" <raa.lkml@gmail.com> writes:

Show 20 quoted lines

> On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:
> ...
>> Exiting with 0 for no-change, 1 for has-change and other value
>> for error is something that falls into the
>>
>>     "I wish if we did it from day one, but now many people's
>>     scripts depend on the behaviour, and heck we ourselves say
>>     that the right way to see if there is difference is to check
>>     if the output is an empty string (look at a few scripts of
>>     our own), so it would be a huge backward compatibility
>>     hassle"
>>
>> category.
>>
>> e.g.
>>
>> git-am.sh:      files=$(git-diff-index --cached --name-only HEAD) || exit
>> git-am.sh:      changed="$(git-diff-index --cached --name-only HEAD)"
>
> Isn't this crazy? Get the information and never really use it?

The information is used.

The first one says "see if we have already difference stashed in the index for our own committing --- by the way, if there is an error, do not do any further harm but error out". If we do not get an error, that line is followed by this:

	files=$(git-diff-index --cached --name-only HEAD) || exit
        if [ "$files" ]
        then
        	echo "Dirty index: ..." >&2
                exit 1
	fi

I've been telling you since the #git session that I know that is *different* from how "diff" works, and I think everybody agrees if we were doing git from scratch today we would have done exit status with 0/1/other to signal no-change, have-diff and error.

But the established way for scripts that use plumbing is

 - to check error with $? (or ... || )
 - to check modified-or-not with output

and people who have been learning from the scripts (we used to have lot more scripts) would have picked up that pattern. That's why I already told you that --exit-status is the right thing to do if we were doing it from scratch, but is a wrong thing to do at this point. Maybe in a release as big as 1.5.0 that we pre-announce a lot of interface changes.

In short, Linus is right in that the current exit code is not useful to see what the end users are interested in (and they are not in the business of debugging git, and diff would error out only when the repository has problems, perhaps a corrupt object or something like that). But being not useful and being currently not relied upon are two different things.

And I am being conservative, especially after a big release.