Bug 1289805 - Ensure the VCSFiles class returns paths that have been absolutized to the repository root, r?smacleod draft
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Tue, 09 Aug 2016 14:49:41 -0400
changeset 401283 7c0e2aad2004f41cd7a317dc8055b6d06099f24b
parent 401282 73facf8f23450ffa8a674849cda880542ae9d969
child 401284 947cba3fd850e62baf36c981bc99ece05b582595
push id26420
push userahalberstadt@mozilla.com
push dateTue, 16 Aug 2016 20:20:10 +0000
reviewerssmacleod
bugs1289805
milestone51.0a1
Bug 1289805 - Ensure the VCSFiles class returns paths that have been absolutized to the repository root, r?smacleod This makes sure we return absolute paths from the VCSFiles class. This is done so we don't accidentally normalize the relative paths onto $CWD in the event the relative path is valid in both places. MozReview-Commit-ID: 2QIuR2YqFos
python/mozlint/mozlint/cli.py
--- a/python/mozlint/mozlint/cli.py
+++ b/python/mozlint/mozlint/cli.py
@@ -57,58 +57,70 @@ class MozlintParser(ArgumentParser):
 
     def __init__(self, **kwargs):
         ArgumentParser.__init__(self, usage=self.__doc__, **kwargs)
 
         for cli, args in self.arguments:
             self.add_argument(*cli, **args)
 
 
-class VCFiles(object):
+class VCSFiles(object):
     def __init__(self):
+        self._root = None
         self._vcs = None
 
     @property
+    def root(self):
+        if self._root:
+            return self._root
+
+        # 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)
+            output = proc.communicate()[0].strip()
+
+            if proc.returncode == 0:
+                self._vcs = cmd[0]
+                self._root = output
+                return self._root
+
+    @property
     def vcs(self):
-        if self._vcs:
-            return self._vcs
-
-        self._vcs = 'none'
-        with open(os.devnull, 'wb') as DEVNULL:
-            if not subprocess.call(['hg', 'root'], stdout=DEVNULL):
-                self._vcs = 'hg'
-            elif not subprocess.call(['git', 'rev-parse'], stdout=DEVNULL):
-                self._vcs = 'git'
-        return self._vcs
+        return self._vcs or (self.root and self._vcs)
 
     @property
     def is_hg(self):
         return self.vcs == 'hg'
 
     @property
     def is_git(self):
         return self.vcs == 'git'
 
+    def _run(self, cmd):
+        files = subprocess.check_output(cmd).split()
+        return [os.path.join(self.root, f) for f in files]
+
     def by_rev(self, rev):
         if self.is_hg:
-            cmd = ['hg', 'log', '-T', '{files % "\\n{file}"}', '-r', rev]
-        elif self.is_git(self):
-            cmd = ['git', 'diff', '--name-only', rev]
-        else:
-            return []
-        return subprocess.check_output(cmd).split()
+            return self._run(['hg', 'log', '-T', '{files % "\\n{file}"}', '-r', rev])
+        elif self.is_git:
+            return self._run(['git', 'diff', '--name-only', rev])
+        return []
 
     def by_workdir(self):
         if self.is_hg:
-            cmd = ['hg', 'status', '-amn']
-        elif self.is_git(self):
-            cmd = ['git', 'diff', '--name-only']
-        else:
-            return []
-        return subprocess.check_output(cmd).split()
+            return self._run(['hg', 'status', '-amn'])
+        elif self.is_git:
+            return self._run(['git', 'diff', '--name-only'])
+        return []
 
 
 def find_linters(linters=None):
     lints = []
     for search_path in SEARCH_PATHS:
         if not os.path.isdir(search_path):
             continue
 
@@ -124,21 +136,21 @@ def find_linters(linters=None):
             lints.append(os.path.join(search_path, f))
     return lints
 
 
 def run(paths, linters, fmt, rev, workdir, **lintargs):
     from mozlint import LintRoller, formatters
 
     # Calculate files from VCS
-    vcfiles = VCFiles()
+    vcs = VCSFiles()
     if rev:
-        paths.extend(vcfiles.by_rev(rev))
+        paths.extend(vcs.by_rev(rev))
     if workdir:
-        paths.extend(vcfiles.by_workdir())
+        paths.extend(vcs.by_workdir())
     paths = paths or ['.']
 
     lint = LintRoller(**lintargs)
     lint.read(find_linters(linters))
 
     # run all linters
     results = lint.roll(paths)