Commit 2a009624 authored by dpranke@chromium.org's avatar dpranke@chromium.org

Add first changes needed for OWNERS file support.

This changes adds the first pass of code needed for OWNERS files.
In total there should probably be maybe four user-visible changes:
  * new gcl/git-cl "suggest-reviewers" command
  * a presubmit hook on upload to automatically add the reviewers
  * an addition to gcl/git-cl status command that tells you
    which files still need review/approval.
  * a presubmit hook on commit to ensure all of the needed reviewers
    have approved the file.

This change implements a core "owners Database" object with the
dumbest possible algorithm for determining a covering set of reviewers,
and the skeleton of the presubmit hooks. This code will not be
used by anything yet, and is also missing unit tests.

Review URL: http://codereview.chromium.org/6581030

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@76342 0039d316-1c4b-4281-b951-d872f2087c98
parent 7ace24df
# Copyright (c) 2010 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
"""A database of OWNERS files."""
class Assertion(AssertionError):
pass
class Database(object):
def __init__(self, root, fopen, os_path):
"""Initializes the database of owners for a repository.
Args:
root: the path to the root of the Repository
all_owners: the list of every owner in the system
open: function callback to open a text file for reading
os_path: module/object callback with fields for 'exists',
'dirname', and 'join'
"""
self.root = root
self.fopen = fopen
self.os_path = os_path
# Mapping of files to authorized owners.
self.files_owned_by = {}
# Mapping of owners to the files they own.
self.owners_for = {}
# In-memory cached map of files to their OWNERS files.
self.owners_file_for = {}
# In-memory cache of OWNERS files and their contents
self.owners_files = {}
def OwnersFor(self, files):
"""Returns a sets of reviewers that will cover the set of files.
The set of files are paths relative to (and under) self.root."""
self._LoadDataNeededFor(files)
return self._CoveringSetOfOwnersFor(files)
def FilesAreCoveredBy(self, files, reviewers):
return not self.FilesNotCoveredBy(files, reviewers)
def FilesNotCoveredBy(self, files, reviewers):
covered_files = set()
for reviewer in reviewers:
covered_files = covered_files.union(self.files_owned_by[reviewer])
return files.difference(covered_files)
def _LoadDataNeededFor(self, files):
for f in files:
self._LoadOwnersFor(f)
def _LoadOwnersFor(self, f):
if f not in self.owners_for:
owner_file = self._FindOwnersFileFor(f)
self.owners_file_for[f] = owner_file
self._ReadOwnersFile(owner_file, f)
def _FindOwnersFileFor(self, f):
# This is really a "do ... until dirname = ''"
dirname = self.os_path.dirname(f)
while dirname:
owner_path = self.os_path.join(dirname, 'OWNERS')
if self.os_path.exists(owner_path):
return owner_path
dirname = self.os_path.dirname(dirname)
owner_path = self.os_path.join(dirname, 'OWNERS')
if self.os_path.exists(owner_path):
return owner_path
raise Assertion('No OWNERS file found for %s' % f)
def _ReadOwnersFile(self, owner_file, affected_file):
owners_for = self.owners_for.setdefault(affected_file, set())
for owner in self.fopen(owner_file):
owner = owner.strip()
self.files_owned_by.setdefault(owner, set()).add(affected_file)
owners_for.add(owner)
def _CoveringSetOfOwnersFor(self, files):
# TODO(dpranke): implement the greedy algorithm for covering sets, and
# consider returning multiple options in case there are several equally
# short combinations of owners.
every_owner = set()
for f in files:
every_owner = every_owner.union(self.owners_for[f])
return every_owner
......@@ -624,3 +624,23 @@ def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings,
'builds are pending.' % max_pendings,
long_text='\n'.join(out))]
return []
def CheckOwners(input_api, output_api):
affected_files = set(input_api.change.AffectedFiles())
owners_db = input_api.owners_db
if input_api.is_commiting:
missing_files = owners_db.FilesNotCoveredBy(affected_files,
input_api.change.approvers)
if missing_files:
return [output_api.PresubmitPromptWarning('Missing approvals for: %s' %
','.join(missing_files))]
return []
else:
if not input_api.change.get('R', None):
suggested_reviewers = owners_db.OwnersFor(affected_files)
# TODO(dpranke): consider getting multiple covering sets of reviewers
# and displaying them somehow?
input_api.change['R'] = ','.join(suggested_reviewers)
......@@ -51,6 +51,7 @@ except ImportError:
# Local imports.
import gclient_utils
import owners
import presubmit_canned_checks
import scm
......@@ -255,6 +256,11 @@ class InputApi(object):
# We carry the canned checks so presubmit scripts can easily use them.
self.canned_checks = presubmit_canned_checks
# TODO(dpranke): figure out a list of all approved owners for a repo
# in order to be able to handle wildcard OWNERS files?
self.owners_db = owners.Database(change.RepositoryRoot(),
fopen=file, os_path=self.os_path)
def PresubmitLocalPath(self):
"""Returns the local path of the presubmit script currently being run.
......@@ -634,6 +640,10 @@ class Change(object):
self._local_root = os.path.abspath(local_root)
self.issue = issue
self.patchset = patchset
# TODO(dpranke): implement - get from the patchset?
self.approvers = set()
self.scm = ''
# From the description text, build up a dictionary of key/value pairs
......
......@@ -97,7 +97,14 @@ def GetPreferredTrySlaves():
"""
def setUp(self):
class FakeChange(object):
root = '/'
def RepositoryRoot(self):
return self.root
SuperMoxTestBase.setUp(self)
self.fake_change = FakeChange()
self.mox.StubOutWithMock(presubmit, 'random')
self.mox.StubOutWithMock(presubmit, 'warn')
presubmit._ASKED_FOR_FEEDBACK = False
......@@ -133,7 +140,7 @@ class PresubmitUnittest(PresubmitTestsBase):
'PresubmitExecuter', 'PromptYesNo', 'ScanSubDirs',
'SvnAffectedFile', 'SvnChange', 'cPickle', 'cStringIO',
'exceptions', 'fnmatch', 'gclient_utils', 'glob', 'json',
'logging', 'marshal', 'normpath', 'optparse', 'os', 'pickle',
'logging', 'marshal', 'normpath', 'optparse', 'os', 'owners', 'pickle',
'presubmit_canned_checks', 'random', 're', 'scm', 'subprocess',
'sys', 'tempfile', 'time', 'traceback', 'types', 'unittest', 'urllib2',
'warn',
......@@ -692,12 +699,13 @@ class InputApiUnittest(PresubmitTestsBase):
'LocalToDepotPath',
'PresubmitLocalPath', 'ReadFile', 'RightHandSideLines', 'ServerPaths',
'basename', 'cPickle', 'cStringIO', 'canned_checks', 'change', 'environ',
'is_committing', 'json', 'marshal', 'os_path', 'pickle', 'platform',
'python_executable', 're', 'subprocess', 'tempfile', 'traceback',
'unittest', 'urllib2', 'version',
'is_committing', 'json', 'marshal', 'os_path', 'owners_db', 'pickle',
'platform', 'python_executable', 're', 'subprocess', 'tempfile',
'traceback', 'unittest', 'urllib2', 'version',
]
# If this test fails, you should add the relevant test.
self.compareMembers(presubmit.InputApi(None, './.', False), members)
self.compareMembers(presubmit.InputApi(self.fake_change, './.', False),
members)
def testDepotToLocalPath(self):
presubmit.scm.SVN.CaptureInfo('svn://foo/smurf').AndReturn(
......@@ -705,32 +713,31 @@ class InputApiUnittest(PresubmitTestsBase):
presubmit.scm.SVN.CaptureInfo('svn:/foo/notfound/burp').AndReturn({})
self.mox.ReplayAll()
path = presubmit.InputApi(None, './p', False).DepotToLocalPath(
path = presubmit.InputApi(self.fake_change, './p', False).DepotToLocalPath(
'svn://foo/smurf')
self.failUnless(path == 'prout')
path = presubmit.InputApi(None, './p', False).DepotToLocalPath(
path = presubmit.InputApi(self.fake_change, './p', False).DepotToLocalPath(
'svn:/foo/notfound/burp')
self.failUnless(path == None)
def testLocalToDepotPath(self):
presubmit.scm.SVN.CaptureInfo('smurf').AndReturn({'URL':
'svn://foo'})
presubmit.scm.SVN.CaptureInfo('smurf').AndReturn({'URL': 'svn://foo'})
presubmit.scm.SVN.CaptureInfo('notfound-food').AndReturn({})
self.mox.ReplayAll()
path = presubmit.InputApi(None, './p', False).LocalToDepotPath('smurf')
path = presubmit.InputApi(self.fake_change, './p', False).LocalToDepotPath(
'smurf')
self.assertEqual(path, 'svn://foo')
path = presubmit.InputApi(None, './p', False).LocalToDepotPath(
path = presubmit.InputApi(self.fake_change, './p', False).LocalToDepotPath(
'notfound-food')
self.failUnless(path == None)
def testInputApiConstruction(self):
self.mox.ReplayAll()
# Fudge the change object, it's not used during construction anyway
api = presubmit.InputApi(change=42, presubmit_path='foo/path/PRESUBMIT.py',
api = presubmit.InputApi(self.fake_change,
presubmit_path='foo/path/PRESUBMIT.py',
is_committing=False)
self.assertEquals(api.PresubmitLocalPath(), 'foo/path')
self.assertEquals(api.change, 42)
self.assertEquals(api.change, self.fake_change)
def testInputApiPresubmitScriptFiltering(self):
join = presubmit.os.path.join
......@@ -871,7 +878,7 @@ class InputApiUnittest(PresubmitTestsBase):
],
),
]
input_api = presubmit.InputApi(None, './PRESUBMIT.py', False)
input_api = presubmit.InputApi(self.fake_change, './PRESUBMIT.py', False)
self.mox.ReplayAll()
self.assertEqual(len(input_api.DEFAULT_WHITE_LIST), 22)
......@@ -1163,7 +1170,7 @@ class GclChangeUnittest(PresubmitTestsBase):
'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTextFiles',
'DescriptionText', 'FullDescriptionText', 'LocalPaths', 'Name',
'RepositoryRoot', 'RightHandSideLines', 'ServerPaths',
'issue', 'patchset', 'scm', 'tags',
'approvers', 'issue', 'patchset', 'scm', 'tags',
]
# If this test fails, you should add the relevant test.
self.mox.ReplayAll()
......@@ -1214,6 +1221,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
'CheckLongLines', 'CheckTreeIsOpen', 'RunPythonUnitTests',
'RunPylint',
'CheckBuildbotPendingBuilds', 'CheckRietveldTryJobExecution',
'CheckOwners',
]
# If this test fails, you should add the relevant test.
self.compareMembers(presubmit_canned_checks, members)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment