[PATCH 2 of 2] keyword: eliminate potential reference cycles from kwrepo

[Simon Heimberg]

I do not understand the whole thread. I try to point out some things I
think I do understand. My comments are below.

Greetings,
Simon

Am Sonntag, den 05.07.2009, 10:40 +0000 schrieb Christian Ebert:
> # HG changeset patch
> # User Christian Ebert <blacktrash at gmx.net>
> # Date 1246790211 -7200
> # Node ID c26ebe8f48b8c4c2b504d9c467d313f30be76fba
> # Parent  fbe2c25a36131fac923b25334a4e576991e64557
> keyword: eliminate potential reference cycles from kwrepo
> 
> Reference member functions of repo.__class__ consistently in
> super(kwrepo, self) at class level.
> Therefore we also need dumb wrappers around sopener and commitctx.
> 
> diff --git a/hgext/keyword.py b/hgext/keyword.py
> --- a/hgext/keyword.py
> +++ b/hgext/keyword.py
> @@ -450,6 +450,12 @@
>      kwtools['templater'] = kwt = kwtemplater(ui, repo)
>  
>      class kwrepo(repo.__class__):
> +        def sopener(self, *args):
> +            return super(kwrepo, self).sopener(*args)
> +
> +        def commitctx(self, *args):
> +            return super(kwrepo, self).commitctx(*args)
> +

I agree with mgeisler that this is a noop. This is what python does
automatically. When self.sopener is called, it is not found in kwrepo
class. In this case the one in the parent class is used. (This is what
inheritance is for.)

>          def file(self, f):
>              if f[0] == '/':
>                  f = f[1:]
> @@ -461,15 +467,15 @@
>  
>          def commit(self, *args, **opts):
>              # use custom commitctx for user commands
> -            # other extensions can still wrap repo.commitctx directly
> -            repo.commitctx = self.kwcommitctx
> +            # other extensions can still wrap commitctx directly
> +            self.commitctx = self.kwcommitctx
>              return super(kwrepo, self).commit(*args, **opts)

what about replacing the last line above with this lines?
               r = super(kwrepo, self).commit(*args, **opts)
               del self.commitctx
               return r

The entry in the object is deleted, so the original method would be
called on the next call. The reference cycle to self is broken.
Maybe it is even better to do the following for breaking the reference
cycle in the case of an error.
               try:
                   r = super(kwrepo, self).commit(*args, **opts)
               finally:
                   del self.commitctx
               return r

>  
>          def kwcommitctx(self, ctx, error=False):
>              wlock = lock = None
>              try:
> -                wlock = self.wlock()
> -                lock = self.lock()
> +                wlock = super(kwrepo, self).wlock()
> +                lock = super(kwrepo, self).lock()

use self.wlock(). wlock is not found in kwrepo, so the one in the parent
class is used automatically.

>                  # store and postpone commit hooks
>                  commithooks = {}
>                  for name, cmd in ui.configitems('hooks'):
> @@ -487,7 +493,8 @@
>                  if commithooks:
>                      for name, cmd in commithooks.iteritems():
>                          ui.setconfig('hooks', name, cmd)
> -                    repo.hook('commit', node=n, parent1=xp1, parent2=xp2)
> +                    super(kwrepo, self).hook('commit',
> +                                             node=n, parent1=xp1, parent2=xp2)

use self here too. (See above.)

>                  return n
>              finally:
>                  release(lock, wlock)