Recursion after installing in Trac 0.13 (dev)

[gary.martin@…]

A change in the component system for 0.13 appears to have broken the ThemeEnginePlugin such that ThemeEngineSystem.__init__() is called recursively. The attached patch stops the ThemeEngineSystem class from implementing IThemeProvider and separates out the code relating to the default theme into its own class.

[gary.martin@…]

patch for recursion error

[Olemis Lang]

Recently I tried to install this plugin in 0.13dev and got similar results i.e. it fails at load time . Fact is that this happens due to the changes committed in 0.12 (more precisely ​trac:changeset:10298 and ​trac:changeset:10300 . The whole story is explained in ​trac:ticket:9418 ;)

I'll be reporting this in t.e.o. issue tracker . I suggest to close this ticket as wontfix .

[anonymous]

The comment above is not accurate. Patch attached to ​trac:ticket:9418 was committed in ​trac:changeset:10295 . Revisions mentioned above add further changes on top of it .

[Olemis Lang]

As long as there's no definitive solution provided by trac-dev , users experiencing this issue can try to apply this patch to Trac distribution .

​https://bitbucket.org/olemis/trac-mq/raw/tho_themeengine/tho/themeengine/t-9580-infinite-recursion.diff

This bug is definitely caused by Trac core rather than the plugin , hence I'm closing the ticket with wontfix (... at least for the moment ;)

[anonymous]

Reopening ticket as this won't be fixed neither in 0.13 nor even later :-/

[anonymous]

    def __init__(self):
        self._info = None

    @property
    def info(self):
        # This can safely be cached because the data can only change on a restart anyway
        if self._info is None:
            self._info = {}
            for provider in self.providers:
                for name in provider.get_theme_names():
                    theme = provider.get_theme_info(name)
                    theme['provider'] = provider
                    theme['module'] = provider.__class__.__module__
                    theme['name'] = name
                    self._info[name.lower()] = theme
        return self._info

[gary.martin@…]

Replying to anonymous

That looks like a really nice solution to me. The idea of deferring calculation of ThemeEngineSystem.info seems sound and a few quick tests suggest to me that it should work.

[anonymous]

​As suggested by cboos you could use @lazy since Trac 1.0 (​or by copying it):

from trac.util import lazy
...
    @lazy 
    def info(self):
        result = {}
        for provider in self.providers:
            for name in provider.get_theme_names():
                theme = provider.get_theme_info(name)
                theme['provider'] = provider
                theme['module'] = provider.__class__.__module__
                theme['name'] = name
                result[name.lower()] = theme
        return result

[Olemis Lang]

I'm proposing the following patch against trunk (see r12162 ) to make it work with Trac >= 0.11 . I'm also raising minor version number (i.e. 2.0.1 => 2.1.0 ) to delimit the exact point before and after Trac >= 0.12 compatibility .

trunk/setup.py

@@ -14 +14 @@
 
 setup(
     name = 'TracThemeEngine',
-    version = '2.0.1',
+    version = '2.1.0',
     packages = ['themeengine'],
     package_data = { 'themeengine': ['templates/*.html', 'htdocs/*.js', 'htdocs/*.css', 'htdocs/img/*.gif',
                                      'htdocs/farbtastic/*.png', 'htdocs/farbtastic/*.js', 'htdocs/farbtastic/*.css' ] },

trunk/themeengine/api.py

@@ -13 +13 @@
 from trac.core import *
 from trac.config import Option
 
+try:
+    from trac.util import lazy
+except ImportError:
+    lazy = None
+
 class ThemeNotFound(TracError):
     """The requested theme isn't found."""
     
@@ -67 +72 @@
     providers = ExtensionPoint(IThemeProvider)
     
     def __init__(self):
-        # This can safely go in here because the data can only change on a restart anyway
-        self.info = {}
+        if lazy is None:
+            # Trac < 0.12 : this can safely go in here because the data can 
+            # only change on a restart anyway
+            self.info = self.info()
+
+    def info(self):
+        # Trac >= 0.12 : Hack needed to deal with infinite recusrion error
+        #    Details : http://trac-hacks.org/ticket/9580#comment:1
+        info = {}
         for provider in self.providers:
             for name in provider.get_theme_names():
                 theme = provider.get_theme_info(name)
                 theme['provider'] = provider
                 theme['module'] = provider.__class__.__module__
                 theme['name'] = name
-                self.info[name.lower()] = theme
-                
+                info[name.lower()] = theme
+        return info
+
+    if lazy is not None:
+        info = lazy(info)
+
     # IThemeProvider methods
     def get_theme_names(self):
         yield 'default'

I'm using lazy instead of property because computation needs to be preformed just once . I'll commit this solution immediately after I confirm that everything will work with aforementioned versions . I've only tested modifications against Trac 0.13dev-r11046 so far . Anyway feedback is welcome .

[Ryan J Ollos]

Replying to olemis:

I'm proposing the following patch against trunk (see r12162 ) to make it work with Trac >= 0.11 . I'm also raising minor version number (i.e. 2.0.1 => 2.1.0 ) to delimit the exact point before and after Trac >= 0.12 compatibility .

Isn't the patch actually fixing Trac >= 1.0 compatibility? The lazy class wasn't available until 1.0 anyway, so on Trac < 1.0, lazy will evaluate to None in __init__.py, and it looks like you'll be falling back to the old way of initializing the attribute. I think this part of the patch is fine, just the comments need to be updated to change reference to 0.12 to 1.0.

I'm using lazy instead of property because computation needs to be preformed just once.

This is not an aspect of Python that I've used a lot, but can't you use lazy as a decorator, as suggest in comment:8?

[Olemis Lang]

Replying to rjollos:

Replying to olemis:

I'm proposing the following patch against trunk (see r12162 ) to make it work with Trac >= 0.11 . I'm also raising minor version number (i.e. 2.0.1 => 2.1.0 ) to delimit the exact point before and after Trac >= 0.12 compatibility .

Isn't the patch actually fixing Trac >= 1.0 compatibility?

It seems plugin should fail also with Trac=0.12 , so that's something to consider as well .

The lazy class wasn't available until 1.0 anyway, so on Trac < 1.0, lazy will evaluate to None in __init__.py, and it looks like you'll be falling back to the old way of initializing the attribute.

Yes I just checked few minutes ago and it seems lazy is not available in 0.12 . So I'll wait for a while before commiting, at least until somebody actually confirms whether this bug can still be reproduced with Trac=0.12 or not .

I think this part of the patch is fine, just the comments need to be updated to change reference to 0.12 to 1.0.

The solution should cover them all 0.11 , 0.12 and 1.0 . Besides I don't want to include copies of lazy nor any other function / class / ... defined in Trac .

I'm using lazy instead of property because computation needs to be preformed just once.

This is not an aspect of Python that I've used a lot, but can't you use lazy as a decorator, as suggest in comment:8?

No , because that will fail for Trac<=0.11 .

[Ryan J Ollos]

Replying to olemis:

This is not an aspect of Python that I've used a lot, but can't you use lazy as a decorator, as suggest in comment:8?

No , because that will fail for Trac<=0.11 .

Okay, yes, I should have noticed that. But you must mean Trac < 1.0, because as you mentioned earlier, the lazy class was introduced in 1.0.

[Ryan J Ollos]

I've tested out the ThemeEnginePlugin with PyTppThemePlugin, and ThemeEnginePlugin 2.0.1 seems to work fine on Trac 0.12. I've also tested after the patch for ThemeEnginePlugin 2.1.0 has been applied and that works fine with Trac 0.12. ​t:#9418 has the milestone of 1.0 and none of the changes were merged to ​0.12-stable, so I think this all makes sense. The issue does not seem to exist in Trac 0.12, and was introduced with Trac 1.0.

Finally, I've tested with Trac 1.0 just to confirm that I can reproduce the issue and wasn't overlooking something when testing with Trac 0.12. I don't see how I could be overlooking the presence of this issue ;)

My only suggestion is to change the two comments in the patch to make reference to Trac 1.0 rather than Trac 0.12.

trunk/themeengine/api.py

@@ -13 +13 @@
 from trac.core import *
 from trac.config import Option
 
+try:
+    from trac.util import lazy
+except ImportError:
+    lazy = None
+
 class ThemeNotFound(TracError):
     """The requested theme isn't found."""
     
@@ -67 +72 @@
     providers = ExtensionPoint(IThemeProvider)
     
     def __init__(self):
-        # This can safely go in here because the data can only change on a restart anyway
-        self.info = {}
+        if lazy is None:
+            # Trac < 1.0: this can safely go in here because the data can 
+            # only change on a restart anyway
+            self.info = self.info()
+
+    def info(self):
+        # Trac >= 1.0 : Hack needed to deal with infinite recusrion error
+        #    Details : http://trac-hacks.org/ticket/9580#comment:1
+        info = {}
         for provider in self.providers:
             for name in provider.get_theme_names():
                 theme = provider.get_theme_info(name)
                 theme['provider'] = provider
                 theme['module'] = provider.__class__.__module__
                 theme['name'] = name
-                self.info[name.lower()] = theme
-                
+                info[name.lower()] = theme
+        return info
+
+    if lazy is not None:
+        info = lazy(info)
+
     # IThemeProvider methods
     def get_theme_names(self):
         yield 'default'

[Olemis Lang]

(In [12173]) ThemeEnginePlugin: Fixes #9580 infinite recursion issue in Trac=1.0