[PATCH] keyword: make repo.commit use a custom commitctx wrapper

[Christian Ebert]

* Matt Mackall on Thursday, July 02, 2009 at 09:40:44 -0500
> On Thu, 2009-07-02 at 11:18 +0200, Christian Ebert wrote:
>> * Christian Ebert on Tuesday, June 30, 2009 at 10:13:28 -0000
>>> # HG changeset patch
>>> # User Christian Ebert <blacktrash at gmx.net>
>>> # Date 1246354203 -7200
>>> # Node ID 02f1d60fe9bf5274d5b4cf409963815694c24338
>>> # Parent  bacb06d1e8b60b6083e3139fc5fb23355612eef8
>>> keyword: make repo.commit use a custom commitctx wrapper
>> 
>> <snip>
>> 
>> A question about my own patch ;-)
>> 
>>> @@ -460,8 +461,14 @@
>>> 
>>>        def commit(self, text='', user=None, date=None, match=None,
>>>                   force=False, editor=None, extra={}):
>>> +            # use custom commitctx for user commands
>>> +            # other extensions can still wrap repo.commitctx directly
>>> +            repo.commitctx = self.kwcommitctx
>> 
>> The above line works also as:
>> 
>>             self.commitctx = self.kwcommitctx
>> 
>> Background:
>> 
>> In reposetup(ui, repo) I override the repo class:
>> 
>>    class kwrepo(repo.__class__):
>> 
>> and later on do things like:
>>   
>>        lock = self.wlock()
>> 
>> even though wlock is not defined in kwrepo. Playing around with
>> pylint, it complained about that "kwrepo has no wlock member",
>> but felt better after I changed the above to
>> 
>>        lock = repo.wlock()
>> 
>> as it would have complained if I had
>> 
>>             self.commitctx = self.kwcommitctx
>> 
>> instead of
>> 
>>             repo.commitctx = self.kwcommitctx
>> 
>> in my patch. 
>> 
>> mq does something similar and uses self consistently (with pylint
>> complaining accordingly). Which is the way to go? Is there a
>> difference wrt to safety? According to test-keyword it doesn't
>> make a difference, but which way should I choose for consistency?
> 
> See cset 5726bb290bfe.

Will try to understand the relevance of this wrt to my question
in the next few days/many months ;-)

> However, member functions turn out to be a special issue, see
> 5726bb290bfe. When you do:
> 
> a = object.func
> 
> you're effectively doing:
> 
> a = lambda *args, **kwargs: object.func(*args, **kwargs)
> 
> which binds a reference to object up inside of a. So when you do:
> 
> self.func2 = self.func1
> 
> you're effectively making a reference cycle inside self and making self
> immortal. A kwrepo wrapping a bundlerepo will thus cause problems
> because bundlerepo won't be able to clean up after itself. So how do we
> wrap an object's method without introducing a cycle? The best way I know
> is to do it at the class level:
> 
> class a(b.__class__):
>    def func(self, *args):
>       super(a, self).func(*args)

Yup. I use that quite often already, but not consistently (mq
doesn't either btw).

A patch for keyword making consistency corrections would look
like this:


# HG changeset patch
# User Christian Ebert <blacktrash at gmx.net>
# Date 1246554781 -7200
# Node ID 8a46927ec390ae31c7a90875dc2837f8e952af47
# Parent  c24c9ce0cdcf74e8377d4e0c5e177682f3d76c32
keyword: use super(kwrepo, self) whereever possible

diff --git a/hgext/keyword.py b/hgext/keyword.py
--- a/hgext/keyword.py
+++ b/hgext/keyword.py
@@ -453,7 +453,7 @@
         def file(self, f):
             if f[0] == '/':
                 f = f[1:]
-            return kwfilelog(self.sopener, kwt, f)
+            return kwfilelog(repo.sopener, kwt, f)
 
         def wread(self, filename):
             data = super(kwrepo, self).wread(filename)
@@ -470,8 +470,8 @@
         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()
                 # store and postpone commit hooks
                 commithooks = {}
                 for name, cmd in ui.configitems('hooks'):
@@ -489,7 +489,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)
                 return n
             finally:
                 release(lock, wlock)


Now look at the sopener thing; the following breaks (changed from
self.opener to repo.sopener above):

class kwrepo(repo.__class__):
        def file(self, f):
            if f[0] == '/':
                f = f[1:]
            return kwfilelog(self.wrapsopener, kwt, f)

        def wrapsopener(self, *args):
            return super(kwrepo, self).sopener(*args)

I know, naïve, sopener comes from store and it's used as an
attribute. And of course the line that sort of triggered my
confusion:

            repo.commitctx = self.kwcommitctx

I don't see how I could do this with super.

Still puzzled

c