Bug 1413922 - [mozversioncontrol] Always use hglib.client if available and fall back to subprocesses if not, r=gps

Most HG commands use subprocesses, even if a context manager (and therefore an
hglib client) has been created. There are only two commands that make use of
the client, but they *only* work inside a context manager. I don't think
there are any technical reason these two commands *need* to use the context
manager.

This patch merges the HgRepository._run_in_client function with
HgRepository._run(). If a client exists, that will be used, otherwise a
subprocess will be used.

Differential Revision: https://phabricator.services.mozilla.com/D1809

--HG--
extra : moz-landing-system : lando

python/mozversioncontrol/mozversioncontrol/__init__.py

@@ -236,10 +236,9 @@ class HgRepository(Repository):
     def __exit__(self, exc_type, exc_val, exc_tb):
         self._client.close()
 
-    def _run_in_client(self, args):
+    def _run(self, *args, **runargs):
         if not self._client.server:
-            raise Exception('active HgRepository context manager required')
-
+            return super(HgRepository, self)._run(*args, **runargs)
         return self._client.rawcommand(args)
 
     def sparse_checkout_present(self):
@@ -304,8 +303,7 @@ class HgRepository(Repository):
     def get_files_in_working_directory(self):
         # Can return backslashes on Windows. Normalize to forward slashes.
         return list(p.replace('\\', '/') for p in
-                    self._run_in_client([b'files', b'-0']).split(b'\0')
-                    if p)
+                    self._run(b'files', b'-0').split(b'\0') if p)
 
     def working_directory_clean(self, untracked=False, ignored=False):
         args = [b'status', b'\0', b'--modified', b'--added', b'--removed',
@@ -317,7 +315,7 @@ class HgRepository(Repository):
 
         # If output is empty, there are no entries of requested status, which
         # means we are clean.
-        return not len(self._run_in_client(args).strip())
+        return not len(self._run(*args).strip())
 
 
 class GitRepository(Repository):

python/mozversioncontrol/test/conftest.py

@@ -0,0 +1,74 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+from __future__ import absolute_import
+
+import os
+import subprocess
+
+import pytest
+
+
+SETUP = {
+    'hg': [
+        """
+        echo "foo" > foo
+        echo "bar" > bar
+        hg init
+        hg add *
+        hg commit -m "Initial commit"
+        """,
+        """
+        echo "[paths]\ndefault = ../remoterepo" > .hg/hgrc
+        """,
+    ],
+    'git': [
+        """
+        echo "foo" > foo
+        echo "bar" > bar
+        git init
+        git add *
+        git commit -am "Initial commit"
+        """,
+        """
+        git remote add upstream ../remoterepo
+        git fetch upstream
+        git branch -u upstream/master
+        """,
+    ]
+}
+
+
+def shell(cmd):
+    subprocess.check_call(cmd, shell=True)
+
+
+@pytest.yield_fixture(params=['git', 'hg'])
+def repo(tmpdir, request):
+    vcs = request.param
+    steps = SETUP[vcs]
+
+    if hasattr(request.module, 'STEPS'):
+        steps.extend(request.module.STEPS[vcs])
+
+    # tmpdir and repo are py.path objects
+    # http://py.readthedocs.io/en/latest/path.html
+    repo = tmpdir.mkdir('repo')
+    repo.vcs = vcs
+
+    # This creates a step iterator. Each time next() is called
+    # on it, the next set of instructions will be executed.
+    repo.step = (shell(cmd) for cmd in steps)
+
+    oldcwd = os.getcwd()
+    os.chdir(repo.strpath)
+
+    next(repo.step)
+
+    repo.copy(tmpdir.join('remoterepo'))
+
+    next(repo.step)
+
+    yield repo
+    os.chdir(oldcwd)

python/mozversioncontrol/test/python.ini

@@ -2,4 +2,5 @@
 subsuite=mozversioncontrol
 skip-if = python == 3
 
+[test_context_manager.py]
 [test_workdir_outgoing.py]

python/mozversioncontrol/test/test_context_manager.py

@@ -0,0 +1,30 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+from __future__ import absolute_import
+
+import mozunit
+
+from mozversioncontrol import get_repository_object
+
+
+def test_context_manager(repo):
+    is_git = repo.vcs == 'git'
+    cmd = ['show', '--no-patch'] if is_git else ['tip']
+
+    vcs = get_repository_object(repo.strpath)
+    output_subprocess = vcs._run(*cmd)
+    assert is_git or vcs._client.server is None
+    assert "Initial commit" in output_subprocess
+
+    with vcs:
+        assert is_git or vcs._client.server is not None
+        output_client = vcs._run(*cmd)
+
+    assert is_git or vcs._client.server is None
+    assert output_subprocess == output_client
+
+
+if __name__ == '__main__':
+    mozunit.main()

python/mozversioncontrol/test/test_workdir_outgoing.py

@@ -5,26 +5,14 @@
 from __future__ import absolute_import
 
 import os
-import subprocess
 
 import mozunit
-import pytest
 
 from mozversioncontrol import get_repository_object
 
 
-setup = {
+STEPS = {
     'hg': [
-        """
-        echo "foo" > foo
-        echo "bar" > bar
-        hg init
-        hg add *
-        hg commit -m "Initial commit"
-        """,
-        """
-        echo "[paths]\ndefault = ../remoterepo" > .hg/hgrc
-        """,
         """
         echo "bar" >> bar
         echo "baz" > baz
@@ -36,18 +24,6 @@ setup = {
         """,
     ],
     'git': [
-        """
-        echo "foo" > foo
-        echo "bar" > bar
-        git init
-        git add *
-        git commit -am "Initial commit"
-        """,
-        """
-        git remote add upstream ../remoterepo
-        git fetch upstream
-        git branch -u upstream/master
-        """,
         """
         echo "bar" >> bar
         echo "baz" > baz
@@ -61,36 +37,6 @@ setup = {
 }
 
 
-def shell(cmd):
-    subprocess.check_call(cmd, shell=True)
-
-
-@pytest.yield_fixture(params=['git', 'hg'])
-def repo(tmpdir, request):
-    vcs = request.param
-
-    # tmpdir and repo are py.path objects
-    # http://py.readthedocs.io/en/latest/path.html
-    repo = tmpdir.mkdir('repo')
-    repo.vcs = vcs
-
-    # This creates a setup iterator. Each time next() is called
-    # on it, the next set of instructions will be executed.
-    repo.setup = (shell(cmd) for cmd in setup[vcs])
-
-    oldcwd = os.getcwd()
-    os.chdir(repo.strpath)
-
-    next(repo.setup)
-
-    repo.copy(tmpdir.join('remoterepo'))
-
-    next(repo.setup)
-
-    yield repo
-    os.chdir(oldcwd)
-
-
 def assert_files(actual, expected):
     assert set(map(os.path.basename, actual)) == set(expected)
 
@@ -101,7 +47,7 @@ def test_workdir_outgoing(repo):
 
     remotepath = '../remoterepo' if repo.vcs == 'hg' else 'upstream/master'
 
-    next(repo.setup)
+    next(repo.step)
 
     assert_files(vcs.get_changed_files('AM', 'all'), ['bar', 'baz'])
     if repo.vcs == 'git':
@@ -111,7 +57,7 @@ def test_workdir_outgoing(repo):
     assert_files(vcs.get_outgoing_files('AM'), [])
     assert_files(vcs.get_outgoing_files('AM', remotepath), [])
 
-    next(repo.setup)
+    next(repo.step)
 
     assert_files(vcs.get_changed_files('AM', 'all'), [])
     assert_files(vcs.get_changed_files('AM', 'staged'), [])