diff options
| -rw-r--r-- | Lib/test/test_clinic.py | 26 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/Build/2020-11-18-11-58-44.bpo-42398.Yt5wO8.rst | 4 | ||||
| -rwxr-xr-x | Tools/clinic/clinic.py | 54 |
3 files changed, 58 insertions, 26 deletions
diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 244c5fe..347ccc0 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -801,17 +801,29 @@ class ClinicExternalTest(TestCase): maxDiff = None def test_external(self): + # bpo-42398: Test that the destination file is left unchanged if the + # content does not change. Moreover, check also that the file + # modification time does not change in this case. source = support.findfile('clinic.test') with open(source, 'r', encoding='utf-8') as f: - original = f.read() - with support.temp_dir() as testdir: - testfile = os.path.join(testdir, 'clinic.test.c') + orig_contents = f.read() + + with support.temp_dir() as tmp_dir: + testfile = os.path.join(tmp_dir, 'clinic.test.c') with open(testfile, 'w', encoding='utf-8') as f: - f.write(original) - clinic.parse_file(testfile, force=True) + f.write(orig_contents) + old_mtime_ns = os.stat(testfile).st_mtime_ns + + clinic.parse_file(testfile) + with open(testfile, 'r', encoding='utf-8') as f: - result = f.read() - self.assertEqual(result, original) + new_contents = f.read() + new_mtime_ns = os.stat(testfile).st_mtime_ns + + self.assertEqual(new_contents, orig_contents) + # Don't change the file modification time + # if the content does not change + self.assertEqual(new_mtime_ns, old_mtime_ns) if __name__ == "__main__": diff --git a/Misc/NEWS.d/next/Build/2020-11-18-11-58-44.bpo-42398.Yt5wO8.rst b/Misc/NEWS.d/next/Build/2020-11-18-11-58-44.bpo-42398.Yt5wO8.rst new file mode 100644 index 0000000..9ab99d0 --- /dev/null +++ b/Misc/NEWS.d/next/Build/2020-11-18-11-58-44.bpo-42398.Yt5wO8.rst @@ -0,0 +1,4 @@ +Fix a race condition in "make regen-all" when make -jN option is used to run +jobs in parallel. The clinic.py script now only use atomic write to write +files. Moveover, generated files are now left unchanged if the content does not +change, to not change the file modification time. diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index c5edc7c..40c95e4 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1756,6 +1756,30 @@ legacy_converters = {} # The callable should not call builtins.print. return_converters = {} + +def write_file(filename, new_contents): + try: + with open(filename, 'r', encoding="utf-8") as fp: + old_contents = fp.read() + + if old_contents == new_contents: + # no change: avoid modifying the file modification time + return + except FileNotFoundError: + pass + + # Atomic write using a temporary file and os.replace() + filename_new = f"{filename}.new" + with open(filename_new, "w", encoding="utf-8") as fp: + fp.write(new_contents) + + try: + os.replace(filename_new, filename) + except: + os.unlink(filename_new) + raise + + clinic = None class Clinic: @@ -1802,7 +1826,7 @@ impl_definition block """ - def __init__(self, language, printer=None, *, force=False, verify=True, filename=None): + def __init__(self, language, printer=None, *, verify=True, filename=None): # maps strings to Parser objects. # (instantiated from the "parsers" global.) self.parsers = {} @@ -1811,7 +1835,6 @@ impl_definition block fail("Custom printers are broken right now") self.printer = printer or BlockPrinter(language) self.verify = verify - self.force = force self.filename = filename self.modules = collections.OrderedDict() self.classes = collections.OrderedDict() @@ -1944,8 +1967,7 @@ impl_definition block block.input = 'preserve\n' printer_2 = BlockPrinter(self.language) printer_2.print_block(block) - with open(destination.filename, "wt") as f: - f.write(printer_2.f.getvalue()) + write_file(destination.filename, printer_2.f.getvalue()) continue text = printer.f.getvalue() @@ -1997,7 +2019,10 @@ impl_definition block return module, cls -def parse_file(filename, *, force=False, verify=True, output=None, encoding='utf-8'): +def parse_file(filename, *, verify=True, output=None): + if not output: + output = filename + extension = os.path.splitext(filename)[1][1:] if not extension: fail("Can't extract file type for file " + repr(filename)) @@ -2007,7 +2032,7 @@ def parse_file(filename, *, force=False, verify=True, output=None, encoding='utf except KeyError: fail("Can't identify file type for file " + repr(filename)) - with open(filename, 'r', encoding=encoding) as f: + with open(filename, 'r', encoding="utf-8") as f: raw = f.read() # exit quickly if there are no clinic markers in the file @@ -2015,19 +2040,10 @@ def parse_file(filename, *, force=False, verify=True, output=None, encoding='utf if not find_start_re.search(raw): return - clinic = Clinic(language, force=force, verify=verify, filename=filename) + clinic = Clinic(language, verify=verify, filename=filename) cooked = clinic.parse(raw) - if (cooked == raw) and not force: - return - - directory = os.path.dirname(filename) or '.' - with tempfile.TemporaryDirectory(prefix="clinic", dir=directory) as tmpdir: - bytes = cooked.encode(encoding) - tmpfilename = os.path.join(tmpdir, os.path.basename(filename)) - with open(tmpfilename, "wb") as f: - f.write(bytes) - os.replace(tmpfilename, output or filename) + write_file(output, cooked) def compute_checksum(input, length=None): @@ -5033,7 +5049,7 @@ For more information see https://docs.python.org/3/howto/clinic.html""") path = os.path.join(root, filename) if ns.verbose: print(path) - parse_file(path, force=ns.force, verify=not ns.force) + parse_file(path, verify=not ns.force) return if not ns.filename: @@ -5049,7 +5065,7 @@ For more information see https://docs.python.org/3/howto/clinic.html""") for filename in ns.filename: if ns.verbose: print(filename) - parse_file(filename, output=ns.output, force=ns.force, verify=not ns.force) + parse_file(filename, output=ns.output, verify=not ns.force) if __name__ == "__main__": |