[PATCH] - CORE - Small modifications for loader

[ilias@…]

The patch modifies the loader in the following way:

• adds optional parameter "further_dirs"
• does not load 'setup.py' files as file-plugins

This is the simplest modification in order to achieve #4183 without many patching.

The patch does not break existent behaviour.

[Christopher Lenz]

Dude, there's no need to open a new ticket to post a patch for an existing ticket.

[ilias@…]

Replying to cmlenz:

Dude, there's no need to open a new ticket to post a patch for an existing ticket.

this patch contains some minimal modifications, in order to make the loader more flexible. It does _not_ implement the functionality described in #4183.

[Christopher Lenz]

Well, it this is not about #4183 (a statement that contradicts with the ticket description), then what is it about? So far, it's a patch without an explanation what is supposed to be achieved (real world use cases).

[Noah Kantrowitz (coderanger) <coderanger@…>]

setuptools already will load plugins from any folder on sys.path. Python offers a huge number of ways to control this list of folders ($PYTHONPATH, .pth files, etc etc). This patch seems largely redundant.

[anonymous]

Replying to cmlenz:

Well, it this is not about #4183 (a statement that contradicts with the ticket description), then what is it about? So far, it's a patch without an explanation what is supposed to be achieved (real world use cases).

I've stated _one_ possible use case: #4183.

(I am at this point not at my machine, thus I cannot check-in the code, to give you the specific usage context).

So, I understand that it's not a priority for the project to implement #4183 (closed as wontfix).

But, of course it would be nice if you make the loader more flexible, thus I am able to reuse the code to implement the functionality myself.

The current loader version does not allow me to pass an own list of directories, because all functionality of the loader is within _one_ function, load_components​.

You should be aware that the current loader implementation makes it impossible to reuse it. This is mainly due to the missing subfunctions.

The patch provided in this ticket helps me, but another way would be to refactor the "load_components" in order to move some functionality to subfunctions (would make it more usable for other developers, too)

If this is the prefered way for you, I could provide an alternative patch.

[ilias@…]

Replying to Noah Kantrowitz (coderanger) <coderanger@yahoo.com>:

setuptools already will load plugins from any folder on sys.path. Python offers a huge number of ways to control this list of folders ($PYTHONPATH, .pth files, etc etc).

The existent loader already provides the ability to load plugins from a specified directory (independend of sys.path)

The patch just adds the flexibility to define further directories (independend of sys.path)

This patch seems largely redundant.

It's not.

Like any other patch which increases reusability, without decreasing usability.

(and, btw: this code need really some refactoring)

[ilias@…]

another even more flexible option would be, to overwrite the plugin-dirs with a passed list (parameter "searchdirs = None")

If a list of dirs is passed, plugins are scanned therein.

Otherwise, the function would behave like usual.

   if searchdirs: 
       plugins_dirs = searchdirs

[ilias@…]

the usage context:

​http://dev.lazaridis.com/base/browser/infra/tracx/tracx/loader.py?rev=156#L58

(currently, i keep a copy of the original loader, modified with the suggested patch, as it was not possibly to apply a dynapatch to this monolithic function. very frustrating situation)

[Matthew Good]

Have you tried simply adding the directories to sys.path.

import sys
sys.path.extend(subdirs)

Setuptools will scan everything on sys.path, so it should be easy to add new directories at runtime without changing the plugin loader.

[Noah Kantrowitz <coderanger@…>]

Replying to mgood:

Setuptools will scan everything on sys.path, so it should be easy to add new directories at runtime without changing the plugin loader.

You could also do SetEnv PYTHONPATH "/my/folder" in the Apache config
(or PythonPath "sys.path + ['/my/folder']" for mod_python).

[ilias@…]

What is going on in this team?

Why are you permanently trying to keep the existent status of the code, which very obviously needs some refactoring?

Even a non-experienced programmer will notice that the implementation of the loader is very inflexible and glued.

Why are you permanently trying to suggest workarounds which interweave other subsystems (sys.path and even apache conf) and this in a use-case which tries to fulfill the opposite?

The creativity, development and evolution blocking behaviour starts to block any of my efforts to use trac as a foundation for my projects.

This is an arrogant behaviour, and it's clearly against the trac project's prime directives.

[Emmanuel Blot]

Replying to ilias@lazaridis.com:

The creativity, development and evolution blocking behaviour starts to block any of my efforts to use trac as a foundation for my projects.

Why don't you give up and use another base for your own projects, or fork from the current code base and write whatever extension suites you?

[Noah Kantrowitz <coderanger@…>]

Replying to ilias@lazaridis.com:

The creativity, development and evolution blocking behaviour starts to block any of my efforts to use trac as a foundation for my projects.

This is an arrogant behaviour, and it's clearly against the trac project's prime directives.

Trac's job is not to do things the way that fit your plans. From what I have seen of your project, you are well outside the design goals of Trac, which is fine, but don't complain about it. I am no stranger to making Trac jump through hoops when needed, so I will be the first to say that doing odd things is a good way to find deficiencies in Trac, but you need to understand that there are limits. Using Trac for something it wasn't designed for, and then complaining that is has problems in your use case is just silly.

[ilias@…]

Replying to eblot:

Replying to ilias@lazaridis.com:

The creativity, development and evolution blocking behaviour starts to block any of my efforts to use trac as a foundation for my projects.

Why don't you give up and use another base for your own projects, or fork from the current code base and write whatever extension suites you?

I have invested many time into trac and I am not interested to produce a fork.

The project should invest some time in order to keep what it promises.

​POLICY - Patch Acceptance

[ilias@…]

Replying to Noah Kantrowitz <coderanger@yahoo.com>:

Replying to ilias@lazaridis.com:

The creativity, development and evolution blocking behaviour starts to block any of my efforts to use trac as a foundation for my projects.

This is an arrogant behaviour, and it's clearly against the trac project's prime directives.

Trac's job is not to do things the way that fit your plans.

You are right.

That's why I'm extending it, whilst using the provided mechanisms.

of course, if I hit on a 'masterpiece of spaghetti-code', I cannot alter the behaviour. thus this patch suggestion.

From what I have seen of your project, you are well outside the design goals of Trac, which is fine, but don't complain about it.

you have missinterpreted it. The project is similar to 'trac-forge', possibly this helps.

​http://dev.lazaridis.com/base/wiki/ProductGuide

I am no stranger to making Trac jump through hoops when needed, so I will be the first to say that doing odd things is a good way to find deficiencies in Trac, but you need to understand that there are limits.

Yes, there are limits, e.g. when the code is available in a single chunk (like in this case).

Using Trac for something it wasn't designed for, and then complaining that is has problems in your use case is just silly.

this is false information. I use trac as a tracking sytem.

-

So, can we fix now this piece of code called "trac.loader".

Discussing this obvious signature change further seems really ridiculous.

Hope the responsible developer is aware of the draft-status of his code, and does not refuse to increase the clarity/usability of it (load_components)

[pacopablo]

Patch wise, I have to side with ilias on this one.

While I don't quite understand the need to disable the setup.py loading stuff, I do think it would be a simple and nice change to be able to specify the plugin directory for loading components when used in another project

Trac's component architecture is simply amazing. It's pretty much the coolest thing since sliced bread. However, it can be kind of hard to use it in another project due to things like the plugin dir and the hard coded value of "trac.plugins", etc.

Making it easier to use in an external project, while not hampering the main project, would be a welcome change.

Also, I have no clue as whether or not this issue will simply be resolved as a result of moving to setuptools or whatever.

[ilias@…]

Replying to mgood:

Have you tried simply adding the directories to sys.path.

import sys
sys.path.extend(subdirs)

Setuptools will scan everything on sys.path, so it should be easy to add new directories at runtime without changing the plugin loader.

this was the first i've tried, but did not work, which has most possibly to do with the working-set:

source:trunk/trac/loader.py@3557#L66​

(note: If I can pass the plugin_dirs, I don't need to know the internal implementation of the loader)

[ilias@…]

related ticket: #4383

[ilias@…]

Possibly the team don't want to touch the method signature.

If so, the functionality could be made available by this procedure:

• apply the suggested patch

• rename existent function load_components to e.g. load_components_alt

• create a load_components function

def load_components(env)
    load_components_alt(env)

[Christopher Lenz]

The merge of setuptools has allowed users of trac.loader to speficy any number of additional directories in addition to the environment plugins directory. Otherwise, we aren't going to make a special case for setup.py files cause that's just asking for trouble IMO.

[ilias@…]

Replying to cmlenz:

The merge of setuptools has allowed users of trac.loader to speficy any number of additional directories in addition to the environment plugins directory.

I am wondering why this one file was not processed independent of the setuptools branch, but better late than never.

Otherwise, we aren't going to make a special case for setup.py files cause that's just asking for trouble IMO.

this is not neccessary anymore, as the loader is now splitted down in 2 parts (egg loader, file loader), thus this problem is eliminated:

​http://dev.lazaridis.com/base/changeset/192