Merge lp:~camptocamp/anybox.recipe.openerp/fix-1358353-yvr into lp:anybox.recipe.openerp

[Yannick Vaucher @ Camptocamp]

        We want to keep ordering of addons path.
        If official addons path is missing, we add just after path of base
        addons.
        Otherwise, we do nothing to keep official addons where they are.

        We preserve the order, because it is important for addons with same
        name.
        Only first found addons of duplicate names is considered.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Nice, can I propose a rewording? (could even be better though)

"""
The ordering of the different paths of addons is important, because when several addons in different paths have the same name, the first of them that is found is used. This can be used, for instance, to replace an official addon by another one by placing a different addons' path before the official one.

If the official addons' path is already set in the config file (e.g. at the end), it will leave it at the end of the paths list, if it is not set, it will be placed at the beginning.
"""

review: Approve

[lp:~camptocamp/anybox.recipe.openerp/fix-1358353-yvr]

557. By Guewen Baconnier @ Camptocamp on 2014-08-18

reword docstring

558. By Yannick Vaucher @ Camptocamp on 2014-08-18

reword docstring

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Added your rewording and did few little changes in 2 commits.

Thanks

[Georges Racinet (gracinet)]

Thanks fort thé MP. I'm currently out of office, will consider it next week.

On 18 août 2014 18:41:44 UTC+02:00, "Yannick Vaucher @ Camptocamp" <email address hidden> wrote:
>Added your rewording and did few little changes in 2 commits.
>
>Thanks
>--
>https://code.launchpad.net/~camptocamp/anybox.recipe.openerp/fix-1358353-yvr/+merge/231218
>You are requested to review the proposed merge of
>lp:~camptocamp/anybox.recipe.openerp/fix-1358353-yvr into
>lp:anybox.recipe.openerp.

--
Sent from my handheld device with K-9 Mail. Please excuse my brevity.

[Georges Racinet (gracinet)]

LGTM, and I appreciate the docstring work. That being said:

- (minor) for now, everything related to the Github layout is acceptable for the stable branch. I can take care of the backporting if you don't feel comfortable about it.

- I think this deserves a unit test of its own. There's already a test_finalize_addons_path() in test_base.py, you could easily make a companion one for this use-case. And this will ease the backporting as well

If you have trouble launching the unit tests, here's the how-to :

http://docs.anybox.fr/anybox.recipe.openerp/stable/contributing.html#launching-static-analysis-and-unit-tests

Thanks again

review: Needs Fixing

[lp:~camptocamp/anybox.recipe.openerp/fix-1358353-yvr]

559. By Yannick Vaucher @ Camptocamp on 2014-09-04

add test for addons path order

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Here we go.

I just added a test for this case.

[Georges Racinet (gracinet)]

> Here we go.
>
> I just added a test for this case.

Unfortunately, the test passes without the main code modification. That's because you forgot to create the 'odoo_dir/addons' directory (hence the github layout method did not work at all).

I added the missing line and it worked as expected.bzr -> merged

review: Approve

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Ok Georges
I see I missed the os.makedirs(odoo_addons) part.

Thanks

=== modified file 'anybox/recipe/openerp/base.py'
--- anybox/recipe/openerp/base.py	2014-07-24 12:44:00 +0000
+++ anybox/recipe/openerp/base.py	2014-09-04 12:27:51 +0000
@@ -1259,6 +1259,17 @@
         If not found (e.g, we are on a nightly for OpenERP <= 7), this method
         does nothing.
 
+        The ordering of the different paths of addons is important.
+        When several addons at different paths have the same name, the first
+        of them being found is used. This can be used, for instance, to
+        replace an official addon by another one by placing a different
+        addons' path before the official one.
+
+        If the official addons' path is already set in the config file
+        (e.g. at the end), it will leave it at the end of the paths list,
+        if it is not set, it will be placed at the beginning just after
+        ``base`` addons' path.
+
         Care is taken not to break configurations that corrected this manually
         with a ``local`` source in the ``addons`` option.
 
@@ -1276,11 +1287,9 @@
         except ValueError:
             insert_at = 0
         try:
-            addons_paths.remove(odoo_git_addons)
+            addons_paths.index(odoo_git_addons)
         except ValueError:
-            pass
-
-        addons_paths.insert(insert_at, odoo_git_addons)
+            addons_paths.insert(insert_at, odoo_git_addons)
 
     def cleanup_openerp_dir(self):
         """Revert local modifications that have been made during installation.

=== modified file 'anybox/recipe/openerp/tests/test_base.py'
--- anybox/recipe/openerp/tests/test_base.py	2014-06-08 21:55:32 +0000
+++ anybox/recipe/openerp/tests/test_base.py	2014-09-04 12:27:51 +0000
@@ -426,6 +426,24 @@
         self.assertEquals(self.recipe.addons_paths, [base_addons,
                                                      '/some/separate/addons'])
 
+    def test_finalize_addons_paths_order(self):
+        """Test finalize_addons_paths keeps addons_path order
+        Ensure we don't move odoo addons in addons_path if it has been
+        set as local
+        """
+        self.make_recipe(
+            version='git http://github.com/odoo/odoo.git odoo 7.0')
+        self.recipe.version_detected = '7.0-somerev'
+        oerp_dir = self.recipe.openerp_dir
+        base_addons = os.path.join(oerp_dir, 'openerp', 'addons')
+        odoo_addons = os.path.join(oerp_dir, 'addons')
+        os.makedirs(base_addons)
+        self.recipe.addons_paths = ['/some/separate/addons',
+                                    odoo_addons]
+        self.recipe.finalize_addons_paths(check_existence=False)
+        self.assertEquals(self.recipe.addons_paths,
+                          [base_addons, '/some/separate/addons', odoo_addons])
+
     def test_finalize_addons_paths_60_layout(self):
         self.make_recipe(version='6.0.4')
         recipe = self.recipe