Fix path filtering to be on the relative path and no on the absolute path.

Add logging to presubmit_support.py.

BUG=
TEST=

Review URL: http://codereview.chromium.org/6004006

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@70378 0039d316-1c4b-4281-b951-d872f2087c98

PRESUBMIT.py

@@ -21,15 +21,27 @@ UNIT_TESTS = [
 
 def CommonChecks(input_api, output_api):
   output = []
+  # Verify that LocalPath() is local, e.g.:
+  #   os.path.join(PresubmitLocalPath(), LocalPath()) == AbsoluteLocalPath()
+  for i in input_api.AffectedFiles():
+    if (input_api.os_path.join(input_api.PresubmitLocalPath(), i.LocalPath()) !=
+        i.AbsoluteLocalPath()):
+      output.append(output_api.PresubmitError('Path inconsistency'))
+      # Return right away because it needs to be fixed first.
+      return output
+
   output.extend(input_api.canned_checks.RunPythonUnitTests(
       input_api,
       output_api,
       UNIT_TESTS))
   output.extend(WasGitClUploadHookModified(input_api, output_api))
 
-  white_list = [r'.*\.py$', r'.*git-try$']
+  white_list = [r'.*\.py$', r'^git-try$']
   black_list = list(input_api.DEFAULT_BLACK_LIST) + [
-      r'.*cpplint\.py$', r'.*git_cl\/.*']
+      r'^cpplint\.py$',
+      r'^git_cl[\/\\].*',
+      r'^git_cl_repo[\/\\].*',
+      r'^git_cl[\/\\]test[\/\\]rietveld.*']
   output.extend(input_api.canned_checks.RunPylint(
       input_api,
       output_api,

presubmit_canned_checks.py

@@ -450,6 +450,34 @@ def RunPythonUnitTests(input_api, output_api, unit_tests):
   return []
 
 
+def _FetchAllFiles(input_api, white_list, black_list):
+  """Hack to fetch all files."""
+  # We cannot use AffectedFiles here because we want to test every python
+  # file on each single python change. It's because a change in a python file
+  # can break another unmodified file.
+  # Use code similar to InputApi.FilterSourceFile()
+  def Find(filepath, filters):
+    for item in filters:
+      if input_api.re.match(item, filepath):
+        return True
+    return False
+
+  import os
+  files = []
+  path_len = len(input_api.PresubmitLocalPath())
+  for dirpath, dirnames, filenames in os.walk(input_api.PresubmitLocalPath()):
+    # Passes dirnames in black list to speed up search.
+    for item in dirnames[:]:
+      filepath = input_api.os_path.join(dirpath, item)[path_len + 1:]
+      if Find(filepath, black_list):
+        dirnames.remove(item)
+    for item in filenames:
+      filepath = input_api.os_path.join(dirpath, item)[path_len + 1:]
+      if Find(filepath, white_list) and not Find(filepath, black_list):
+        files.append(filepath)
+  return files
+
+
 def RunPylint(input_api, output_api, white_list=None, black_list=None):
   """Run pylint on python files.
 
@@ -468,28 +496,9 @@ def RunPylint(input_api, output_api, white_list=None, black_list=None):
   import warnings
   warnings.filterwarnings('ignore', category=DeprecationWarning)
   try:
-    # We cannot use AffectedFiles here because we want to test every python
-    # file on each single python change. It's because a change in a python file
-    # can break another unmodified file.
-    # Use code similar to InputApi.FilterSourceFile()
-    def Find(filepath, filters):
-      for item in filters:
-        if input_api.re.match(item, filepath):
-          return True
-      return False
-
-    import os
-    files = []
-    for dirpath, dirnames, filenames in os.walk(input_api.PresubmitLocalPath()):
-      # Passes dirnames in black list to speed up search.
-      for item in dirnames[:]:
-        if Find(input_api.os_path.join(dirpath, item), black_list):
-          dirnames.remove(item)
-      for item in filenames:
-        filepath = input_api.os_path.join(dirpath, item)
-        if Find(filepath, white_list) and not Find(filepath, black_list):
-          files.append(filepath)
-
+    files = _FetchAllFiles(input_api, white_list, black_list)
+    if not files:
+      return []
     # Now that at least one python file was modified and all the python files
     # were listed, try to run pylint.
     try:
@@ -509,7 +518,11 @@ def RunPylint(input_api, output_api, white_list=None, black_list=None):
           'sudo easy_install pylint"\n'
           'Cannot do static analysis of python files.')]
     if result:
-      return [output_api.PresubmitPromptWarning('Fix pylint errors first.')]
+      if input_api.is_committing:
+        error_type = output_api.PresubmitError
+      else:
+        error_type = output_api.PresubmitPromptWarning
+      return [error_type('Fix pylint errors first.')]
     return []
   finally:
     warnings.filterwarnings('default', category=DeprecationWarning)

presubmit_support.py

@@ -211,8 +211,8 @@ class InputApi(object):
       # All caps files like README and LICENCE.
       r".*\b[A-Z0-9_]+$",
       # SCM (can happen in dual SCM configuration). (Slightly over aggressive)
-      r".*\.git[\\\/].*",
-      r".*\.svn[\\\/].*",
+      r"(|.*[\\\/])\.git[\\\/].*",
+      r"(|.*[\\\/])\.svn[\\\/].*",
   )
 
   def __init__(self, change, presubmit_path, is_committing):
@@ -405,6 +405,7 @@ class AffectedFile(object):
     self._local_root = repository_root
     self._is_directory = None
     self._properties = {}
+    logging.debug('%s(%s)' % (self.__class__.__name__, self._path))
 
   def ServerPath(self):
     """Returns a path string that identifies the file in the SCM system.
@@ -802,6 +803,7 @@ def ListRelevantPresubmitFiles(files, root):
     if os.path.isfile(p):
       results.append(p)
 
+  logging.debug('Presubmit files: %s' % ','.join(results))
   return results
 
 
@@ -917,7 +919,9 @@ class PresubmitExecuter(object):
       function_name = 'CheckChangeOnUpload'
     if function_name in context:
       context['__args'] = (input_api, OutputApi())
+      logging.debug('Running %s in %s' % (function_name, presubmit_path))
       result = eval(function_name + '(*__args)', context)
+      logging.debug('Running %s done.' % function_name)
       if not (isinstance(result, types.TupleType) or
               isinstance(result, types.ListType)):
         raise exceptions.RuntimeError(