From 15bf463f6722646a34a730a2de99ecbadb88503b Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 15 May 2023 15:00:03 +0200 Subject: [PATCH 1/2] gh-64595: Fix write file logic in Argument Clinic Check if any clinic output actually changes any of the output files before deciding if we should touch the source file. This needs to be done before we eventually update the source file. --- Tools/clinic/clinic.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 4270fb3cc56613..ea5c1e08d8d6f6 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1965,16 +1965,20 @@ def dump(self): return_converters = {} -def write_file(filename, new_contents, force=False): +def file_changed(filename: str, new_contents: str) -> bool: + """Return true if file contents changed (meaning we must update it)""" try: with open(filename, 'r', encoding="utf-8") as fp: old_contents = fp.read() - - if old_contents == new_contents and not force: - # no change: avoid modifying the file modification time - return + return old_contents != new_contents except FileNotFoundError: - pass + return True + + +def write_file(filename: str, new_contents: str, force=False): + if not force and not file_changed(filename, new_contents): + # no change: avoid modifying the file modification time + return # Atomic write using a temporary file and os.replace() filename_new = f"{filename}.new" @@ -2238,7 +2242,7 @@ def parse_file(filename, *, verify=True, output=None): src_out, clinic_out = clinic.parse(raw) # If clinic output changed, force updating the source file as well. - force = bool(clinic_out) + force = any(file_changed(fn, data) for fn, data in clinic_out) write_file(output, src_out, force=force) for fn, data in clinic_out: write_file(fn, data) From 2a20f908c6b26969a9e76135604a35a7b84d4ccc Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 15 May 2023 15:10:52 +0200 Subject: [PATCH 2/2] Alternative implementation --- Tools/clinic/clinic.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index ea5c1e08d8d6f6..13fd66b0406f26 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -1975,11 +1975,7 @@ def file_changed(filename: str, new_contents: str) -> bool: return True -def write_file(filename: str, new_contents: str, force=False): - if not force and not file_changed(filename, new_contents): - # no change: avoid modifying the file modification time - return - +def write_file(filename: str, new_contents: str): # Atomic write using a temporary file and os.replace() filename_new = f"{filename}.new" with open(filename_new, "w", encoding="utf-8") as fp: @@ -2241,11 +2237,12 @@ def parse_file(filename, *, verify=True, output=None): clinic = Clinic(language, verify=verify, filename=filename) src_out, clinic_out = clinic.parse(raw) - # If clinic output changed, force updating the source file as well. - force = any(file_changed(fn, data) for fn, data in clinic_out) - write_file(output, src_out, force=force) - for fn, data in clinic_out: - write_file(fn, data) + changes = [(fn, data) for fn, data in clinic_out if file_changed(fn, data)] + if changes: + # Always (re)write the source file. + write_file(output, src_out) + for fn, data in clinic_out: + write_file(fn, data) def compute_checksum(input, length=None):