diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 73bb942af7c0a1..5e3593fe27f0e4 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -8,6 +8,7 @@ from test.support.os_helper import TESTFN, unlink, rmtree from textwrap import dedent from unittest import TestCase +import difflib import inspect import os.path import re @@ -2893,6 +2894,148 @@ def test_cli_force(self): generated = f.read() self.assertEndsWith(generated, checksum) + DRY_RUN_CODE = dedent(""" + /*[clinic input] + func + a: int + / + + Docstring. + [clinic start generated code]*/ + """) + + def make_dry_run_file(self, tmp_dir): + fn = os.path.join(tmp_dir, "test.c") + with open(fn, "w", encoding="utf-8") as f: + f.write(self.DRY_RUN_CODE) + return fn + + @staticmethod + def dest_file(fn): + # The default destination for the generated code. Its path is + # built from the "{dirname}/clinic/{basename}.h" template, so it + # always uses forward slashes, even on Windows. + dirname, basename = os.path.split(fn) + return f"{dirname}/clinic/{basename}.h" + + def check_unchanged(self, tmp_dir, fn, pre_mtime): + # Neither the source file nor the destination file + # nor its directory is created or modified. + with open(fn, encoding="utf-8") as f: + self.assertEqual(f.read(), self.DRY_RUN_CODE) + self.assertEqual(os.stat(fn).st_mtime_ns, pre_mtime) + self.assertEqual(os.listdir(tmp_dir), ["test.c"]) + + def test_cli_dry_run(self): + with os_helper.temp_dir() as tmp_dir: + fn = self.make_dry_run_file(tmp_dir) + pre_mtime = os.stat(fn).st_mtime_ns + out = self.expect_success("--dry-run", fn) + self.assertEqual(out.splitlines(), [ + f"would create {self.dest_file(fn)}", + f"would update {fn}", + ]) + self.check_unchanged(tmp_dir, fn, pre_mtime) + + def test_cli_dry_run_no_change(self): + with os_helper.temp_dir() as tmp_dir: + fn = self.make_dry_run_file(tmp_dir) + self.expect_success(fn) + self.assertEqual(self.expect_success("--dry-run", fn), "") + self.assertEqual(self.expect_success("--diff", fn), "") + + def test_cli_dry_run_no_clinic_block(self): + with os_helper.temp_dir() as tmp_dir: + fn = os.path.join(tmp_dir, "test.c") + with open(fn, "w", encoding="utf-8") as f: + f.write("int x;\n") + self.assertEqual(self.expect_success("--dry-run", fn), "") + + def test_cli_dry_run_output(self): + with os_helper.temp_dir() as tmp_dir: + fn = self.make_dry_run_file(tmp_dir) + out_fn = os.path.join(tmp_dir, "output.c") + out = self.expect_success("--dry-run", "-o", out_fn, fn) + self.assertIn(f"would create {out_fn}", out) + self.assertNotIn(f"would update {fn}", out) + self.assertFalse(os.path.exists(out_fn)) + + def test_cli_dry_run_make(self): + with os_helper.temp_dir() as tmp_dir: + fn = self.make_dry_run_file(tmp_dir) + pre_mtime = os.stat(fn).st_mtime_ns + out = self.expect_success("--dry-run", "--make", "--srcdir", tmp_dir) + self.assertIn(f"would update {fn}", out) + self.check_unchanged(tmp_dir, fn, pre_mtime) + + def test_cli_dry_run_verbose(self): + with os_helper.temp_dir() as tmp_dir: + fn = self.make_dry_run_file(tmp_dir) + out, err, code = self.run_clinic("-v", "--dry-run", fn) + self.assertEqual(code, 0) + # The progress goes to stderr, so that the standard output + # contains only the report. + self.assertEqual(err.splitlines(), [fn]) + self.assertEqual(out.splitlines(), [ + f"would create {self.dest_file(fn)}", + f"would update {fn}", + ]) + + def test_cli_dry_run_checksum_mismatch(self): + invalid_input = dedent(""" + /*[clinic input] + output preset block + module test + test.fn + a: int + [clinic start generated code]*/ + /*[clinic end generated code: output=bogus input=bogus]*/ + """) + with os_helper.temp_dir() as tmp_dir: + fn = os.path.join(tmp_dir, "test.c") + with open(fn, "w", encoding="utf-8") as f: + f.write(invalid_input) + pre_mtime = os.stat(fn).st_mtime_ns + # The dry run does not disable the checksum verification. + _, err = self.expect_failure("--dry-run", fn) + self.assertIn("Checksum mismatch!", err) + # With -f the change is reported, but still not written. + out = self.expect_success("--dry-run", "-f", fn) + self.assertIn(f"would update {fn}", out) + with open(fn, encoding="utf-8") as f: + self.assertEqual(f.read(), invalid_input) + self.assertEqual(os.stat(fn).st_mtime_ns, pre_mtime) + + def test_cli_diff(self): + with os_helper.temp_dir() as tmp_dir: + fn = self.make_dry_run_file(tmp_dir) + pre_mtime = os.stat(fn).st_mtime_ns + out = self.expect_success("--diff", fn) + self.check_unchanged(tmp_dir, fn, pre_mtime) + + # A new file is created by the patch. + dest_fn = self.dest_file(fn) + self.assertStartsWith(out, f"--- /dev/null\n+++ {dest_fn}\n@@ -0,0 +1,") + self.assertIn(f"--- {fn}\n+++ {fn}\n", out) + self.assertIn("+/*[clinic end generated code:", out) + + # The patch is what clinic would have written. + self.expect_success(fn) + with open(fn, encoding="utf-8") as f: + new_contents = f.read() + expected = "".join(difflib.unified_diff( + self.DRY_RUN_CODE.splitlines(keepends=True), + new_contents.splitlines(keepends=True), + fromfile=fn, tofile=fn)) + self.assertEndsWith(out, expected) + + def test_cli_fail_converters_and_dry_run(self): + for opt in "--dry-run", "--diff": + with self.subTest(opt=opt): + _, err = self.expect_failure("--converters", opt) + msg = "can't use --dry-run or --diff with --converters" + self.assertIn(msg, err) + def test_cli_make(self): c_code = dedent(""" /*[clinic input] diff --git a/Misc/NEWS.d/next/Tools-Demos/2026-08-05-10-24-17.gh-issue-155207.Wv3nKq.rst b/Misc/NEWS.d/next/Tools-Demos/2026-08-05-10-24-17.gh-issue-155207.Wv3nKq.rst new file mode 100644 index 00000000000000..392ab42bffa726 --- /dev/null +++ b/Misc/NEWS.d/next/Tools-Demos/2026-08-05-10-24-17.gh-issue-155207.Wv3nKq.rst @@ -0,0 +1,3 @@ +Argument Clinic now supports the ``--dry-run`` and ``--diff`` options. +They list the files which would be changed, or write a unified diff of the +changes to the standard output, without modifying any file. diff --git a/Tools/clinic/libclinic/__init__.py b/Tools/clinic/libclinic/__init__.py index 742f1448146a0f..4adabd0e90c274 100644 --- a/Tools/clinic/libclinic/__init__.py +++ b/Tools/clinic/libclinic/__init__.py @@ -26,6 +26,8 @@ is_legal_py_identifier, ) from .utils import ( + FileChange, + FileWriter, FormatCounterFormatter, NULL, NullType, @@ -33,6 +35,7 @@ VersionTuple, compute_checksum, create_regex, + read_file, unknown, unspecified, write_file, @@ -66,6 +69,8 @@ "is_legal_py_identifier", # Utility functions + "FileChange", + "FileWriter", "FormatCounterFormatter", "NULL", "NullType", @@ -73,6 +78,7 @@ "VersionTuple", "compute_checksum", "create_regex", + "read_file", "unknown", "unspecified", "write_file", diff --git a/Tools/clinic/libclinic/app.py b/Tools/clinic/libclinic/app.py index 632bed3ce53dde..d81874e26967cf 100644 --- a/Tools/clinic/libclinic/app.py +++ b/Tools/clinic/libclinic/app.py @@ -87,6 +87,7 @@ def __init__( filename: str, limited_capi: bool, verify: bool = True, + writer: libclinic.FileWriter | None = None, ) -> None: # maps strings to Parser objects. # (instantiated from the "parsers" global.) @@ -95,6 +96,7 @@ def __init__( if printer: fail("Custom printers are broken right now") self.printer = printer or BlockPrinter(language) + self.writer = writer or libclinic.FileWriter() self.verify = verify self.limited_capi = limited_capi self.filename = filename @@ -213,7 +215,7 @@ def parse(self, input: str) -> str: try: dirname = os.path.dirname(destination.filename) try: - os.makedirs(dirname) + self.writer.makedirs(dirname) except FileExistsError: if not os.path.isdir(dirname): fail(f"Can't write to destination " @@ -234,8 +236,8 @@ def parse(self, input: str) -> str: printer_2 = BlockPrinter(self.language) printer_2.print_block(block, header_includes=includes) - libclinic.write_file(destination.filename, - printer_2.f.getvalue()) + self.writer.write(destination.filename, + printer_2.f.getvalue()) continue return printer.f.getvalue() diff --git a/Tools/clinic/libclinic/cli.py b/Tools/clinic/libclinic/cli.py index f36c6d04efd383..678edc8ca28944 100644 --- a/Tools/clinic/libclinic/cli.py +++ b/Tools/clinic/libclinic/cli.py @@ -1,6 +1,7 @@ from __future__ import annotations import argparse +import difflib import inspect import os import re @@ -52,9 +53,12 @@ def parse_file( limited_capi: bool, output: str | None = None, verify: bool = True, + writer: libclinic.FileWriter | None = None, ) -> None: if not output: output = filename + if writer is None: + writer = libclinic.FileWriter() extension = os.path.splitext(filename)[1][1:] if not extension: @@ -80,10 +84,11 @@ def parse_file( clinic = Clinic(language, verify=verify, filename=filename, - limited_capi=limited_capi) + limited_capi=limited_capi, + writer=writer) cooked = clinic.parse(raw) - libclinic.write_file(output, cooked) + writer.write(output, cooked) def create_cli() -> argparse.ArgumentParser: @@ -102,6 +107,12 @@ def create_cli() -> argparse.ArgumentParser: help="redirect file output to OUTPUT") cmdline.add_argument("-v", "--verbose", action='store_true', help="enable verbose mode") + cmdline.add_argument("--dry-run", action='store_true', + help=("don't write any file, only list the files " + "which would be changed")) + cmdline.add_argument("--diff", action='store_true', + help=("don't write any file, write a unified diff " + "of the changes to the standard output")) cmdline.add_argument("--converters", action='store_true', help=("print a list of all supported converters " "and return converters")) @@ -119,12 +130,43 @@ def create_cli() -> argparse.ArgumentParser: return cmdline +def print_diff(change: libclinic.FileChange) -> None: + if change.old_contents is None: + fromfile = "/dev/null" + old_lines: list[str] = [] + else: + fromfile = change.filename + old_lines = change.old_contents.splitlines(keepends=True) + sys.stdout.writelines(difflib.unified_diff( + old_lines, + change.new_contents.splitlines(keepends=True), + fromfile=fromfile, + tofile=change.filename, + )) + + +def report_changes(writer: libclinic.FileWriter, *, diff: bool) -> None: + for change in sorted(writer.changes, key=lambda change: change.filename): + if diff: + print_diff(change) + else: + action = "create" if change.old_contents is None else "update" + print(f"would {action} {change.filename}") + + def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: + dry_run = ns.dry_run or ns.diff + # The report is written to the standard output, so the progress + # is written to the standard error stream to not mix them. + verbose_file = sys.stderr if dry_run else sys.stdout + if ns.converters: if ns.filename: parser.error( "can't specify --converters and a filename at the same time" ) + if dry_run: + parser.error("can't use --dry-run or --diff with --converters") AnyConverterType = ConverterType | ReturnConverterType converter_list: list[tuple[str, AnyConverterType]] = [] return_converter_list: list[tuple[str, AnyConverterType]] = [] @@ -188,6 +230,7 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: excludes = [os.path.normpath(f) for f in excludes] else: excludes = [] + writer = libclinic.FileWriter(dry_run=dry_run) for root, dirs, files in os.walk(ns.srcdir): for rcs_dir in ('.svn', '.git', '.hg', 'build', 'externals'): if rcs_dir in dirs: @@ -201,9 +244,11 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: if path in excludes: continue if ns.verbose: - print(path) + print(path, file=verbose_file) parse_file(path, - verify=not ns.force, limited_capi=ns.limited_capi) + verify=not ns.force, limited_capi=ns.limited_capi, + writer=writer) + report_changes(writer, diff=ns.diff) return if not ns.filename: @@ -212,11 +257,14 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: if ns.output and len(ns.filename) > 1: parser.error("can't use -o with multiple filenames") + writer = libclinic.FileWriter(dry_run=dry_run) for filename in ns.filename: if ns.verbose: - print(filename) + print(filename, file=verbose_file) parse_file(filename, output=ns.output, - verify=not ns.force, limited_capi=ns.limited_capi) + verify=not ns.force, limited_capi=ns.limited_capi, + writer=writer) + report_changes(writer, diff=ns.diff) def main(argv: list[str] | None = None) -> NoReturn: diff --git a/Tools/clinic/libclinic/utils.py b/Tools/clinic/libclinic/utils.py index 3df64f270dd074..8fc8748f0f9ae1 100644 --- a/Tools/clinic/libclinic/utils.py +++ b/Tools/clinic/libclinic/utils.py @@ -1,4 +1,5 @@ import collections +import dataclasses as dc import enum import hashlib import os @@ -7,17 +8,20 @@ from typing import Literal, Final -def write_file(filename: str, new_contents: str) -> None: - """Write new content to file, iff the content changed.""" +def read_file(filename: str) -> str | None: + """Return the content of the file, or None if it does not exist.""" try: with open(filename, encoding="utf-8") as fp: - old_contents = fp.read() - - if old_contents == new_contents: - # no change: avoid modifying the file modification time - return + return fp.read() except FileNotFoundError: - pass + return None + + +def write_file(filename: str, new_contents: str) -> None: + """Write new content to file, iff the content changed.""" + if read_file(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" with open(filename_new, "w", encoding="utf-8") as fp: @@ -29,6 +33,42 @@ def write_file(filename: str, new_contents: str) -> None: raise +@dc.dataclass(slots=True, frozen=True) +class FileChange: + filename: str + # None if the file does not exist yet. + old_contents: str | None + new_contents: str + + +@dc.dataclass(slots=True) +class FileWriter: + """Write the generated files. + + In the dry run mode no file is written, the changes are only recorded. + """ + + dry_run: bool = False + changes: list[FileChange] = dc.field(default_factory=list) + + def makedirs(self, dirname: str) -> None: + if not self.dry_run: + os.makedirs(dirname) + elif os.path.exists(dirname): + # Create nothing, but fail as os.makedirs() does, so that + # the caller can report an existing non-directory. + raise FileExistsError(dirname) + + def write(self, filename: str, new_contents: str) -> None: + if not self.dry_run: + write_file(filename, new_contents) + return + old_contents = read_file(filename) + if old_contents != new_contents: + self.changes.append( + FileChange(filename, old_contents, new_contents)) + + def compute_checksum(input_: str, length: int | None = None) -> str: checksum = hashlib.sha1(input_.encode("utf-8")).hexdigest() if length: