Skip to content

Commit 656a64b

Browse files
authored
gh-141930: Use the regular IO stack to write .pyc files for a better error message on failure (GH-141931)
* Use open() to write the bytecode * Convert to unittest style asserts * Tweak news, thanks @vstinner * Tidy * reword NEWS, avoid word "retried"
1 parent 69f54ce commit 656a64b

File tree

3 files changed

+59
-22
lines changed

3 files changed

+59
-22
lines changed

Lib/importlib/_bootstrap_external.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,8 @@ def _write_atomic(path, data, mode=0o666):
208208
try:
209209
# We first write data to a temporary file, and then use os.replace() to
210210
# perform an atomic rename.
211-
with _io.FileIO(fd, 'wb') as file:
212-
bytes_written = file.write(data)
213-
if bytes_written != len(data):
214-
# Raise an OSError so the 'except' below cleans up the partially
215-
# written file.
216-
raise OSError("os.write() didn't write the full pyc file")
211+
with _io.open(fd, 'wb') as file:
212+
file.write(data)
217213
_os.replace(path_tmp, path)
218214
except OSError:
219215
try:

Lib/test/test_importlib/test_util.py

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -788,31 +788,70 @@ def test_complete_multi_phase_init_module(self):
788788
self.run_with_own_gil(script)
789789

790790

791-
class MiscTests(unittest.TestCase):
792-
def test_atomic_write_should_notice_incomplete_writes(self):
791+
class PatchAtomicWrites:
792+
def __init__(self, truncate_at_length, never_complete=False):
793+
self.truncate_at_length = truncate_at_length
794+
self.never_complete = never_complete
795+
self.seen_write = False
796+
self._children = []
797+
798+
def __enter__(self):
793799
import _pyio
794800

795801
oldwrite = os.write
796-
seen_write = False
797-
798-
truncate_at_length = 100
799802

800803
# Emulate an os.write that only writes partial data.
801804
def write(fd, data):
802-
nonlocal seen_write
803-
seen_write = True
804-
return oldwrite(fd, data[:truncate_at_length])
805+
if self.seen_write and self.never_complete:
806+
return None
807+
self.seen_write = True
808+
return oldwrite(fd, data[:self.truncate_at_length])
805809

806810
# Need to patch _io to be _pyio, so that io.FileIO is affected by the
807811
# os.write patch.
808-
with (support.swap_attr(_bootstrap_external, '_io', _pyio),
809-
support.swap_attr(os, 'write', write)):
810-
with self.assertRaises(OSError):
811-
# Make sure we write something longer than the point where we
812-
# truncate.
813-
content = b'x' * (truncate_at_length * 2)
814-
_bootstrap_external._write_atomic(os_helper.TESTFN, content)
815-
assert seen_write
812+
self.children = [
813+
support.swap_attr(_bootstrap_external, '_io', _pyio),
814+
support.swap_attr(os, 'write', write)
815+
]
816+
for child in self.children:
817+
child.__enter__()
818+
return self
819+
820+
def __exit__(self, exc_type, exc_val, exc_tb):
821+
for child in self.children:
822+
child.__exit__(exc_type, exc_val, exc_tb)
823+
824+
825+
class MiscTests(unittest.TestCase):
826+
827+
def test_atomic_write_retries_incomplete_writes(self):
828+
truncate_at_length = 100
829+
length = truncate_at_length * 2
830+
831+
with PatchAtomicWrites(truncate_at_length=truncate_at_length) as cm:
832+
# Make sure we write something longer than the point where we
833+
# truncate.
834+
content = b'x' * length
835+
_bootstrap_external._write_atomic(os_helper.TESTFN, content)
836+
self.assertTrue(cm.seen_write)
837+
838+
self.assertEqual(os.stat(support.os_helper.TESTFN).st_size, length)
839+
os.unlink(support.os_helper.TESTFN)
840+
841+
def test_atomic_write_errors_if_unable_to_complete(self):
842+
truncate_at_length = 100
843+
844+
with (
845+
PatchAtomicWrites(
846+
truncate_at_length=truncate_at_length, never_complete=True,
847+
) as cm,
848+
self.assertRaises(OSError)
849+
):
850+
# Make sure we write something longer than the point where we
851+
# truncate.
852+
content = b'x' * (truncate_at_length * 2)
853+
_bootstrap_external._write_atomic(os_helper.TESTFN, content)
854+
self.assertTrue(cm.seen_write)
816855

817856
with self.assertRaises(OSError):
818857
os.stat(support.os_helper.TESTFN) # Check that the file did not get written.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
When importing a module, use Python's regular file object to ensure that
2+
writes to ``.pyc`` files are complete or an appropriate error is raised.

0 commit comments

Comments
 (0)