Remove over 100 lines from checkperms.py.

tools/checkperms/checkperms.py

Index: tools/checkperms/checkperms.py
diff --git a/tools/checkperms/checkperms.py b/tools/checkperms/checkperms.py
index c3959f99ce9289fabcfae1a39588947cf89b9797..38bf05e7217dc2d8be4afffe9dc43a2ea87b551c 100755
--- a/tools/checkperms/checkperms.py
+++ b/tools/checkperms/checkperms.py
@@ -11,79 +11,29 @@ see .cc files in green and get confused.
 
 To ignore a particular file, add it to WHITELIST_FILES.
 To ignore a particular extension, add it to WHITELIST_EXTENSIONS.
-To ignore whatever regexps your heart desires, add it WHITELIST_REGEX.
+To ignore a particular path, add it WHITELIST_PATHS.
 
 Note that all directory separators must be slashes (Unix-style) and not
 backslashes. All directories should be relative to the source root and all
 file paths should be only lowercase.
 """
 
+import logging
 import optparse
 import os
-import pipes
-import re
 import stat
+import subprocess
 import sys
 
 #### USER EDITABLE SECTION STARTS HERE ####
 
 # Files with these extensions are allowed to have executable permissions.
-WHITELIST_EXTENSIONS = [
-    'bash',
-    'bat',
-    'dll',
-    'dylib',
-    'exe',
-    'pl',
-    'py',
-    'rb',
-    'sed',
-    'sh',
-]
-
-# Files that end the following paths are whitelisted too.
-WHITELIST_FILES = [
-    '/build/gyp_chromium',
-    '/build/linux/dump_app_syms',
-    '/build/linux/pkg-config-wrapper',
-    '/build/mac/strip_from_xcode',
-    '/build/mac/strip_save_dsym',
-    '/chrome/installer/mac/pkg-dmg',
-    '/chrome/tools/build/linux/chrome-wrapper',
-    '/chrome/tools/build/mac/build_app_dmg',
-    '/chrome/tools/build/mac/clean_up_old_versions',
-    '/chrome/tools/build/mac/dump_product_syms',
-    '/chrome/tools/build/mac/generate_localizer',
-    '/chrome/tools/build/mac/make_sign_sh',
-    '/chrome/tools/build/mac/verify_order',
-    '/o3d/build/gyp_o3d',
-    '/o3d/gypbuild',
-    '/o3d/installer/linux/debian.in/rules',
-    '/third_party/icu/source/runconfigureicu',
-    '/third_party/gold/gold32',
-    '/third_party/gold/gold64',
-    '/third_party/gold/ld',
-    '/third_party/gold/ld.bfd',
-    '/third_party/lcov/bin/gendesc',
-    '/third_party/lcov/bin/genhtml',
-    '/third_party/lcov/bin/geninfo',
-    '/third_party/lcov/bin/genpng',
-    '/third_party/lcov/bin/lcov',
-    '/third_party/lcov/bin/mcov',
-    '/third_party/lcov-1.9/bin/gendesc',
-    '/third_party/lcov-1.9/bin/genhtml',
-    '/third_party/lcov-1.9/bin/geninfo',
-    '/third_party/lcov-1.9/bin/genpng',
-    '/third_party/lcov-1.9/bin/lcov',
-    '/third_party/libxml/linux/xml2-config',
-    '/third_party/lzma_sdk/executable/7za.exe',
-    '/third_party/swig/linux/swig',
-    '/third_party/tcmalloc/chromium/src/pprof',
-    '/tools/deep_memory_profiler/dmprof',
-    '/tools/git/post-checkout',
-    '/tools/git/post-merge',
-    '/tools/ld_bfd/ld',
-]
+WHITELIST_EXTENSIONS = (
+  'bat',
+  'dll',
+  'dylib',
+  'exe',
+)
 
 # File names that are always whitelisted.  (These are all autoconf spew.)
 WHITELIST_FILENAMES = set((
@@ -94,251 +44,193 @@ WHITELIST_FILENAMES = set((
   'install-sh',
   'missing',
   'mkinstalldirs',
-  'scons',
   'naclsdk',
+  'scons',
 ))
 
 # File paths that contain these regexps will be whitelisted as well.
-WHITELIST_REGEX = [
-    re.compile('/third_party/openssl/'),
-    re.compile('/third_party/sqlite/'),
-    re.compile('/third_party/xdg-utils/'),
-    re.compile('/third_party/yasm/source/patched-yasm/config'),
-    re.compile('/third_party/ffmpeg/tools'),
-]
+WHITELIST_PATHS = (
+  '/third_party/openssl/',
+  '/third_party/sqlite/',
+  '/third_party/xdg-utils/',
+  '/third_party/yasm/source/patched-yasm/config',
+  '/third_party/ffmpeg/tools',
+)
 
 #### USER EDITABLE SECTION ENDS HERE ####
 
-WHITELIST_EXTENSIONS_REGEX = re.compile(r'\.(%s)' %
-                                        '|'.join(WHITELIST_EXTENSIONS))
-
-WHITELIST_FILES_REGEX = re.compile(r'(%s)$' % '|'.join(WHITELIST_FILES))
-
-# Set to true for more output. This is set by the command line options.
-VERBOSE = False
-
-# Using forward slashes as directory separators, ending in a forward slash.
-# Set by the command line options.
-BASE_DIRECTORY = ''
-
-# The default if BASE_DIRECTORY is not set on the command line.
-DEFAULT_BASE_DIRECTORY = '../../..'
-
-# The directories which contain the sources managed by git.
-GIT_SOURCE_DIRECTORY = set()
-
-# The SVN repository url.
-SVN_REPO_URL = ''
-
-# Whether we are using SVN or GIT.
-IS_SVN = True
-
-# Executable permission mask
-EXECUTABLE_PERMISSION = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH
 
+def is_verbose():
+  return logging.getLogger().isEnabledFor(logging.DEBUG)
 
-def IsWhiteListed(file_path):
-  """Returns True if file_path is in our whitelist of files to ignore."""
-  if WHITELIST_EXTENSIONS_REGEX.match(os.path.splitext(file_path)[1]):
-    return True
-  if WHITELIST_FILES_REGEX.search(file_path):
-    return True
-  if os.path.basename(file_path) in WHITELIST_FILENAMES:
-    return True
-  for regex in WHITELIST_REGEX:
-    if regex.search(file_path):
-      return True
-  return False
 
+def capture(cmd, cwd):
+  """Returns the output of a command.
 
-def CheckFile(file_path):
-  """Checks file_path's permissions.
-
-  Args:
-    file_path: The file path to check.
-
-  Returns:
-    Either a string describing the error if there was one, or None if the file
-    checked out OK.
+  Ignores the error code or stderr.
   """
-  if VERBOSE:
-    print 'Checking file: ' + file_path
-
-  file_path_lower = file_path.lower()
-  if IsWhiteListed(file_path_lower):
-    return None
-
-  # Not whitelisted, stat the file and check permissions.
-  try:
-    st_mode = os.stat(file_path).st_mode
-  except IOError, e:
-    return 'Failed to stat file: %s' % e
-  except OSError, e:
-    return 'Failed to stat file: %s' % e
-
-  if EXECUTABLE_PERMISSION & st_mode:
-    # Look if the file starts with #!/
+  env = os.environ.copy()
+  env['LANG'] = 'en_us.UTF-8'
+  p = subprocess.Popen(
+      cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, env=env)
+  return p.communicate()[0]
+
+
+class ApiBase(object):
+  def __init__(self, root_dir):
+    self.root_dir = root_dir
+
+  @staticmethod
+  def is_whitelisted(file_path):
+    """Returns True if file_path is in our whitelist of files to ignore.
+
+    file_path is a relative directory to the checkout root.
+    """
+    file_path = file_path.lower()
+    return (
+        os.path.splitext(file_path)[1][1:] in WHITELIST_EXTENSIONS or
+        os.path.basename(file_path) in WHITELIST_FILENAMES or
+        file_path.startswith(WHITELIST_PATHS))
+
+  @staticmethod
+  def has_executable_bit(file_path):
+    """Returns if any executable bit is set.
+
+    file_path is the absolute path to the file.
+    """
+    permission = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH
+    return bool(permission & os.stat(file_path).st_mode)
+
+  @staticmethod
+  def has_shebang(file_path):
+    """Returns if the file starts with #!/.
+
+    file_path is the absolute path to the file.
+    """
     with open(file_path, 'rb') as f:
-      if f.read(3) == '#!/':
-        # That's fine.
+      return f.read(3) == '#!/'
+
+  def check_file(self, file_path):
+    """Checks file_path's permissions and returns an error if it is
+    inconsistent.
+    """
+    if is_verbose():
+      print 'Checking file: %s' % file_path
+
+    if self.is_whitelisted(file_path):
+      return None
+
+    full_path = os.path.join(self.root_dir, file_path)
+    bit = self.has_executable_bit(full_path)
+    shebang = self.has_shebang(full_path)
+    if bit != shebang:
+      if bit:
+        return '%s: Has executable bit but not shebang' % file_path
+      else:
+        return '%s: Has shebang but not executable bit' % file_path
+
+  def check(self, start_dir):
+    """Check the files in start_dir, recursively check its subdirectories."""
+    errors = []
+    for root, dirs, files in os.walk(start_dir):
+      for f in files:
+        error = check_file(root_dir, os.path.join(root, f))
+        if error:
+          errors.append(error)
+    return errors
+
+
+class ApiSvn(ApiBase):
+  @staticmethod
+  def get_info(dir_path):
+    """Returns svn meta-data for a svn checkout."""
+    if not os.path.isdir(dir_path):
+      return {}
+    out = capture(['svn', 'info', '.'], dir_path)
+    return dict(l.split(': ', 1) for l in out.splitlines() if l)
+
+  @staticmethod
+  def get_root(dir_path):
+    """Returns the svn checkout root or None."""
+    svn_url = get_svn_info(dir_path).get('Repository Root:')
+    if not svn_url:
+      return None
+    while True:
+      parent = os.path.dirname(dir_path)
+      if parent == dir_path:
         return None
-    # TODO(maruel): Check that non-executable file do not start with a shebang.
-    error = 'Contains executable permission'
-    if VERBOSE:
-      return '%s: %06o' % (error, st_mode)
-    return error
-  return None
-
-
-def ShouldCheckDirectory(dir_path):
-  """Determine if we should check the content of dir_path."""
-  if not IS_SVN:
-    return dir_path in GIT_SOURCE_DIRECTORY
-  repo_url = GetSvnRepositoryRoot(dir_path)
-  if not repo_url:
-    return False
-  return repo_url == SVN_REPO_URL
-
-
-def CheckDirectory(dir_path):
-  """Check the files in dir_path; recursively check its subdirectories."""
-  # Collect a list of all files and directories to check.
-  files_to_check = []
-  dirs_to_check = []
-  success = True
-  contents = os.listdir(dir_path)
-  for cur in contents:
-    full_path = os.path.join(dir_path, cur)
-    if os.path.isdir(full_path) and ShouldCheckDirectory(full_path):
-      dirs_to_check.append(full_path)
-    elif os.path.isfile(full_path):
-      files_to_check.append(full_path)
-
-  # First check all files in this directory.
-  for cur_file in files_to_check:
-    file_status = CheckFile(cur_file)
-    if file_status is not None:
-      print 'ERROR in %s\n%s' % (cur_file, file_status)
-      success = False
-
-  # Next recurse into the subdirectories.
-  for cur_dir in dirs_to_check:
-    if not CheckDirectory(cur_dir):
-      success = False
-  return success
-
-
-def GetGitSourceDirectory(root):
-  """Returns a set of the directories to be checked.
-
-  Args:
-    root: The repository root where a .git directory must exist.
-
-  Returns:
-    A set of directories which contain sources managed by git.
-  """
-  git_source_directory = set()
-  popen_out = os.popen('cd %s && git ls-files --full-name .' %
-                       pipes.quote(root))
-  for line in popen_out:
-    dir_path = os.path.join(root, os.path.dirname(line))
-    git_source_directory.add(dir_path)
-  git_source_directory.add(root)
-  return git_source_directory
+      if svn_url != get_svn_info(parent).get('Repository Root:'):
+        return dir_path
+      dir_path = parent
 
+  def check(self, start_dir):
+    """Like ApiBase.check() excepts that it skips non-versioned directories."""
+    return super(ApiSvn, self).check(start_dir)
 
-def GetSvnRepositoryRoot(dir_path):
-  """Returns the repository root for a directory.
 
-  Args:
-    dir_path: A directory where a .svn subdirectory should exist.
+class ApiGit(ApiBase):
+  @staticmethod
+  def get_root(dir_path):
+    """Returns the git checkout root or None."""
+    root = capture(['git', 'rev-parse', '--show-toplevel'], dir_path).strip()
+    if root:
+      return root
+
+  def check(self, start_dir):
+    """Like ApiBase.check() excepts that it skips non-versioned directories."""
+    return super(ApiSvn, self).check(start_dir)
 
-  Returns:
-    The svn repository that contains dir_path or None.
-  """
-  svn_dir = os.path.join(dir_path, '.svn')
-  if not os.path.isdir(svn_dir):
-    return None
-  popen_out = os.popen('cd %s && svn info' % pipes.quote(dir_path))
-  for line in popen_out:
-    if line.startswith('Repository Root: '):
-      return line[len('Repository Root: '):].rstrip()
-  return None
 
+def get_scm(dir_path):
+  """Returns a properly configured ApiBase instance."""
+  root = ApiSvn.get_root(dir_path)
+  if root:
+    return ApiSvn(root)
+  root = ApiGit.get_root(dir_path)
+  if root:
+    return ApiGit(root)
 
-def main(argv):
+  # Returns a non-scm aware checker.
+  logging.warn('Failed to determine the SCM for %s' % dir_path)
+  return ApiBase(root)
+
+
+def main():
   usage = """Usage: python %prog [--root <root>] [tocheck]
   tocheck  Specifies the directory, relative to root, to check. This defaults
            to "." so it checks everything.
 
 Examples:
-  python checkperms.py
-  python checkperms.py --root /path/to/source chrome"""
-
-  option_parser = optparse.OptionParser(usage=usage)
-  option_parser.add_option('--root', dest='base_directory',
-                           default=DEFAULT_BASE_DIRECTORY,
-                           help='Specifies the repository root. This defaults '
-                           'to %default relative to the script file, which '
-                           'will normally be the repository root.')
-  option_parser.add_option('-v', '--verbose', action='store_true',
-                           help='Print debug logging')
-  options, args = option_parser.parse_args()
-
-  global VERBOSE
-  if options.verbose:
-    VERBOSE = True
-
-  # Optional base directory of the repository.
-  global BASE_DIRECTORY
-  if (not options.base_directory or
-      options.base_directory == DEFAULT_BASE_DIRECTORY):
-    BASE_DIRECTORY = os.path.abspath(
-        os.path.join(os.path.abspath(argv[0]), DEFAULT_BASE_DIRECTORY))
-  else:
-    BASE_DIRECTORY = os.path.abspath(argv[2])
-
-  # Figure out which directory we have to check.
-  if not args:
-    # No directory to check specified, use the repository root.
-    start_dir = BASE_DIRECTORY
-  elif len(args) == 1:
-    # Directory specified. Start here. It's supposed to be relative to the
-    # base directory.
-    start_dir = os.path.abspath(os.path.join(BASE_DIRECTORY, args[0]))
-  else:
-    # More than one argument, we don't handle this.
-    option_parser.print_help()
-    return 1
-
-  print 'Using base directory:', BASE_DIRECTORY
-  print 'Checking directory:', start_dir
-
-  BASE_DIRECTORY = BASE_DIRECTORY.replace('\\', '/')
-  start_dir = start_dir.replace('\\', '/')
-
-  success = True
-  if os.path.exists(os.path.join(BASE_DIRECTORY, '.svn')):
-    global SVN_REPO_URL
-    SVN_REPO_URL = GetSvnRepositoryRoot(BASE_DIRECTORY)
-    if not SVN_REPO_URL:
-      print 'Cannot determine the SVN repo URL'
-      success = False
-  elif os.path.exists(os.path.join(BASE_DIRECTORY, '.git')):
-    global IS_SVN
-    IS_SVN = False
-    global GIT_SOURCE_DIRECTORY
-    GIT_SOURCE_DIRECTORY = GetGitSourceDirectory(BASE_DIRECTORY)
-    if not GIT_SOURCE_DIRECTORY:
-      print 'Cannot determine the list of GIT directories'
-      success = False
-  else:
-    print 'Cannot determine the SCM used in %s' % BASE_DIRECTORY
-    success = False
-
-  if success:
-    success = CheckDirectory(start_dir)
-  if not success:
+  python %prog
+  python %prog --root /path/to/source chrome"""
+
+  parser = optparse.OptionParser(usage=usage)
+  parser.add_option(
+      '--root',
+      default=get_scm('.')[0],
+      help='Specifies the repository root. This defaults '
+           'to %default relative to the script file, which '
+           'will normally be the repository root.')
+  parser.add_option(
+      '-v', '--verbose', action='store_true', help='Print debug logging')
+  options, args = parser.parse_args()
+
+  logging.basicConfig(
+      level=(logging.DEBUG if options.verbose else logging.ERROR))
+
+  if len(args) > 1:
+    parser.error('Too many arguments used')
+  if args:
+    start_dir = args[0]
+
+  if not options.root:
+    parser.error('Must specify --root')
+  options.root = os.path.abspath(options.root)
+
+  # Guess again the SCM used.
+  api = get_scm(options.root)
+
+  if not api.check(start_dir):
     print '\nFAILED\n'
     return 1
   print '\nSUCCESS\n'
@@ -346,4 +238,4 @@ Examples:
 
 
 if '__main__' == __name__:
-  sys.exit(main(sys.argv))
+  sys.exit(main())