Bug 1401309 - [mozlint] Remove vcs.py and use mozversioncontrol instead, r=gps

This also migrates the vcs.py test to mozversioncontrol and adds a new task for
it.

MozReview-Commit-ID: 9jTRkjNupVA

--HG--
extra : rebase_source : 400f27498e00ea45234ad7c951770b098e916b8e

python/moz.build

@@ -40,6 +40,7 @@ PYTHON_UNITTEST_MANIFESTS += [
     'mach/mach/test/python.ini',
     'mozbuild/dumbmake/test/python.ini',
     'mozlint/test/python.ini',
+    'mozversioncontrol/test/python.ini',
 ]
 
 if CONFIG['MOZ_BUILD_APP']:

python/mozlint/mozlint/roller.py

@@ -11,11 +11,13 @@ import traceback
 from collections import defaultdict
 from concurrent.futures import ProcessPoolExecutor
 from multiprocessing import cpu_count
+from subprocess import CalledProcessError
+
+from mozversioncontrol import get_repository_object, MissingUpstreamRepo, InvalidRepoPath
 
 from .errors import LintersNotConfigured
 from .parser import Parser
 from .types import supported_types
-from .vcs import VCSHelper
 
 
 def _run_linters(config, paths, **lintargs):
@@ -55,13 +57,16 @@ class LintRoller(object):
     :param lintargs: Arguments to pass to the underlying linter(s).
     """
 
-    def __init__(self, root=None, **lintargs):
+    def __init__(self, root, **lintargs):
         self.parse = Parser()
-        self.vcs = VCSHelper.create()
+        try:
+            self.vcs = get_repository_object(root)
+        except InvalidRepoPath:
+            self.vcs = None
 
         self.linters = []
         self.lintargs = lintargs
-        self.lintargs['root'] = root or self.vcs.root or os.getcwd()
+        self.lintargs['root'] = root
 
         # linters that return non-zero
         self.failed = None
@@ -98,11 +103,23 @@ class LintRoller(object):
         if not self.linters:
             raise LintersNotConfigured
 
+        if not self.vcs and (workdir or outgoing):
+            print("error: '{}' is not a known repository, can't use "
+                  "--workdir or --outgoing".format(self.lintargs['root']))
+
         # Calculate files from VCS
-        if workdir:
-            paths.update(self.vcs.by_workdir(workdir))
-        if outgoing:
-            paths.update(self.vcs.by_outgoing(outgoing))
+        try:
+            if workdir:
+                paths.update(self.vcs.get_changed_files('AM', mode=workdir))
+            if outgoing:
+                try:
+                    paths.update(self.vcs.get_outgoing_files('AM', upstream=outgoing))
+                except MissingUpstreamRepo:
+                    print("warning: could not find default push, specify a remote for --outgoing")
+        except CalledProcessError as e:
+            print("error running: {}".format(' '.join(e.cmd)))
+            if e.output:
+                print(e.output)
 
         if not paths and (workdir or outgoing):
             print("warning: no files linted")

python/mozlint/mozlint/vcs.py

@@ -1,113 +0,0 @@
-# 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, print_function
-
-import os
-import subprocess
-
-
-class VCSHelper(object):
-    """A base VCS helper that always returns an empty list
-    for the case when no version control was found.
-    """
-    def __init__(self, root):
-        self.root = root
-
-    @classmethod
-    def find_vcs(cls):
-        # First check if we're in an hg repo, if not try git
-        commands = (
-            ['hg', 'root'],
-            ['git', 'rev-parse', '--show-toplevel'],
-        )
-
-        for cmd in commands:
-            proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-            output = proc.communicate()[0].strip()
-
-            if proc.returncode == 0:
-                return cmd[0], output
-        return 'none', ''
-
-    @classmethod
-    def create(cls):
-        vcs, root = cls.find_vcs()
-        return vcs_class[vcs](root)
-
-    def run(self, cmd):
-        try:
-            files = subprocess.check_output(cmd, stderr=subprocess.STDOUT).split()
-        except subprocess.CalledProcessError as e:
-            if e.output:
-                print(' '.join(cmd))
-                print(e.output)
-            return []
-        return [os.path.join(self.root, f) for f in files if f]
-
-    def by_workdir(self, mode):
-        return []
-
-    def by_outgoing(self, dest='default'):
-        return []
-
-
-class HgHelper(VCSHelper):
-    """A helper to find files to lint from Mercurial."""
-
-    def by_outgoing(self, dest='default'):
-        return self.run(['hg', 'outgoing', '--quiet', '--template',
-                         "{file_mods % '\\n{file}'}{file_adds % '\\n{file}'}", '-r', '.', dest])
-
-    def by_workdir(self, mode):
-        return self.run(['hg', 'status', '-amn'])
-
-
-class GitHelper(VCSHelper):
-    """A helper to find files to lint from Git."""
-    _default = None
-
-    @property
-    def default(self):
-        if self._default:
-            return self._default
-
-        ref = subprocess.check_output(['git', 'symbolic-ref', '-q', 'HEAD']).strip()
-        dest = subprocess.check_output(
-            ['git', 'for-each-ref', '--format=%(upstream:short)', ref]).strip()
-
-        if not dest:
-            branches = subprocess.check_output(['git', 'branch', '--list'])
-            for b in ('master', 'central', 'default'):
-                if b in branches and not ref.endswith(b):
-                    dest = b
-                    break
-
-        self._default = dest
-        return self._default
-
-    def by_outgoing(self, dest='default'):
-        if dest == 'default':
-            if not self.default:
-                print("warning: could not find default push, specify a remote for --outgoing")
-                return []
-            dest = self.default
-        comparing = '{}..HEAD'.format(self.default)
-        return self.run(['git', 'log', '--name-only', '--diff-filter=AM',
-                         '--oneline', '--pretty=format:', comparing])
-
-    def by_workdir(self, mode):
-        cmd = ['git', 'diff', '--name-only', '--diff-filter=AM']
-        if mode == 'staged':
-            cmd.append('--cached')
-        else:
-            cmd.append('HEAD')
-        return self.run(cmd)
-
-
-vcs_class = {
-    'git': GitHelper,
-    'hg': HgHelper,
-    'none': VCSHelper,
-}

python/mozlint/test/python.ini

@@ -7,7 +7,3 @@ subsuite = mozlint, os == "linux"
 [test_parser.py]
 [test_roller.py]
 [test_types.py]
-[test_vcs.py]
-# these tests run in the build images on non-linux, which have old
-# versions of mercurial and git installed
-skip-if = os != "linux"

python/mozversioncontrol/test/python.ini

@@ -0,0 +1,4 @@
+[DEFAULT]
+subsuite=mozversioncontrol
+
+[test_workdir_outgoing.py]

python/mozversioncontrol/test/test_workdir_outgoing.py

@@ -10,7 +10,7 @@ import subprocess
 import mozunit
 import pytest
 
-from mozlint.vcs import VCSHelper, vcs_class
+from mozversioncontrol import get_repository_object
 
 
 setup = {
@@ -95,29 +95,28 @@ def assert_files(actual, expected):
     assert set(map(os.path.basename, actual)) == set(expected)
 
 
-def test_vcs_helper(repo):
-    vcs = VCSHelper.create()
-    assert vcs.__class__ == vcs_class[repo.vcs]
-    assert vcs.root == repo.strpath
+def test_workdir_outgoing(repo):
+    vcs = get_repository_object(repo.strpath)
+    assert vcs.path == repo.strpath
 
     remotepath = '../remoterepo' if repo.vcs == 'hg' else 'upstream/master'
 
     next(repo.setup)
 
-    assert_files(vcs.by_workdir('all'), ['bar', 'baz'])
+    assert_files(vcs.get_changed_files('AM', 'all'), ['bar', 'baz'])
     if repo.vcs == 'git':
-        assert_files(vcs.by_workdir('staged'), ['baz'])
+        assert_files(vcs.get_changed_files('AM', mode='staged'), ['baz'])
     elif repo.vcs == 'hg':
-        assert_files(vcs.by_workdir('staged'), ['bar', 'baz'])
-    assert_files(vcs.by_outgoing(), [])
-    assert_files(vcs.by_outgoing(remotepath), [])
+        assert_files(vcs.get_changed_files('AM', 'staged'), ['bar', 'baz'])
+    assert_files(vcs.get_outgoing_files('AM'), [])
+    assert_files(vcs.get_outgoing_files('AM', remotepath), [])
 
     next(repo.setup)
 
-    assert_files(vcs.by_workdir('all'), [])
-    assert_files(vcs.by_workdir('staged'), [])
-    assert_files(vcs.by_outgoing(), ['bar', 'baz'])
-    assert_files(vcs.by_outgoing(remotepath), ['bar', 'baz'])
+    assert_files(vcs.get_changed_files('AM', 'all'), [])
+    assert_files(vcs.get_changed_files('AM', 'staged'), [])
+    assert_files(vcs.get_outgoing_files('AM'), ['bar', 'baz'])
+    assert_files(vcs.get_outgoing_files('AM', remotepath), ['bar', 'baz'])
 
 
 if __name__ == '__main__':

taskcluster/ci/source-test/python.yml

@@ -150,6 +150,29 @@ mozlint:
             - 'python/mozlint/**'
             - 'python/mach_commands.py'
 
+mozversioncontrol:
+    description: python/mozversioncontrol unit tests
+    platform: linux64/opt
+    treeherder:
+        symbol: py(vcs)
+        kind: test
+        tier: 2
+    worker-type:
+        by-platform:
+            linux64.*: aws-provisioner-v1/gecko-t-linux-xlarge
+    worker:
+        by-platform:
+            linux64.*:
+                docker-image: {in-tree: "lint"}
+                max-run-time: 3600
+    run:
+        using: mach
+        mach: python-test --subsuite mozversioncontrol
+    when:
+        files-changed:
+            - 'python/mozversioncontrol/**'
+            - 'python/mach_commands.py'
+
 reftest-harness:
     description: layout/tools/reftest unittests
     platform: