Opened 5 years ago

Closed 4 years ago

# Recursion after installing in Trac 0.13 (dev)

Reported by: Owned by: gary.martin@… olemis normal ThemeEnginePlugin normal rjollos 0.12

### Description

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.

### Changed 5 years ago by gary.martin@…

patch for recursion error

### comment:1 Changed 5 years ago by olemis

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 .

### comment:2 Changed 5 years ago by 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 .

### comment:3 Changed 5 years ago by olemis

• Resolution set to wontfix
• Status changed from new to closed

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 .

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

### comment:4 Changed 5 years ago by anonymous

• Resolution wontfix deleted
• Status changed from closed to reopened

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

### comment:5 follow-up: ↓ 7 Changed 4 years ago by 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


### comment:6 Changed 4 years ago by rjollos

• Cc rjollos added; anonymous removed

### comment:7 in reply to: ↑ 5 Changed 4 years ago by gary.martin@…

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.

### comment:8 Changed 4 years ago by 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


### comment:9 follow-up: ↓ 15 Changed 4 years ago by olemis

• Owner changed from coderanger to olemis
• Status changed from reopened to new

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

 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

 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.""" 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 .

### comment:14 Changed 4 years ago by olemis

• Status changed from new to assigned

### comment:15 in reply to: ↑ 9 ; follow-up: ↓ 16 Changed 4 years ago by rjollos

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?

### comment:16 in reply to: ↑ 15 ; follow-up: ↓ 17 Changed 4 years ago by 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 .

### comment:17 in reply to: ↑ 16 Changed 4 years ago by rjollos

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.

### comment:18 Changed 4 years ago by rjollos

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

 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.""" 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'

### comment:15 follow-up: ↓ 16 Changed 4 years ago by olemis

• Resolution set to fixed
• Status changed from assigned to closed

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