[PATCH] tags: don't allow environment errors to be raised from _writetagscache
adrian at cadifra.com
Fri Jun 17 02:07:43 CDT 2011
On 2011-06-17 08:54, Adrian Buehlmann wrote:
> 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.
>>>> 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).
I forgot to add that I *am* reading the TortoiseHg bugreports coming in
and the tracebacks pasted in particular.
It's not like we get reports of this class of error on a daily basis and
it doesn't really feel like we get tens of them per month - Even though
I don't have statistics here myself.
Compare this with the >14000 downloads for the thg x64 installer alone
(in two weeks).
> 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.
More information about the Mercurial-devel