Bug 1401309 - [mozlint] Remove vcs.py and use mozversioncontrol instead, r?gps draft
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Mon, 25 Sep 2017 16:30:27 -0400
changeset 671959 e350889b001a917c8db4df8298c31d27c7c27899
parent 671958 ea4c86bcb01c486e947c71164e69c707895abdb3
child 672125 7eff4cf44b9e15199c4b3c298f61c0928c0e4f0d
child 672132 a26cc9b26f15dc234671bad10d9a6a5090c53f3c
child 672134 6bc474d6b60cb085d1f6015ff4acf11272c0dea1
child 672607 f130e69db189da6c5bc1fa392eeb67b13bd94432
push id82100
push userahalberstadt@mozilla.com
push dateThu, 28 Sep 2017 14:54:56 +0000
reviewersgps
bugs1401309
milestone58.0a1
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
python/moz.build
python/mozlint/mozlint/roller.py
python/mozlint/mozlint/vcs.py
python/mozlint/test/python.ini
python/mozlint/test/test_vcs.py
python/mozversioncontrol/test/python.ini
python/mozversioncontrol/test/test_workdir_outgoing.py
taskcluster/ci/source-test/python.yml
--- a/python/moz.build
+++ b/python/moz.build
@@ -35,15 +35,16 @@ SPHINX_PYTHON_PACKAGE_DIRS += [
 ]
 
 SPHINX_TREES['mach'] = 'mach/docs'
 
 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_UNITTEST_MANIFESTS += [
         'mozbuild/mozbuild/test/python.ini',
         'mozbuild/mozpack/test/python.ini',
     ]
--- a/python/mozlint/mozlint/roller.py
+++ b/python/mozlint/mozlint/roller.py
@@ -6,21 +6,23 @@ from __future__ import absolute_import, 
 
 import os
 import signal
 import sys
 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):
     results = defaultdict(list)
     failed = []
 
     func = supported_types[config['type']]
     res = func(paths, config, **lintargs) or []
@@ -50,23 +52,26 @@ class LintRoller(object):
     """Registers and runs linters.
 
     :param root: Path to which relative paths will be joined. If
                  unspecified, root will either be determined from
                  version control or cwd.
     :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
 
     def read(self, paths):
         """Parse one or more linters and add them to the registry.
 
         :param paths: A path or iterable of paths to linter definitions.
@@ -93,21 +98,33 @@ class LintRoller(object):
         if isinstance(paths, basestring):
             paths = set([paths])
         elif isinstance(paths, (list, tuple)):
             paths = set(paths)
 
         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")
             return {}
 
         paths = paths or ['.']
 
         # This will convert paths back to a list, but that's ok since
deleted file mode 100644
--- a/python/mozlint/mozlint/vcs.py
+++ /dev/null
@@ -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,
-}
--- a/python/mozlint/test/python.ini
+++ b/python/mozlint/test/python.ini
@@ -2,12 +2,8 @@
 subsuite = mozlint, os == "linux"
 
 [test_cli.py]
 [test_filterpaths.py]
 [test_formatters.py]
 [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"
deleted file mode 100644
--- a/python/mozlint/test/test_vcs.py
+++ /dev/null
@@ -1,124 +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
-
-import os
-import subprocess
-
-import mozunit
-import pytest
-
-from mozlint.vcs import VCSHelper, vcs_class
-
-
-setup = {
-    '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
-        hg add baz
-        hg rm foo
-        """,
-        """
-        hg commit -m "Remove foo; modify bar; add baz"
-        """,
-    ],
-    '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
-        git add baz
-        git rm foo
-        """,
-        """
-        git commit -am "Remove foo; modify bar; add baz"
-        """
-    ]
-}
-
-
-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)
-
-
-def test_vcs_helper(repo):
-    vcs = VCSHelper.create()
-    assert vcs.__class__ == vcs_class[repo.vcs]
-    assert vcs.root == repo.strpath
-
-    remotepath = '../remoterepo' if repo.vcs == 'hg' else 'upstream/master'
-
-    next(repo.setup)
-
-    assert_files(vcs.by_workdir('all'), ['bar', 'baz'])
-    if repo.vcs == 'git':
-        assert_files(vcs.by_workdir('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), [])
-
-    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'])
-
-
-if __name__ == '__main__':
-    mozunit.main()
new file mode 100644
--- /dev/null
+++ b/python/mozversioncontrol/test/python.ini
@@ -0,0 +1,4 @@
+[DEFAULT]
+subsuite=mozversioncontrol
+
+[test_workdir_outgoing.py]
new file mode 100644
--- /dev/null
+++ b/python/mozversioncontrol/test/test_workdir_outgoing.py
@@ -0,0 +1,123 @@
+# 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 mozunit
+import pytest
+
+from mozversioncontrol import get_repository_object
+
+
+setup = {
+    '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
+        hg add baz
+        hg rm foo
+        """,
+        """
+        hg commit -m "Remove foo; modify bar; add baz"
+        """,
+    ],
+    '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
+        git add baz
+        git rm foo
+        """,
+        """
+        git commit -am "Remove foo; modify bar; add baz"
+        """
+    ]
+}
+
+
+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)
+
+
+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.get_changed_files('AM', 'all'), ['bar', 'baz'])
+    if repo.vcs == 'git':
+        assert_files(vcs.get_changed_files('AM', mode='staged'), ['baz'])
+    elif repo.vcs == 'hg':
+        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.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__':
+    mozunit.main()
--- a/taskcluster/ci/source-test/python.yml
+++ b/taskcluster/ci/source-test/python.yml
@@ -145,16 +145,39 @@ mozlint:
     run:
         using: mach
         mach: python-test --subsuite mozlint
     when:
         files-changed:
             - '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:
         - linux64/opt
         - linux64/debug
         - linux64-asan/opt
     require-build: true
     treeherder: