[PATCH 02 of 10] localrepo: introduce "checkoutgoing()" to extend outgoing revision check easily

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Fri Mar 7 09:59:24 CST 2014


At Thu, 06 Mar 2014 15:48:31 +0100,
Mads Kiilerich wrote:
> 
> On 03/05/2014 01:12 PM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1394019743 -32400
> > #      Wed Mar 05 20:42:23 2014 +0900
> > # Node ID 202d835ca3a173f808262677fccc796257945268
> > # Parent  655cacfc3311c174c995e4bab13755b498cb43dc
> > localrepo: introduce "checkoutgoing()" to extend outgoing revision check easily
> >
> > diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> > --- a/mercurial/exchange.py
> > +++ b/mercurial/exchange.py
> > @@ -103,6 +103,7 @@
> >           try:
> >               _pushdiscovery(pushop)
> >               if _pushcheckoutgoing(pushop):
> > +                pushop.repo.checkoutgoing(pushop.remote, pushop.outgoing)
> >                   _pushchangeset(pushop)
> >               _pushcomputecommonheads(pushop)
> >               _pushsyncphase(pushop)
> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > --- a/mercurial/localrepo.py
> > +++ b/mercurial/localrepo.py
> > @@ -1677,6 +1677,15 @@
> >           """
> >           pass
> >   
> > +    def checkoutgoing(self, remote, outgoing):
> 
> Yes, pretty please, let's get some nice hooks so we can get rid of some 
> of the ugly hacks largefiles had to use when it was a standalone extension.
> 
> The 'check' name indicates to me that it can return a check value ... 
> which it can't.
> 
> To me it sounds more like a 'hook'. Could we call it something like 
> 'prepushchangesethook'? I don't know if it could be some kind of 'real' 
> hooks like the other ones we have. If not, could we not just
> override _pushchangeset or _pushcheckoutgoing instead?

I just followed the style of existing hook point "checkpush()" in
localrepository class.

    def checkpush(self, force, revs):
        """Extensions can override this function if additional checks have
        to be performed before pushing, or call it if they override push
        command.
        """
        pass

'prepushchangesethook' seems to be good, at least for me.

IMHO, overriding "_pushchangeset()" or "_pushcheckoutgoing()" needs
checking whether source repo is lfilesrepo or not, and looks worse
than adding hook point(s) to localrepository class.


BTW, hook function overriding requires overrider classes to invoke
"super(overrider, self).hookfunc(...)" correctly to follow chain of
overridings. But it can be broken easily and silently by careless
extensions.

Should I add new variable instantiated from "util.hooks" (like
"cmdutil.summaryhooks") instead of adding new hook method to be
overridden ?


> In general, largefiles and subrepos have very similar needs. I think we 
> could make both much more pretty if subrepos could get better "extension 
> points" and if largefiles started using the subrepo hooks.
> 
> /Mads
> 
> > +        """Extensions can override this function if additional checks have
> > +        to be performed on outgoing revisions before pushing, or call it
> > +        if they override push command.
> > +
> > +        'outgoing' argument is the result of 'discovery.findcommonoutgoing()'.
> > +        """
> > +        pass
> > +
> >       def push(self, remote, force=False, revs=None, newbranch=False):
> >           return exchange.push(self, remote, force, revs, newbranch)
> >   
> > _______________________________________________
> > 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