[PATCH STABLE] dirstate: ignore symlinks when fs cannot handle them (issue1888)

[Martin Geisler]

Matt Mackall <mpm at selenic.com> writes:

> On Fri, 2010-07-23 at 15:33 +0200, Martin Geisler wrote:
>> Matt Mackall <mpm at selenic.com> writes:
>> 
>> > On Thu, 2010-07-22 at 14:20 +0200, Martin Geisler wrote:
>> >> # HG changeset patch
>> >> # User Martin Geisler <mg at aragost.com>
>> >> # Date 1279801072 -7200
>> >> # Branch stable
>> >> # Node ID 16b70e8b69d3175079fc34857890e483bf37f480
>> >> # Parent  91af149b5cd72dc91c1e3ae4ee018caf7203323e
>> >> dirstate: ignore symlinks when fs cannot handle them (issue1888)
>> >
>> > Looks good.
>> 
>> I'm afraid the patch is buggy -- this line
>> 
>>   not (mode & lnkkind) or self._checklink
>> 
>> should be
>> 
>>   getkind(mode) != lnkkind or self._checklink)
>
> That's just this:
>
> def S_IFMT(mode):
>     return mode & 0170000
>
> So perhaps (mode & lnkkind != lnkkind) is sufficient.

Yes, that works! The performance penalty is now pretty benign as shown
here where I ran 'hg perfstatus' 10 times:

  Before:          After:
  min: 0.544703    min: 0.546549
  med: 0.547592    med: 0.548881
  avg: 0.549146    avg: 0.548549
  max: 0.564112    max: 0.551504

The median time is increased about 0.24%. I pushed the patch as
changeset ca6cebd8734e.

I also played around with a version of the patch that did

  not (mode & 0020000)

to check if the file is not a symlink. The 0020000 mask will also hit
character and block devices, but since we don't version those, only
symlinks are left. It was not faster, though, so I went with the more
straight forward version you suggested.

-- 
Martin Geisler

aragost Trifork
Professional Mercurial support
http://aragost.com/mercurial/