Bug 1401309 - [mozlint] Remove vcs.py and use mozversioncontrol instead, r=gps
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Mon, 25 Sep 2017 16:30:27 -0400
changeset 383486 4a92efa8e87d65ae1f468cbc93e7bb3c70f48012
parent 383485 f148c6eeb1d9f8a63c3901cf86dc12d1b52eb531
child 383487 e595d08c3ee222648f788e2f5b1ea29e9284fafc
push id32594
push userkwierso@gmail.com
push dateThu, 28 Sep 2017 22:49:33 +0000
treeherdermozilla-central@6dea0ee45b66 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs1401309
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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: