[Success!] Re: [MERGE] Implement bzrlib.lock with CreateFile rather than LockFileEx on win32 (operational part)

[Martin (gzlist)]

On 02/07/2009, John Arbash Meinel <john at arbash-meinel.com> wrote:
> John Szakmeister wrote:
>> Does this help with the shelve not working on Windows
>> (https://bugs.launchpad.net/bzr/+bug/305006)?
>
> no

To expand on that, this only changes how the low-level file locking is
implemented, not its behaviour or its use in any of the higher level
apis. So, `bzr selftest -s bt.test_shelf` has the same 25 errors
before and after this change.

My understanding of the current state of affairs, and hopefully John
A. M. will correct me if I get any of the details wrong, is as
follows.

Write locks are intended to prevent readers getting partial data while
the file is being written to. On nix, this doesn't happen, they only
exclude each other, but file writes are short enough that it's rare
that anything will *actually* try and read a partially written file.
There are a number of lock tests that are disabled because of this
non-exclusive behaviour on the primary platform. The windows code
works as advertised, and thus falls over in places like the shelf
code, that creates two DirState objects over the same bit of the
filesystem at the same time, and tries to take a write lock out on one
and a read lock out on the other. That's what really needs fixing.

In summary:
* The code is broken on nix.
* The devs use nix and don't test against other platforms.
* Therefore, bazaar relies on the code being broken.

I could actually submit a patch that cripples write locks in a similar
way to they are on nix. That seems to fix selftest test_shelf (except
one symlink test that's lacking a missing feature guard). Whether
that's actually a good idea or not, is not for me to judge.

Martin