[PATCH STABLE] largefiles: access to specific fields only if largefiles enabled (issue4547)

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Mon Mar 2 05:01:19 CST 2015


At Mon, 02 Mar 2015 01:30:47 -0600,
Matt Mackall wrote:
> 
> On Thu, 2015-02-26 at 06:06 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1424898219 -32400
> > #      Thu Feb 26 06:03:39 2015 +0900
> > # Branch stable
> > # Node ID 8049e948fa335fb6cfcce88197dc9963cf4698b7
> > # Parent  942a5a34b2d0611ab284380fbe45b9bb1897af98
> > largefiles: access to specific fields only if largefiles enabled (issue4547)
> 
> When I merged this into stable, I got excessive recursion depth in
> test-largefiles-misc.t. I ended up pushing it anyway because I'd already
> pushed out stable and was unlikely to figure out what was going on on my
> own. Please take a look.

Until my this patch, there is no test with subrepos in which
largefiles is disabled locally.

In such case, "openlfdirstate" invocation in "overriderevert"
introduced by 79c2c29c71ae causes "excessive recursion depth", because
"openlfdirstate" invokes already overridden "scmutil.match" when there
is no "lfdirstate" file: "disabling largefiles" also satisfies this
condition.

  --- a/hgext/largefiles/overrides.py
  +++ b/hgext/largefiles/overrides.py
  @@ -716,10 +716,17 @@ def overriderevert(orig, ui, repo, *pats
                   default='relpath'):
               match = oldmatch(ctx, pats, opts, globbed, default)
               m = copy.copy(match)
  +
  +            # revert supports recursing into subrepos, and though largefiles
  +            # currently doesn't work correctly in that case, this match is
  +            # called, so the lfdirstate above may not be the correct one for
  +            # this invocation of match.
  +            lfdirstate = lfutil.openlfdirstate(ctx._repo.ui, ctx._repo)
  +
               def tostandin(f):
                   if lfutil.standin(f) in ctx:
                       return lfutil.standin(f)
  -                elif lfutil.standin(f) in repo[None]:
  +                elif lfutil.standin(f) in repo[None] or lfdirstate[f] == 'r':
                       return None
                   return f
               m._files = [tostandin(f) for f in m._files]


IMHO, we should do:

(1) avoid invocation of "overriding 'match' function" in repos
    disabling largefiles for efficiency

  ====================
  diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
  --- a/hgext/largefiles/overrides.py
  +++ b/hgext/largefiles/overrides.py
  @@ -61,8 +61,13 @@ def installmatchfn(f):
       '''monkey patch the scmutil module with a custom match function.
       Warning: it is monkey patching the _module_ on runtime! Not thread safe!''$
       oldmatch = scmutil.match
  -    setattr(f, 'oldmatch', oldmatch)
  -    scmutil.match = f
  +    def switchmatch(ctx, *args, **kwargs):
  +        if not util.safehasattr(ctx._repo, '_largefilesenabled'):
  +            return oldmatch(ctx, *args, **kwargs)
  +        else:
  +            return f(ctx, *args, **kwargs)
  +    setattr(switchmatch, 'oldmatch', oldmatch)
  +    scmutil.match = switchmatch
       return oldmatch
  
   def restorematchfn():
  ====================

(2) avoid accidental recursive invocation

  Even if largefiles is enabled in the repository, unexpected missing
  lfdirstate file also causes "getstandinmatcher()" invocation in
  "openlfdirstate()".

  To avoid accidental recursive invocation in such case, we have to
  pass "False" as "create" to "openlfdirstate()".

  ====================
  diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
  --- a/hgext/largefiles/overrides.py
  +++ b/hgext/largefiles/overrides.py
  @@ -721,7 +721,7 @@ def overriderevert(orig, ui, repo, *pats
               # currently doesn't work correctly in that case, this match is
               # called, so the lfdirstate above may not be the correct one for
               # this invocation of match.
  -            lfdirstate = lfutil.openlfdirstate(ctx._repo.ui, ctx._repo)
  +            lfdirstate = lfutil.openlfdirstate(ctx._repo.ui, ctx._repo, False)
  
               def tostandin(f):
                   if lfutil.standin(f) in ctx:
  ====================


> -- 
> Mathematics is the supreme nostalgia of our time.
> 
> 
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp


More information about the Mercurial-devel mailing list