Add checkdeps presubmit check. Warns on new #includes of dependencies

PRESUBMIT.py

Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index ba38fedca0196e563c275661bb01302b3ff8329a..eb93d4f158ee7ca0a6c5c7c2efee350e7e2d164b 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -9,7 +9,9 @@ for more details about the presubmit API built into gcl.
 """
 
 
+import os
 import re
+import sys
 
 
 _EXCLUDED_PATHS = (
@@ -374,6 +376,52 @@ def _CheckNoPragmaOnce(input_api, output_api):
   return []
 
 
+def _CheckUnwantedDependencies(input_api, output_api):
+  """Runs checkdeps on #include statements added in this
+  change. Breaking - rules is an error, breaking ! rules is a
+  warning.
+  """
+  # We need to wait until we have an input_api object and use this

[M-A Ruel, 2012/07/26 12:37:42]

old_sys = sys.path
try:
  sys.path = sys.path + [os.path.join(input_api.PresubmitLocalPath(), 'tools',
'checkdeps')]

# This creates a new list.

finally:
  sys.path = old_sys_path

[Jói, 2012/07/26 13:00:34]

Done.

+  # roundabout construct to import checkdeps because this file is
+  # eval-ed and thus doesn't have __file__.
+  sys.path.append(os.path.join(input_api.change.RepositoryRoot(),
+                               'tools/checkdeps'))
+  import checkdeps
+
+  added_includes = []
+  for f in input_api.AffectedFiles():
+    if not checkdeps.ShouldCheckAddedIncludesInFile(f.LocalPath()):
+      continue
+
+    include_lines = []

[M-A Ruel, 2012/07/26 12:37:42]

include_lines = [line for line_num, line in f.ChangedContents()]

but is that what you want to do?

[Jói, 2012/07/26 13:00:34]

It should be named changed_lines rather than include_lines; I fixed that and
used your suggestion.

+    for line_num, line in f.ChangedContents():
+      include_lines.append(line)
+    added_includes.append([f.LocalPath(), include_lines])
+
+  error_descriptions = []
+  warning_descriptions = []
+  for path, rule_type, rule_description in checkdeps.CheckAddedIncludes(
+      added_includes):
+    description_with_path = '%s\n    %s' % (path, rule_description)
+    if rule_type == checkdeps.Rule.DISALLOW:
+      error_descriptions.append(description_with_path)
+    else:
+      warning_descriptions.append(description_with_path)
+
+  results = []
+  if error_descriptions:
+    results.append(output_api.PresubmitError(
+        'You added one or more #includes that violate checkdeps rules.',
+        error_descriptions))
+  if warning_descriptions:
+    results.append(output_api.PresubmitPromptWarning(
+        'You added one or more #includes of files that are temporarily\n'
+        'allowed but being removed. Can you avoid introducing the\n'
+        '#include? See relevant DEPS file(s) for details and contacts.',
+        warning_descriptions))
+  return results

[M-A Ruel, 2012/07/26 12:37:42]

Personally, I would probably have just input_api.subprocess.check_call() the
script with the list of files to check. To work around command line length
limitation, use '-' as parameter and have the script files =
sys.stdin.readlines(). Feels easier to me.

[Jói, 2012/07/26 13:00:34]

It's more efficient to check only the changed lines, so coupling via Python
calls seems a lot easier than packing everything into a script invocation.  If
you don't object I will leave this as an import thing with a direct method call.

+
+
 def _CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   results = []
@@ -388,6 +436,7 @@ def _CommonChecks(input_api, output_api):
   results.extend(_CheckNoDEPSGIT(input_api, output_api))
   results.extend(_CheckNoBannedFunctions(input_api, output_api))
   results.extend(_CheckNoPragmaOnce(input_api, output_api))
+  results.extend(_CheckUnwantedDependencies(input_api, output_api))
   return results