Merge lp:~sebastiken/anybox.recipe.openerp/group_standalone_addons into lp:anybox.recipe.openerp

[Sebastian Kennedy]

Added addons option called group. It allows to group different standalone addons in the same directory.
It helps for addons that are traked in differents repositories.
The use of this new option is like 'subdir' option. Below there is an example:

--- Content of buildout.cfg ---
[openerp]
recipe = anybox.recipe.openerp:server
serie = 7.0
version = nightly 7.0 latest
addons = git git://repoexample.com/module1.git module1 7.0 group=grouped_addons
         git git://repoexample.com/module2.git module2 master group=grouped_addons
         git git://repoexample.com/module3.git module3 master group=other_grouped_addons
--- End ---

Then, there will be two directories named grouped_addons and other_grouped_addons. First one, will contain module1 and module2 and the second one will contain module3.

[Georges Racinet (gracinet)]

Thanks for the contribution !

The idea sounds nice, and this may even become the successor of the existing magic for standalones
(which took us a long while to really work and is a source of problems in itself).
About this, see also https://bugs.launchpad.net/anybox.recipe.openerp/+bug/1214832 which I didn't have the courage to start yet.

But it'll take me a while to truly evaluate it. I wonder in particular if all the features that need to instanciate a VCS subclass will support it out of the box with so simple a diff.

Also, like most features, it'll need its own unit tests.

[Sebastian Kennedy (sebastiken)]

Georges, maybe this merge could solve that bug, although is not the solution proposed in the bug description.
I want to write the test, but when I execute nosetests I have an error that says "zc.buildout" cannot be imported. If you tell me how to solve this, I will write the tests too. Should I install zc.buildout system wide module?
I have created a virtual environment to isolate developing.

Thanks!

[Georges Racinet (gracinet)]

Sebastian, yes you'll need to install zc.buildout, but it's more that enough to install it in your virtualenv.

Normally, the 'develop' command as explained in this paragraph: http://docs.anybox.fr/anybox.recipe.openerp/trunk/contributing.html#development-setup
should take care of all the dependencies.

By the way, I don't care much if that's not the solution I proposed a while ago. This concept of 'group' seems to solve the 'standalone' use-case while being more general.

Also, we should think of how buildout configuration evolves. For instance, what if one such addon changes group ? If the repo stops being a standalone one (such things can happen, and be outside the control of the person maintaining that configuration, if the answer is to start with a new target directory, we should say so in doc).

Don't worry, these can be taken care of in later improvements.

[Sebastian Kennedy (sebastiken)]

Georges, thanks, I've just installed zc.buildout with pip and tests started to run. So, I will write a test for this new option. If I make the test and commit it to my branch, Have I to do a new merge request? I usually used github's pull requests which subsequents commits are grouped and discussed in the same pull request.
About your question, if you change the value of a 'group' option on a repo, this will not be a problem, because a new directory is created and append to addons_paths. The only thing this code doesn't do is to delete old repo addon (if it exists in an another group) and it will lead to a problem because of a duplicated addon. But it seems to be imposible to check this, only checking the existance of the addon in all folders and it so impractical. I think will be simpler if user cleans all before change group value.
The second question is not contemplated. I don't check if it is an standalone addon or if stops being standalone. So I see a new problem, if user puts 'group' option to addons that are not standalone, I really don't know what happens, I suppose there will nested in three level, i.e., group_folder/addons_folder/ <-- here will be the addons. It is also a problem because OpenERP will not find those. I think it is simple to solve, maybe adding again "some magic" to know if it is standalone or not detecting __openerp__.py or __terp__.py. But I understand you complain about that in bug description above.
Tell me what you think about these problems. I am going to write the test, ignoring this troubles for now.

> Sebastian, yes you'll need to install zc.buildout, but it's more that enough
> to install it in your virtualenv.
>
> Normally, the 'develop' command as explained in this paragraph:
> http://docs.anybox.fr/anybox.recipe.openerp/trunk/contributing.html
> #development-setup
> should take care of all the dependencies.
>
> By the way, I don't care much if that's not the solution I proposed a while
> ago. This concept of 'group' seems to solve the 'standalone' use-case while
> being more general.
>
> Also, we should think of how buildout configuration evolves. For instance,
> what if one such addon changes group ? If the repo stops being a standalone
> one (such things can happen, and be outside the control of the person
> maintaining that configuration, if the answer is to start with a new target
> directory, we should say so in doc).
>
> Don't worry, these can be taken care of in later improvements.

[lp:~sebastiken/anybox.recipe.openerp/group_standalone_addons]

557. By Sebastian Kennedy on 2014-09-23

Test written for new addons option 'group'

[Sebastian Kennedy (sebastiken)]

Georges, I have written a tests for this new option. Could you review it?
Thanks

[Georges Racinet (gracinet)]

Sebastian, sorry to be so slow (had lot of other work to do), I'm getting back to it right now.

[Sebastian Kennedy (sebastiken)]

Thanks, Georges. I understand you about lot of work.
See you!

=== modified file 'anybox/recipe/openerp/base.py'
--- anybox/recipe/openerp/base.py	2014-09-04 23:08:25 +0000
+++ anybox/recipe/openerp/base.py	2014-09-23 18:22:26 +0000
@@ -599,6 +599,14 @@
                            clean=self.clean)
             options.update(addons_options)
 
+            group = addons_options.get('group')
+            group_dir = None
+            if group:
+                l0, l1 = os.path.split(local_dir)
+                group_dir = join(l0, group)
+                local_dir = join(group_dir, l1)
+                if not os.path.exists(group_dir):
+                    os.mkdir(group_dir)
             if loc_type != 'local':
                 for k, v in self.options.items():
                     if k.startswith(loc_type + '-'):
@@ -612,7 +620,13 @@
                 utils.clean_object_files(local_dir)
 
             subdir = addons_options.get('subdir')
-            addons_dir = join(local_dir, subdir) if subdir else local_dir
+            if group_dir:
+                addons_dir = group_dir
+            else:
+                addons_dir = local_dir
+
+            if subdir:
+                addons_dir = join(addons_dir, subdir)
 
             manifest = os.path.join(addons_dir, '__openerp__.py')
             manifest_pre_v6 = os.path.join(addons_dir, '__terp__.py')
@@ -635,7 +649,8 @@
                 os.mkdir(addons_dir)
                 new_dir = join(addons_dir, name)
                 os.rename(tmp, new_dir)
-            self.addons_paths.append(addons_dir)
+            if not addons_dir in self.addons_paths:
+                self.addons_paths.append(addons_dir)
 
     def revert_sources(self):
         """Revert all sources to the revisions specified in :attr:`sources`.

=== modified file 'anybox/recipe/openerp/tests/test_server.py'
--- anybox/recipe/openerp/tests/test_server.py	2014-09-05 07:18:36 +0000
+++ anybox/recipe/openerp/tests/test_server.py	2014-09-23 18:22:26 +0000
@@ -125,6 +125,29 @@
                           ])
         self.assertEquals(paths, [web_addons_dir])
 
+    def test_retrieve_addons_standalone_grouped(self):
+        self.make_recipe(version='6.1', addons='fakevcs lp:my-addons1 addons1 '
+                         'last:1 group=grouped\nfakevcs lp:my-addons2 addons2 '
+                         'last:1 group=grouped')
+        # manual creation because fakevcs does nothing but retrieve_addons
+        # has assertions on existence of target directories
+        group_dir = os.path.join(self.buildout_dir, 'grouped')
+        addons1_dir = os.path.join(group_dir, 'addons1')
+        addons2_dir = os.path.join(group_dir, 'addons2')
+
+        self.recipe.retrieve_addons()
+        paths = self.recipe.addons_paths
+        print get_vcs_log()
+        self.assertEquals(get_vcs_log(), [
+                          (addons1_dir, 'lp:my-addons1', 'last:1',
+                           dict(offline=False, clear_locks=False, clean=False,
+                                    group="grouped")),
+                          (addons2_dir, 'lp:my-addons2', 'last:1',
+                           dict(offline=False, clear_locks=False, clean=False,
+                                    group="grouped"))
+                          ])
+        self.assertEquals(paths, [group_dir])
+
     def check_retrieve_addons_single(self, dirname):
         """The VCS is a whole addon."""
         self.make_recipe(version='6.1',