[trunk] [rev 1975] startup option --addons-path can't handle more than one addons path

first patch wasn't working when creating a new db, second one is okay

Problem of the patch is that since OpenERP currently has no API to access & manipulate (physical) addons (most of their content anyway, there are a few functions in bin/addons but they're not really documented and usually not used), pretty much all addons and big parts of the core hit directly into config['addons_path'], considering it a single, physical path (e.g. os.path.join(config['addons_path'], 'crm', 'some', 'file', 'path'), also note that the possibility of the addon being zipped is completely ignored).

Therefore at this time and unless it's fixed everywhere (and there are lots of places, let me tell you) addons_path are completely broken.

Xavier (Open ERP) a écrit :
> Problem of the patch is that since OpenERP currently has no API to
> access & manipulate (physical) addons (most of their content anyway,
> there are a few functions in bin/addons but they're not really
> documented and usually not used), pretty much all addons and big parts
> of the core hit directly into config['addons_path'], considering it a
> single, physical path (e.g. os.path.join(config['addons_path'], 'crm',
> 'some', 'file', 'path'), also note that the possibility of the addon
> being zipped is completely ignored).
>
> Therefore at this time and unless it's fixed everywhere (and there are
> lots of places, let me tell you) addons_path are completely broken.
>
OK thanks xavier for giving more infos.

I've checked the core + addons usage of config['addons_path'] and i
don't think it's a big deal to make something in order to have a way to
add arbitrary modules path on the fly(put) and a method to access
modules path for other usages(get).

Actually when config['addons_path'] is used in addons it's to physically
access to server/bin/addons folder to get ressources, ie access a file
within a folder.

When config['addons_path'] is used in core parts, it's for appending to
python path with sys.path.insert

We also have in bin/addons/init.py some functions that deal with modules
management but doesn't use config['addons_path'] really...

For now i don't really know what's the best way but basically we need :

- a method for adding extra addons path on startup (command line) or
server config file. This should make useless the usage of symbolic links
and in develppement process a more efficient way for switching between
different setup environnement or configuration for testing extra-addons
or community modules for example.

What's important is to have a way to add arbitrary module folder or
parent modules folder.

Maybe a new command line arg like --extra-addons-path can be a pretty
way for doing it.

I'm gonna make a blueprint to see what other dev thinks.

> Actually when config['addons_path'] is used in addons it's to physically access to server/bin/addons folder to get ressources, ie access a file within a folder.

Yeah, but if config['addons_path'] becomes a list of paths, then everything breaks. And even if we keep it as a single string, the addon being looked for through `config['addons_path'] + '/' + addon_name` might not be in the folder we randomly selected among all our addon paths. So I really don't think it's acceptable to not fix this.

> For now i don't really know what's the best way but basically we need [...] a method for adding extra addons path on startup (command line) or server config file

I agree, and this method already kind-of exists via ``--addons-path``, it just doesn't work. But before that we need to insulate addons from addons_path.

> I'm gonna make a blueprint to see what other dev thinks.

I wrote a small spec on an API (multiple addons path should be trivial to add once that part's done and folded into trunk + addons), I'm running it before chs and stw to weed out basic stupidity as I'm not a great spec writer, but I should be able to ask you for feedback within a pair of days, if you're interested.

On Monday 18 January 2010, you wrote:
> Yeah, but if config['addons_path'] becomes a list of paths, then
> everything breaks. And even if we keep it as a single string, the addon
> being looked for through `config['addons_path'] + '/' + addon_name`
> might not be in the folder we randomly selected among all our addon
> paths. So I really don't think it's acceptable to not fix this.
>

That functionality comes from my 5.0.x hacks. I've been using that one for
quite some time now (also in my 5.1.x branch). Are you sure it had been merged
right in launchpad?
The only quirk, as far as I remember, was that base_module_quality could have
had some bugs with multiple addons paths.

From my conf (all this time):
addons_path = /home/panos/build/openerp/addons/,
/home/panos/build/openerp/addons/etl/lib/, /home/panos/build/openerp/client-
kde/server-modules

See, it takes a comma and a space (did I trim that?) .. Feel free to improve.

Note that it has been quite some time since I wrote that hack. So, I may have
forgotten some details..

On Monday 18 January 2010, you wrote:

> i know it's been merged, I don't know about right, but from what I've read
> the situation with addons hitting directly in tools.config['addons_path']
> as a path is basically ignored isn't it?
I remember that no module should be using the tools.config['addons_path']
value, but instead call the find_module() or sth like that.

> > From my conf (all this time):
> > addons_path = /home/panos/build/openerp/addons/,
> > /home/panos/build/openerp/addons/etl/lib/,
> > /home/panos/build/openerp/client- kde/server-modules
> >
> > See, it takes a comma and a space (did I trim that?) .. Feel free to
> > improve.
>
> As I said, I'm floating a spec among the colleagues to solve the issue in a
> cleaner (and more extensible) way in the future, I can send you the link
> if you'd like (though I don't know if it'll end up folded in trunk in the
> end).

Yes, a single line in the config is not the best way, but was easy to code at
that time.

>
> Also, I think we should use the platform's os.pathsep to split paths.
pathsep has to do with directories notation, doesn't it? The delimiter we
could use in a string, such as the comma, colon or semicolon (like in $PATH)
is another thing to decide.

On 18 Jan 2010, at 12:16 , P. Christeas wrote:
>
> Note that it has been quite some time since I wrote that hack. So, I may have
> forgotten some details..
>
> On Monday 18 January 2010, you wrote:
>
>> i know it's been merged, I don't know about right, but from what I've read
>> the situation with addons hitting directly in tools.config['addons_path']
>> as a path is basically ignored isn't it?
> I remember that no module should be using the tools.config['addons_path']
> value, but instead call the find_module() or sth like that.
>
yep. the operative word, sadly is "should". The truth of the matter, however, is that many addons do directly hit tools.config['addons_path']. And I think I saw a few eval()ing __terp__.py manually as well.

>
>>> From my conf (all this time):
>>> addons_path = /home/panos/build/openerp/addons/,
>>> /home/panos/build/openerp/addons/etl/lib/,
>>> /home/panos/build/openerp/client- kde/server-modules
>>>
>>> See, it takes a comma and a space (did I trim that?) .. Feel free to
>>> improve.
>>
>> As I said, I'm floating a spec among the colleagues to solve the issue in a
>> cleaner (and more extensible) way in the future, I can send you the link
>> if you'd like (though I don't know if it'll end up folded in trunk in the
>> end).
>
> Yes, a single line in the config is not the best way, but was easy to code at
> that time.
>
Oh no, the single config line is perfect (along with the --addons-path option), I don't want to change that.

>>
>> Also, I think we should use the platform's os.pathsep to split paths.
> pathsep has to do with directories notation, doesn't it?

That's what one would think yeah, but it's not the case. The directory separator is os.path.sep, os.path.pathsep is the $PATH separator, and is defined in the various platform-specific *path.py files:

Lib/macpath.py
21:pathsep = '\n'

Lib/ntpath.py
28:pathsep = ';'

Lib/os2emxpath.py
27:pathsep = ';'

Lib/plat-riscos/riscospath.py
20:pathsep = ','

Lib/posixpath.py
32:pathsep = ':'

(note that macpath is for "Classic" MacOS, OSX uses posixpath)

os.path.pathsep is pretty badly named, I guess it stands for "$PATH separator" but it's not exactly clear.

On Monday 18 January 2010, you wrote:
> >
> > pathsep has to do with directories notation, doesn't it?
>
> That's what one would think yeah, but it's not the case. The directory
> separator is os.path.sep, os.path.pathsep is the $PATH separator, and is
> defined in the various platform-specific *path.py files:
>
Now that I think of it, it would be a genuinely bad idea to have a system-
specific delimiter, instead of a global (for all installations of OpenERP).
If we just admit that we wouldn't install OpenERP in a strange path
(containing spaces, commas or else), we can reserve those characters for our
addons list delimiter.

On 18 Jan 2010, at 15:05 , P. Christeas wrote:
>
> On Monday 18 January 2010, you wrote:
>>>
>>> pathsep has to do with directories notation, doesn't it?
>>
>> That's what one would think yeah, but it's not the case. The directory
>> separator is os.path.sep, os.path.pathsep is the $PATH separator, and is
>> defined in the various platform-specific *path.py files:
>>
> Now that I think of it, it would be a genuinely bad idea to have a system-
> specific delimiter, instead of a global (for all installations of OpenERP).
> If we just admit that we wouldn't install OpenERP in a strange path
> (containing spaces, commas or else), we can reserve those characters for our
> addons list delimiter.

Why? File (and directory) paths are already system-specific, and those are OS-wide conventions and there is no reason to break them. Using nonstandard path separators means having to document them and having to deal with sysadmins who want to setup openerp servers and have to break their own OS's conventions (which should be pretty much embedded in their bones).

xrg, I finally created the blueprint for the spec I was drafting (there's also a bit of demonstration code in the bitbucket repository, but I can't for the life of me find out where to put a second link on the blueprint page): https://blueprints.launchpad.net/openobject-server/+spec/addons-access-api

Please do tell me what you think of it (likewise for Franck). Integrating that in the server and migrating addons to it should make handling multiple addons path cleanly possible (and pretty easy).

important for the loempia project.

Hello Franck,

This has been fixed.

Thanks.

Hello,

Hum It's seems that it doesn't work anymore in the last trunk version

You make a test on the addons_path you raised an error if the path doesn't exist.

it's good but this test occurs before the split with the comma, so an error is always raised because (/home/vincent/addons,/home/vincent/extra-addons doesn't exist).

Hope this help,

Vincent

And other comment, it's not a wishlist as soon as openerp server doesn't follow symlink anymore.

Hello,

I am reopening this issue for trunk.When I am starting the server from server directory and giving the multiple path then server will not start.

Thanks.

Hello,

The resolution of this bug is critical for the execution of the aeroo reporting module and any module that includes the function
config ['addons_path']
They do not work when there are multiple addons paths.

I tested the new server revision:
https://code.launchpad.net/~openerp-groupes/openobject-server/6.0-cli-allows-multiple-addons-path

with aeroo reports and multiple addons paths

but the same error occurs when i try to print the sample report:

"hello_world" not defined For more reference inspect error logs. (<type 'exceptions.Exception'>, Exception(u'Aeroo Reports: Error while generating the report.', UndefinedError('"hello_world" not defined',), '"hello_world" not defined', u'For more reference inspect error logs.'), <traceback object at 0xa416f2c>)

and the traceback is:

ERROR:report_aeroo:[01]: Traceback (most recent call last):
ERROR:report_aeroo:[02]: File "/home/david/Escritorio/prueba/addons2/report_aeroo/report_aeroo.py", line 396, in create_aeroo_report
ERROR:report_aeroo:[03]: data = basic.generate(**oo_parser.localcontext).render().getvalue()
ERROR:report_aeroo:[04]: File "/usr/local/lib/python2.6/dist-packages/aeroolib-1.0.0-py2.6.egg/aeroolib/plugins/base.py", line 48, in render
ERROR:report_aeroo:[05]: return self.serializer(self.events)
ERROR:report_aeroo:[06]: File "/usr/local/lib/python2.6/dist-packages/aeroolib-1.0.0-py2.6.egg/aeroolib/plugins/opendocument.py", line 1156, in __call__
ERROR:report_aeroo:[07]: for kind, data, pos in stream:
ERROR:report_aeroo:[08]: File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/base.py", line 605, in _include
ERROR:report_aeroo:[09]: for event in stream:
ERROR:report_aeroo:[10]: File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/markup.py", line 327, in _match
ERROR:report_aeroo:[11]: for event in stream:
ERROR:report_aeroo:[12]: File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/base.py", line 565, in _flatten
ERROR:report_aeroo:[13]: result = _eval_expr(data, ctxt, vars)
ERROR:report_aeroo:[14]: File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/base.py", line 277, in _eval_expr
ERROR:report_aeroo:[15]: retval = expr.evaluate(ctxt)
ERROR:report_aeroo:[16]: File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/eval.py", line 178, in evaluate
ERROR:report_aeroo:[17]: return eval(self.code, _globals, {'__data__': data})
ERROR:report_aeroo:[18]: File "<string>", line 1, in <Expression u'__filter(hello_world(o.name))'>
ERROR:report_aeroo:[19]: File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/eval.py", line 309, in lookup_name
ERROR:report_aeroo:[20]: val = cls.undefined(name)
ERROR:report_aeroo:[21]: File "/usr/local/lib/python2.6/dist-packages/Genshi-0.6-py2.6.egg/genshi/template/eval.py", line 410, in undefined
ERROR:report_aeroo:[22]: raise UndefinedError(key, owner=owner)
ERROR:report_aeroo:[23]: UndefinedError: "hello_world" not defined
ERROR:web-services:[01]: Exception: (u'Aeroo Reports: Error while generating the report.', UndefinedError('"hello_world" not defined',), '"hello_world" not defined', u'For more reference inspect error logs.')
ERROR:web-services:[02]: Traceback (most recent call last):
ERROR:web-services:[03]: File "/home/davi...

The following fix has bee merged in trunk at revision 3420.
https://code.launchpad.net/~openerp-groupes/openobject-server/6.0-cli-allows-multiple-addons-path/+merge/59588