[PATCH STABLE] largefiles: access to specific fields only if largefiles enabled (issue4547)
Matt Harbison
mharbison72 at gmail.com
Mon Mar 2 19:58:10 CST 2015
On Mon, 02 Mar 2015 08:42:32 -0500, FUJIWARA Katsunori
<foozy at lares.dti.ne.jp> wrote:
> At Mon, 02 Mar 2015 20:01:19 +0900,
> FUJIWARA Katsunori wrote:
>>
>> 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
Is it just the monkey patched scmutil.match() that needs to be avoided?
It looks like revert and copy are the only things left that use it. I'm
not sure how to avoid the largefile tweaking done to the other matchers
that are passed in via narrowmatcher() (though maybe that's harmless).
>> ====================
>> 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():
>> ====================
>
> I'm also working to override "cmdutil.revert" instead of
> "commands.revert" for backout, histedit, shelve and
> hgsubrepo.filerevert invoking the former directly.
+1 on that, and thanks for mentioning it, because I was toying with that
too. I think everything I did so far has been landed, except an
unpolished change to pass matchers instead of pattern lists to
cmdutil.revert(). If you need it, I can toss it out on the ML. There's
some other stuff I'm working on in the next few days anyway, and I don't
want to disrupt you from fixing this.
I'm a bit baffled by the revert implementation. It just clicked now that
subrepo.filerevert --all hardcodes 'set:modified()' solely to generate the
backup files. I still don't understand why it is changing the subrepo
state in hgsubrepo.revert(). It seems it should only be touching files,
not potentially changing branches, etc. Not that any of this is specific
to largefiles, but maybe it needs consideration when changing pats ->
matcher on cmdutil.revert()?
--Matt
> And I noticed that it causes nested wrapping match function
> unexpectedly in nested subrepositories.
>
> I'll post changes (1) above or so, after more investigation around
> "cmdutil.revert" and "installmatchfn".
>
> On the other hand, I'll post changes (2) below soon, to pass
> test-largefiles-misc.t successfully.
>
>
>> (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
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
>
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori] foozy at lares.dti.ne.jp
More information about the Mercurial-devel
mailing list