Re: [PATCH] filter-branch: return 2 when nothing to rewrite

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 15, 2018, 15:42 UTC
Message-ID: <xmqqa7v973b5.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180315141220.GB27748@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

> On Thu, Mar 15, 2018 at 02:03:59PM +0100, Michele Locati wrote:
>
>> Using the --state-branch option allows us to perform incremental filtering.
>> This may lead to having nothing to rewrite in subsequent filtering, so we need
>> a way to recognize this case.
>> So, let's exit with 2 instead of 1 when this "error" occurs.
>
> That sounds like a good feature. It doesn't look like we use "2" for
> anything else currently.

I do not want to sound overly negative against the first contribution from a new contributor, but I am not sure if this is a good idea. While I do agree that the caller of filter-branch would want _some_ way to tell if the call

 - got some new stuff,
 - got no error but did not get anything new, or
 - failed

and act accordingly, changing the exit code to a non-zero value for the second case above would mean that existing scripts that have happily been working would suddenly start failing. Due to the lack of an easy way to tell the first two cases apart, they may have been doing _extra_ work after calling filter-branch when it found no new development (resulting in an expensive no-op), or perhaps they implemented their own way to tell the second case apart from the first one and efficiently omitting extra work in the second case already. In either case, these scripts will get broken with this change.

So I'd respond with a mild "no" with "can't we allow callers to tell the first two cases apart in some other way so that we do not have to break existing scripts?".

Show 6 quoted lines

>> ---
>>  git-filter-branch.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> This should probably get a mention in the manpage at
> Documentation/git-filter-branch.txt, too.

Whatever solution we eventually end up with, it needs to be documented.

Thanks.