Use OldContents() for OWNERS files to do checks

BUG=141253
R=dpranke@chromium.org

Change-Id: Iacbc2f0571e725e4f2ccf5ea7878f101972289bb
Reviewed-on: https://chromium-review.googlesource.com/476610Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Jochen Eisinger <jochen@chromium.org>

git_cl.py

@@ -5553,7 +5553,8 @@ def CMDowners(parser, args):
           cl.GetChange(base_branch, None).AffectedFiles()],
       change.RepositoryRoot(),
       author, fopen=file, os_path=os.path,
-      disable_color=options.no_color).run()
+      disable_color=options.no_color,
+      override_files=change.OriginalOwnersFiles()).run()
 
 
 def BuildGitDiffCmd(diff_type, upstream_commit, args):

owners.py

@@ -114,6 +114,11 @@ class Database(object):
     # Pick a default email regexp to use; callers can override as desired.
     self.email_regexp = re.compile(BASIC_EMAIL_REGEXP)
 
+    # Replacement contents for the given files. Maps the file name of an
+    # OWNERS file (relative to root) to an iterator returning the replacement
+    # file contents.
+    self.override_files = {}
+
     # Mapping of owners to the paths or globs they own.
     self._owners_to_paths = {EVERYONE: set()}
 
@@ -240,7 +245,13 @@ class Database(object):
     dirpath = self.os_path.dirname(path)
     in_comment = False
     lineno = 0
-    for line in self.fopen(owners_path):
+
+    if path in self.override_files:
+      file_iter = self.override_files[path]
+    else:
+      file_iter = self.fopen(owners_path)
+
+    for line in file_iter:
       lineno += 1
       line = line.strip()
       if line.startswith('#'):
@@ -395,6 +406,8 @@ class Database(object):
     dirs_remaining = set(self._enclosing_dir_with_owners(f) for f in files)
     all_possible_owners = self.all_possible_owners(dirs_remaining, author)
     suggested_owners = set()
+    if len(all_possible_owners) == 0:
+      return suggested_owners
     while dirs_remaining:
       owner = self.lowest_cost_owner(all_possible_owners, dirs_remaining)
       suggested_owners.add(owner)

owners_finder.py

@@ -25,7 +25,8 @@ class OwnersFinder(object):
   def __init__(self, files, local_root, author,
                fopen, os_path,
                email_postfix='@chromium.org',
-               disable_color=False):
+               disable_color=False,
+               override_files=None):
     self.email_postfix = email_postfix
 
     if os.name == 'nt' or disable_color:
@@ -35,6 +36,7 @@ class OwnersFinder(object):
       self.COLOR_RESET = ''
 
     self.db = owners_module.Database(local_root, fopen, os_path)
+    self.db.override_files = override_files or {}
     self.db.load_data_needed_for(files)
 
     self.os_path = os_path
@@ -52,6 +54,7 @@ class OwnersFinder(object):
       files = filtered_files
       # Reload the database.
       self.db = owners_module.Database(local_root, fopen, os_path)
+      self.db.override_files = override_files or {}
       self.db.load_data_needed_for(files)
 
     self.all_possible_owners = self.db.all_possible_owners(files, None)

presubmit_canned_checks.py

@@ -883,6 +883,7 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
       input_api.change.AffectedFiles(file_filter=source_file_filter)])
 
   owners_db = input_api.owners_db
+  owners_db.override_files = input_api.change.OriginalOwnersFiles()
   owner_email, reviewers = GetCodereviewOwnerAndReviewers(
       input_api,
       owners_db.email_regexp,
@@ -903,11 +904,11 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
                   (needed, '\n    '.join(sorted(missing_files))))]
     if not input_api.is_committing:
       suggested_owners = owners_db.reviewers_for(missing_files, owner_email)
-      finder = input_api.owners_finder(missing_files,
-                                       input_api.change.RepositoryRoot(),
-                                       owner_email,
-                                       fopen=file, os_path=input_api.os_path,
-                                       email_postfix='', disable_color=True)
+      finder = input_api.owners_finder(
+          missing_files, input_api.change.RepositoryRoot(),
+          owner_email, fopen=file, os_path=input_api.os_path,
+          email_postfix='', disable_color=True,
+          override_files=input_api.change.OriginalOwnersFiles())
       owners_with_comments = []
       def RecordComments(text):
         owners_with_comments.append(finder.print_indent() + text)

presubmit_support.py

@@ -916,6 +916,13 @@ class Change(object):
         x for x in self.AffectedFiles(include_deletes=False)
         if x.IsTestableFile())
 
+  def OriginalOwnersFiles(self):
+    """A map from path names of affected OWNERS files to their old content."""
+    def owners_file_filter(f):
+      return 'OWNERS' in os.path.split(f.LocalPath())[1]
+    files = self.AffectedFiles(file_filter=owners_file_filter)
+    return dict([(f.LocalPath(), f.OldContents()) for f in files])
+
 
 class GitChange(Change):
   _AFFECTED_FILES = GitAffectedFile

tests/owners_unittest.py

@@ -18,6 +18,7 @@ import owners
 ben = 'ben@example.com'
 brett = 'brett@example.com'
 darin = 'darin@example.com'
+jochen = 'jochen@example.com'
 john = 'john@example.com'
 ken = 'ken@example.com'
 peter = 'peter@example.com'
@@ -345,8 +346,9 @@ class OwnersDatabaseTest(_BaseTestCase):
 
 class ReviewersForTest(_BaseTestCase):
   def assert_reviewers_for(self, files, potential_suggested_reviewers,
-                           author=None):
+                           author=None, override_files=None):
     db = self.db()
+    db.override_files = override_files or {}
     suggested_reviewers = db.reviewers_for(set(files), author)
     self.assertTrue(suggested_reviewers in
         [set(suggestion) for suggestion in potential_suggested_reviewers])
@@ -463,6 +465,12 @@ class ReviewersForTest(_BaseTestCase):
     self.assert_reviewers_for(['content/garply/bar.cc'],
                               [[brett]])
 
+  def test_override_files(self):
+      self.assert_reviewers_for(['content/baz/froboz.h'], [[jochen]],
+                                override_files={'content/baz/OWNERS': [jochen]})
+      self.assert_reviewers_for(['content/baz/froboz.h'], [[john],[darin]],
+                                override_files={'content/baz/OWNERS': []})
+
 
 class LowestCostOwnersTest(_BaseTestCase):
   # Keep the data in the test_lowest_cost_owner* methods as consistent with

tests/presubmit_unittest.py

@@ -1538,7 +1538,7 @@ class ChangeUnittest(PresubmitTestsBase):
         'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTestableFiles',
         'AffectedTextFiles',
         'AllFiles', 'DescriptionText', 'FullDescriptionText',
-        'LocalPaths', 'Name', 'RepositoryRoot',
+        'LocalPaths', 'Name', 'OriginalOwnersFiles', 'RepositoryRoot',
         'RightHandSideLines', 'SetDescriptionText', 'TAG_LINE_RE',
         'author_email', 'issue', 'patchset', 'scm', 'tags',
     ]
@@ -2276,6 +2276,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
     if not is_committing or (not tbr and issue):
       affected_file.LocalPath().AndReturn('foo/xyz.cc')
       change.AffectedFiles(file_filter=None).AndReturn([affected_file])
+      change.OriginalOwnersFiles().AndReturn({})
       if issue and not rietveld_response and not gerrit_response:
         rietveld_response = {
           "owner_email": change.author_email,
@@ -2305,6 +2306,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
       if not is_committing and uncovered_files:
         fake_db.reviewers_for(set(['foo']),
             change.author_email).AndReturn(change.author_email)
+        change.OriginalOwnersFiles().AndReturn({})
 
     self.mox.ReplayAll()
     output = presubmit.PresubmitOutput()