Show a list of directories missing OWNER reviewers on upload, and show directories rather than file…

owners.py

 # 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."""
 
 import collections
 import re
 
 
@@ skipping 61 matching lines @@
 
     files is a sequence of paths relative to (and under) self.root."""
     self._check_paths(files)
     self._load_data_needed_for(files)
     return self._covering_set_of_owners_for(files)
 
   def files_are_covered_by(self, files, reviewers):
     """Returns whether every file is owned by at least one reviewer."""
     return not self.files_not_covered_by(files, reviewers)
 
-  def files_not_covered_by(self, files, reviewers):
-    """Returns the set of files that are not owned by at least one reviewer.
+  def directories_not_covered_by(self, files, reviewers):
+    """Returns the set of directories that are not owned by a reviewer.
+
+    See files_not_covered_by() for details.
+    """
+    return self.files_not_covered_by(files, reviewers, directories=True)
+
+  def files_not_covered_by(self, files, reviewers, directories=False):

[Dirk Pranke, 2012/03/07 21:51:47]

With your change in presubmit_canned_checks, I think we don't actually need
files_not_covered_by, do we? I also don't think files_are_covered_by() is called
by anyone at this point, so you can probably get rid of these two routines, and
just rename files_not_covered_by to directories_not_covered_by and only have the
directories=True path).

[Pam (message me for reviews), 2012/03/07 22:21:11]

Done. It slightly changes the special-cased behavior when there are no
reviewers, but the only place that could have been used was in
files_are_covered_by(), so I moved the no-reviewers check there.

+    """Returns the set of items that are not owned by at least one reviewer.
 
     Args:
         files is a sequence of paths relative to (and under) self.root.
-        reviewers is a sequence of strings matching self.email_regexp."""
+        reviewers is a sequence of strings matching self.email_regexp.
+        if directories is True, returns a set of directories; otherwise,
+          returns a set of files.
+    """
     self._check_paths(files)
     self._check_reviewers(reviewers)
-    if not reviewers:
+    if not reviewers and not directories:
       return files
 
     self._load_data_needed_for(files)
     files_by_dir = self._files_by_dir(files)
-    covered_dirs = self._dirs_covered_by(reviewers)
+    if reviewers:
+      covered_dirs = self._dirs_covered_by(reviewers)
+    else:
+      covered_dirs = set()
     uncovered_files = []
     for d, files_in_d in files_by_dir.iteritems():
       if not self._is_dir_covered_by(d, covered_dirs):
-        uncovered_files.extend(files_in_d)
+       if directories:
+          uncovered_files.append(self._enclosing_dir_with_owners(d))
+       else:
+          uncovered_files.extend(files_in_d)
     return set(uncovered_files)
 
   def _check_paths(self, files):
     def _is_under(f, pfx):
       return self.os_path.abspath(self.os_path.join(pfx, f)).startswith(pfx)
     _assert_is_collection(files)
     assert all(_is_under(f, self.os_path.abspath(self.root)) for f in files)
 
   def _check_reviewers(self, reviewers):
     _assert_is_collection(reviewers)
@@ skipping 12 matching lines @@
     return dirs
 
   def _stop_looking(self, dirname):
     return dirname in self.stop_looking
 
   def _is_dir_covered_by(self, dirname, covered_dirs):
     while not dirname in covered_dirs and not self._stop_looking(dirname):
       dirname = self.os_path.dirname(dirname)
     return dirname in covered_dirs
 
+  def _enclosing_dir_with_owners(self, directory):
+    """Returns the innermost enclosing directory that has an OWNERS file."""
+    dirpath = directory
+    while not dirpath in self.owners_for:
+      if self._stop_looking(dirpath):
+        break
+      dirpath = self.os_path.dirname(dirpath)
+    return dirpath
+
   def _load_data_needed_for(self, files):
     for f in files:
       dirpath = self.os_path.dirname(f)
       while not dirpath in self.owners_for:
         self._read_owners_in_dir(dirpath)
         if self._stop_looking(dirpath):
           break
         dirpath = self.os_path.dirname(dirpath)
 
   def _read_owners_in_dir(self, dirpath):
@@ skipping 27 matching lines @@
     # short combinations of owners.
     every_owner = set()
     for f in files:
       dirname = self.os_path.dirname(f)
       while dirname in self.owners_for:
         every_owner |= self.owners_for[dirname]
         if self._stop_looking(dirname):
           break
         dirname = self.os_path.dirname(dirname)
     return every_owner