diff --git a/presubmit_support.py b/presubmit_support.py index b38652406..d4400554d 100755 --- a/presubmit_support.py +++ b/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')] diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 9df0b830e..39b64b828 100755 --- a/tests/presubmit_unittest.py +++ b/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."""