[PATCH 2 of 2 RFC] localrepo: make it possible to use flock in a repo

[Jun Wu]

Thanks for the detailed comment! Replied inline.

Excerpts from Gregory Szorc's message of 2017-05-19 18:03:58 -0700:
> From a high level, this approach seems reasonable. There are some obvious
> missing components to the implementation:
> 
> * Docs for the new requirement in internals/requirements.txt
> * Ability to create new repos with the requirement
>
> I initially thought it surprising that we didn't check for flock support at
> repo open time as part of validating requirements. But your commit message
> explains that we'll error at lock time upon missing flock support. Since we
> don't have reader locks and I think it is user hostile to lock readers out
> of repos no longer supporting flock, I think this is fine. But there should
> be a test demonstrating the behavior of a flock requirement without flock
> support.

Lacking of tests, docs, debug commands (to turn this on and off) will be
addressed once this graduates from RFC.

> Also, I'm going to bikeshed the requirement name. Requirements are global
> and will persist for all of time. With that context in mind, "flock" is
> arguably too generic. What this feature/requirement actually resembles is
> "use flock() for the locking strategy used by the 'store'

More precisely, the requirement is "svfs.base" being a different directory
from "vfs.base".

> layout/requirement which implies use of flock() on the lock and wlock
> files." The meaning of the "flock" requirement can thus only be interpreted
> in the presence of another requirement, such as "store." After all, if
> there is a "store2" requirement, then "flock" likely takes on new meaning

If "store2" provides a directory, like repo.svfs = vfs('.hg/store2').
The existing code will just work without modification.

So I don't think it's necessary to distinguish "store-flock" and
"store2-flock".

> (since we'd likely use a different locking strategy). I believe
> requirements should be narrowly tailored to the exact feature they support.
> Otherwise if their meaning changes over time, that opens us up to bugs and
> possibly repo corruption. So I think a more appropriate requirement name is
> something like "store-flock" so the limits of its application (to the lock
> and wlock files for the "store" requirement) are explicit and not subject
> to reinterpretation. Semantically, this is probably ideally expressed as a
> sub-requirement. e.g. "store+flock." But I'd rather punt on that discussion
> until we have a few more requirements and the relationship between them is
> less clear.

With a second thought, I think it may be possible to just unify the locks if
svfs.base == vfs.base. That could be done by letting repo.lock returns
repo.wlock() in this special case (no store + flock).