Fix checkdeps

checkdeps assumes that no DEPS rule ends in a slash. If a DEPS rule
ends in a slash, it is ignored and not enforced. This CL fixes this
bug by making DEPS rules that end in a slash an error.

There are five DEPS files that have a rule ending in a slash in the
repo, though only two of them (fortunately!) accidentally committed
files violating the DEPS.

Bugs are filed.

Bug: 1084817, 1084826, 1084827
Change-Id: Ifbfdc4c16b89a36b26df51eee1897d0385689181
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208299Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770669}

buildtools/checkdeps/builddeps.py

@@ -93,7 +93,8 @@ class DepsBuilder(object):
     Args:
       base_directory: local 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.
+      being_tested: Set to True to ignore the DEPS file at
+                    buildtools/checkdeps/DEPS.
       ignore_temp_rules: Ignore rules that start with Rule.TEMP_ALLOW ("!").
     """
     base_directory = (base_directory or
@@ -236,7 +237,7 @@ class DepsBuilder(object):
     deps_file_path = os.path.join(dir_path_local_abs, 'DEPS')
 
     # The second conditional here is to disregard the
-    # tools/checkdeps/DEPS file while running tests.  This DEPS file
+    # buildtools/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

buildtools/checkdeps/checkdeps.py

@@ -51,7 +51,8 @@ class DepsChecker(DepsBuilder):
     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.
+      being_tested: Set to true to ignore the DEPS file at
+                    buildtools/checkdeps/DEPS.
       ignore_temp_rules: Ignore rules that start with Rule.TEMP_ALLOW ("!").
     """
     DepsBuilder.__init__(
@@ -188,7 +189,7 @@ def PrintUsage():
 
   --root 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".
+              normal location of the script in "<root>/buildtools/checkdeps".
 
   --(others)  There are a few lesser-used options; run with --help to show them.
 

buildtools/checkdeps/rules.py

@@ -74,6 +74,14 @@ def ParseRuleString(rule_string, source):
       'The rule string "%s" does not begin with a "+", "-" or "!".' %
       rule_string)
 
+  # If a directory is specified in a DEPS file with a trailing slash, then it
+  # will not match as a parent directory in Rule's [Parent|Child]OrMatch above.
+  # Ban them.
+  if rule_string[-1] == '/':
+    raise Exception(
+      'The rule string "%s" ends with a "/" which is not allowed' %
+      rule_string)
+
   return rule_string[0], rule_string[1:]
 
 

chrome/browser/android/crypto/DEPS

 include_rules = [
-  "-chrome/browser/",
+  "-chrome/browser",
   "-content/public/android",
   "+content/public/android/java/src/org/chromium/content_public",
 ]

chrome/browser/extensions/DEPS

@@ -10,7 +10,7 @@ include_rules = [
   # The extensions system should not depend on Chrome Platform Apps (or any
   # other component of the apps system).
   # https://crbug.com/873872.
-  "-chrome/browser/apps/",
+  "-chrome/browser/apps",
 ]
 specific_include_rules = {
   ".*test(_chromeos)?\.(cc|h)$": [
@@ -37,4 +37,58 @@ specific_include_rules = {
   "webstore_private_apitest.cc" : [
     "+chrome/browser/ui/views/parent_permission_dialog_view.h",
   ],
+
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "browsertest_util\.cc": [
+    "+chrome/browser/apps/app_service/app_launch_params.h",
+    "+chrome/browser/apps/app_service/app_service_proxy.h",
+    "+chrome/browser/apps/app_service/app_service_proxy_factory.h",
+    "+chrome/browser/apps/app_service/browser_app_launcher.h",
+  ],
+
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "chrome_extension_host_delegate\.cc": [
+    "+chrome/browser/apps/platform_apps/audio_focus_web_contents_observer.h",
+  ],
+
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "corb_and_cors_extension_browsertest\.cc": [
+    "+chrome/browser/apps/app_service/app_launch_params.h",
+    "+chrome/browser/apps/app_service/app_service_proxy.h",
+    "+chrome/browser/apps/app_service/app_service_proxy_factory.h",
+    "+chrome/browser/apps/app_service/browser_app_launcher.h",
+  ],
+
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "extension_apitest\.cc": [
+    "+chrome/browser/apps/app_service/app_launch_params.h",
+    "+chrome/browser/apps/app_service/app_service_proxy.h",
+    "+chrome/browser/apps/app_service/app_service_proxy_factory.h",
+    "+chrome/browser/apps/app_service/browser_app_launcher.h",
+  ],
+
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "extension_browser_window_helper\.cc": [
+    "+chrome/browser/apps/app_service/launch_utils.h",
+  ],
+
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "extension_browsertest\.cc": [
+    "+chrome/browser/apps/app_service/app_launch_params.h",
+    "+chrome/browser/apps/app_service/app_service_proxy.h",
+    "+chrome/browser/apps/app_service/app_service_proxy_factory.h",
+    "+chrome/browser/apps/app_service/browser_app_launcher.h",
+  ],
+
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "external_pref_loader\.cc": [
+    "+chrome/browser/apps/user_type_filter.h",
+  ],
 }

chrome/browser/extensions/api/developer_private/DEPS

@@ -7,4 +7,10 @@ specific_include_rules = {
     # Allow the unittest to create a data_decoder service.
     "+services/data_decoder"
   ],
+
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "developer_private_api\.cc": [
+    "+chrome/browser/apps/app_service/app_launch_params.h",
+  ],
 }

chrome/browser/extensions/api/management/DEPS

+specific_include_rules = {
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "chrome_management_api_delegate\.cc": [
+    "+chrome/browser/apps/app_service/app_launch_params.h",
+    "+chrome/browser/apps/app_service/app_service_proxy.h",
+    "+chrome/browser/apps/app_service/app_service_proxy_factory.h",
+    "+chrome/browser/apps/app_service/browser_app_launcher.h",
+  ],
+}

chrome/browser/extensions/api/notifications/DEPS

+specific_include_rules = {
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084826
+  "notifications_apitest\.cc": [
+    "+chrome/browser/apps/app_service/app_launch_params.h",
+    "+chrome/browser/apps/app_service/app_service_proxy.h",
+    "+chrome/browser/apps/app_service/app_service_proxy_factory.h",
+    "+chrome/browser/apps/app_service/browser_app_launcher.h",
+  ],
+}

chrome/browser/profiles/android/DEPS

 include_rules = [
-  "-chrome/browser/",
-  "+chrome/browser/android/crypto/",
+  "-chrome/browser",
+  "+chrome/browser/android/crypto",
   "-content/public/android",
   "+content/public/android/java/src/org/chromium/content_public",
 ]
+specific_include_rules = {
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084827
+  "profile_downloader_android\.cc": [
+    "+chrome/browser/browser_process.h",
+    "+chrome/browser/profiles/profile_android.h",
+    "+chrome/browser/profiles/profile_attributes_entry.h",
+    "+chrome/browser/profiles/profile_attributes_storage.h",
+    "+chrome/browser/profiles/profile_avatar_icon_util.h",
+    "+chrome/browser/profiles/profile_downloader.h",
+    "+chrome/browser/profiles/profile_downloader_delegate.h",
+    "+chrome/browser/profiles/profile_manager.h",
+    "+chrome/browser/signin/identity_manager_factory.h",
+  ],
+
+  # This DEPS violation snuck in while there was a bug in the checkdeps tool.
+  # https://crbug.com/1084827
+  "profile_manager_utils\.cc": [
+    "+chrome/browser/browser_process.h",
+    "+chrome/browser/profiles/android/jni_headers/ProfileManagerUtils_jni.h",
+    "+chrome/browser/profiles/profile.h",
+    "+chrome/browser/profiles/profile_manager.h",
+  ],
+}

components/previews/DEPS

@@ -2,7 +2,7 @@ include_rules = [
   # Previews is a layered component; subdirectories must introduce
   # allowances of //content dependencies as appropriate.
   "-components",
-  "-components/data_reduction_proxy/",
-  "+components/data_reduction_proxy/core/browser/",
+  "-components/data_reduction_proxy",
+  "+components/data_reduction_proxy/core/browser",
   "-components/previews/content",
 ]
\ No newline at end of file

content/public/common/DEPS

@@ -9,7 +9,7 @@ specific_include_rules = {
     "+content/common",
   ],
   "simple_url_loader\.cc": [
-    "-content/",
+    "-content",
     "+content/public/common/simple_url_loader\.h",
     "+content/public/common/resource_request\.h",
     "+content/public/common/resource_response\.h",