Re: [PATCH] git: add --no-optional-locks option

[Jeff King <peff@peff.net>]

On Thu, Sep 21, 2017 at 01:55:58PM +0900, Junio C Hamano wrote:

> The phrase 'optional lock' does not answer this question clearly,
> though: does it make sense to extend the coverage of this option in
> the future to things more than the "opportunistic update to the
> index file"?
> 
> If the answer is no, then having 'index' instead of 'lock' in the
> name of the option would make more sense (and 'opportunistic' over
> 'optional', too), because what the change is about is to allow other
> processes that are directly interacting with the user to update the
> index, and 'lock' being hindrance is merely an implementation
> detail.  The comment on the "test" in the log message mentions as if
> it were a short-coming that it does not check the lock but checks
> if the index is written, but I think that is testing what matters
> and preferable than testing "did we lock and then unlock it?"
> 
> On the other hand, if the answer is yes, then I am curious what
> other things this may extend to, and if these other things are also
> opportunistic optimizations.

I left it intentionally vague exactly because I thought we might want to
leave room for the answer to change to "yes" eventually.  For instance,
imagine that we had a ref storage format that required periodic
compaction, and readers might sometimes choose to compact in order to
save future readers from repeating some work they've done. If that
compaction means holding a lock even for a brief period, I think it
would fall under this option.

I admit that's just adding more hand-waving to the pile. But I don't
think it really _hurts_ to leave that door open (aside from making the
documentation a bit wishy-washy). And it helps because callers would
pick up the new features automatically, without having to learn about a
new option.

And I think that's really what this option is. It is less about the
caller asking for some specific behavior, and more about them telling
Git about the context in which it's executing so it can make intelligent
decisions.

And in that sense, something descriptive like --background-process
perhaps would be a better name. Except that I couldn't come up with a
name that isn't confusing (certainly --background-process implies to me
that Git would itself run in the background, which makes no sense here).

I also considered something like "--read-only" to tell Git that we
should avoid writing to the repository. But that's really not what this
does. It just avoids writes that may cause contention, not all writes.

I also considered using the word "opportunistic" in the option name, but
decided it was too long and hard to spell.

So there. I am open to a better name, but I could not come up with one.

> Thanks (and sorry for not being Johannes ;-).

You lack his rugged good looks, but your review was still welcome.

-Peff