Look for PRESUBMIT.py beyond root if inherit-review-settings-ok is present.

Patch contributed by Darin Petkov

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@49868 0039d316-1c4b-4281-b951-d872f2087c98

presubmit_support.py

@@ -6,7 +6,7 @@
 """Enables directory-specific presubmit checks to run at upload and/or commit.
 """
 
-__version__ = '1.3.4'
+__version__ = '1.3.5'
 
 # TODO(joi) Add caching where appropriate/needed. The API is designed to allow
 # caching (between all different invocations of presubmit scripts for a given
@@ -740,6 +740,9 @@ class GitChange(Change):
 def ListRelevantPresubmitFiles(files, root):
   """Finds all presubmit files that apply to a given set of source files.
 
+  If inherit-review-settings-ok is present right under root, looks for
+  PRESUBMIT.py in directories enclosing root.
+
   Args:
     files: An iterable container containing file paths.
     root: Path where to stop searching.
@@ -747,19 +750,38 @@ def ListRelevantPresubmitFiles(files, root):
   Return:
     List of absolute paths of the existing PRESUBMIT.py scripts.
   """
-  entries = []
-  for f in files:
-    f = normpath(os.path.join(root, f))
-    while f:
-      f = os.path.dirname(f)
-      if f in entries:
+  files = [normpath(os.path.join(root, f)) for f in files]
+
+  # List all the individual directories containing files.
+  directories = set([os.path.dirname(f) for f in files])
+
+  # Ignore root if inherit-review-settings-ok is present.
+  if os.path.isfile(os.path.join(root, 'inherit-review-settings-ok')):
+    root = None
+
+  # Collect all unique directories that may contain PRESUBMIT.py.
+  candidates = set()
+  for directory in directories:
+    while True:
+      if directory in candidates:
+        break
+      candidates.add(directory)
+      if directory == root:
         break
-      entries.append(f)
-      if f == root:
+      parent_dir = os.path.dirname(directory)
+      if parent_dir == directory:
+        # We hit the system root directory.
         break
-  entries.sort()
-  entries = map(lambda x: os.path.join(x, 'PRESUBMIT.py'), entries)
-  return filter(lambda x: os.path.isfile(x), entries)
+      directory = parent_dir
+
+  # Look for PRESUBMIT.py in all candidate directories.
+  results = []
+  for directory in sorted(list(candidates)):
+    p = os.path.join(directory, 'PRESUBMIT.py')
+    if os.path.isfile(p):
+      results.append(p)
+
+  return results
 
 
 class GetTrySlavesExecuter(object):
@@ -1024,12 +1046,14 @@ def Main(argv):
   parser.add_option("--description", default='')
   parser.add_option("--issue", type='int', default=0)
   parser.add_option("--patchset", type='int', default=0)
-  parser.add_option("--root", default='')
+  parser.add_option("--root", default=os.getcwd(),
+                    help="Search for PRESUBMIT.py up to this directory. "
+                    "If inherit-review-settings-ok is present in this "
+                    "directory, parent directories up to the root file "
+                    "system directories will also be searched.")
   parser.add_option("--default_presubmit")
   parser.add_option("--may_prompt", action='store_true', default=False)
   options, args = parser.parse_args(argv[1:])
-  if not options.root:
-    options.root = os.getcwd()
   if os.path.isdir(os.path.join(options.root, '.git')):
     change_class = GitChange
     if not options.files:

tests/presubmit_unittest.py

@@ -57,6 +57,9 @@ def GetPreferredTrySlaves():
 
 class PresubmitUnittest(PresubmitTestsBase):
   """General presubmit_support.py tests (excluding InputApi and OutputApi)."""
+
+  _INHERIT_SETTINGS = 'inherit-review-settings-ok'
+
   def testMembersChanged(self):
     self.mox.ReplayAll()
     members = [
@@ -83,6 +86,9 @@ class PresubmitUnittest(PresubmitTestsBase):
       join('moo', 'mat', 'gat', 'yo.h'),
       join('foo', 'luck.h'),
     ]
+    inherit_path = presubmit.os.path.join(self.fake_root_dir,
+                                          self._INHERIT_SETTINGS)
+    presubmit.os.path.isfile(inherit_path).AndReturn(False)
     presubmit.os.path.isfile(join(self.fake_root_dir,
                                   'PRESUBMIT.py')).AndReturn(True)
     presubmit.os.path.isfile(join(self.fake_root_dir, 'foo',
@@ -109,6 +115,36 @@ class PresubmitUnittest(PresubmitTestsBase):
                'PRESUBMIT.py')
         ])
 
+  def testListRelevantPresubmitFilesInheritSettings(self):
+    join = presubmit.os.path.join
+    sys_root_dir = self._OS_SEP
+    root_dir = join(sys_root_dir, 'foo', 'bar')
+    files = [
+      'test.cc',
+      join('moo', 'test2.cc'),
+      join('zoo', 'test3.cc')
+    ]
+    inherit_path = presubmit.os.path.join(root_dir, self._INHERIT_SETTINGS)
+    presubmit.os.path.isfile(inherit_path).AndReturn(True)
+    presubmit.os.path.isfile(join(sys_root_dir,
+                                  'PRESUBMIT.py')).AndReturn(False)
+    presubmit.os.path.isfile(join(sys_root_dir, 'foo',
+                                  'PRESUBMIT.py')).AndReturn(True)
+    presubmit.os.path.isfile(join(sys_root_dir, 'foo', 'bar',
+                                  'PRESUBMIT.py')).AndReturn(False)
+    presubmit.os.path.isfile(join(sys_root_dir, 'foo', 'bar', 'moo',
+                                  'PRESUBMIT.py')).AndReturn(True)
+    presubmit.os.path.isfile(join(sys_root_dir, 'foo', 'bar', 'zoo',
+                                  'PRESUBMIT.py')).AndReturn(False)
+    self.mox.ReplayAll()
+
+    presubmit_files = presubmit.ListRelevantPresubmitFiles(files, root_dir)
+    self.assertEqual(presubmit_files,
+        [
+          join(sys_root_dir, 'foo', 'PRESUBMIT.py'),
+          join(sys_root_dir, 'foo', 'bar', 'moo', 'PRESUBMIT.py')
+        ])
+
   def testTagLineRe(self):
     self.mox.ReplayAll()
     m = presubmit.Change._TAG_LINE_RE.match(' BUG =1223, 1445  \t')
@@ -286,6 +322,9 @@ class PresubmitUnittest(PresubmitTestsBase):
     ]
     haspresubmit_path = join(self.fake_root_dir, 'haspresubmit', 'PRESUBMIT.py')
     root_path = join(self.fake_root_dir, 'PRESUBMIT.py')
+    inherit_path = presubmit.os.path.join(self.fake_root_dir,
+                                          self._INHERIT_SETTINGS)
+    presubmit.os.path.isfile(inherit_path).AndReturn(False)
     presubmit.os.path.isfile(root_path).AndReturn(True)
     presubmit.os.path.isfile(haspresubmit_path).AndReturn(True)
     presubmit.gclient_utils.FileRead(root_path,
@@ -313,7 +352,10 @@ class PresubmitUnittest(PresubmitTestsBase):
     ]
     presubmit_path = join(self.fake_root_dir, 'PRESUBMIT.py')
     haspresubmit_path = join(self.fake_root_dir, 'haspresubmit', 'PRESUBMIT.py')
+    inherit_path = presubmit.os.path.join(self.fake_root_dir,
+                                          self._INHERIT_SETTINGS)
     for i in range(2):
+      presubmit.os.path.isfile(inherit_path).AndReturn(False)
       presubmit.os.path.isfile(presubmit_path).AndReturn(True)
       presubmit.os.path.isfile(haspresubmit_path).AndReturn(True)
       presubmit.gclient_utils.FileRead(presubmit_path, 'rU'
@@ -350,6 +392,9 @@ class PresubmitUnittest(PresubmitTestsBase):
     presubmit_path = join(self.fake_root_dir, 'PRESUBMIT.py')
     haspresubmit_path = join(self.fake_root_dir, 'haspresubmit',
                              'PRESUBMIT.py')
+    inherit_path = presubmit.os.path.join(self.fake_root_dir,
+                                          self._INHERIT_SETTINGS)
+    presubmit.os.path.isfile(inherit_path).AndReturn(False)
     presubmit.os.path.isfile(presubmit_path).AndReturn(True)
     presubmit.os.path.isfile(haspresubmit_path).AndReturn(True)
     presubmit.gclient_utils.FileRead(presubmit_path, 'rU'
@@ -383,6 +428,9 @@ def CheckChangeOnUpload(input_api, output_api):
 def CheckChangeOnCommit(input_api, output_api):
   raise Exception("Test error")
 """
+    inherit_path = presubmit.os.path.join(self.fake_root_dir,
+                                          self._INHERIT_SETTINGS)
+    presubmit.os.path.isfile(inherit_path).AndReturn(False)
     presubmit.os.path.isfile(join(self.fake_root_dir, 'PRESUBMIT.py')
         ).AndReturn(False)
     presubmit.os.path.isfile(join(self.fake_root_dir,
@@ -458,6 +506,9 @@ def CheckChangeOnCommit(input_api, output_api):
   raise Exception("Test error")
 """
     presubmit.random.randint(0, 4).AndReturn(1)
+    inherit_path = presubmit.os.path.join(self.fake_root_dir,
+                                          self._INHERIT_SETTINGS)
+    presubmit.os.path.isfile(inherit_path).AndReturn(False)
     self.mox.ReplayAll()
 
     output = StringIO.StringIO()
@@ -504,11 +555,15 @@ def CheckChangeOnCommit(input_api, output_api):
     filename_linux = join('linux_only', 'penguin.cc')
     root_presubmit = join(self.fake_root_dir, 'PRESUBMIT.py')
     linux_presubmit = join(self.fake_root_dir, 'linux_only', 'PRESUBMIT.py')
+    inherit_path = presubmit.os.path.join(self.fake_root_dir,
+                                          self._INHERIT_SETTINGS)
 
+    presubmit.os.path.isfile(inherit_path).AndReturn(False)
     presubmit.os.path.isfile(root_presubmit).AndReturn(True)
     presubmit.gclient_utils.FileRead(root_presubmit, 'rU').AndReturn(
         self.presubmit_tryslave % '["win"]')
 
+    presubmit.os.path.isfile(inherit_path).AndReturn(False)
     presubmit.os.path.isfile(root_presubmit).AndReturn(True)
     presubmit.os.path.isfile(linux_presubmit).AndReturn(True)
     presubmit.gclient_utils.FileRead(root_presubmit, 'rU').AndReturn(