[PATCH] tags: don't allow environment errors to be raised from _writetagscache

[Adrian Buehlmann]

On 2011-06-17 01:12, Steve Borho wrote:
> On Thu, Jun 16, 2011 at 3:45 PM, Adrian Buehlmann <adrian at cadifra.com> wrote:
>> On 2011-06-16 22:26, Steve Borho wrote:
>>> # HG changeset patch
>>> # User Steve Borho <steve at borho.org>
>>> # Date 1308255977 18000
>>> # Node ID 4196f029b6b4a1f05dd0eac9a9df9d19424af4cc
>>> # Parent  ba3c36cea66e7a9cc0bb274b7f7b73ad65212df7
>>> tags: don't allow environment errors to be raised from _writetagscache
>>>
>>> See https://bitbucket.org/tortoisehg/thg/issue/719
>>
>> What is an "environment error"?
> 
> EnvironmentError is the base class of IOError and OSError.
> 
> http://docs.python.org/library/exceptions.html

ok

>>> diff -r ba3c36cea66e -r 4196f029b6b4 mercurial/tags.py
>>> --- a/mercurial/tags.py       Thu Jun 16 14:33:06 2011 -0500
>>> +++ b/mercurial/tags.py       Thu Jun 16 15:26:17 2011 -0500
>>> @@ -286,4 +286,7 @@
>>>      for (name, (node, hist)) in cachetags.iteritems():
>>>          cachefile.write("%s %s\n" % (hex(node), name))
>>>
>>> -    cachefile.rename()
>>> +    try:
>>> +        cachefile.rename()
>>> +    except (OSError, IOError):
>>> +        pass
>>
>> This still feels like papering over a problem not clearly understood (at
>> least on my part).
> 
> It could be, but this still leads to exceptions being raised from just
> about any API call.  The tags cache is updated from a lot of common

..

> code paths.  We already capture and ignore exceptions when we create
> the tempfile (failure to write the cache file is not a fatal error).

Yes, this capture is fine and it should cover the later rename already
for the cases we cover with that (not being able to write to the target
directory in general).

> This is just acknowledging that on Windows it is often the rename that
> throws the error.

I haven't seen a conclusive theory yet. And I would like to see one
before papering over that error and risking later getting complaints
that Mercurial is slow. Later discovering that the cache is defunct
*because* we papered over the error is much harder.

>> And, in total, I've pondered about file access on Windows for quite a
>> while now.
> 
> It is most likely a bad virus checker, but the people who file these
> bugs are quite un-responsive to questions.  I've lost count of how
> many of these we've received (bitbucket search finds 8 issues, but
> some bug reports are attached and aren't searchable).

The problem I have is the term "most likely". I think it's a sign that
we haven't understood the real cause of the problem yet.

Today, there are two camps of virus checkers on Windows

(A) opening files with shared delete flag (allows renames and unlinks)
(B) opening files without or locking them by mapping into RAM

My goal is to make Mercurial work fine in the presence of (A) and I have
even installed one of these here myself for that purpose and I've never
seen that traceback here.

Mercurial is already pretty much compatible with class (A) checkers and
claiming such a problem is a problem with a virus checker doesn't work
that well any more these days (with current Mercurial).

At least I would like to see what we get with 1.9 on that front, that
is, releasing 1.9 without papering over this problem. Or I would like to
see a theory why the error happens before papering over it.