diff --git a/gclient_utils.py b/gclient_utils.py index 9f4de36ad1..4d9e9860ff 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -170,8 +170,8 @@ def FileRead(filename, mode='rbU'): return s -def FileWrite(filename, content, mode='w'): - with codecs.open(filename, mode=mode, encoding='utf-8') as f: +def FileWrite(filename, content, mode='w', encoding='utf-8'): + with codecs.open(filename, mode=mode, encoding=encoding) as f: f.write(content) @@ -185,6 +185,35 @@ def temporary_directory(**kwargs): rmtree(tdir) +@contextlib.contextmanager +def temporary_file(): + """Creates a temporary file. + + On Windows, a file must be closed before it can be opened again. This function + allows to write something like: + + with gclient_utils.temporary_file() as tmp: + gclient_utils.FileWrite(tmp, foo) + useful_stuff(tmp) + + Instead of something like: + + with tempfile.NamedTemporaryFile(delete=False) as tmp: + tmp.write(foo) + tmp.close() + try: + useful_stuff(tmp) + finally: + os.remove(tmp.name) + """ + handle, name = tempfile.mkstemp() + os.close(handle) + try: + yield name + finally: + os.remove(name) + + def safe_rename(old, new): """Renames a file reliably. diff --git a/git_cl.py b/git_cl.py index 8ecafb3932..90818561fe 100755 --- a/git_cl.py +++ b/git_cl.py @@ -2342,12 +2342,10 @@ class Changelist(object): parent = self._ComputeParent(remote, upstream_branch, custom_cl_base, options.force, change_desc) tree = RunGit(['rev-parse', 'HEAD:']).strip() - with tempfile.NamedTemporaryFile(delete=False) as desc_tempfile: - desc_tempfile.write(change_desc.description.encode('utf-8', 'replace')) - desc_tempfile.close() - ref_to_push = RunGit(['commit-tree', tree, '-p', parent, - '-F', desc_tempfile.name]).strip() - os.remove(desc_tempfile.name) + with gclient_utils.temporary_file() as desc_tempfile: + gclient_utils.FileWrite(desc_tempfile, change_desc.description) + ref_to_push = RunGit( + ['commit-tree', tree, '-p', parent, '-F', desc_tempfile]).strip() else: # if not options.squash change_desc = ChangeDescription( options.message or _create_description_from_log(git_diff_args)) diff --git a/split_cl.py b/split_cl.py index b4c508272c..9d16b1c6e0 100644 --- a/split_cl.py +++ b/split_cl.py @@ -15,6 +15,7 @@ import subprocess2 import sys import tempfile +import gclient_utils import git_footers import owners import owners_finder @@ -119,13 +120,11 @@ def UploadCl(cl_index, num_cls, refactor_branch, refactor_branch_upstream, # Commit changes. The temporary file is created with delete=False so that it # can be deleted manually after git has read it rather than automatically # when it is closed. - with tempfile.NamedTemporaryFile(delete=False) as tmp_file: - tmp_file.write( + with gclient_utils.temporary_file() as tmp_file: + gclient_utils.FileWrite( + tmp_file, FormatDescriptionOrComment(description, directory, cl_index, num_cls)) - # Close the file to let git open it at the next line. - tmp_file.close() - git.run('commit', '-F', tmp_file.name) - os.remove(tmp_file.name) + git.run('commit', '-F', tmp_file) # Upload a CL. upload_args = ['-f', '-r', reviewer] diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index 14ec878ccc..740f095b73 100755 --- a/tests/gclient_utils_test.py +++ b/tests/gclient_utils_test.py @@ -337,24 +337,21 @@ class GClientUtilsTest(trial_dir.TestCase): expected, gclient_utils.ParseCodereviewSettingsContent(content)) def testFileRead_Bytes(self): - with tempfile.NamedTemporaryFile(delete=False) as tmp: - tmp.write(b'foo \xe2\x9c bar') - # NamedTemporaryFiles must be closed on Windows before being opened again. - tmp.close() - try: - self.assertEqual('foo \ufffd bar', gclient_utils.FileRead(tmp.name)) - finally: - os.remove(tmp.name) + with gclient_utils.temporary_file() as tmp: + gclient_utils.FileWrite( + tmp, b'foo \xe2\x9c bar', mode='wb', encoding=None) + self.assertEqual('foo \ufffd bar', gclient_utils.FileRead(tmp)) def testFileRead_Unicode(self): - with tempfile.NamedTemporaryFile(delete=False) as tmp: - tmp.write(b'foo \xe2\x9c\x94 bar') - # NamedTemporaryFiles must be closed on Windows before being opened again. - tmp.close() - try: - self.assertEqual('foo ✔ bar', gclient_utils.FileRead(tmp.name)) - finally: - os.remove(tmp.name) + with gclient_utils.temporary_file() as tmp: + gclient_utils.FileWrite(tmp, 'foo ✔ bar') + self.assertEqual('foo ✔ bar', gclient_utils.FileRead(tmp)) + + def testTemporaryFile(self): + with gclient_utils.temporary_file() as tmp: + gclient_utils.FileWrite(tmp, 'test') + self.assertEqual('test', gclient_utils.FileRead(tmp)) + self.assertFalse(os.path.exists(tmp)) if __name__ == '__main__': diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 4c1818207d..8378ee3eaf 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -32,7 +32,9 @@ import metrics # We have to disable monitoring before importing git_cl. metrics.DISABLE_METRICS_COLLECTION = True +import contextlib import clang_format +import gclient_utils import gerrit_util import git_cl import git_common @@ -55,26 +57,14 @@ def _constantFn(return_value): CERR1 = callError(1) -def MakeNamedTemporaryFileMock(test, expected_content): - class NamedTemporaryFileMock(object): - def __init__(self, *args, **kwargs): - self.name = '/tmp/named' - self.expected_content = expected_content.encode('utf-8', 'replace') - - def __enter__(self): - return self - - def __exit__(self, _type, _value, _tb): - pass - - def write(self, content): - if self.expected_content: - test.assertEqual(self.expected_content, content) - - def close(self): - pass +class TemporaryFileMock(object): + def __init__(self): + self.suffix = 0 - return NamedTemporaryFileMock + @contextlib.contextmanager + def __call__(self): + self.suffix += 1 + yield '/tmp/fake-temp' + str(self.suffix) class ChangelistMock(object): @@ -897,8 +887,9 @@ class TestGitCl(unittest.TestCase): calls += [ ((['git', 'rev-parse', 'HEAD:'],), # `HEAD:` means HEAD's tree hash. '0123456789abcdef'), + ((['FileWrite', '/tmp/fake-temp1', description],), None), ((['git', 'commit-tree', '0123456789abcdef', '-p', parent, - '-F', '/tmp/named'],), + '-F', '/tmp/fake-temp1'],), ref_to_push), ] else: @@ -1178,9 +1169,7 @@ class TestGitCl(unittest.TestCase): change_id=change_id) if fetched_status != 'ABANDONED': mock.patch( - 'tempfile.NamedTemporaryFile', - MakeNamedTemporaryFileMock( - self, expected_content=description)).start() + 'gclient_utils.temporary_file', TemporaryFileMock()).start() mock.patch('os.remove', return_value=True).start() self.calls += self._gerrit_upload_calls( description, reviewers, squash, diff --git a/tests/git_footers_test.py b/tests/git_footers_test.py index 619b51b27c..e04354e45d 100755 --- a/tests/git_footers_test.py +++ b/tests/git_footers_test.py @@ -17,6 +17,7 @@ else: sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +import gclient_utils import git_footers class GitFootersTest(unittest.TestCase): @@ -256,15 +257,10 @@ My commit message is my best friend. It is my life. I must master it. 'sys.stdin', StringIO('line\r\nany spaces\r\n\r\n\r\nFoo: 1\nBar: 2\nFoo: 3')) def testToJson(self): - with tempfile.NamedTemporaryFile(delete=False) as tmp: - try: - # NamedTemporaryFiles must be closed on Windows before being opened - # again. - tmp.close() - self.assertEqual(git_footers.main(['--json', tmp.name]), 0) - js = json.load(open(tmp.name)) - finally: - os.remove(tmp.name) + with gclient_utils.temporary_file() as tmp: + self.assertEqual(git_footers.main(['--json', tmp]), 0) + with open(tmp) as f: + js = json.load(f) self.assertEqual(js, {'Foo': ['3', '1'], 'Bar': ['2']}) diff --git a/tests/git_hyper_blame_test.py b/tests/git_hyper_blame_test.py index 80843ed80d..44b3db8574 100755 --- a/tests/git_hyper_blame_test.py +++ b/tests/git_hyper_blame_test.py @@ -28,6 +28,7 @@ sys.path.insert(0, DEPOT_TOOLS_ROOT) from testing_support import coverage_utils from testing_support import git_test_utils +import gclient_utils import git_common GitRepo = git_test_utils.GitRepo @@ -186,16 +187,16 @@ class GitHyperBlameMainTest(GitHyperBlameTestBase): self.blame_line('A', '2*) line 2.1')] outbuf = BytesIO() - with tempfile.NamedTemporaryFile(mode='w+', prefix='ignore') as ignore_file: - ignore_file.write('# Line comments are allowed.\n') - ignore_file.write('\n') - ignore_file.write('{}\n'.format(self.repo['B'])) - # A revision that is not in the repo (should be ignored). - ignore_file.write('xxxx\n') - ignore_file.flush() + with gclient_utils.temporary_file() as ignore_file: + gclient_utils.FileWrite( + ignore_file, + '# Line comments are allowed.\n' + '\n' + '{}\n' + 'xxxx\n'.format(self.repo['B'])) retval = self.repo.run( self.git_hyper_blame.main, - ['--ignore-file', ignore_file.name, 'tag_C', 'some/files/file'], + ['--ignore-file', ignore_file, 'tag_C', 'some/files/file'], outbuf) self.assertEqual(0, retval)