From 1a61eb625d4b062bb2d6f0902b4979b48def4d33 Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Thu, 18 Apr 2024 21:18:32 +0000 Subject: [PATCH] Provide useful submodule info for all Change classes GitChange uses 'scm.GIT.ListSubmodules' to filter out files under repo root that belong to a submodule. ProvidedDiffChange does not use this so doesn't filter out submoduled files. 'scm.GIT.ListSubmodules' just reads '.gitmodules' at repo root and can be run outside a git workspace. Use this to provide useful submodule info for Change/ProvidedDiffChange. Bug: b/333744051 Change-Id: Idaead11c69681e86276a29b0dc58090e7c4ec2c6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5466527 Reviewed-by: Josip Sokcevic Commit-Queue: Gavin Mak --- presubmit_support.py | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/presubmit_support.py b/presubmit_support.py index 237dae58f..b72d33c2e 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -1199,6 +1199,9 @@ class Change(object): self._description_without_tags = '' self.SetDescriptionText(description) + # List of submodule paths in the repo. + self._submodules = None + assert all((isinstance(f, (list, tuple)) and len(f) == 2) for f in files), files @@ -1359,11 +1362,11 @@ class Change(object): return list(filter(lambda x: x.Action() != 'D', affected)) def AffectedSubmodules(self): - """Returns a list of AffectedFile instances for submodules in the change. - - There is no SCM and no submodules, so return an empty list. - """ - return [] + """Returns a list of AffectedFile instances for submodules in the change.""" + return [ + af for af in self._affected_files + if af.LocalPath() in self._repo_submodules() + ] def AffectedTestableFiles(self, include_deletes=None, **kwargs): """Return a list of the existing text files in a change.""" @@ -1423,11 +1426,10 @@ class Change(object): return {f.LocalPath(): f.OldContents() for f in files} def _repo_submodules(self): - """Returns submodule paths for current change's repo. - - There is no SCM, so return an empty list. - """ - return [] + """Returns submodule paths for current change's repo.""" + if not self._submodules: + self._submodules = scm.GIT.ListSubmodules(self.RepositoryRoot()) + return self._submodules class GitChange(Change): @@ -1438,18 +1440,9 @@ class GitChange(Change): self._upstream = upstream super(GitChange, self).__init__(*args) - # List of submodule paths in the repo. - self._submodules = None - def _diff_cache(self): return self._AFFECTED_FILES.DIFF_CACHE(self._upstream) - def _repo_submodules(self): - """Returns submodule paths for current change's repo.""" - if not self._submodules: - self._submodules = scm.GIT.ListSubmodules(self.RepositoryRoot()) - return self._submodules - def UpstreamBranch(self): """Returns the upstream branch for the change.""" return self._upstream @@ -1481,13 +1474,6 @@ class GitChange(Change): return affected return list(filter(lambda x: x.Action() != 'D', affected)) - def AffectedSubmodules(self): - """Returns a list of AffectedFile instances for submodules in the change.""" - return [ - af for af in self._affected_files - if af.LocalPath() in self._repo_submodules() - ] - class ProvidedDiffChange(Change): _AFFECTED_FILES = ProvidedDiffAffectedFile