[IMP] Give precedence to module directories instead of zips while locating resources
authorOlivier Dony <odo@openerp.com>
Wed, 8 Feb 2012 17:39:32 +0000 (18:39 +0100)
committerOlivier Dony <odo@openerp.com>
Wed, 8 Feb 2012 17:39:32 +0000 (18:39 +0100)
The previous behavior gave the precedence to zipped
modules, without any apparent reason, and this is
sub-optimal for several reasons:

1. The default is to have regular modules, not
zipped modules, so looking first for a regular
module is more efficient.
2. Keeping a zipped module next to a regular
module with the same name is not a documented
or supported feature.
3. Even if you were relying on this behavior
having the extracted module take precedence
is more practical: you could simply extract
the zipped module to test a quick fix.

We have another issue related to this feature
because the code looking for zipped modules
escapes the addons paths chroot and goes
up to the filesystem root looking for a zip
module at each step. This is described in
bug 928376 and a fix for it should follow.

lp bug: https://launchpad.net/bugs/928376 fixed

bzr revid: odo@openerp.com-20120208173932-pwhz53vxxdzbo8ja

openerp/modules/module.py
openerp/tools/misc.py

index 58a4547..3d3bc0d 100644 (file)
@@ -280,25 +280,28 @@ def get_module_as_zip(modulename, b64enc=True, src=True):
 def get_module_resource(module, *args):
     """Return the full path of a resource of the given module.
 
-    @param module: the module
-    @param args: the resource path components
+    :param module: module name
+    :param list(str) args: resource path components within module
 
-    @return: absolute path to the resource
+    :rtype: str
+    :return: absolute path to the resource
 
     TODO name it get_resource_path
     TODO make it available inside on osv object (self.get_resource_path)
     """
-    a = get_module_path(module)
-    if not a: return False
-    resource_path = opj(a, *args)
-    if zipfile.is_zipfile( a +'.zip') :
-        zip = zipfile.ZipFile( a + ".zip")
+    mod_path = get_module_path(module)
+    if not mod_path: return False
+    resource_path = opj(mod_path, *args)
+    if os.path.isdir(mod_path):
+        # the module is a directory - ignore zip behavior
+        if os.path.exists(resource_path):
+            return resource_path
+    elif zipfile.is_zipfile(mod_path + '.zip'):
+        zip = zipfile.ZipFile( mod_path + ".zip")
         files = ['/'.join(f.split('/')[1:]) for f in zip.namelist()]
         resource_path = '/'.join(args)
         if resource_path in files:
-            return opj(a, resource_path)
-    elif os.path.exists(resource_path):
-        return resource_path
+            return opj(mod_path, resource_path)
     return False
 
 def get_module_icon(module):
index a5c7290..e0be5ff 100644 (file)
@@ -175,10 +175,22 @@ def file_open(name, mode="r", subdir='addons', pathinfo=False):
 
     name = os.path.normpath(name)
 
-    # Check for a zipfile in the path
+    # Give higher priority to module directories, which is
+    # a more common case than zipped modules.
+    if os.path.isfile(name):
+        fo = file(name, mode)
+        if pathinfo:
+            return fo, name
+        return fo
+
+    # Support for loading modules in zipped form.
+    # This will not work for zipped modules that are sitting
+    # outside of known addons paths. 
     head = name
     zipname = False
-    name2 = False
+    # FIXME: implement chrooting inside addons paths and fix
+    # for incorrect path_info behavior. Work in progress by
+    # Florent X linked to bug 928376
     while True:
         head, tail = os.path.split(head)
         if not tail:
@@ -200,14 +212,8 @@ def file_open(name, mode="r", subdir='addons', pathinfo=False):
                     return fo, name
                 return fo
             except Exception:
-                name2 = os.path.normpath(os.path.join(head + '.zip', zipname))
                 pass
-    for i in (name2, name):
-        if i and os.path.isfile(i):
-            fo = file(i, mode)
-            if pathinfo:
-                return fo, i
-            return fo
+
     if os.path.splitext(name)[1] == '.rml':
         raise IOError, 'Report %s doesn\'t exist or deleted : ' %str(name)
     raise IOError, 'File not found : %s' % name