[PATCH STABLE] match: fix a caseonly rename + explicit path commit on icasefs (issue4768)

[Matt Harbison]

On Mon, 10 Aug 2015 03:10:03 -0400, Pierre-Yves David  
<pierre-yves.david at ens-lyon.org> wrote:

>
>
> On 08/06/2015 07:15 PM, Matt Harbison wrote:
>> On Thu, 06 Aug 2015 22:03:09 -0400, Pierre-Yves David
>> <pierre-yves.david at ens-lyon.org> wrote:
>>
>>>
>>>
>>> On 08/06/2015 06:56 PM, Matt Harbison wrote:
>>>> # HG changeset patch
>>>> # User Matt Harbison <matt_harbison at yahoo.com>
>>>> # Date 1438909216 14400
>>>> #      Thu Aug 06 21:00:16 2015 -0400
>>>> # Branch stable
>>>> # Node ID c54ca3f987fd1eaaba3a26d62806270fff535346
>>>> # Parent  79f0cb97d7537a7c2948f8f9b0a89148825a3a1d
>>>> match: fix a caseonly rename + explicit path commit on icasefs
>>>> (issue4768)
>>>>
>> [snip]
>>>>
>>>> diff --git a/mercurial/match.py b/mercurial/match.py
>>>> --- a/mercurial/match.py
>>>> +++ b/mercurial/match.py
>>>> @@ -386,7 +386,8 @@
>>>>       def __init__(self, root, cwd, patterns, include, exclude,
>>>> default, auditor,
>>>>                    ctx, listsubrepos=False, badfn=None):
>>>>           init = super(icasefsmatcher, self).__init__
>>>> -        self._dsnormalize = ctx.repo().dirstate.normalize
>>>> +        self._dirstate = ctx.repo().dirstate
>>>
>>> Can we keep a dirstate reference in the matcher without trouble?
>>
>> It's safe for the icasefsmatcher class because that is only created
>> inside workingctx, so the optional ctx arg is always given.  But there
>> are some creators of matcher that don't pass ctx, so we should probably
>> avoid a maybe-its-valid-or-maybe-not member there.  (Not to mention that
>> the base matcher may not be for a wdir()).
>
> So, I'm now more confused, I initially asked the question with two  
> possible issue in mind:
>
> - reference cycle,
> - layer violation,
>
> But reading your reply, it seems like the code in your patch "Is working  
> fine now" but unprotected againt other (valid) way to call this code  
> that could crash.
>
> Did I got this right? If so, this looks like a bad idea, isn't it?

I misunderstood then- I thought you were nudging me to move dirstate into  
the base matcher class for some reason.

But basically, what Yuya said.  I originally did all of this normalizing  
in dirstate [1], but sid0 had performance concerns.  I don't think there's  
any real danger of this special matcher class being used anywhere else  
though.

[1] https://selenic.com/pipermail/mercurial-devel/2015-April/068183.html