git: replaced some raw subprocess commands with dedicated GIT vcsserver commands.

rhodecode/lib/vcs/backends/git/commit.py

@@ -223,17 +223,9 b' class GitCommit(base.BaseCommit):'
         """
         Returns list of child commits.
         """
-        rev_filter = settings.GIT_REV_FILTER
-        output, __ = self.repository.run_git_command(
-            ['rev-list', '--children'] + rev_filter)
 
-        child_ids = []
-        pat = re.compile(r'^%s' % self.raw_id)
-        for l in output.splitlines():
-            if pat.match(l):
-                found_ids = l.split(' ')[1:]
-                child_ids.extend(found_ids)
-        return self._make_commits(child_ids)
+        children = self._remote.children(self.raw_id)
+        return self._make_commits(children)
 
     def _make_commits(self, commit_ids):
         def commit_maker(_commit_id):
@@ -271,52 +263,27 b' class GitCommit(base.BaseCommit):'
         """
         Returns history of file as reversed list of `GitCommit` objects for
         which file at given `path` has been modified.
-
-        TODO: This function now uses an underlying 'git' command which works
-        quickly but ideally we should replace with an algorithm.
         """
-        self._get_filectx(path)
-        f_path = safe_str(path)
 
-        # optimize for n==1, rev-list is much faster for that use-case
-        if limit == 1:
-            cmd = ['rev-list', '-1', self.raw_id, '--', f_path]
-        else:
-            cmd = ['log']
-            if limit:
-                cmd.extend(['-n', str(safe_int(limit, 0))])
-            cmd.extend(['--pretty=format: %H', '-s', self.raw_id, '--', f_path])
-
-        output, __ = self.repository.run_git_command(cmd)
-        commit_ids = re.findall(r'[0-9a-fA-F]{40}', output)
-
+        path = self._get_filectx(path)
+        hist = self._remote.node_history(self.raw_id, path, limit)
         return [
             self.repository.get_commit(commit_id=commit_id, pre_load=pre_load)
-            for commit_id in commit_ids]
+            for commit_id in hist]
 
     def get_file_annotate(self, path, pre_load=None):
         """
         Returns a generator of four element tuples with
             lineno, commit_id, commit lazy loader and line
+        """
 
-        TODO: This function now uses os underlying 'git' command which is
-        generally not good. Should be replaced with algorithm iterating
-        commits.
-        """
-        cmd = ['blame', '-l', '--root', '-r', self.raw_id, '--', path]
-        # -l     ==> outputs long shas (and we need all 40 characters)
-        # --root ==> doesn't put '^' character for bounderies
-        # -r commit_id ==> blames for the given commit
-        output, __ = self.repository.run_git_command(cmd)
+        result = self._remote.node_annotate(self.raw_id, path)
 
-        for i, blame_line in enumerate(output.split('\n')[:-1]):
-            line_no = i + 1
-            commit_id, line = re.split(r' ', blame_line, 1)
+        for ln_no, commit_id, content in result:
             yield (
-                line_no, commit_id,
-                lambda: self.repository.get_commit(commit_id=commit_id,
-                                                   pre_load=pre_load),
-                line)
+                ln_no, commit_id,
+                lambda: self.repository.get_commit(commit_id=commit_id, pre_load=pre_load),
+                content)
 
     def get_nodes(self, path):
 

rhodecode/lib/vcs/backends/git/repository.py

@@ -549,33 +549,16 b' class GitRepository(BaseRepository):'
         if path1 is not None and path1 != path:
             raise ValueError("Diff of two different paths not supported.")
 
-        flags = [
-            '-U%s' % context, '--full-index', '--binary', '-p',
-            '-M', '--abbrev=40']
-        if ignore_whitespace:
-            flags.append('-w')
-
-        if commit1 == self.EMPTY_COMMIT:
-            cmd = ['show'] + flags + [commit2.raw_id]
+        if path:
+            file_filter = path
         else:
-            cmd = ['diff'] + flags + [commit1.raw_id, commit2.raw_id]
-
-        if path:
-            cmd.extend(['--', path])
+            file_filter = None
 
-        stdout, __ = self.run_git_command(cmd)
-        # If we used 'show' command, strip first few lines (until actual diff
-        # starts)
-        if commit1 == self.EMPTY_COMMIT:
-            lines = stdout.splitlines()
-            x = 0
-            for line in lines:
-                if line.startswith('diff'):
-                    break
-                x += 1
-            # Append new line just like 'diff' command do
-            stdout = '\n'.join(lines[x:]) + '\n'
-        return GitDiff(stdout)
+        diff = self._remote.diff(
+            commit1.raw_id, commit2.raw_id, file_filter=file_filter,
+            opt_ignorews=ignore_whitespace,
+            context=context)
+        return GitDiff(diff)
 
     def strip(self, commit_id, branch_name):
         commit = self.get_commit(commit_id=commit_id)

rhodecode/lib/vcs/backends/hg/commit.py

@@ -266,8 +266,7 b' class MercurialCommit(base.BaseCommit):'
         for ln_no, commit_id, content in result:
             yield (
                 ln_no, commit_id,
-                lambda: self.repository.get_commit(commit_id=commit_id,
-                                                   pre_load=pre_load),
+                lambda: self.repository.get_commit(commit_id=commit_id, pre_load=pre_load),
                 content)
 
     def get_nodes(self, path):

rhodecode/tests/vcs/test_git.py

@@ -1065,37 +1065,35 b' class TestGitSpecificWithRepo(BackendTes'
 
     def test_paths_fast_traversing(self):
         commit = self.repo.get_commit()
-        assert (
-            commit.get_node('foobar/static/js/admin/base.js').content ==
-            'base')
+        assert commit.get_node('foobar/static/js/admin/base.js').content == 'base'
 
     def test_get_diff_runs_git_command_with_hashes(self):
         comm1 = self.repo[0]
         comm2 = self.repo[1]
-        self.repo.run_git_command = mock.Mock(return_value=['', ''])
-        self.repo.get_diff(comm1, comm2)
+
+        with mock.patch.object(self.repo, '_remote') as remote_mock:
+            self.repo.get_diff(comm1, comm2)
 
-        self.repo.run_git_command.assert_called_once_with(
-            ['diff', '-U3', '--full-index', '--binary', '-p', '-M',
-             '--abbrev=40', comm1.raw_id, comm2.raw_id])
+        remote_mock.diff.assert_called_once_with(
+            comm1.raw_id, comm2.raw_id,
+            file_filter=None, opt_ignorews=False, context=3)
 
     def test_get_diff_runs_git_command_with_str_hashes(self):
         comm2 = self.repo[1]
-        self.repo.run_git_command = mock.Mock(return_value=['', ''])
-        self.repo.get_diff(self.repo.EMPTY_COMMIT, comm2)
-        self.repo.run_git_command.assert_called_once_with(
-            ['show', '-U3', '--full-index', '--binary', '-p', '-M',
-             '--abbrev=40', comm2.raw_id])
+        with mock.patch.object(self.repo, '_remote') as remote_mock:
+            self.repo.get_diff(self.repo.EMPTY_COMMIT, comm2)
+        remote_mock.diff.assert_called_once_with(
+            self.repo.EMPTY_COMMIT.raw_id, comm2.raw_id,
+            file_filter=None, opt_ignorews=False, context=3)
 
     def test_get_diff_runs_git_command_with_path_if_its_given(self):
         comm1 = self.repo[0]
         comm2 = self.repo[1]
-        self.repo.run_git_command = mock.Mock(return_value=['', ''])
-        self.repo.get_diff(comm1, comm2, 'foo')
-        self.repo.run_git_command.assert_called_once_with(
-            ['diff', '-U3', '--full-index', '--binary', '-p', '-M',
-             '--abbrev=40', self.repo._lookup_commit(0),
-             comm2.raw_id, '--', 'foo'])
+        with mock.patch.object(self.repo, '_remote') as remote_mock:
+            self.repo.get_diff(comm1, comm2, 'foo')
+        remote_mock.diff.assert_called_once_with(
+            self.repo._lookup_commit(0), comm2.raw_id,
+            file_filter='foo', opt_ignorews=False, context=3)
 
 
 @pytest.mark.usefixtures("vcs_repository_support")

rhodecode/tests/vcs/test_hg.py

@@ -57,7 +57,7 b' def repo(request, baseapp):'
     return repo
 
 
-class TestMercurialRepository:
+class TestMercurialRepository(object):
 
     # pylint: disable=protected-access