Re: Re* [BUG] Index.lock error message regression in git 2.11.0

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 7, 2016, 18:21 UTC
Message-ID: <xmqqd1h3y506.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqq4m2gzotn.fsf_-_@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 6 quoted lines

> ...
> I would not be surprised if some existing calls to hold_lock*()
> functions that pass die_on_error=0 need to be updated to pass
> LOCK_SILENT_ON_ERROR, and when this fix is taken alone, it may look
> like a regression, but we are better off starting louder and squelch
> the ones that we find safe to make silent than the other way around.

I actually take this part back, for two reasons.

 * Before the recent js/sequencer-wo-die topic that made this
   failure mode of 'git merge' more dangerous by accident, there
   were already callers that passed die_on_error=0 to hold_lock*
   family of functions, and we can trust these callers.  They either
   have been silent upon lock failure sensibly (e.g. a caller that
   tries to acquire the lock to opportunistically update the index
   can safely choose not to do anything and be silent) or they have
   had their own way of reporting the errors to the users.  The "you
   need to ask to be totally quiet" approach in my rerolled patch
   (the one I am responding to) will introduce new regressions to
   these codepaths.

 * Among the ones that stopped passing die_on_error=1 when the topic
   was merged, there are
   ones that give sensible error messages.  Again, they do not need
   extra message with the "you need to ask to be totally quiet"
   approach [*1*].

We need to instead go through the latter, i.e. the ones that appear in "git show --first-parent 2a4062a4a8", with fine-toothed comb to see which 0 made an error totally silent (like the one Robbie spotted in merge.c) and fix them to ask hold_lock*() functions not to die but still report an error.