git-cl: Add yapfignore support to git cl format

Adds support for .yapfignore files to "git cl format" when formatting
Python files. yapf is supposed to parse the .yapfignore file in the
current working directory, but this appears to not work when files
are explicitly passed to yapf for formatting like they are in git cl
format. Instead, parse the .yapfignore file ourselves and skip over any
matching files.

Bug: angleproject:3985
Change-Id: I5e8469470fb8ddbaa914005b012ac1f39dfdd223
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1849807Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>

git_cl.py

@@ -14,6 +14,7 @@ from multiprocessing.pool import ThreadPool
 import base64
 import collections
 import datetime
+import glob
 import httplib
 import itertools
 import json
@@ -763,6 +764,44 @@ def _FindYapfConfigFile(fpath, yapf_config_cache, top_dir=None):
   return ret
 
 
+def _GetYapfIgnoreFilepaths(top_dir):
+  """Returns all filepaths that match the ignored files in the .yapfignore file.
+
+  yapf is supposed to handle the ignoring of files listed in .yapfignore itself,
+  but this functionality appears to break when explicitly passing files to
+  yapf for formatting. According to
+  https://github.com/google/yapf/blob/master/README.rst#excluding-files-from-formatting-yapfignore,
+  the .yapfignore file should be in the directory that yapf is invoked from,
+  which we assume to be the top level directory in this case.
+
+  Args:
+    top_dir: The top level directory for the repository being formatted.
+
+  Returns:
+    A set of all filepaths that should be ignored by yapf.
+  """
+  yapfignore_file = os.path.join(top_dir, '.yapfignore')
+  ignore_filepaths = set()
+  if not os.path.exists(yapfignore_file):
+    return ignore_filepaths
+
+  # glob works relative to the current working directory, so we need to ensure
+  # that we're at the top level directory.
+  old_cwd = os.getcwd()
+  try:
+    os.chdir(top_dir)
+    with open(yapfignore_file) as f:
+      for line in f.readlines():
+        stripped_line = line.strip()
+        # Comments and blank lines should be ignored.
+        if stripped_line.startswith('#') or stripped_line == '':
+          continue
+        ignore_filepaths |= set(glob.glob(stripped_line))
+    return ignore_filepaths
+  finally:
+    os.chdir(old_cwd)
+
+
 def print_stats(args):
   """Prints statistics about the change to the user."""
   # --no-ext-diff is broken in some versions of Git, so try to work around
@@ -5202,7 +5241,11 @@ def CMDformat(parser, args):
     if not opts.full and filtered_py_files:
       py_line_diffs = _ComputeDiffLineRanges(filtered_py_files, upstream_commit)
 
+    ignored_yapf_files = _GetYapfIgnoreFilepaths(top_dir)
+
     for f in filtered_py_files:
+      if f in ignored_yapf_files:
+        continue
       yapf_config = _FindYapfConfigFile(f, yapf_configs, top_dir)
       if yapf_config is None:
         yapf_config = chromium_default_yapf_style

tests/git_cl_test.py

@@ -9,6 +9,7 @@ import datetime
 import json
 import logging
 import os
+import shutil
 import StringIO
 import sys
 import tempfile
@@ -3388,6 +3389,93 @@ class CMDUploadTestCase(CMDTestCaseBase):
     git_cl._trigger_try_jobs.assert_called_once_with(
         mock.ANY, mock.ANY, expected_buckets, mock.ANY, 8)
 
+
+class CMDFormatTestCase(TestCase):
+
+  def setUp(self):
+    super(CMDFormatTestCase, self).setUp()
+    self._top_dir = tempfile.mkdtemp()
+
+  def tearDown(self):
+    shutil.rmtree(self._top_dir)
+    super(CMDFormatTestCase, self).tearDown()
+
+  def _make_yapfignore(self, contents):
+    with open(os.path.join(self._top_dir, '.yapfignore'), 'w') as yapfignore:
+      yapfignore.write('\n'.join(contents))
+
+  def _make_files(self, file_dict):
+    for directory, files in file_dict.iteritems():
+      subdir = os.path.join(self._top_dir, directory)
+      if not os.path.exists(subdir):
+        os.makedirs(subdir)
+      for f in files:
+        with open(os.path.join(subdir, f), 'w'):
+          pass
+
+  def testYapfignoreBasic(self):
+    self._make_yapfignore(['test.py', '*/bar.py'])
+    self._make_files({
+        '.': ['test.py', 'bar.py'],
+        'foo': ['bar.py'],
+    })
+    self.assertEqual(
+        set(['test.py', 'foo/bar.py']),
+        git_cl._GetYapfIgnoreFilepaths(self._top_dir))
+
+  def testYapfignoreMissingYapfignore(self):
+    self.assertEqual(set(), git_cl._GetYapfIgnoreFilepaths(self._top_dir))
+
+  def testYapfignoreMissingFile(self):
+    self._make_yapfignore(['test.py', 'test2.py', 'test3.py'])
+    self._make_files({
+        '.': ['test.py', 'test3.py'],
+    })
+    self.assertEqual(
+        set(['test.py', 'test3.py']),
+        git_cl._GetYapfIgnoreFilepaths(self._top_dir))
+
+  def testYapfignoreComments(self):
+    self._make_yapfignore(['test.py', '#test2.py'])
+    self._make_files({
+        '.': ['test.py', 'test2.py'],
+    })
+    self.assertEqual(
+        set(['test.py']), git_cl._GetYapfIgnoreFilepaths(self._top_dir))
+
+  def testYapfignoreBlankLines(self):
+    self._make_yapfignore(['test.py', '', '', 'test2.py'])
+    self._make_files({'.': ['test.py', 'test2.py']})
+    self.assertEqual(
+        set(['test.py', 'test2.py']),
+        git_cl._GetYapfIgnoreFilepaths(self._top_dir))
+
+  def testYapfignoreWhitespace(self):
+    self._make_yapfignore([' test.py '])
+    self._make_files({'.': ['test.py']})
+    self.assertEqual(
+        set(['test.py']), git_cl._GetYapfIgnoreFilepaths(self._top_dir))
+
+  def testYapfignoreMultiWildcard(self):
+    self._make_yapfignore(['*es*.py'])
+    self._make_files({
+        '.': ['test.py', 'test2.py'],
+    })
+    self.assertEqual(
+        set(['test.py', 'test2.py']),
+        git_cl._GetYapfIgnoreFilepaths(self._top_dir))
+
+  def testYapfignoreRestoresDirectory(self):
+    self._make_yapfignore(['test.py'])
+    self._make_files({
+        '.': ['test.py'],
+    })
+    old_cwd = os.getcwd()
+    self.assertEqual(
+        set(['test.py']), git_cl._GetYapfIgnoreFilepaths(self._top_dir))
+    self.assertEqual(old_cwd, os.getcwd())
+
+
 if __name__ == '__main__':
   logging.basicConfig(
       level=logging.DEBUG if '-v' in sys.argv else logging.ERROR)