aiden
/
depot_tools
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[owners][git-cl] Use owners client to show all owners for a file.

Browse Source 
Change-Id: I04949d95ca466452a58ea91cf2db86852c0621bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2551378
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Auto-Submit: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Anthony Polito <apolito@google.com>
changes/78/2551378/5
Edward Lesmes 5 years ago committed by LUCI CQ
parent e7d1862b15
commit eeca9c6ee7
4 changed files with 41 additions and 37 deletions
Show Stats Download Patch File Download Diff File

62
git_cl.py
Unescape Escape View File

@ -43,6 +43,7 @@ import git_new_branch
import metrics import metrics
import metrics_utils import metrics_utils
import owners import owners
import owners_client
import owners_finder import owners_finder
import presubmit_canned_checks import presubmit_canned_checks
import presubmit_support import presubmit_support
@ -1251,7 +1252,7 @@ class Changelist(object):
assert self.GetIssue(), 'issue is required to update description' assert self.GetIssue(), 'issue is required to update description'
if gerrit_util.HasPendingChangeEdit( if gerrit_util.HasPendingChangeEdit(
self._GetGerritHost(), self._GerritChangeIdentifier()): self.GetGerritHost(), self._GerritChangeIdentifier()):
if not force: if not force:
confirm_or_exit( confirm_or_exit(
'The description cannot be modified while the issue has a pending ' 'The description cannot be modified while the issue has a pending '
@ -1259,9 +1260,9 @@ class Changelist(object):
'or delete it.\n\n', action='delete the unpublished edit') 'or delete it.\n\n', action='delete the unpublished edit')
gerrit_util.DeletePendingChangeEdit( gerrit_util.DeletePendingChangeEdit(
self._GetGerritHost(), self._GerritChangeIdentifier()) self.GetGerritHost(), self._GerritChangeIdentifier())
gerrit_util.SetCommitMessage( gerrit_util.SetCommitMessage(
self._GetGerritHost(), self._GerritChangeIdentifier(), self.GetGerritHost(), self._GerritChangeIdentifier(),
description, notify='NONE') description, notify='NONE')
self.description = description self.description = description
@ -1307,7 +1308,7 @@ class Changelist(object):
gclient_utils.FileWrite(description_file, description) gclient_utils.FileWrite(description_file, description)
args.extend(['--json_output', json_output]) args.extend(['--json_output', json_output])
args.extend(['--description_file', description_file]) args.extend(['--description_file', description_file])
args.extend(['--gerrit_project', self._GetGerritProject()]) args.extend(['--gerrit_project', self.GetGerritProject()])
start = time_time() start = time_time()
cmd = ['vpython', PRESUBMIT_SUPPORT] + args cmd = ['vpython', PRESUBMIT_SUPPORT] + args
@ -1479,7 +1480,7 @@ class Changelist(object):
labels = {'Commit-Queue': vote_map[new_state]} labels = {'Commit-Queue': vote_map[new_state]}
notify = False if new_state == _CQState.DRY_RUN else None notify = False if new_state == _CQState.DRY_RUN else None
gerrit_util.SetReview( gerrit_util.SetReview(
self._GetGerritHost(), self._GerritChangeIdentifier(), self.GetGerritHost(), self._GerritChangeIdentifier(),
labels=labels, notify=notify) labels=labels, notify=notify)
return 0 return 0
except KeyboardInterrupt: except KeyboardInterrupt:
@ -1497,7 +1498,7 @@ class Changelist(object):
# Still raise exception so that stack trace is printed. # Still raise exception so that stack trace is printed.
raise raise
def _GetGerritHost(self): def GetGerritHost(self):
# Lazy load of configs. # Lazy load of configs.
self.GetCodereviewServer() self.GetCodereviewServer()
if self._gerrit_host and '.' not in self._gerrit_host: if self._gerrit_host and '.' not in self._gerrit_host:
@ -1536,7 +1537,7 @@ class Changelist(object):
self._gerrit_server = 'https://%s' % self._gerrit_host self._gerrit_server = 'https://%s' % self._gerrit_host
return self._gerrit_server return self._gerrit_server
def _GetGerritProject(self): def GetGerritProject(self):
"""Returns Gerrit project name based on remote git URL.""" """Returns Gerrit project name based on remote git URL."""
remote_url = self.GetRemoteUrl() remote_url = self.GetRemoteUrl()
if remote_url is None: if remote_url is None:
@ -1561,7 +1562,7 @@ class Changelist(object):
Not to be confused by value of "Change-Id:" footer. Not to be confused by value of "Change-Id:" footer.
If Gerrit project can be determined, this will speed up Gerrit HTTP API RPC. If Gerrit project can be determined, this will speed up Gerrit HTTP API RPC.
""" """
project = self._GetGerritProject() project = self.GetGerritProject()
if project: if project:
return gerrit_util.ChangeIdentifier(project, self.GetIssue()) return gerrit_util.ChangeIdentifier(project, self.GetIssue())
# Fall back on still unique, but less efficient change number. # Fall back on still unique, but less efficient change number.
@ -1647,14 +1648,14 @@ class Changelist(object):
if not isinstance(cookies_auth, gerrit_util.CookiesAuthenticator): if not isinstance(cookies_auth, gerrit_util.CookiesAuthenticator):
return return
cookies_user = cookies_auth.get_auth_email(self._GetGerritHost()) cookies_user = cookies_auth.get_auth_email(self.GetGerritHost())
if self.GetIssueOwner() == cookies_user: if self.GetIssueOwner() == cookies_user:
return return
logging.debug('change %s owner is %s, cookies user is %s', logging.debug('change %s owner is %s, cookies user is %s',
self.GetIssue(), self.GetIssueOwner(), cookies_user) self.GetIssue(), self.GetIssueOwner(), cookies_user)
# Maybe user has linked accounts or something like that, # Maybe user has linked accounts or something like that,
# so ask what Gerrit thinks of this user. # so ask what Gerrit thinks of this user.
details = gerrit_util.GetAccountDetails(self._GetGerritHost(), 'self') details = gerrit_util.GetAccountDetails(self.GetGerritHost(), 'self')
if details['email'] == self.GetIssueOwner(): if details['email'] == self.GetIssueOwner():
return return
if not force: if not force:
@ -1757,7 +1758,7 @@ class Changelist(object):
def AddComment(self, message, publish=None): def AddComment(self, message, publish=None):
gerrit_util.SetReview( gerrit_util.SetReview(
self._GetGerritHost(), self._GerritChangeIdentifier(), self.GetGerritHost(), self._GerritChangeIdentifier(),
msg=message, ready=publish) msg=message, ready=publish)
def GetCommentsSummary(self, readable=True): def GetCommentsSummary(self, readable=True):
@ -1768,9 +1769,9 @@ class Changelist(object):
options=['MESSAGES', 'DETAILED_ACCOUNTS', options=['MESSAGES', 'DETAILED_ACCOUNTS',
'CURRENT_REVISION']).get('messages', []) 'CURRENT_REVISION']).get('messages', [])
file_comments = gerrit_util.GetChangeComments( file_comments = gerrit_util.GetChangeComments(
self._GetGerritHost(), self._GerritChangeIdentifier()) self.GetGerritHost(), self._GerritChangeIdentifier())
robot_file_comments = gerrit_util.GetChangeRobotComments( robot_file_comments = gerrit_util.GetChangeRobotComments(
self._GetGerritHost(), self._GerritChangeIdentifier()) self.GetGerritHost(), self._GerritChangeIdentifier())
# Add the robot comments onto the list of comments, but only # Add the robot comments onto the list of comments, but only
# keep those that are from the latest patchset. # keep those that are from the latest patchset.
@ -1796,7 +1797,7 @@ class Changelist(object):
patchset = 'PS%d' % comment['patch_set'] patchset = 'PS%d' % comment['patch_set']
line = comment.get('line', 0) line = comment.get('line', 0)
url = ('https://%s/c/%s/%s/%s#%s%s' % url = ('https://%s/c/%s/%s/%s#%s%s' %
(self._GetGerritHost(), self.GetIssue(), comment['patch_set'], path, (self.GetGerritHost(), self.GetIssue(), comment['patch_set'], path,
'b' if comment.get('side') == 'PARENT' else '', 'b' if comment.get('side') == 'PARENT' else '',
str(line) if line else '')) str(line) if line else ''))
comments[key][path][patchset][line] = (url, comment['message']) comments[key][path][patchset][line] = (url, comment['message'])
@ -1856,11 +1857,11 @@ class Changelist(object):
def CloseIssue(self): def CloseIssue(self):
gerrit_util.AbandonChange( gerrit_util.AbandonChange(
self._GetGerritHost(), self._GerritChangeIdentifier(), msg='') self.GetGerritHost(), self._GerritChangeIdentifier(), msg='')
def SubmitIssue(self, wait_for_merge=True): def SubmitIssue(self, wait_for_merge=True):
gerrit_util.SubmitChange( gerrit_util.SubmitChange(
self._GetGerritHost(), self._GerritChangeIdentifier(), self.GetGerritHost(), self._GerritChangeIdentifier(),
wait_for_merge=wait_for_merge) wait_for_merge=wait_for_merge)
def _GetChangeDetail(self, options=None): def _GetChangeDetail(self, options=None):
@ -1888,7 +1889,7 @@ class Changelist(object):
try: try:
data = gerrit_util.GetChangeDetail( data = gerrit_util.GetChangeDetail(
self._GetGerritHost(), self._GerritChangeIdentifier(), options_set) self.GetGerritHost(), self._GerritChangeIdentifier(), options_set)
except gerrit_util.GerritError as e: except gerrit_util.GerritError as e:
if e.http_status == 404: if e.http_status == 404:
raise GerritChangeNotExists(self.GetIssue(), self.GetCodereviewServer()) raise GerritChangeNotExists(self.GetIssue(), self.GetCodereviewServer())
@ -1901,7 +1902,7 @@ class Changelist(object):
assert self.GetIssue(), 'issue must be set to query Gerrit' assert self.GetIssue(), 'issue must be set to query Gerrit'
try: try:
data = gerrit_util.GetChangeCommit( data = gerrit_util.GetChangeCommit(
self._GetGerritHost(), self._GerritChangeIdentifier()) self.GetGerritHost(), self._GerritChangeIdentifier())
except gerrit_util.GerritError as e: except gerrit_util.GerritError as e:
if e.http_status == 404: if e.http_status == 404:
raise GerritChangeNotExists(self.GetIssue(), self.GetCodereviewServer()) raise GerritChangeNotExists(self.GetIssue(), self.GetCodereviewServer())
@ -2189,7 +2190,7 @@ class Changelist(object):
remote, remote_branch = self.GetRemoteBranch() remote, remote_branch = self.GetRemoteBranch()
should_retry = remote_branch == DEFAULT_OLD_BRANCH and \ should_retry = remote_branch == DEFAULT_OLD_BRANCH and \
gerrit_util.GetProjectHead( gerrit_util.GetProjectHead(
self._gerrit_host, self._GetGerritProject()) == 'refs/heads/main' self._gerrit_host, self.GetGerritProject()) == 'refs/heads/main'
if not should_retry: if not should_retry:
DieWithError(str(e), change_desc) DieWithError(str(e), change_desc)
@ -2273,12 +2274,12 @@ class Changelist(object):
cc = [email.strip() for email in cc if email.strip()] cc = [email.strip() for email in cc if email.strip()]
if change_desc.get_cced(): if change_desc.get_cced():
cc.extend(change_desc.get_cced()) cc.extend(change_desc.get_cced())
if self._GetGerritHost() == 'chromium-review.googlesource.com': if self.GetGerritHost() == 'chromium-review.googlesource.com':
valid_accounts = set(reviewers + cc) valid_accounts = set(reviewers + cc)
# TODO(crbug/877717): relax this for all hosts. # TODO(crbug/877717): relax this for all hosts.
else: else:
valid_accounts = gerrit_util.ValidAccounts( valid_accounts = gerrit_util.ValidAccounts(
self._GetGerritHost(), reviewers + cc) self.GetGerritHost(), reviewers + cc)
logging.info('accounts %s are recognized, %s invalid', logging.info('accounts %s are recognized, %s invalid',
sorted(valid_accounts), sorted(valid_accounts),
set(reviewers + cc).difference(set(valid_accounts))) set(reviewers + cc).difference(set(valid_accounts)))
@ -2341,8 +2342,8 @@ class Changelist(object):
if change_desc.get_reviewers(tbr_only=True): if change_desc.get_reviewers(tbr_only=True):
score = gerrit_util.GetCodeReviewTbrScore( score = gerrit_util.GetCodeReviewTbrScore(
self._GetGerritHost(), self.GetGerritHost(),
self._GetGerritProject()) self.GetGerritProject())
refspec_opts.append('l=Code-Review+%s' % score) refspec_opts.append('l=Code-Review+%s' % score)
# Gerrit sorts hashtags, so order is not important. # Gerrit sorts hashtags, so order is not important.
@ -2359,7 +2360,7 @@ class Changelist(object):
refspec = '%s:refs/for/%s%s' % (ref_to_push, branch, refspec_suffix) refspec = '%s:refs/for/%s%s' % (ref_to_push, branch, refspec_suffix)
git_push_metadata = { git_push_metadata = {
'gerrit_host': self._GetGerritHost(), 'gerrit_host': self.GetGerritHost(),
'title': options.title or '<untitled>', 'title': options.title or '<untitled>',
'change_id': change_id, 'change_id': change_id,
'description': change_desc.description, 'description': change_desc.description,
@ -2383,7 +2384,7 @@ class Changelist(object):
# GetIssue() is not set in case of non-squash uploads according to tests. # GetIssue() is not set in case of non-squash uploads according to tests.
# TODO(crbug.com/751901): non-squash uploads in git cl should be removed. # TODO(crbug.com/751901): non-squash uploads in git cl should be removed.
gerrit_util.AddReviewers( gerrit_util.AddReviewers(
self._GetGerritHost(), self.GetGerritHost(),
self._GerritChangeIdentifier(), self._GerritChangeIdentifier(),
reviewers, cc, reviewers, cc,
notify=bool(options.send_mail)) notify=bool(options.send_mail))
@ -4769,12 +4770,15 @@ def CMDowners(parser, args):
if len(args) == 0: if len(args) == 0:
print('No files specified for --show-all. Nothing to do.') print('No files specified for --show-all. Nothing to do.')
return 0 return 0
project = cl.GetGerritProject()
branch = cl.GetCommonAncestorWithUpstream()
client = owners_client.DepotToolsClient(
host=cl.GetGerritHost(),
root=settings.GetRoot(),
branch=branch)
for arg in args: for arg in args:
base_branch = cl.GetCommonAncestorWithUpstream()
database = owners.Database(settings.GetRoot(), open, os.path)
database.load_data_needed_for([arg])
print('Owners for %s:' % arg) print('Owners for %s:' % arg)
for owner in sorted(database.all_possible_owners([arg], None)): for owner in client.ListOwnersForFile(project, branch, arg):
print(' - %s' % owner) print(' - %s' % owner)
return 0 return 0

2
owners_client.py
Unescape Escape View File

@ -79,7 +79,7 @@ class OwnersClient(object):
class DepotToolsClient(OwnersClient): class DepotToolsClient(OwnersClient):
"""Implement OwnersClient using owners.py Database.""" """Implement OwnersClient using owners.py Database."""
def __init__(self, host, root, fopen, os_path, branch): def __init__(self, host, root, branch, fopen=open, os_path=os.path):
super(DepotToolsClient, self).__init__(host) super(DepotToolsClient, self).__init__(host)
self._root = root self._root = root
self._fopen = fopen self._fopen = fopen

12
tests/git_cl_test.py
Unescape Escape View File

@ -231,7 +231,7 @@ class TestGitClBasic(unittest.TestCase):
side_effect=[git_cl.GitPushError(), None]).start() side_effect=[git_cl.GitPushError(), None]).start()
mock.patch('git_cl.Changelist.GetRemoteBranch', mock.patch('git_cl.Changelist.GetRemoteBranch',
return_value=('foo', 'bar')).start() return_value=('foo', 'bar')).start()
mock.patch('git_cl.Changelist._GetGerritProject', mock.patch('git_cl.Changelist.GetGerritProject',
return_value='foo').start() return_value='foo').start()
mock.patch('git_cl.gerrit_util.GetProjectHead', mock.patch('git_cl.gerrit_util.GetProjectHead',
return_value='refs/heads/main').start() return_value='refs/heads/main').start()
@ -253,7 +253,7 @@ class TestGitClBasic(unittest.TestCase):
side_effect=[git_cl.GitPushError(), None]).start() side_effect=[git_cl.GitPushError(), None]).start()
mock.patch('git_cl.Changelist.GetRemoteBranch', mock.patch('git_cl.Changelist.GetRemoteBranch',
return_value=('foo', git_cl.DEFAULT_OLD_BRANCH)).start() return_value=('foo', git_cl.DEFAULT_OLD_BRANCH)).start()
mock.patch('git_cl.Changelist._GetGerritProject', mock.patch('git_cl.Changelist.GetGerritProject',
return_value='foo').start() return_value='foo').start()
mock.patch('git_cl.gerrit_util.GetProjectHead', mock.patch('git_cl.gerrit_util.GetProjectHead',
return_value='refs/heads/old_default').start() return_value='refs/heads/old_default').start()
@ -275,7 +275,7 @@ class TestGitClBasic(unittest.TestCase):
side_effect=[git_cl.GitPushError(), None]).start() side_effect=[git_cl.GitPushError(), None]).start()
mock.patch('git_cl.Changelist.GetRemoteBranch', mock.patch('git_cl.Changelist.GetRemoteBranch',
return_value=('foo', git_cl.DEFAULT_OLD_BRANCH)).start() return_value=('foo', git_cl.DEFAULT_OLD_BRANCH)).start()
mock.patch('git_cl.Changelist._GetGerritProject', mock.patch('git_cl.Changelist.GetGerritProject',
return_value='foo').start() return_value='foo').start()
mock.patch('git_cl.gerrit_util.GetProjectHead', mock.patch('git_cl.gerrit_util.GetProjectHead',
return_value='refs/heads/main').start() return_value='refs/heads/main').start()
@ -2236,7 +2236,7 @@ class TestGitCl(unittest.TestCase):
sys.stdout.getvalue()) sys.stdout.getvalue())
def _mock_gerrit_changes_for_detail_cache(self): def _mock_gerrit_changes_for_detail_cache(self):
mock.patch('git_cl.Changelist._GetGerritHost', lambda _: 'host').start() mock.patch('git_cl.Changelist.GetGerritHost', lambda _: 'host').start()
def test_gerrit_change_detail_cache_simple(self): def test_gerrit_change_detail_cache_simple(self):
self._mock_gerrit_changes_for_detail_cache() self._mock_gerrit_changes_for_detail_cache()
@ -2760,7 +2760,7 @@ class ChangelistTest(unittest.TestCase):
mock.patch('git_cl.time_time').start() mock.patch('git_cl.time_time').start()
mock.patch('metrics.collector').start() mock.patch('metrics.collector').start()
mock.patch('subprocess2.Popen').start() mock.patch('subprocess2.Popen').start()
mock.patch('git_cl.Changelist._GetGerritProject', mock.patch('git_cl.Changelist.GetGerritProject',
return_value='https://chromium-review.googlesource.com').start() return_value='https://chromium-review.googlesource.com').start()
self.addCleanup(mock.patch.stopall) self.addCleanup(mock.patch.stopall)
self.temp_count = 0 self.temp_count = 0
@ -2987,7 +2987,7 @@ class CMDTestCaseBase(unittest.TestCase):
'git_cl.Changelist.GetCodereviewServer', 'git_cl.Changelist.GetCodereviewServer',
return_value='https://chromium-review.googlesource.com').start() return_value='https://chromium-review.googlesource.com').start()
mock.patch( mock.patch(
'git_cl.Changelist._GetGerritHost', 'git_cl.Changelist.GetGerritHost',
return_value='chromium-review.googlesource.com').start() return_value='chromium-review.googlesource.com').start()
mock.patch( mock.patch(
'git_cl.Changelist.GetMostRecentPatchset', 'git_cl.Changelist.GetMostRecentPatchset',

2
tests/owners_client_test.py
Unescape Escape View File

@ -78,7 +78,7 @@ class DepotToolsClientTest(unittest.TestCase):
return_value={}).start() return_value={}).start()
self.addCleanup(mock.patch.stopall) self.addCleanup(mock.patch.stopall)
self.client = owners_client.DepotToolsClient( self.client = owners_client.DepotToolsClient(
'host', '/', self.fopen, self.repo, 'branch') 'host', '/', 'branch', self.fopen, self.repo)
def testListOwners(self): def testListOwners(self):
self.assertEquals( self.assertEquals(

Write Preview
Loading…
Cancel
Save