Implement ability to specify temporarily-allowed dependencies in DEPS

files, and use this ability in a few DEPS files where appropriate.
This has no effect on the normal running of checkdeps; "!"
dependencies are treated just like "+" dependencies when checkdeps is
run on our bots.

An upcoming change will use the new checkdeps.CheckAddedIncludes
function, and will error out if you add a new include that violates a
"-" rule, and show a presubmit warning when you add a new include that
violates a "!" rule (the warning will say something like "We are in
the process of removing dependencies from this directory to that file,
can you avoid adding more?"

While I was in there, fixed path handling so that checkdeps will work
on case-sensitive platforms with paths that include upper-case
characters (e.g. a checkout of Chrome at ~/c/Chrome/src rather than
~/c/chrome/src).

Since the pipes.quote method seems unreliable on Windows (it failed on
my setup), switched to subprocess.list2cmdline which I believe is
stable.

Added a small manual testing mode to checkdeps.  It currently only
verifies the CheckAddedIncludes function.

TBR=jam@chromium.org
BUG=138280

Review URL: https://chromiumcodereview.appspot.com/10805042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149163 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 592 insertions, 295 deletions

diff --git a/tools/checkdeps/DEPS b/tools/checkdeps/DEPS
new file mode 100644
index 0000000..7a57b0b
--- /dev/null
+++ b/tools/checkdeps/DEPS
@@ -0,0 +1,3 @@
+skip_child_includes = [
+  "testdata",
+]
diff --git a/tools/checkdeps/PRESUBMIT.py b/tools/checkdeps/PRESUBMIT.py
new file mode 100644
index 0000000..10ef632
--- /dev/null
+++ b/tools/checkdeps/PRESUBMIT.py
@@ -0,0 +1,25 @@
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Presubmit script for checkdeps tool.
+"""
+
+
+def CheckChange(input_api, output_api):
+  results = []
+  results.extend(input_api.canned_checks.RunUnitTests(
+      input_api, output_api,
+      [input_api.os_path.join(input_api.PresubmitLocalPath(),
+                              'checkdeps_test.py')]))
+  return results
+
+
+# Mandatory entrypoint.
+def CheckChangeOnUpload(input_api, output_api):
+  return CheckChange(input_api, output_api)
+
+
+# Mandatory entrypoint.
+def CheckChangeOnCommit(input_api, output_api):
+  return CheckChange(input_api, output_api)
diff --git a/tools/checkdeps/checkdeps.py b/tools/checkdeps/checkdeps.py
index 9624812..7661d9d 100755
--- a/tools/checkdeps/checkdeps.py
+++ b/tools/checkdeps/checkdeps.py
@@ -18,9 +18,17 @@ gclient. An example would be:
     "base":"http://foo.bar/trunk/base"
   }
 
-DEPS files not in the top-level of a module won't need this. Then you have
-any additional include rules. You can add (using "+") or subtract (using "-")
-from the previously specified rules (including module-level deps).
+DEPS files not in the top-level of a module won't need this. Then you
+have any additional include rules. You can add (using "+") or subtract
+(using "-") from the previously specified rules (including
+module-level deps). You can also specify a path that is allowed for
+now but that we intend to remove, using "!"; this is treated the same
+as "+" when check_deps is run by our bots, but a presubmit step will
+show a warning if you add a new include of a file that is only allowed
+by "!".
+
+Note that for .java files, there is currently no difference between
+"+" and "!", even in the presubmit step.
 
   include_rules = {
     # Code should be able to use base (it's specified in the module-level
@@ -32,7 +40,11 @@ from the previously specified rules (including module-level deps).
 
     # And it can include files from this other directory even though there is
     # no deps rule for it.
-    "+tools/crime_fighter"
+    "+tools/crime_fighter",
+
+    # This dependency is allowed for now but work is ongoing to remove it,
+    # so you shouldn't add further dependencies on it.
+    "!base/evil/ok_for_now.h",
   }
 
 DEPS files may be placed anywhere in the tree. Each one applies to all
@@ -54,12 +66,13 @@ only lowercase.
 
 import os
 import optparse
-import pipes
+import subprocess
 import sys
 import copy
 
 import cpp_checker
 import java_checker
+from rules import Rule, Rules
 
 
 # Variable name used in the DEPS file to add or subtract include files from
@@ -70,257 +83,296 @@ INCLUDE_RULES_VAR_NAME = "include_rules"
 # be checked. This allows us to skip third party code, for example.
 SKIP_SUBDIRS_VAR_NAME = "skip_child_includes"
 
-# Set to true for more output. This is set by the command line options.
-VERBOSE = False
 
-# In lowercase, using forward slashes as directory separators, ending in a
-# forward slash. Set by the command line options.
-BASE_DIRECTORY = ""
+def NormalizePath(path):
+  """Returns a path normalized to how we write DEPS rules and compare paths.
+  """
+  return path.lower().replace('\\', '/')
 
-# The directories which contain the sources managed by git.
-GIT_SOURCE_DIRECTORY = set()
 
+class DepsChecker(object):
+  """Parses include_rules from DEPS files and can verify files in the
+  source tree against them.
+  """
 
-# Specifies a single rule for an include, which can be either allow or disallow.
-class Rule(object):
-  def __init__(self, allow, directory, source):
-    self.allow = allow
-    self._dir = directory
-    self._source = source
+  def __init__(self, base_directory=None, verbose=False, being_tested=False):
+    """Creates a new DepsChecker.
 
-  def __str__(self):
-    if (self.allow):
-      return '"+%s" from %s.' % (self._dir, self._source)
-    return '"-%s" from %s.' % (self._dir, self._source)
+    Args:
+      base_directory: OS-compatible path to root of checkout, e.g. C:\chr\src.
+      verbose: Set to true for debug output.
+      being_tested: Set to true to ignore the DEPS file at tools/checkdeps/DEPS.
+    """
+    self.base_directory = base_directory
+    if not base_directory:
+      self.base_directory = os.path.abspath(
+        os.path.join(os.path.abspath(os.path.dirname(__file__)), '..', '..'))
 
-  def ParentOrMatch(self, other):
-    """Returns true if the input string is an exact match or is a parent
-    of the current rule. For example, the input "foo" would match "foo/bar"."""
-    return self._dir == other or self._dir.startswith(other + "/")
+    self.verbose = verbose
 
-  def ChildOrMatch(self, other):
-    """Returns true if the input string would be covered by this rule. For
-    example, the input "foo/bar" would match the rule "foo"."""
-    return self._dir == other or other.startswith(self._dir + "/")
+    self.git_source_directories = set()
+    self._AddGitSourceDirectories()
 
+    self._under_test = being_tested
 
-def ParseRuleString(rule_string, source):
-  """Returns a tuple of a boolean indicating whether the directory is an allow
-  rule, and a string holding the directory name.
-  """
-  if len(rule_string) < 1:
-    raise Exception('The rule string "%s" is too short\nin %s' %
-                    (rule_string, source))
+  def _ApplyRules(self, existing_rules, includes, cur_dir):
+    """Applies the given include rules, returning the new rules.
 
-  if rule_string[0] == "+":
-    return (True, rule_string[1:])
-  if rule_string[0] == "-":
-    return (False, rule_string[1:])
-  raise Exception('The rule string "%s" does not begin with a "+" or a "-"' %
-                  rule_string)
+    Args:
+      existing_rules: A set of existing rules that will be combined.
+      include: The list of rules from the "include_rules" section of DEPS.
+      cur_dir: The current directory, normalized path. We will create an
+               implicit rule that allows inclusion from this directory.
 
+    Returns: A new set of rules combining the existing_rules with the other
+             arguments.
+    """
+    rules = copy.copy(existing_rules)
 
-class Rules:
-  def __init__(self):
-    """Initializes the current rules with an empty rule list."""
-    self._rules = []
+    # First apply the implicit "allow" rule for the current directory.
+    if cur_dir.startswith(
+          NormalizePath(os.path.normpath(self.base_directory))):
+      relative_dir = cur_dir[len(self.base_directory) + 1:]
 
-  def __str__(self):
-    ret = "Rules = [\n"
-    ret += "\n".join([" %s" % x for x in self._rules])
-    ret += "]\n"
-    return ret
+      source = relative_dir
+      if len(source) == 0:
+        source = "top level"  # Make the help string a little more meaningful.
+      rules.AddRule("+" + relative_dir, "Default rule for " + source)
+    else:
+      raise Exception("Internal error: base directory is not at the beginning" +
+                      " for\n  %s and base dir\n  %s" %
+                      (cur_dir, self.base_directory))
 
-  def AddRule(self, rule_string, source):
-    """Adds a rule for the given rule string.
+    # Last, apply the additional explicit rules.
+    for (_, rule_str) in enumerate(includes):
+      if not relative_dir:
+        rule_description = "the top level include_rules"
+      else:
+        rule_description = relative_dir + "'s include_rules"
+      rules.AddRule(rule_str, rule_description)
 
-    Args:
-      rule_string: The include_rule string read from the DEPS file to apply.
-      source: A string representing the location of that string (filename, etc.)
-              so that we can give meaningful errors.
-    """
-    (add_rule, rule_dir) = ParseRuleString(rule_string, source)
-    # Remove any existing rules or sub-rules that apply. For example, if we're
-    # passed "foo", we should remove "foo", "foo/bar", but not "foobar".
-    self._rules = [x for x in self._rules if not x.ParentOrMatch(rule_dir)]
-    self._rules.insert(0, Rule(add_rule, rule_dir, source))
-
-  def DirAllowed(self, allowed_dir):
-    """Returns a tuple (success, message), where success indicates if the given
-    directory is allowed given the current set of rules, and the message tells
-    why if the comparison failed."""
-    for rule in self._rules:
-      if rule.ChildOrMatch(allowed_dir):
-        # This rule applies.
-        if rule.allow:
-          return (True, "")
-        return (False, rule.__str__())
-    # No rules apply, fail.
-    return (False, "no rule applying")
-
-
-def ApplyRules(existing_rules, includes, cur_dir):
-  """Applies the given include rules, returning the new rules.
-
-  Args:
-    existing_rules: A set of existing rules that will be combined.
-    include: The list of rules from the "include_rules" section of DEPS.
-    cur_dir: The current directory. We will create an implicit rule that
-             allows inclusion from this directory.
-
-  Returns: A new set of rules combining the existing_rules with the other
-           arguments.
-  """
-  rules = copy.copy(existing_rules)
-
-  # First apply the implicit "allow" rule for the current directory.
-  if cur_dir.lower().startswith(BASE_DIRECTORY):
-    relative_dir = cur_dir[len(BASE_DIRECTORY) + 1:]
-    # Normalize path separators to slashes.
-    relative_dir = relative_dir.replace("\\", "/")
-    source = relative_dir
-    if len(source) == 0:
-      source = "top level"  # Make the help string a little more meaningful.
-    rules.AddRule("+" + relative_dir, "Default rule for " + source)
-  else:
-    raise Exception("Internal error: base directory is not at the beginning" +
-                    " for\n  %s and base dir\n  %s" %
-                    (cur_dir, BASE_DIRECTORY))
-
-  # Last, apply the additional explicit rules.
-  for (_, rule_str) in enumerate(includes):
-    if not len(relative_dir):
-      rule_description = "the top level include_rules"
-    else:
-      rule_description = relative_dir + "'s include_rules"
-    rules.AddRule(rule_str, rule_description)
+    return rules
 
-  return rules
+  def _ApplyDirectoryRules(self, existing_rules, dir_name):
+    """Combines rules from the existing rules and the new directory.
 
+    Any directory can contain a DEPS file. Toplevel DEPS files can contain
+    module dependencies which are used by gclient. We use these, along with
+    additional include rules and implicit rules for the given directory, to
+    come up with a combined set of rules to apply for the directory.
 
-def ApplyDirectoryRules(existing_rules, dir_name):
-  """Combines rules from the existing rules and the new directory.
+    Args:
+      existing_rules: The rules for the parent directory. We'll add-on to these.
+      dir_name: The directory name that the deps file may live in (if
+                it exists).  This will also be used to generate the
+                implicit rules.  This is a non-normalized path.
+
+    Returns: A tuple containing: (1) the combined set of rules to apply to the
+             sub-tree, and (2) a list of all subdirectories that should NOT be
+             checked, as specified in the DEPS file (if any).
+    """
+    norm_dir_name = NormalizePath(dir_name)
+
+    # Check for a .svn directory in this directory or check this directory is
+    # contained in git source direcotries. This will tell us if it's a source
+    # directory and should be checked.
+    if not (os.path.exists(os.path.join(dir_name, ".svn")) or
+            (norm_dir_name in self.git_source_directories)):
+      return (None, [])
+
+    # Check the DEPS file in this directory.
+    if self.verbose:
+      print "Applying rules from", dir_name
+    def FromImpl(_unused, _unused2):
+      pass  # NOP function so "From" doesn't fail.
+
+    def FileImpl(_unused):
+      pass  # NOP function so "File" doesn't fail.
+
+    class _VarImpl:
+      def __init__(self, local_scope):
+        self._local_scope = local_scope
+
+      def Lookup(self, var_name):
+        """Implements the Var syntax."""
+        if var_name in self._local_scope.get("vars", {}):
+          return self._local_scope["vars"][var_name]
+        raise Exception("Var is not defined: %s" % var_name)
+
+    local_scope = {}
+    global_scope = {
+        "File": FileImpl,
+        "From": FromImpl,
+        "Var": _VarImpl(local_scope).Lookup,
+        }
+    deps_file = os.path.join(dir_name, "DEPS")
+
+    # The second conditional here is to disregard the
+    # tools/checkdeps/DEPS file while running tests.  This DEPS file
+    # has a skip_child_includes for 'testdata' which is necessary for
+    # running production tests, since there are intentional DEPS
+    # violations under the testdata directory.  On the other hand when
+    # running tests, we absolutely need to verify the contents of that
+    # directory to trigger those intended violations and see that they
+    # are handled correctly.
+    if os.path.isfile(deps_file) and (
+        not self._under_test or not os.path.split(dir_name)[1] == 'checkdeps'):
+      execfile(deps_file, global_scope, local_scope)
+    elif self.verbose:
+      print "  No deps file found in", dir_name
+
+    # Even if a DEPS file does not exist we still invoke ApplyRules
+    # to apply the implicit "allow" rule for the current directory
+    include_rules = local_scope.get(INCLUDE_RULES_VAR_NAME, [])
+    skip_subdirs = local_scope.get(SKIP_SUBDIRS_VAR_NAME, [])
+
+    return (self._ApplyRules(existing_rules, include_rules, norm_dir_name),
+            skip_subdirs)
+
+  def CheckDirectory(self, start_dir):
+    """Checks all relevant source files in the specified directory and
+    its subdirectories for compliance with DEPS rules throughout the
+    tree (starting at |self.base_directory|).  |start_dir| must be a
+    subdirectory of |self.base_directory|.
+
+    Returns an empty array on success.  On failure, the array contains
+    strings that can be printed as human-readable error messages.
+    """
+    # TODO(joi): Make this work for start_dir != base_dir (I have a
+    # subsequent change in flight to do this).
+    base_rules = Rules()
+    java = java_checker.JavaChecker(self.base_directory, self.verbose)
+    cpp = cpp_checker.CppChecker(self.verbose)
+    checkers = dict(
+        (extension, checker)
+        for checker in [java, cpp] for extension in checker.EXTENSIONS)
+    return self._CheckDirectoryImpl(base_rules, checkers, start_dir)
+
+  def _CheckDirectoryImpl(self, parent_rules, checkers, dir_name):
+    (rules, skip_subdirs) = self._ApplyDirectoryRules(parent_rules, dir_name)
+    if rules == None:
+      return []
+
+    # Collect a list of all files and directories to check.
+    files_to_check = []
+    dirs_to_check = []
+    results = []
+    contents = os.listdir(dir_name)
+    for cur in contents:
+      if cur in skip_subdirs:
+        continue  # Don't check children that DEPS has asked us to skip.
+      full_name = os.path.join(dir_name, cur)
+      if os.path.isdir(full_name):
+        dirs_to_check.append(full_name)
+      elif os.path.splitext(full_name)[1] in checkers:
+        files_to_check.append(full_name)
+
+    # First check all files in this directory.
+    for cur in files_to_check:
+      checker = checkers[os.path.splitext(cur)[1]]
+      file_status = checker.CheckFile(rules, cur)
+      if file_status:
+        results.append("ERROR in " + cur + "\n" + file_status)
+
+    # Next recurse into the subdirectories.
+    for cur in dirs_to_check:
+      results.extend(self._CheckDirectoryImpl(rules, checkers, cur))
+    return results
+
+  def CheckAddedCppIncludes(self, added_includes):
+    """This is used from PRESUBMIT.py to check new #include statements added in
+    the change being presubmit checked.
 
-  Any directory can contain a DEPS file. Toplevel DEPS files can contain
-  module dependencies which are used by gclient. We use these, along with
-  additional include rules and implicit rules for the given directory, to
-  come up with a combined set of rules to apply for the directory.
+    Args:
+      added_includes: ((file_path, (include_line, include_line, ...), ...)
 
-  Args:
-    existing_rules: The rules for the parent directory. We'll add-on to these.
-    dir_name: The directory name that the deps file may live in (if it exists).
-              This will also be used to generate the implicit rules.
+    Return:
+      A list of tuples, (bad_file_path, rule_type, rule_description)
+      where rule_type is one of Rule.DISALLOW or Rule.TEMP_ALLOW and
+      rule_description is human-readable. Empty if no problems.
+    """
+    # Map of normalized directory paths to rules to use for those
+    # directories, or None for directories that should be skipped.
+    directory_rules = {}
+
+    def ApplyDirectoryRulesAndSkipSubdirs(parent_rules, dir_path):
+      rules_tuple = self._ApplyDirectoryRules(parent_rules, dir_path)
+      directory_rules[NormalizePath(dir_path)] = rules_tuple[0]
+      for subdir in rules_tuple[1]:
+        # We skip this one case for running tests.
+        directory_rules[NormalizePath(
+            os.path.normpath(os.path.join(dir_path, subdir)))] = None
+
+    ApplyDirectoryRulesAndSkipSubdirs(Rules(), self.base_directory)
+
+    def GetDirectoryRules(dir_path):
+      """Returns a Rules object to use for the given directory, or None
+      if the given directory should be skipped.
+      """
+      norm_dir_path = NormalizePath(dir_path)
+
+      if not dir_path.startswith(
+          NormalizePath(os.path.normpath(self.base_directory))):
+        dir_path = os.path.join(self.base_directory, dir_path)
+        norm_dir_path = NormalizePath(dir_path)
+
+      parent_dir = os.path.dirname(dir_path)
+      parent_rules = None
+      if not norm_dir_path in directory_rules:
+        parent_rules = GetDirectoryRules(parent_dir)
+
+      # We need to check for an entry for our dir_path again, in case we
+      # are at a path e.g. A/B/C where A/B/DEPS specifies the C
+      # subdirectory to be skipped; in this case, the invocation to
+      # GetDirectoryRules(parent_dir) has already filled in an entry for
+      # A/B/C.
+      if not norm_dir_path in directory_rules:
+        if not parent_rules:
+          # If the parent directory should be skipped, then the current
+          # directory should also be skipped.
+          directory_rules[norm_dir_path] = None
+        else:
+          ApplyDirectoryRulesAndSkipSubdirs(parent_rules, dir_path)
+      return directory_rules[norm_dir_path]
+
+    cpp = cpp_checker.CppChecker(self.verbose)
+
+    problems = []
+    for file_path, include_lines in added_includes:
+      # TODO(joi): Make this cover Java as well.
+      if not os.path.splitext(file_path)[1] in cpp.EXTENSIONS:
+        pass
+      rules_for_file = GetDirectoryRules(os.path.dirname(file_path))
+      if rules_for_file:
+        for line in include_lines:
+          is_include, line_status, rule_type = cpp.CheckLine(
+              rules_for_file, line, True)
+          if rule_type != Rule.ALLOW:
+            problems.append((file_path, rule_type, line_status))
+    return problems
+
+  def _AddGitSourceDirectories(self):
+    """Adds any directories containing sources managed by git to
+    self.git_source_directories.
+    """
+    if not os.path.exists(os.path.join(self.base_directory, ".git")):
+      return
 
-  Returns: A tuple containing: (1) the combined set of rules to apply to the
-           sub-tree, and (2) a list of all subdirectories that should NOT be
-           checked, as specified in the DEPS file (if any).
-  """
-  # Check for a .svn directory in this directory or check this directory is
-  # contained in git source direcotries. This will tell us if it's a source
-  # directory and should be checked.
-  if not (os.path.exists(os.path.join(dir_name, ".svn")) or
-          (dir_name.lower() in GIT_SOURCE_DIRECTORY)):
-    return (None, [])
-
-  # Check the DEPS file in this directory.
-  if VERBOSE:
-    print "Applying rules from", dir_name
-  def FromImpl(_unused, _unused2):
-    pass  # NOP function so "From" doesn't fail.
-
-  def FileImpl(_unused):
-    pass  # NOP function so "File" doesn't fail.
-
-  class _VarImpl:
-    def __init__(self, local_scope):
-      self._local_scope = local_scope
-
-    def Lookup(self, var_name):
-      """Implements the Var syntax."""
-      if var_name in self._local_scope.get("vars", {}):
-        return self._local_scope["vars"][var_name]
-      raise Exception("Var is not defined: %s" % var_name)
-
-  local_scope = {}
-  global_scope = {
-      "File": FileImpl,
-      "From": FromImpl,
-      "Var": _VarImpl(local_scope).Lookup,
-      }
-  deps_file = os.path.join(dir_name, "DEPS")
-
-  if os.path.isfile(deps_file):
-    execfile(deps_file, global_scope, local_scope)
-  elif VERBOSE:
-    print "  No deps file found in", dir_name
-
-  # Even if a DEPS file does not exist we still invoke ApplyRules
-  # to apply the implicit "allow" rule for the current directory
-  include_rules = local_scope.get(INCLUDE_RULES_VAR_NAME, [])
-  skip_subdirs = local_scope.get(SKIP_SUBDIRS_VAR_NAME, [])
-
-  return (ApplyRules(existing_rules, include_rules, dir_name), skip_subdirs)
-
-
-def CheckDirectory(parent_rules, checkers, dir_name):
-  (rules, skip_subdirs) = ApplyDirectoryRules(parent_rules, dir_name)
-  if rules == None:
-    return True
-
-  # Collect a list of all files and directories to check.
-  files_to_check = []
-  dirs_to_check = []
-  success = True
-  contents = os.listdir(dir_name)
-  for cur in contents:
-    if cur in skip_subdirs:
-      continue  # Don't check children that DEPS has asked us to skip.
-    full_name = os.path.join(dir_name, cur)
-    if os.path.isdir(full_name):
-      dirs_to_check.append(full_name)
-    elif os.path.splitext(full_name)[1] in checkers:
-      files_to_check.append(full_name)
-
-  # First check all files in this directory.
-  for cur in files_to_check:
-    checker = checkers[os.path.splitext(cur)[1]]
-    file_status = checker.CheckFile(rules, cur)
-    if file_status:
-      print "ERROR in " + cur + "\n" + file_status
-      success = False
-
-  # Next recurse into the subdirectories.
-  for cur in dirs_to_check:
-    if not CheckDirectory(rules, checkers, cur):
-      success = False
-
-  return success
-
-
-def GetGitSourceDirectory(root):
-  """Returns a set of the directories to be checked.
-
-  Args:
-    root: The repository root where .git directory exists.
-
-  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.readlines():
-    dir_name = os.path.join(root, os.path.dirname(line))
-    # Add the directory as well as all the parent directories.
-    while dir_name != root:
-      git_source_directory.add(dir_name)
-      dir_name = os.path.dirname(dir_name)
-  git_source_directory.add(root)
-  return git_source_directory
+    popen_out = os.popen("cd %s && git ls-files --full-name ." %
+                         subprocess.list2cmdline([self.base_directory]))
+    for line in popen_out.readlines():
+      dir_name = os.path.join(self.base_directory, os.path.dirname(line))
+      # Add the directory as well as all the parent directories. Use
+      # forward slashes and lower case to normalize paths.
+      while dir_name != self.base_directory:
+        self.git_source_directories.add(NormalizePath(dir_name))
+        dir_name = os.path.dirname(dir_name)
+    self.git_source_directories.add(NormalizePath(self.base_directory))
 
 
 def PrintUsage():
   print """Usage: python checkdeps.py [--root <root>] [tocheck]
+
   --root   Specifies the repository root. This defaults to "../../.." relative
            to the script file. This will be correct given the normal location
            of the script in "<root>/tools/checkdeps".
@@ -334,73 +386,42 @@ Examples:
   python checkdeps.py --root c:\\source chrome"""
 
 
-def checkdeps(options, args):
-  global VERBOSE
-  if options.verbose:
-    VERBOSE = True
+def main():
+  option_parser = optparse.OptionParser()
+  option_parser.add_option("", "--root", default="", dest="base_directory",
+                           help='Specifies the repository root. This defaults '
+                           'to "../../.." relative to the script file, which '
+                           'will normally be the repository root.')
+  option_parser.add_option("-v", "--verbose", action="store_true",
+                           default=False, help="Print debug logging")
+  options, args = option_parser.parse_args()
 
-  # Optional base directory of the repository.
-  global BASE_DIRECTORY
-  if not options.base_directory:
-    BASE_DIRECTORY = os.path.abspath(
-        os.path.join(os.path.abspath(os.path.dirname(__file__)), "../.."))
-  else:
-    BASE_DIRECTORY = os.path.abspath(options.base_directory)
+  deps_checker = DepsChecker(options.base_directory, options.verbose)
 
   # Figure out which directory we have to check.
-  if len(args) == 0:
-    # No directory to check specified, use the repository root.
-    start_dir = BASE_DIRECTORY
-  elif len(args) == 1:
+  start_dir = deps_checker.base_directory
+  if 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:
+    start_dir = os.path.abspath(
+        os.path.join(deps_checker.base_directory, args[0]))
+  elif len(args) >= 2:
     # More than one argument, we don't handle this.
     PrintUsage()
     return 1
 
-  print "Using base directory:", BASE_DIRECTORY
+  print "Using base directory:", deps_checker.base_directory
   print "Checking:", start_dir
 
-  base_rules = Rules()
-
-  # The base directory should be lower case from here on since it will be used
-  # for substring matching on the includes, and we compile on case-insensitive
-  # systems. Plus, we always use slashes here since the include parsing code
-  # will also normalize to slashes.
-  BASE_DIRECTORY = BASE_DIRECTORY.lower()
-  BASE_DIRECTORY = BASE_DIRECTORY.replace("\\", "/")
-  start_dir = start_dir.replace("\\", "/")
-
-  if os.path.exists(os.path.join(BASE_DIRECTORY, ".git")):
-    global GIT_SOURCE_DIRECTORY
-    GIT_SOURCE_DIRECTORY = GetGitSourceDirectory(BASE_DIRECTORY)
-
-  java = java_checker.JavaChecker(BASE_DIRECTORY, VERBOSE)
-  cpp = cpp_checker.CppChecker(VERBOSE)
-  checkers = dict(
-      (extension, checker)
-      for checker in [java, cpp] for extension in checker.EXTENSIONS)
-  success = CheckDirectory(base_rules, checkers, start_dir)
-  if not success:
+  results = deps_checker.CheckDirectory(start_dir)
+  if results:
+    for result in results:
+      print result
     print "\nFAILED\n"
     return 1
   print "\nSUCCESS\n"
   return 0
 
 
-def main():
-  option_parser = optparse.OptionParser()
-  option_parser.add_option("", "--root", default="", dest="base_directory",
-                           help='Specifies the repository root. This defaults '
-                           'to "../../.." relative to the script file, which '
-                           'will normally be the repository root.')
-  option_parser.add_option("-v", "--verbose", action="store_true",
-                           default=False, help="Print debug logging")
-  options, args = option_parser.parse_args()
-  return checkdeps(options, args)
-
-
 if '__main__' == __name__:
   sys.exit(main())
diff --git a/tools/checkdeps/checkdeps_test.py b/tools/checkdeps/checkdeps_test.py
new file mode 100755
index 0000000..e650baf
--- /dev/null
+++ b/tools/checkdeps/checkdeps_test.py
@@ -0,0 +1,91 @@
+#!/usr/bin/env python
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Tests for checkdeps.
+"""
+
+import os
+import unittest
+
+
+import checkdeps
+
+
+class CheckDepsTest(unittest.TestCase):
+
+  def setUp(self):
+    self.deps_checker = checkdeps.DepsChecker(being_tested=True)
+
+  def testRegularCheckDepsRun(self):
+    problems = self.deps_checker.CheckDirectory(
+        os.path.join(self.deps_checker.base_directory,
+                     'tools/checkdeps/testdata'))
+    self.failUnlessEqual(3, len(problems))
+
+    def VerifySubstringsInProblems(key_path, substrings_in_sequence):
+      found = False
+      key_path = os.path.normpath(key_path)
+      for problem in problems:
+        index = problem.find(key_path)
+        if index != -1:
+          for substring in substrings_in_sequence:
+            index = problem.find(substring, index + 1)
+            self.failUnless(index != -1)
+          found = True
+          break
+      if not found:
+        self.fail('Found no problem for file %s' % key_path)
+
+    VerifySubstringsInProblems('testdata/allowed/test.h',
+                               ['-tools/checkdeps/testdata/disallowed',
+                                '-third_party/explicitly_disallowed',
+                                'Because of no rule applying'])
+    VerifySubstringsInProblems('testdata/disallowed/test.h',
+                               ['-third_party/explicitly_disallowed',
+                                'Because of no rule applying',
+                                'Because of no rule applying'])
+    VerifySubstringsInProblems('disallowed/allowed/test.h',
+                               ['-third_party/explicitly_disallowed',
+                                'Because of no rule applying',
+                                'Because of no rule applying'])
+
+  def testCheckAddedIncludesAllGood(self):
+    problems = self.deps_checker.CheckAddedCppIncludes(
+      [['tools/checkdeps/testdata/allowed/test.cc',
+        ['#include "tools/checkdeps/testdata/allowed/good.h"',
+         '#include "tools/checkdeps/testdata/disallowed/allowed/good.h"']
+      ]])
+    self.failIf(problems)
+
+  def testCheckAddedIncludesManyGarbageLines(self):
+    garbage_lines = ["My name is Sam%d\n" % num for num in range(50)]
+    problems = self.deps_checker.CheckAddedCppIncludes(
+      [['tools/checkdeps/testdata/allowed/test.cc', garbage_lines]])
+    self.failIf(problems)
+
+  def testCheckAddedIncludesNoRule(self):
+    problems = self.deps_checker.CheckAddedCppIncludes(
+      [['tools/checkdeps/testdata/allowed/test.cc',
+        ['#include "no_rule_for_this/nogood.h"']
+      ]])
+    self.failUnless(problems)
+
+  def testCheckAddedIncludesSkippedDirectory(self):
+    problems = self.deps_checker.CheckAddedCppIncludes(
+      [['tools/checkdeps/testdata/disallowed/allowed/skipped/test.cc',
+        ['#include "whatever/whocares.h"']
+      ]])
+    self.failIf(problems)
+
+  def testCheckAddedIncludesTempAllowed(self):
+    problems = self.deps_checker.CheckAddedCppIncludes(
+      [['tools/checkdeps/testdata/allowed/test.cc',
+        ['#include "tools/checkdeps/testdata/disallowed/temporarily_allowed.h"']
+      ]])
+    self.failUnless(problems)
+
+
+if __name__ == '__main__':
+  unittest.main()
diff --git a/tools/checkdeps/cpp_checker.py b/tools/checkdeps/cpp_checker.py
index d32c6be..0f51909 100644
--- a/tools/checkdeps/cpp_checker.py
+++ b/tools/checkdeps/cpp_checker.py
@@ -7,6 +7,8 @@
 import codecs
 import re
 
+from rules import Rule
+
 
 class CppChecker(object):
 
@@ -32,38 +34,50 @@ class CppChecker(object):
   def __init__(self, verbose):
     self._verbose = verbose
 
-  def _CheckLine(self, rules, line):
-    """Checks the given file with the given rule set.
-    Returns a tuple (is_include, illegal_description).
+  def CheckLine(self, rules, line, fail_on_temp_allow=False):
+    """Checks the given line with the given rule set.
+    Returns a triplet (is_include, illegal_description, rule_type).
+
     If the line is an #include directive the first value will be True.
-    If it is also an illegal include, the second value will be a string
-    describing the error.  Otherwise, it will be None."""
+    If it is also an illegal include, the second value will be a
+    string describing the error.  Otherwise, it will be None.  If
+    fail_on_temp_allow is False, only Rule.DISALLOW rules will cause a
+    problem to be reported.  If it is true, both Rule.DISALLOW and
+    Rule.TEMP_ALLOW will cause an error.
+
+    The last item in the triplet returns the type of rule that
+    applied, one of Rule.ALLOW (which implies the second item is
+    None), Rule.DISALLOW (which implies that the second item is not
+    None) and Rule.TEMP_ALLOW (in which case the second item will be
+    None only if fail_on_temp_allow is False).
+    """
     found_item = self._EXTRACT_INCLUDE_PATH.match(line)
     if not found_item:
-      return False, None  # Not a match
+      return False, None, Rule.ALLOW  # Not a match
 
     include_path = found_item.group(1)
 
     if '\\' in include_path:
-      return True, 'Include paths may not include backslashes'
+      return True, 'Include paths may not include backslashes', Rule.DISALLOW
 
     if '/' not in include_path:
       # Don't fail when no directory is specified. We may want to be more
       # strict about this in the future.
       if self._verbose:
         print ' WARNING: directory specified with no path: ' + include_path
-      return True, None
+      return True, None, Rule.ALLOW
 
     (allowed, why_failed) = rules.DirAllowed(include_path)
-    if not allowed:
+    if (allowed == Rule.DISALLOW or
+        (fail_on_temp_allow and allowed == Rule.TEMP_ALLOW)):
       if self._verbose:
         retval = '\nFor %s' % rules
       else:
         retval = ''
       return True, retval + ('Illegal include: "%s"\n    Because of %s' %
-          (include_path, why_failed))
+          (include_path, why_failed)), allowed
 
-    return True, None
+    return True, None, allowed
 
   def CheckFile(self, rules, filepath):
     if self._verbose:
@@ -90,7 +104,7 @@ class CppChecker(object):
             in_if0 -= 1
           continue
 
-        is_include, line_status = self._CheckLine(rules, line)
+        is_include, line_status, rule_type = self.CheckLine(rules, line)
         if is_include:
           last_include = line_num
         if line_status is not None:
diff --git a/tools/checkdeps/java_checker.py b/tools/checkdeps/java_checker.py
index 933fd84..cc6fe66 100644
--- a/tools/checkdeps/java_checker.py
+++ b/tools/checkdeps/java_checker.py
@@ -8,6 +8,8 @@ import codecs
 import os
 import re
 
+from rules import Rule
+
 
 class JavaChecker(object):
   """Import checker for Java files.
@@ -94,7 +96,7 @@ class JavaChecker(object):
           # Convert Windows paths to Unix style, as used in DEPS files.
           include_path = include_path.replace(os.path.sep, '/')
           (allowed, why_failed) = rules.DirAllowed(include_path)
-          if not allowed:
+          if allowed == Rule.DISALLOW:
             if self._verbose:
               result += '\nFor %s' % rules
             result += 'Illegal include: "%s"\n    Because of %s\n' % (
diff --git a/tools/checkdeps/rules.py b/tools/checkdeps/rules.py
new file mode 100644
index 0000000..08dd13e
--- /dev/null
+++ b/tools/checkdeps/rules.py
@@ -0,0 +1,89 @@
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Base classes to represent dependency rules, used by checkdeps.py"""
+
+
+class Rule(object):
+  """Specifies a single rule for an include, which can be one of
+  ALLOW, DISALLOW and TEMP_ALLOW.
+  """
+
+  # These are the prefixes used to indicate each type of rule. These
+  # are also used as values for self.allow to indicate which type of
+  # rule this is.
+  ALLOW = "+"
+  DISALLOW = "-"
+  TEMP_ALLOW = "!"
+
+  def __init__(self, allow, directory, source):
+    self.allow = allow
+    self._dir = directory
+    self._source = source
+
+  def __str__(self):
+    return '"%s%s" from %s.' % (self.allow, self._dir, self._source)
+
+  def ParentOrMatch(self, other):
+    """Returns true if the input string is an exact match or is a parent
+    of the current rule. For example, the input "foo" would match "foo/bar"."""
+    return self._dir == other or self._dir.startswith(other + "/")
+
+  def ChildOrMatch(self, other):
+    """Returns true if the input string would be covered by this rule. For
+    example, the input "foo/bar" would match the rule "foo"."""
+    return self._dir == other or other.startswith(self._dir + "/")
+
+
+def ParseRuleString(rule_string, source):
+  """Returns a tuple of a boolean indicating whether the directory is an allow
+  rule, and a string holding the directory name.
+  """
+  if not rule_string:
+    raise Exception('The rule string "%s" is empty\nin %s' %
+                    (rule_string, source))
+
+  if not rule_string[0] in [Rule.ALLOW, Rule.DISALLOW, Rule.TEMP_ALLOW]:
+    raise Exception(
+      'The rule string "%s" does not begin with a "+", "-" or "!".' %
+      rule_string)
+
+  return (rule_string[0], rule_string[1:])
+
+
+class Rules(object):
+  def __init__(self):
+    """Initializes the current rules with an empty rule list."""
+    self._rules = []
+
+  def __str__(self):
+    return 'Rules = [\n%s]' % '\n'.join(' %s' % x for x in self._rules)
+
+  def AddRule(self, rule_string, source):
+    """Adds a rule for the given rule string.
+
+    Args:
+      rule_string: The include_rule string read from the DEPS file to apply.
+      source: A string representing the location of that string (filename, etc.)
+              so that we can give meaningful errors.
+    """
+    (add_rule, rule_dir) = ParseRuleString(rule_string, source)
+    # Remove any existing rules or sub-rules that apply. For example, if we're
+    # passed "foo", we should remove "foo", "foo/bar", but not "foobar".
+    self._rules = [x for x in self._rules if not x.ParentOrMatch(rule_dir)]
+    self._rules.insert(0, Rule(add_rule, rule_dir, source))
+
+  def DirAllowed(self, allowed_dir):
+    """Returns a tuple (success, message), where success indicates if the given
+    directory is allowed given the current set of rules, and the message tells
+    why if the comparison failed."""
+    for rule in self._rules:
+      if rule.ChildOrMatch(allowed_dir):
+        # This rule applies.
+        why_failed = ""
+        if rule.allow != Rule.ALLOW:
+          why_failed = str(rule)
+        return (rule.allow, why_failed)
+    # No rules apply, fail.
+    return (Rule.DISALLOW, "no rule applying")
diff --git a/tools/checkdeps/testdata/DEPS b/tools/checkdeps/testdata/DEPS
new file mode 100644
index 0000000..8cb5054
--- /dev/null
+++ b/tools/checkdeps/testdata/DEPS
@@ -0,0 +1,5 @@
+include_rules = [
+  "-tools/checkdeps/testdata/disallowed",
+  "+tools/checkdeps/testdata/allowed",
+  "-third_party/explicitly_disallowed",
+]
diff --git a/tools/checkdeps/testdata/allowed/DEPS b/tools/checkdeps/testdata/allowed/DEPS
new file mode 100644
index 0000000..ae750e2
--- /dev/null
+++ b/tools/checkdeps/testdata/allowed/DEPS
@@ -0,0 +1,5 @@
+include_rules = [
+  "+tools/checkdeps/testdata/disallowed/allowed",
+  "!tools/checkdeps/testdata/disallowed/temporarily_allowed.h",
+  "+third_party/allowed_may_use",
+]
diff --git a/tools/checkdeps/testdata/allowed/test.h b/tools/checkdeps/testdata/allowed/test.h
new file mode 100644
index 0000000..ea33090
--- /dev/null
+++ b/tools/checkdeps/testdata/allowed/test.h
@@ -0,0 +1,11 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "tools/checkdeps/testdata/allowed/good.h"
+#include "tools/checkdeps/testdata/disallowed/bad.h"
+#include "tools/checkdeps/testdata/disallowed/allowed/good.h"
+#include "tools/checkdeps/testdata/disallowed/temporarily_allowed.h"
+#include "third_party/explicitly_disallowed/bad.h"
+#include "third_party/allowed_may_use/good.h"
+#include "third_party/no_rule/bad.h"
diff --git a/tools/checkdeps/testdata/disallowed/allowed/DEPS b/tools/checkdeps/testdata/disallowed/allowed/DEPS
new file mode 100644
index 0000000..2be72b8
--- /dev/null
+++ b/tools/checkdeps/testdata/disallowed/allowed/DEPS
@@ -0,0 +1,3 @@
+skip_child_includes = [
+  "skipped",
+]
diff --git a/tools/checkdeps/testdata/disallowed/allowed/skipped/test.h b/tools/checkdeps/testdata/disallowed/allowed/skipped/test.h
new file mode 100644
index 0000000..8010596
--- /dev/null
+++ b/tools/checkdeps/testdata/disallowed/allowed/skipped/test.h
@@ -0,0 +1,5 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "whatever/whocares/ok.h"
diff --git a/tools/checkdeps/testdata/disallowed/allowed/test.h b/tools/checkdeps/testdata/disallowed/allowed/test.h
new file mode 100644
index 0000000..f7dc822
--- /dev/null
+++ b/tools/checkdeps/testdata/disallowed/allowed/test.h
@@ -0,0 +1,11 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "tools/checkdeps/testdata/allowed/good.h"
+// Always allowed to include self and parents.
+#include "tools/checkdeps/testdata/disallowed/good.h"
+#include "tools/checkdeps/testdata/disallowed/allowed/good.h"
+#include "third_party/explicitly_disallowed/bad.h"
+#include "third_party/allowed_may_use/bad.h"
+#include "third_party/no_rule/bad.h"
diff --git a/tools/checkdeps/testdata/disallowed/test.h b/tools/checkdeps/testdata/disallowed/test.h
new file mode 100644
index 0000000..d0128e3
--- /dev/null
+++ b/tools/checkdeps/testdata/disallowed/test.h
@@ -0,0 +1,12 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "tools/checkdeps/testdata/allowed/good.h"
+// Always allowed to include self.
+#include "tools/checkdeps/testdata/disallowed/good.h"
+#include "tools/checkdeps/testdata/disallowed/allowed/good.h"
+#include "third_party/explicitly_disallowed/bad.h"
+// Only allowed for code under allowed/.
+#include "third_party/allowed_may_use/bad.h"
+#include "third_party/no_rule/bad.h"