Added orphaned compiled Python file checks.

There have been a handful of cases where orphaned compiled Python files (.pyc
files without an accompanying .py file) have been loaded preferentially by
the presubmit scripts, causing weird problems. This adds a check and a mode for
such files, either warning the user or deleting them.

BUG=None
TEST=local
  - Tested against local repository.

R=iannucci@chromium.org, szager@chromium.org

Review URL: https://codereview.chromium.org/621533002

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

presubmit_support.py

@@ -1287,7 +1287,6 @@ class PresubmitExecuter(object):
     Return:
       A list of result objects, empty if no problems.
     """
-
     # Change to the presubmit file's directory to support local imports.
     main_path = os.getcwd()
     os.chdir(os.path.dirname(presubmit_path))
@@ -1336,7 +1335,8 @@ def DoPresubmitChecks(change,
                       input_stream,
                       default_presubmit,
                       may_prompt,
-                      rietveld_obj):
+                      rietveld_obj,
+                      cleanup=False):
   """Runs all presubmit checks that apply to the files in the change.
 
   This finds all PRESUBMIT.py files in directories enclosing the files in the
@@ -1355,6 +1355,8 @@ def DoPresubmitChecks(change,
     default_presubmit: A default presubmit script to execute in any case.
     may_prompt: Enable (y/n) questions on warning or error.
     rietveld_obj: rietveld.Rietveld object.
+    cleanup: If True, filesystem modifications may be made as necessary to
+             clean up repository state to ensure reliable checks.
 
   Warning:
     If may_prompt is true, output_stream SHOULD be sys.stdout and input_stream
@@ -1366,6 +1368,10 @@ def DoPresubmitChecks(change,
   """
   old_environ = os.environ
   try:
+    # Clear all orphaned compiled Python files.
+    CleanOrphanedCompiledPython(output_stream, change.RepositoryRoot(),
+                                remove=cleanup)
+
     # Make sure python subprocesses won't generate .pyc files.
     os.environ = os.environ.copy()
     os.environ['PYTHONDONTWRITEBYTECODE'] = '1'
@@ -1442,6 +1448,46 @@ def DoPresubmitChecks(change,
     os.environ = old_environ
 
 
+def IsCompiledPython(path):
+  """Tests whether a given path looks like a compiled Python file."""
+  # Validate the magic number.
+  with open(path, 'rb') as fd:
+    magic = fd.read(8)
+  return len(magic) == 8 and magic[2:4] == '\x0d\x0a'
+
+
+def CleanOrphanedCompiledPython(output, path, remove):
+  """Clears all compiled Python files in a directory recursively."""
+  pyc_files = []
+  for dirpath, _, filenames in os.walk(path):
+    for filename in filenames:
+      name, ext = os.path.splitext(filename)
+      if ext != '.pyc':
+        continue
+
+      # Is there an accompanying Python script? If so, leave the compiled file.
+      py_path = os.path.join(dirpath, '%s.py' % (name,))
+      if os.path.isfile(py_path):
+        continue
+
+      # Make sure this is compiled Python before we delete it.
+      path = os.path.join(dirpath, filename)
+      if not IsCompiledPython(path):
+        continue
+      pyc_files.append(path)
+  if pyc_files and not remove:
+    output.write(
+        'Warning: Found orphaned compiled Python files. These should be\n'
+        'removed before running PRESUBMIT, as they can have unexpected impact\n'
+        'on checks and tests:\n')
+    for pyc_file in pyc_files:
+      output.write('  %s\n' % (pyc_file,))
+  else:
+    for pyc_file in pyc_files:
+      output.write('Removing orphaned compiled Python file: %s\n' % (pyc_file,))
+      os.remove(pyc_file)
+
+
 def ScanSubDirs(mask, recursive):
   if not recursive:
     return [x for x in glob.glob(mask) if x not in ('.svn', '.git')]

tests/presubmit_unittest.py

@@ -7,6 +7,9 @@
 
 # pylint: disable=E1101,E1103
 
+import __builtin__
+
+import contextlib
 import functools
 import itertools
 import logging
@@ -177,7 +180,8 @@ class PresubmitUnittest(PresubmitTestsBase):
       'presubmit_canned_checks', 'random', 're', 'rietveld', 'scm',
       'subprocess', 'sys', 'tempfile', 'time', 'traceback', 'types', 'unittest',
       'urllib2', 'warn', 'multiprocessing', 'DoGetTryMasters',
-      'GetTryMastersExecuter', 'itertools',
+      'GetTryMastersExecuter', 'itertools', 'CleanOrphanedCompiledPython',
+      'IsCompiledPython',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(presubmit, members)
@@ -682,6 +686,7 @@ class PresubmitUnittest(PresubmitTestsBase):
     presubmit.gclient_utils.FileRead(haspresubmit_path,
                                      'rU').AndReturn(self.presubmit_text)
     presubmit.random.randint(0, 4).AndReturn(1)
+    presubmit.os.walk(self.fake_root_dir).AndReturn([])
     self.mox.ReplayAll()
 
     input_buf = StringIO.StringIO('y\n')
@@ -722,6 +727,8 @@ class PresubmitUnittest(PresubmitTestsBase):
           ).AndReturn(self.presubmit_text)
     presubmit.random.randint(0, 4).AndReturn(1)
     presubmit.random.randint(0, 4).AndReturn(1)
+    presubmit.os.walk(self.fake_root_dir).AndReturn([])
+    presubmit.os.walk(self.fake_root_dir).AndReturn([])
     self.mox.ReplayAll()
 
     input_buf = StringIO.StringIO('n\n')  # say no to the warning
@@ -768,6 +775,7 @@ class PresubmitUnittest(PresubmitTestsBase):
     presubmit.gclient_utils.FileRead(haspresubmit_path, 'rU').AndReturn(
         self.presubmit_text)
     presubmit.random.randint(0, 4).AndReturn(1)
+    presubmit.os.walk(self.fake_root_dir).AndReturn([])
     self.mox.ReplayAll()
 
     change = presubmit.Change(
@@ -809,6 +817,7 @@ def CheckChangeOnCommit(input_api, output_api):
                                   'haspresubmit',
                                   'PRESUBMIT.py')).AndReturn(False)
     presubmit.random.randint(0, 4).AndReturn(0)
+    presubmit.os.walk(self.fake_root_dir).AndReturn([])
     self.mox.ReplayAll()
 
     input_buf = StringIO.StringIO('y\n')
@@ -891,6 +900,7 @@ def CheckChangeOnCommit(input_api, output_api):
     inherit_path = presubmit.os.path.join(self.fake_root_dir,
                                           self._INHERIT_SETTINGS)
     presubmit.os.path.isfile(inherit_path).AndReturn(False)
+    presubmit.os.walk(self.fake_root_dir).AndReturn([])
     self.mox.ReplayAll()
 
     output = StringIO.StringIO()
@@ -1175,6 +1185,88 @@ def CheckChangeOnCommit(input_api, output_api):
     except SystemExit, e:
       self.assertEquals(2, e.code)
 
+  def testOrphanedCompiledPythonDoesntRemoveFiles(self):
+    self.mox.StubOutWithMock(presubmit, 'IsCompiledPython')
+    py_path = os.path.join(self.fake_root_dir, 'myfile.py')
+    pyc_path = os.path.join(self.fake_root_dir, 'myfile.pyc')
+
+    presubmit.os.walk(self.fake_root_dir).AndReturn([
+        (self.fake_root_dir, [], ['myfile.pyc']),
+    ])
+    presubmit.os.path.isfile(py_path).AndReturn(False)
+    presubmit.IsCompiledPython(pyc_path).AndReturn(True)
+    self.mox.ReplayAll()
+
+    output = StringIO.StringIO()
+    presubmit.CleanOrphanedCompiledPython(output, self.fake_root_dir, False)
+
+  def testCleanOrphanedCompiledPython(self):
+    self.mox.StubOutWithMock(presubmit, 'IsCompiledPython')
+    py_path = os.path.join(self.fake_root_dir, 'myfile.py')
+    pyc_path = os.path.join(self.fake_root_dir, 'myfile.pyc')
+
+    presubmit.os.walk(self.fake_root_dir).AndReturn([
+        (self.fake_root_dir, [], ['myfile.pyc']),
+    ])
+    presubmit.os.path.isfile(py_path).AndReturn(False)
+    presubmit.IsCompiledPython(pyc_path).AndReturn(True)
+    presubmit.os.remove(pyc_path)
+    self.mox.ReplayAll()
+
+    output = StringIO.StringIO()
+    presubmit.CleanOrphanedCompiledPython(output, self.fake_root_dir, True)
+
+  def testCleanNonOrphanedCompiledPython(self):
+    self.mox.StubOutWithMock(presubmit, 'IsCompiledPython')
+    py_path = os.path.join(self.fake_root_dir, 'myfile.py')
+
+    presubmit.os.walk(self.fake_root_dir).AndReturn([
+        (self.fake_root_dir, [], ['myfile.pyc', 'myfile.py']),
+    ])
+    presubmit.os.path.isfile(py_path).AndReturn(True)
+    self.mox.ReplayAll()
+
+    output = StringIO.StringIO()
+    presubmit.CleanOrphanedCompiledPython(output, self.fake_root_dir, True)
+
+  def testCleanOrphanedCompiledPythonAvoidsOtherFiles(self):
+    self.mox.StubOutWithMock(presubmit, 'IsCompiledPython')
+
+    presubmit.os.walk(self.fake_root_dir).AndReturn([
+        (self.fake_root_dir, [], ['otherfile.txt']),
+    ])
+    self.mox.ReplayAll()
+
+    output = StringIO.StringIO()
+    presubmit.CleanOrphanedCompiledPython(output, self.fake_root_dir, True)
+
+  def testIsCompiledPython(self):
+    self.mox.StubOutWithMock(__builtin__, 'open')
+    pyc_path = os.path.join(self.fake_root_dir, 'testfile.pyc')
+    open(pyc_path, 'rb').AndReturn(contextlib.closing(
+        StringIO.StringIO('\x00\x00\x0d\x0a\x00\x00\x00\x00')))
+    self.mox.ReplayAll()
+
+    self.assertTrue(presubmit.IsCompiledPython(pyc_path))
+
+  def testIsCompiledPythonFailsForInvalidMagic(self):
+    self.mox.StubOutWithMock(__builtin__, 'open')
+    pyc_path = os.path.join(self.fake_root_dir, 'testfile.pyc')
+    open(pyc_path, 'rb').AndReturn(contextlib.closing(
+        StringIO.StringIO('\xca\xfe\xba\xbe\x00\x00\x00\x00')))
+    self.mox.ReplayAll()
+
+    self.assertFalse(presubmit.IsCompiledPython(pyc_path))
+
+  def testIsCompiledPythonFailsForInvalidSize(self):
+    self.mox.StubOutWithMock(__builtin__, 'open')
+    pyc_path = os.path.join(self.fake_root_dir, 'testfile.pyc')
+    open(pyc_path, 'rb').AndReturn(contextlib.closing(
+        StringIO.StringIO('\x00\x00\x0d\x0a\x00\x00\x00')))
+    self.mox.ReplayAll()
+
+    self.assertFalse(presubmit.IsCompiledPython(pyc_path))
+
 
 class InputApiUnittest(PresubmitTestsBase):
   """Tests presubmit.InputApi."""