From f8e16bdbd6c5621a106396fcf2b35ba3a75db995 Mon Sep 17 00:00:00 2001 From: Josip Sokcevic Date: Fri, 7 Mar 2025 14:15:25 -0800 Subject: [PATCH] Reland^2: Fix CheckForRecursedeps to work for submodule based gclient DEPS This reverts commit 7d6d4424b5b3968665dd5cf86bbca01517e94dfe. Change-Id: Ibb05a6b6db2babef525a4230c01785570e074161 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6336451 Auto-Submit: Josip Sokcevic Reviewed-by: Robbie Iannucci Commit-Queue: Robbie Iannucci --- presubmit_canned_checks.py | 51 ++++++++++++++++++++++++++++++++------ presubmit_support.py | 4 +++ 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index e78588f75e..04a8dccfac 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -2081,13 +2081,12 @@ def CheckForCommitObjects(input_api, output_api): url = dep if isinstance(dep, str) else dep['url'] commit_hash = url.split('@')[-1] - # Two exceptions were in made in two projects prior to this check - # enforcement. We need to address those exceptions, but in the meantime - # we can't fail this global presubmit check - # https://chromium.googlesource.com/infra/infra/+/refs/heads/main/DEPS#45 - if dep_path == 'recipes-py' and commit_hash == 'refs/heads/main': - continue + # One exception is made prior to this check enforcement. + # + # We need to address this exception, but in the meantime we can't fail + # this global presubmit check. + # # https://chromium.googlesource.com/angle/angle/+/refs/heads/main/DEPS#412 if dep_path == 'third_party/dummy_chromium': continue @@ -2168,7 +2167,44 @@ def CheckForRecursedeps(input_api, output_api): # No recursedeps entry, carry on! return [] - existing_deps = deps.get('deps', {}) + existing_deps: set[str] = set() + # If git_dependencies is SYNC or SUBMODULES, prefer the submodule data. + if deps.get('git_dependencies', 'DEPS') == 'DEPS': + existing_deps = set(deps.get('deps', {})) + else: + existing_deps = input_api.change.AllLocalSubmodules() + # existing_deps is 'local paths' i.e. repo-relative. However, if deps is + # NOT use_relative_paths, then we need to adjust these to be + # client-root-relative. + # + # Sadly, as of 25Q1, the default is still False. + if not deps.get('use_relative_paths', False): + # Find the relative path between the gclient root and the repo root. + gclient_relpath: str = input_api.os_path.relpath( + input_api.change.RepositoryRoot(), + start=input_api.gclient_paths.FindGclientRoot( + input_api.change.RepositoryRoot())) + + # relpath will use native os.path.sep, so split and clean it. + # + # NOTE: relpath can make ./path/to/something, so trim off the '.' + gclient_relpath_toks = tuple( + gclient_relpath.split(input_api.os_path.sep)) + if gclient_relpath_toks[0] == '.': + gclient_relpath_toks = gclient_relpath_toks[1:] + + # All submodules are relative to the repo root, so join + # + # gclient-to-repo ++ repo-to-submodule + # + # To get path strings which should appear in recursedeps. + # + # We must join with '/', not os_path.sep, because the paths in + # DEPS.recursedeps always use forward slashes. + existing_deps = set( + '/'.join(gclient_relpath_toks + + (p.replace(input_api.os_path.sep, '/'), )) + for p in existing_deps) errors = [] for check_dep in deps['recursedeps']: @@ -2178,6 +2214,7 @@ def CheckForRecursedeps(input_api, output_api): f'Found recuredep entry {check_dep} but it is not found ' 'in\n deps itself. Remove it from recurcedeps or add ' 'deps entry.')) + return errors diff --git a/presubmit_support.py b/presubmit_support.py index 2feb6b414b..0e30c9c10e 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -1435,6 +1435,10 @@ class Change(object): """Returns local paths for affected submodules.""" return [af.LocalPath() for af in self.AffectedSubmodules()] + def AllLocalSubmodules(self) -> set[str]: + """Returns local paths for all submodules.""" + return set(self._repo_submodules()) + def AbsoluteLocalPaths(self): """Convenience function.""" return [af.AbsoluteLocalPath() for af in self.AffectedFiles()]