From e6f40ea0341552c3e78fb107fe9f37b87d95f321 Mon Sep 17 00:00:00 2001
From: Josip Sokcevic <sokcevic@chromium.org>
Date: Mon, 28 Aug 2023 23:15:48 +0000
Subject: [PATCH] Check if DEPS git is not in git submodules

If a new git dependency is added to DEPS file, presubmit check should
fail if there's no corresponding git submodule entry if git_dependencies
is set to SYNC.

R=jojwang@google.com

Bug: 1476115
Change-Id: I0fdebb036c129c2f97524b86ee4d70c07e5b0091
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4818792
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
---
 presubmit_canned_checks.py | 62 ++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py
index 2ffcfb2eed..060379eabf 100644
--- a/presubmit_canned_checks.py
+++ b/presubmit_canned_checks.py
@@ -1712,6 +1712,24 @@ def CheckForCommitObjects(input_api, output_api):
   Returns:
     A presubmit error if a commit object is not expected.
   """
+  # Get DEPS file.
+  deps_file = input_api.os_path.join(input_api.PresubmitLocalPath(), 'DEPS')
+  if not input_api.os_path.isfile(deps_file):
+    # No DEPS file, carry on!
+    return []
+
+  with open(deps_file) as f:
+    deps_content = f.read()
+  deps = _ParseDeps(deps_content)
+  # set default
+  if 'deps' not in deps:
+    deps['deps'] = {}
+  if 'git_dependencies' not in deps:
+    deps['git_dependencies'] = 'DEPS'
+
+  if deps['git_dependencies'] == 'SUBMODULES':
+    # git submodule is source of truth, so no further action needed.
+    return []
 
   def parse_tree_entry(ent):
     """Splits a tree entry into components
@@ -1744,17 +1762,7 @@ def CheckForCommitObjects(input_api, output_api):
   if len(commit_tree_entries) == 0:
     return []
 
-  # This gets DEPS file.
-  deps_file = input_api.os_path.join(input_api.PresubmitLocalPath(), 'DEPS')
-  if not input_api.os_path.isfile(deps_file):
-    # No DEPS file, carry on!
-    return []
-
-  with open(deps_file) as f:
-    deps_content = f.read()
-  deps = _ParseDeps(deps_content)
-
-  if not 'git_dependencies' in deps or deps['git_dependencies'] == 'DEPS':
+  if deps['git_dependencies'] == 'DEPS':
     commit_tree_entries = [x[3] for x in commit_tree_entries]
     return [
         output_api.PresubmitError(
@@ -1766,19 +1774,30 @@ def CheckForCommitObjects(input_api, output_api):
             'again:\n', commit_tree_entries)
     ]
 
-  if deps['git_dependencies'] == 'SUBMODULES':
-    # git submodule is source of truth, so no further action needed.
-    return []
-
   assert deps['git_dependencies'] == 'SYNC', 'unexpected git_dependencies.'
 
+  # Create mapping HASH -> PATH
+  git_submodules = {}
+  for commit_tree_entry in commit_tree_entries:
+    git_submodules[commit_tree_entry[2]] = commit_tree_entry[3]
+
   mismatch_entries = []
   deps_msg = ""
-  for commit_tree_entry in commit_tree_entries:
-    # Search for commit hashes in DEPS file - they must be present
-    if commit_tree_entry[2] not in deps_content:
-      mismatch_entries.append(commit_tree_entry[3])
-      deps_msg += f"\n    {commit_tree_entry[3]} -> {commit_tree_entry[2]}"
+  for dep_path, dep in deps['deps'].items():
+    if 'dep_type' in dep and dep['dep_type'] != 'git':
+      continue
+
+    url = dep if isinstance(dep, str) else dep['url']
+    commit_hash = url.split('@')[-1]
+    if commit_hash in git_submodules:
+      git_submodules.pop(commit_hash)
+    else:
+      mismatch_entries.append(dep_path)
+      deps_msg += f"\n [DEPS]      {dep_path} -> {commit_hash}"
+
+  for commit_hash, path in git_submodules.items():
+    mismatch_entries.append(path)
+    deps_msg += f"\n [gitlink]   {path} -> {commit_hash}"
 
   if mismatch_entries:
     return [
@@ -1789,8 +1808,7 @@ def CheckForCommitObjects(input_api, output_api):
             'the following command in the root of this repository:\n'
             '    gclient gitmodules'
             '\n\n'
-            'If git submodule changes are correct, update the following DEPS '
-            'entries to: ' + deps_msg)
+            'The following entries diverged: ' + deps_msg)
     ]
 
   return []