| Submitter | Katsunori FUJIWARA |
|---|---|
| Date | Feb. 25, 2015, 9:06 p.m. |
| Message ID | <8049e948fa335fb6cfcc.1424898406@feefifofum> |
| Download | mbox | patch |
| Permalink | /patch/7830/ |
| State | Accepted |
| Commit | d414c28db84d592f9f27c683414bc02d18a5e344 |
| Headers | show |
Comments
On Thu, 2015-02-26 at 06:06 +0900, FUJIWARA Katsunori wrote: > # HG changeset patch > # User FUJIWARA Katsunori <foozy@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) Queued for stable, thanks.
On Thu, 2015-02-26 at 06:06 +0900, FUJIWARA Katsunori wrote: > # HG changeset patch > # User FUJIWARA Katsunori <foozy@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.
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@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@lares.dti.ne.jp
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@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(): > ==================== I'm also working to override "cmdutil.revert" instead of "commands.revert" for backout, histedit, shelve and hgsubrepo.filerevert invoking the former directly. 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@lares.dti.ne.jp > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel ---------------------------------------------------------------------- [FUJIWARA Katsunori] foozy@lares.dti.ne.jp
On Mon, 02 Mar 2015 08:42:32 -0500, FUJIWARA Katsunori <foozy@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@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@lares.dti.ne.jp >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@selenic.com >> http://selenic.com/mailman/listinfo/mercurial-devel > > ---------------------------------------------------------------------- > [FUJIWARA Katsunori] foozy@lares.dti.ne.jp
At Mon, 02 Mar 2015 20:58:10 -0500, Matt Harbison wrote: > > On Mon, 02 Mar 2015 08:42:32 -0500, FUJIWARA Katsunori > <foozy@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@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). Sorry for late response. "(1)" was proposed just for fixing "excessive recursion depth". I think that overriding functions other than one for "scmutil.match" should be also avoided in repos disabling largefiles for efficiency in the future. > > 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. In fact, I have to finish some other patches for largefiles before working for "overriderevert". So, you would have enough time to work for it :-) > 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. Me too, about "set:modified()" for "--all" ! > 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. Current revert implementation of hgsubrepo/gitsubrepo seems to support not "revert specified files in subrepo to specified revision" but only "revert whole subrepo to specified revision", even though "revert" of them takes "pats" argument. It may be reason why "cmdutil.revert" still doesn't take "match" argument instead of patterns: partial reverting should want "cmdutil.revert" to take "match" argument for easily narrowing. > 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: > >> ==================== ---------------------------------------------------------------------- [FUJIWARA Katsunori] foozy@lares.dti.ne.jp
Patch
diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py --- a/hgext/largefiles/lfutil.py +++ b/hgext/largefiles/lfutil.py @@ -577,7 +577,7 @@ Otherwise, this returns the function to always write out (or ignore if ``not forcibly``) status. ''' - if forcibly is None: + if forcibly is None and util.safehasattr(repo, '_largefilesenabled'): return repo._lfstatuswriters[-1] else: if forcibly: diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py --- a/hgext/largefiles/overrides.py +++ b/hgext/largefiles/overrides.py @@ -832,6 +832,9 @@ return result def overriderebase(orig, ui, repo, **opts): + if not util.safehasattr(repo, '_largefilesenabled'): + return orig(ui, repo, **opts) + resuming = opts.get('continue') repo._lfcommithooks.append(lfutil.automatedcommithook(resuming)) repo._lfstatuswriters.append(lambda *msg, **opts: None) diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py --- a/hgext/largefiles/reposetup.py +++ b/hgext/largefiles/reposetup.py @@ -24,6 +24,9 @@ return class lfilesrepo(repo.__class__): + # the mark to examine whether "repo" object enables largefiles or not + _largefilesenabled = True + lfstatus = False def status_nolfiles(self, *args, **kwargs): return super(lfilesrepo, self).status(*args, **kwargs) diff --git a/tests/test-largefiles-misc.t b/tests/test-largefiles-misc.t --- a/tests/test-largefiles-misc.t +++ b/tests/test-largefiles-misc.t @@ -907,6 +907,77 @@ $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS #endif +Test overridden functions work correctly even for repos disabling +largefiles (issue4547) + + $ hg showconfig extensions | grep largefiles + extensions.largefiles=! + +(test updating implied by clone) + + $ hg init enabled-but-no-largefiles + $ echo normal1 > enabled-but-no-largefiles/normal1 + $ hg -R enabled-but-no-largefiles add enabled-but-no-largefiles/normal1 + $ hg -R enabled-but-no-largefiles commit -m '#0@enabled-but-no-largefiles' + Invoking status precommit hook + A normal1 + $ cat >> enabled-but-no-largefiles/.hg/hgrc <<EOF + > [extensions] + > # enable locally + > largefiles= + > EOF + $ hg clone -q enabled-but-no-largefiles no-largefiles + +(test rebasing implied by pull: precommit while rebasing unexpectedly +shows "normal3" as "?", because lfdirstate isn't yet written out at +that time) + + $ echo normal2 > enabled-but-no-largefiles/normal2 + $ hg -R enabled-but-no-largefiles add enabled-but-no-largefiles/normal2 + $ hg -R enabled-but-no-largefiles commit -m '#1@enabled-but-no-largefiles' + Invoking status precommit hook + A normal2 + + $ echo normal3 > no-largefiles/normal3 + $ hg -R no-largefiles add no-largefiles/normal3 + $ hg -R no-largefiles commit -m '#1@no-largefiles' + Invoking status precommit hook + A normal3 + + $ hg -R no-largefiles -q pull --rebase + Invoking status precommit hook + ? normal3 + +(test reverting) + + $ hg init subrepo-root + $ cat >> subrepo-root/.hg/hgrc <<EOF + > [extensions] + > # enable locally + > largefiles= + > EOF + $ echo large > subrepo-root/large + $ hg -R subrepo-root add --large subrepo-root/large + $ hg clone -q no-largefiles subrepo-root/no-largefiles + $ cat > subrepo-root/.hgsub <<EOF + > no-largefiles = no-largefiles + > EOF + $ hg -R subrepo-root add subrepo-root/.hgsub + $ hg -R subrepo-root commit -m '#0' + Invoking status precommit hook + A .hgsub + A large + ? .hgsubstate + $ echo dirty >> subrepo-root/large + $ echo dirty >> subrepo-root/no-largefiles/normal1 + $ hg -R subrepo-root status -S + M large + M no-largefiles/normal1 + $ hg -R subrepo-root revert --all + reverting subrepo-root/.hglf/large (glob) + reverting subrepo no-largefiles + reverting subrepo-root/no-largefiles/normal1 (glob) + $ cd ..
# HG changeset patch # User FUJIWARA Katsunori <foozy@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) Even if largefiles extension is enabled in a repository, "repo" object, which isn't "largefiles.reposetup()"-ed, is passed to overridden functions in the cases below unexpectedly, because extensions are enabled for each repositories strictly. (1) clone without -U: (2) pull with -U: (3) pull with --rebase: combination of "enabled@src", "disabled@dst" and "not-required@src" cause this situation. largefiles requirement @src @dst @src result -------- -------- --------------- -------------------- enabled disabled not-required aborted unexpectedly required requirement error (intentional) -------- -------- --------------- -------------------- enabled enabled * success -------- -------- --------------- -------------------- disabled enabled * success (only for "pull") -------- -------- --------------- -------------------- disabled disabled not-required success required requirement error (intentional) -------- -------- --------------- -------------------- (4) update/revert with a subrepo disabling largefiles In these cases, overridden functions cause accessing to largefiles specific fields of not "largefiles.reposetup()"-ed "repo" object, and execution is aborted. - (1), (2), (4) cause accessing to "_lfstatuswriters" in "getstatuswriter()" invoked via "updatelfiles()" - (3) causes accessing to "_lfcommithooks" in "overriderebase()" For safe accessing to these fields, this patch examines whether passed "repo" object is "largefiles.reposetup()"-ed or not before accessing to them. This patch chooses examining existence of newly introduced "_largefilesenabled" instead of "_lfcommithooks" and "_lfstatuswriters" directly, because the former is better name for the generic "largefiles is enabled in this repo" mark than the latter. In the future, all other overridden functions should avoid largefiles specific processing for efficiency, and "_largefilesenabled" is better also for such purpose. BTW, "lfstatus" can't be used for such purpose, because some code paths set it forcibly regardless of existence of it in specified "repo" object.