From bef085068bd086c28b35fc8f0834207df0fa9ed3 Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Fri, 5 Feb 2021 14:08:32 -0800 Subject: [PATCH] PRESUBMIT.py: Use input_api.owners_client to check DEPS includes owners. Replace input_api.owners_db with input_api.owners_client, a common interface to check owners using Depot Tools owners implementation and Gerrit Code-Owners plugin. Bug: chromium:1175847 No-Presubmit: true Change-Id: I6f526cdee7676b3fb85bcfacc579f43445e5fb0d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206140 Commit-Queue: Edward Lemur Reviewed-by: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#33209} --- PRESUBMIT.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 12f87d7ff1..678e504cb0 100755 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -1319,10 +1319,10 @@ def _CalculateAddedDeps(os_path, old_contents, new_contents): def CheckAddedDepsHaveTargetApprovals(input_api, output_api): """When a dependency prefixed with + is added to a DEPS file, we - want to make sure that the change is reviewed by an OWNER of the - target file or directory, to avoid layering violations from being - introduced. This check verifies that this happens. - """ + want to make sure that the change is reviewed by an OWNER of the + target file or directory, to avoid layering violations from being + introduced. This check verifies that this happens. + """ virtual_depended_on_files = set() file_filter = lambda f: not input_api.re.match( @@ -1362,20 +1362,19 @@ def CheckAddedDepsHaveTargetApprovals(input_api, output_api): else: output = output_api.PresubmitNotifyResult - owners_db = input_api.owners_db owner_email, reviewers = ( input_api.canned_checks.GetCodereviewOwnerAndReviewers( input_api, - owners_db.email_regexp, + None, approval_needed=input_api.is_committing)) owner_email = owner_email or input_api.change.author_email - reviewers_plus_owner = set(reviewers) - if owner_email: - reviewers_plus_owner.add(owner_email) - missing_files = owners_db.files_not_covered_by(virtual_depended_on_files, - reviewers_plus_owner) + approval_status = input_api.owners_client.GetFilesApprovalStatus( + virtual_depended_on_files, reviewers.union([owner_email]), []) + missing_files = [ + f for f in virtual_depended_on_files + if approval_status[f] != input_api.owners_client.APPROVED] # We strip the /DEPS part that was added by # _FilesToCheckForIncomingDeps to fake a path to a file in a @@ -1398,7 +1397,8 @@ def CheckAddedDepsHaveTargetApprovals(input_api, output_api): 'modified in this CL:\n %s' % '\n '.join(sorted(unapproved_dependencies))) ] - suggested_owners = owners_db.reviewers_for(missing_files, owner_email) + suggested_owners = input_api.owners_client.SuggestOwners( + missing_files, exclude=[owner_email]) output_list.append( output('Suggested missing target path OWNERS:\n %s' % '\n '.join(suggested_owners or [])))