[damned-lies] Added cherry-picking commit capability (no UI yet)

[Claude Paroz <claudep src gnome org>]

commit 8ea9927c9ef6c44cb58abd8d5940fb689b2d5c71
Author: Claude Paroz <claude 2xlibre net>
Date:   Sat Oct 17 15:46:09 2015 +0200

    Added cherry-picking commit capability (no UI yet)
    
    Refs bug #726786.

 stats/models.py      |   97 +++++++++++++++++++++++++++++++++----------------
 stats/tests/tests.py |   93 +++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 141 insertions(+), 49 deletions(-)
---
diff --git a/stats/models.py b/stats/models.py
index 6117165..93fdaa5 100644
--- a/stats/models.py
+++ b/stats/models.py
@@ -164,6 +164,7 @@ class Module(models.Model):
             return True
         return False
 
+
 class ModuleLock(object):
     """ Weird things happen when multiple updates run in parallel for the same module
         We use filesystem directories creation/deletion to act as global lock mecanism
@@ -179,7 +180,8 @@ class ModuleLock(object):
                 os.mkdir(self.dirpath)
                 break;
             except OSError:
-                sleep(30)
+                # The directory exists, something is happening on the module, let's wait
+                sleep(10)
 
     def __exit__(self, *exc_info):
         os.rmdir(self.dirpath)
@@ -211,7 +213,6 @@ class Branch(models.Model):
 
     def __init__(self, *args, **kwargs):
         super(Branch, self).__init__(*args, **kwargs)
-        self.checkout_lock = threading.Lock()
         self._ui_stats = None
         self._doc_stats = None
 
@@ -582,23 +583,7 @@ class Branch(models.Model):
 
         command_list = []
         if self._exists():
-            # Path exists, update repos
-            if vcs_type == "cvs":
-                command_list.append((modulepath, ['cvs', '-z4', 'up', '-Pd']))
-            elif vcs_type == "svn":
-                command_list.append((modulepath, ['svn', 'up', '--non-interactive']))
-            elif vcs_type == "hg":
-                command_list.append((modulepath, ['hg', 'revert', '--all']))
-            elif vcs_type == "git":
-                # tester "git checkout %(branch)s && git clean -dfq && git pull origin/%(branch)s"
-                command_list.append((modulepath, ['git', 'checkout', '-f', self.name]))
-                command_list.append((modulepath, ['git', 'fetch']))
-                command_list.append((modulepath, ['git', 'reset', '--hard', 'origin/%s' % self.name]))
-                command_list.append((modulepath, ['git', 'clean', '-dfq']))
-                # check if there are any submodules and init & update them
-                command_list.append((modulepath, "if [ -e .gitmodules ]; then git submodule update --init; 
fi"))
-            elif vcs_type == "bzr":
-                command_list.append((modulepath, ['bzr', 'up']))
+            command_list.extend(self.update_repo(execute=False))
         else:
             # Checkout
             vcs_path = self.get_vcs_url()
@@ -633,21 +618,49 @@ class Branch(models.Model):
         # Run command(s)
         logging.debug("Checking '%s.%s' out to '%s'..." % (module_name, self.name, modulepath))
         # Do not allow 2 checkouts to run in parallel on the same branch
-        self.checkout_lock.acquire()
-        try:
+        with ModuleLock(self.module):
             for working_dir, command in command_list:
                 utils.run_shell_command(command, raise_on_error=True, cwd=working_dir)
-        finally:
-            self.checkout_lock.release()
         return 1
 
-    def commit_po(self, po_file, domain, language, author):
+    def update_repo(self, execute=True):
+        """
+        Update existing repository checkout.
+        WARNING: the calling method should acquire a lock for the module to not
+        mix checkouts in different threads/processes.
+        """
+        modulepath = self.co_path()
+        logging.debug("Updating '%s.%s' (in '%s')..." % (self.module.name, self.name, modulepath))
+        command_list = []
+        if self.module.vcs_type == "cvs":
+            command_list.append((modulepath, ['cvs', '-z4', 'up', '-Pd']))
+        elif self.module.vcs_type == "svn":
+            command_list.append((modulepath, ['svn', 'up', '--non-interactive']))
+        elif self.module.vcs_type == "hg":
+            command_list.append((modulepath, ['hg', 'revert', '--all']))
+        elif self.module.vcs_type == "git":
+            # tester "git checkout %(branch)s && git clean -dfq && git pull origin/%(branch)s"
+            command_list.append((modulepath, ['git', 'checkout', '-f', self.name]))
+            command_list.append((modulepath, ['git', 'fetch']))
+            command_list.append((modulepath, ['git', 'reset', '--hard', 'origin/%s' % self.name]))
+            command_list.append((modulepath, ['git', 'clean', '-dfq']))
+            # check if there are any submodules and init & update them
+            command_list.append((modulepath, "if [ -e .gitmodules ]; then git submodule update --init; fi"))
+        elif self.module.vcs_type == "bzr":
+            command_list.append((modulepath, ['bzr', 'up']))
+        if execute:
+            for working_dir, command in command_list:
+                utils.run_shell_command(command, raise_on_error=True, cwd=working_dir)
+        else:
+            return command_list
+
+    def commit_po(self, po_file, domain, language, author, sync_master=False):
         """ Commit the file 'po_file' in the branch VCS repository """
         if self.is_vcs_readonly():
             raise Exception("This branch is in read-only mode. Unable to commit")
         vcs_type = self.module.vcs_type
         if vcs_type not in ("git",):
-            raise Exception("Commit is not implemented for '%s'" % vcs_type)
+            raise NotImplementedError("Commit is not implemented for '%s'" % vcs_type)
 
         locale = language.locale
         commit_dir = os.path.join(self.co_path(), domain.directory)
@@ -655,13 +668,9 @@ class Branch(models.Model):
         dest_filename = os.path.join(prefix, "%s.po" % locale)
         dest_path = os.path.join(commit_dir, dest_filename)
 
-        if vcs_type == "git":
-            with ModuleLock(self.module):
-                utils.run_shell_command(
-                    ['git', 'checkout', self.name], raise_on_error=True, cwd=commit_dir)
-                utils.run_shell_command(
-                    ['git', 'pull'], raise_on_error=True, cwd=commit_dir)
-
+        with ModuleLock(self.module):
+            self.update_repo()
+            if vcs_type == "git":
                 already_exist = os.access(dest_path, os.F_OK)
                 if not already_exist and domain.dtype != 'ui':
                     raise Exception("Sorry, adding new translations for documentation is not yet supported.")
@@ -696,6 +705,11 @@ class Branch(models.Model):
                     utils.run_shell_command(
                         ['git', 'reset', '--hard', 'origin/%s' % self.name], cwd=commit_dir)
                     raise
+                else:
+                    _, out, _ = utils.run_shell_command(
+                        ['git', 'log', '-n1', '--format=oneline'], cwd=commit_dir)
+                    commit_hash = out.split()[0] if out else ''
+
         # Finish by updating stats
         if already_exist:
             stat = Statistics.objects.get(language=language, branch=self, domain=domain)
@@ -703,6 +717,25 @@ class Branch(models.Model):
         else:
             self.update_stats(force=False, checkout=False, domain=domain)
 
+        if sync_master and not self.is_head():
+            # Cherry-pick the commit on the master branch
+            self.module.get_head_branch().cherrypick_commit(commit_hash, domain)
+
+    def cherrypick_commit(self, commit_hash, domain):
+        if self.module.vcs_type != "git":
+            raise NotImplementedError("Commit cherry-pick is not implemented for '%s'" % 
self.module.vcs_type)
+        with ModuleLock(self.module):
+            self.update_repo()
+            commit_dir = os.path.join(self.co_path(), domain.directory)
+            result = utils.run_shell_command(
+                ['git', 'cherry-pick', '-x', commit_hash], cwd=commit_dir)
+            if result[0] == utils.STATUS_OK:
+                utils.run_shell_command(
+                    ['git', 'push', 'origin', self.name], raise_on_error=True, cwd=commit_dir)
+            else:
+                # Revert
+                utils.run_shell_command(['git', 'cherry-pick', '--abort'], cwd=commit_dir)
+
 
 DOMAIN_TYPE_CHOICES = (
     ('ui', 'User Interface'),
diff --git a/stats/tests/tests.py b/stats/tests/tests.py
index 5d7e73f..fb45b46 100644
--- a/stats/tests/tests.py
+++ b/stats/tests/tests.py
@@ -66,18 +66,29 @@ def mocked_checkout(branch):
 
 class patch_shell_command:
     """
-    Mock utils.run_shell_commands and gather all passed commands
+    Mock utils.run_shell_commands and gather all passed commands.
+    `only` is an optional list of commands to limit mocking to (empty -> all).
     """
+    def __init__(self, only=None):
+        self.only = only
+
     def __enter__(self):
         self.cmds = []
-        def mocked_run_shell_command(cmd, *args, **kwargs):
-            self.cmds.append(" ".join(cmd) if isinstance(cmd, list) else cmd)
-            return 0, '', ''
         self.saved_run_shell_command = utils.run_shell_command
-        utils.run_shell_command = mocked_run_shell_command
+        utils.run_shell_command = self.mocked_run_shell_command
         return self.cmds
 
-    def __exit__(self, type, value, traceback):
+    def mocked_run_shell_command(self, cmd, *args, **kwargs):
+        cmd_str = " ".join(cmd) if isinstance(cmd, list) else cmd
+        self.cmds.append(cmd_str)
+        if self.only is not None and not any(needle in cmd_str for needle in self.only):
+            # Pass the command to the real utils.run_shell_command
+            return self.saved_run_shell_command(cmd, *args, **kwargs)
+        else:
+            # Pretend the command was successfull
+            return 0, '', ''
+
+    def __exit__(self, *args):
         utils.run_shell_command = self.saved_run_shell_command
 
 
@@ -89,7 +100,7 @@ class ModuleTestCase(TestCase):
         ('gnome-doc-utils', 'xml2po'),
     )
     def __init__(self, name):
-        TestCase.__init__(self, name)
+        super(ModuleTestCase, self).__init__(name)
         for package, prog in self.SYS_DEPENDENCIES:
             if not utils.check_program_presence(prog):
                 raise Exception("You are missing a required system package needed by Damned Lies (%s)" % 
package)
@@ -292,11 +303,17 @@ class ModuleTestCase(TestCase):
         self.mod.vcs_root = self.mod.vcs_root.replace('git://', 'ssh://')
         self.mod.save()
 
+        update_repo_sequence = (
+            'git checkout -f master', 'git fetch', 'git reset --hard origin/master',
+            'git clean -dfq', 'if [ -e .gitmodules ]; then git submodule update --init; fi',
+        )
         # User interface (existing language)
-        git_ops = ('git checkout master', 'git pull', 'git add fr.po',
-                   # Quoting is done at the Popen level
-                   'git commit -m Updated French translation --author Author <someone example org>',
-                   'git push origin master')
+        git_ops = update_repo_sequence + (
+            'git add fr.po',
+            # Quoting is done at the Popen level
+            'git commit -m Updated French translation --author Author <someone example org>',
+            'git push origin master'
+        )
         with patch_shell_command() as cmds:
             branch.commit_po(po_file, domain, fr_lang, 'Author <someone example org>')
             for idx, cmd in enumerate(git_ops):
@@ -304,9 +321,11 @@ class ModuleTestCase(TestCase):
 
         # User interface (new language)
         bem_lang = Language.objects.get(locale='bem')
-        git_ops = ('git checkout master', 'git pull', 'git add bem.po', 'git add LINGUAS',
-                   'git commit -m Added Bemba translation --author Author <someone example org>',
-                   'git push origin master')
+        git_ops = update_repo_sequence + (
+            'git add bem.po', 'git add LINGUAS',
+            'git commit -m Added Bemba translation --author Author <someone example org>',
+            'git push origin master'
+        )
         with patch_shell_command() as cmds:
             branch.commit_po(po_file, domain, bem_lang, 'Author <someone example org>')
             for idx, cmd in enumerate(git_ops):
@@ -319,15 +338,55 @@ class ModuleTestCase(TestCase):
 
         # Documentation
         domain = self.mod.domain_set.get(name='help')
-        git_ops = ('git checkout master', 'git pull', 'git add fr/fr.po',
-                   'git commit -m Updated French translation --author Author <someone example org>',
-                   'git push origin master')
+        git_ops = update_repo_sequence + (
+            'git add fr/fr.po',
+            'git commit -m Updated French translation --author Author <someone example org>',
+            'git push origin master'
+        )
         with patch_shell_command() as cmds:
             branch.commit_po(po_file, domain, fr_lang, 'Author <someone example org>')
             for idx, cmd in enumerate(git_ops):
                 self.assertIn(cmd, cmds[idx])
 
     @test_scratchdir
+    def test_commit_and_cherrypick(self):
+        """
+        Committing in non-HEAD branch and checking "sync with master" will
+        cherry-pick the branch commit on the master branch.
+        """
+        domain = self.mod.domain_set.get(name='po')
+        commit_dir = os.path.join(self.branch.co_path(), domain.directory)
+        utils.run_shell_command(['git', 'checkout', '-b', 'gnome-3-18', 'origin/master'], cwd=commit_dir)
+        branch = Branch.objects.create(module=self.mod, name='gnome-3-18')
+        fr_lang = Language.objects.get(locale='fr')
+        # Copy stats from master
+        stat = Statistics.objects.get(language=fr_lang, branch=self.branch, domain=domain)
+        stat.pk, stat.full_po, stat.part_po = None, None, None
+        stat.branch = branch
+        stat.save()
+        po_file = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'test.po')
+        self.mod.vcs_root = self.mod.vcs_root.replace('git://', 'ssh://')
+        self.mod.save()
+
+        update_repo_sequence = (
+            'git checkout -f gnome-3-18', 'git fetch', 'git reset --hard origin/gnome-3-18',
+            'git clean -dfq', 'if [ -e .gitmodules ]; then git submodule update --init; fi',
+        )
+        update_repo_sequence_master = tuple(cmd.replace('gnome-3-18', 'master') for cmd in 
update_repo_sequence)
+        git_ops = update_repo_sequence + (
+            'git add fr.po',
+            'git commit -m Updated French translation --author Author <someone example org>',
+            'git push origin gnome-3-18', 'git log -n1 --format=oneline', 'msgfmt --statistics -o /dev/null',
+        ) + update_repo_sequence_master + (
+            'git cherry-pick -x',
+            'git push origin master',
+        )
+        with patch_shell_command(only=['git pull', 'git push', 'git fetch', 'git reset']) as cmds:
+            branch.commit_po(po_file, domain, fr_lang, 'Author <someone example org>', sync_master=True)
+            for idx, cmd in enumerate(git_ops):
+                self.assertIn(cmd, cmds[idx])
+
+    @test_scratchdir
     def test_branch_file_changed(self):
         # file_hashes is empty in fixture, so first call should always return True
         self.assertTrue(self.mod.get_head_branch().file_changed("gnome-hello.doap"))