Re: [PATCH] rerere: exit silently on "forget" when rerere is disabled

From: Junio C Hamano <gitster@pobox.com>
Date: May 14, 2015, 19:33 UTC
Message-ID: <xmqqmw17j6oh.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20150514192052.GB9329@peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

> On Thu, May 14, 2015 at 12:19:25PM -0700, Junio C Hamano wrote:
>
>> > It looks like we need to pay more attention to the return value of
>> > setup_rerere, which is what is supposed to take the lock.
>> 
>> Good spotting.  The normal rerere does check, but rerere-forget
>> codepath seems to forget it.
>
> Here's a patch.

Thanks.  This is obviously correct to fix your "init -q" one.

I am still puzzled by the original, though. I assumed that rerere was enabled and working correctly (in the sense that it correctly replayed a mistaken resolution recorded earlier, which Alex wanted to correct by forgetting).

Show 34 quoted lines

>
> -- >8 --
> Subject: rerere: exit silently on "forget" when rerere is disabled
>
> If you run "git rerere forget foo" in a repository that does
> not have rerere enabled, git hits an internal error:
>
>   $ git init -q
>   $ git rerere forget foo
>   fatal: BUG: attempt to commit unlocked object
>
> The problem is that setup_rerere() will not actually take
> the lock if the rerere system is disabled. We should notice
> this and return early. We can return with a success code
> here, because we know there is nothing to forget.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  rerere.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/rerere.c b/rerere.c
> index 31644de..94aea9a 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -659,6 +659,8 @@ int rerere_forget(struct pathspec *pathspec)
>  		return error("Could not read index");
>  
>  	fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
> +	if (fd < 0)
> +		return 0;
>  
>  	unmerge_cache(pathspec);
>  	find_conflict(&conflict);