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

PRESUBMIT.py

 # 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.
 
 """Top-level presubmit script for Chromium.
 
 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 for more details about the presubmit API built into gcl.
 """
 
 
+import os

[M-A Ruel, 2012/07/26 13:09:38]

Remove

[Jói, 2012/07/26 13:37:21]

Done.

 import re
+import sys
 
 
 _EXCLUDED_PATHS = (
     r"^breakpad[\\\/].*",
     r"^native_client_sdk[\\\/].*",
     r"^net[\\\/]tools[\\\/]spdyshark[\\\/].*",
     r"^skia[\\\/].*",
     r"^v8[\\\/].*",
     r".*MakeFile$",
     r".+_autogen\.h$",
@@ skipping 344 matching lines @@
       files.append(f)
 
   if files:
     return [output_api.PresubmitError(
         'Do not use #pragma once in header files.\n'
         'See http://www.chromium.org/developers/coding-style#TOC-File-headers',
         files)]
   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
+  # roundabout construct to import checkdeps because this file is
+  # eval-ed and thus doesn't have __file__.
+  original_sys_path = sys.path
+  try:
+    sys.path = sys.path + [os.path.join(

[M-A Ruel, 2012/07/26 13:09:38]

input_api.os_path.join(

[Jói, 2012/07/26 13:37:21]

Done.

+        input_api.change.RepositoryRoot(), 'tools', 'checkdeps')]

[M-A Ruel, 2012/07/26 13:09:38]

It is relatively unsafe to use RepositoryRoot(). With svn, it could be an
arbitrary directory. It's better to use PresubmitLocalPath() as I noted earlier.

[Jói, 2012/07/26 13:37:21]

Ah, sorry I missed that.  Thanks for catching.

+    import checkdeps
+  finally:
+    # Restore sys.path to what it was before.
+    sys.path = original_sys_path
+
+  added_includes = []
+  for f in input_api.AffectedFiles():
+    if not checkdeps.ShouldCheckAddedIncludesInFile(f.LocalPath()):
+      continue
+
+    changed_lines = [line for line_num, line in f.ChangedContents()]
+    added_includes.append([f.LocalPath(), changed_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(

[M-A Ruel, 2012/07/26 13:09:38]

On commit, it would need to be a notification otherwise it'll break the CQ.

[Jói, 2012/07/26 13:37:21]

I would prefer if people need to jump through hoops to land a change that adds a
dependency we are trying to get rid of.  I wanted to go mid-way by making it a
warning, John suggested making it an error.  I'd prefer not to have this be a
no-problem event when submitting via the CQ.

+        '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
+
+
 def _CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   results = []
   results.extend(input_api.canned_checks.PanProjectChecks(
       input_api, output_api, excluded_paths=_EXCLUDED_PATHS))
   results.extend(_CheckAuthorizedAuthor(input_api, output_api))
   results.extend(
     _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
   results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
   results.extend(_CheckNoUNIT_TESTInSourceFiles(input_api, output_api))
   results.extend(_CheckNoNewWStrings(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
 
 
 def _CheckSubversionConfig(input_api, output_api):
   """Verifies the subversion config file is correctly setup.
 
   Checks that autoprops are enabled, returns an error otherwise.
   """
   join = input_api.os_path.join
   if input_api.platform == 'win32':
@@ skipping 109 matching lines @@
   if all(re.search('[/_]android[/_.]', f) for f in files):
     return ['android']
 
   trybots = ['win_rel', 'linux_rel', 'mac_rel', 'linux_clang:compile',
              'android']
   # match things like aurax11.cc or aura_oak.cc
   if any(re.search('[/_]aura', f) for f in files):
     trybots.append('linux_chromeos')
 
   return trybots