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

Yuya Nishihara yuya at tcha.org
Mon Aug 10 10:16:07 UTC 2015


On Mon, 10 Aug 2015 00:10:03 -0700, Pierre-Yves David 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?

There's already a reference to dirstate.normalize, which is a bound method
of dirstate, so I think this patch won't make things worse.



More information about the Mercurial-devel mailing list