Recursion after installing in Trac 0.13 (dev)

patch for recursion error

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 .

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 .

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 ;)

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

    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

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.

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

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 .

Index: trunk/setup.py
===================================================================
--- trunk/setup.py	(revision 12163)
+++ trunk/setup.py	(working copy)
@@ -14,7 +14,7 @@
 
 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' ] },
Index: trunk/themeengine/api.py
===================================================================
--- trunk/themeengine/api.py	(revision 12163)
+++ trunk/themeengine/api.py	(working copy)
@@ -13,6 +13,11 @@
 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,16 +72,27 @@
     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 .

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?

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 .

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.

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

  old	new
  13	13	from trac.core import *
  14	14	from trac.config import Option
  15	15
  	16	try:
  	17	from trac.util import lazy
  	18	except ImportError:
  	19	lazy = None
  	20
  16	21	class ThemeNotFound(TracError):
  17	22	"""The requested theme isn't found."""
  18	23
  …	…
  67	72	providers = ExtensionPoint(IThemeProvider)
  68	73
  69	74	def __init__(self):
  70		# This can safely go in here because the data can only change on a restart anyway
  71		self.info = {}
  	75	if lazy is None:
  	76	# Trac < 1.0: this can safely go in here because the data can
  	77	# only change on a restart anyway
  	78	self.info = self.info()
  	79
  	80	def info(self):
  	81	# Trac >= 1.0 : Hack needed to deal with infinite recusrion error
  	82	#    Details : http://trac-hacks.org/ticket/9580#comment:1
  	83	info = {}
  72	84	for provider in self.providers:
  73	85	for name in provider.get_theme_names():
  74	86	theme = provider.get_theme_info(name)
  75	87	theme['provider'] = provider
  76	88	theme['module'] = provider.__class__.__module__
  77	89	theme['name'] = name
  78		self.info[name.lower()] = theme
  79
  	90	info[name.lower()] = theme
  	91	return info
  	92
  	93	if lazy is not None:
  	94	info = lazy(info)
  	95
  80	96	# IThemeProvider methods
  81	97	def get_theme_names(self):
  82	98	yield 'default'

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