From bb701b73236be0da633bbfa0bad4f33dbf9843b9 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 8 Aug 2023 12:19:14 +0200 Subject: [PATCH 1/7] gh-104683: Add --exclude option to Argument Clinic CLI Explicitly exclude Lib/test/clinic.test.c when running make clinic. --- Doc/howto/clinic.rst | 5 +++++ Makefile.pre.in | 2 +- .../2023-08-08-12-21-41.gh-issue-104683.DRsAQE.rst | 1 + Tools/clinic/clinic.py | 5 +++++ 4 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Tools-Demos/2023-08-08-12-21-41.gh-issue-104683.DRsAQE.rst diff --git a/Doc/howto/clinic.rst b/Doc/howto/clinic.rst index 743c7c9cb3000e..47ebcbbd7d5a85 100644 --- a/Doc/howto/clinic.rst +++ b/Doc/howto/clinic.rst @@ -188,6 +188,11 @@ The CLI supports the following options: The directory tree to walk in :option:`--make` mode. +.. option:: --exclude FILES + + Comma-separated list of files to exclude in :option:`--make` mode. + This option has no effect unless :option:`--make` is also given. + .. option:: FILE ... The list of files to process. diff --git a/Makefile.pre.in b/Makefile.pre.in index 12409774746a30..2a626504c7aef9 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -775,7 +775,7 @@ coverage-report: regen-token regen-frozen # Run "Argument Clinic" over all source files .PHONY: clinic clinic: check-clean-src $(srcdir)/Modules/_blake2/blake2s_impl.c - $(PYTHON_FOR_REGEN) $(srcdir)/Tools/clinic/clinic.py --make --srcdir $(srcdir) + $(PYTHON_FOR_REGEN) $(srcdir)/Tools/clinic/clinic.py --make --exclude Lib/test/clinic.test.c --srcdir $(srcdir) $(PYTHON_FOR_REGEN) $(srcdir)/Tools/build/generate_global_objects.py # Build the interpreter diff --git a/Misc/NEWS.d/next/Tools-Demos/2023-08-08-12-21-41.gh-issue-104683.DRsAQE.rst b/Misc/NEWS.d/next/Tools-Demos/2023-08-08-12-21-41.gh-issue-104683.DRsAQE.rst new file mode 100644 index 00000000000000..ee3a70967b098b --- /dev/null +++ b/Misc/NEWS.d/next/Tools-Demos/2023-08-08-12-21-41.gh-issue-104683.DRsAQE.rst @@ -0,0 +1 @@ +Add ``--exclude`` option to Argument Clinic CLI. diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 82e5b804c2e3ea..1b4afa250a75e0 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5832,6 +5832,8 @@ def create_cli() -> argparse.ArgumentParser: help="walk --srcdir to run over all relevant files") cmdline.add_argument("--srcdir", type=str, default=os.curdir, help="the directory tree to walk in --make mode") + cmdline.add_argument("--exclude", type=str, + help="a list of files to exclude --make mode") cmdline.add_argument("filename", metavar="FILE", type=str, nargs="*", help="the list of files to process") return cmdline @@ -5903,6 +5905,7 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: parser.error("can't use -o or filenames with --make") if not ns.srcdir: parser.error("--srcdir must not be empty with --make") + excludes = [os.path.join(ns.srcdir, f) for f in ns.exclude.split(",")] for root, dirs, files in os.walk(ns.srcdir): for rcs_dir in ('.svn', '.git', '.hg', 'build', 'externals'): if rcs_dir in dirs: @@ -5912,6 +5915,8 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: if not filename.endswith(('.c', '.cpp', '.h')): continue path = os.path.join(root, filename) + if path in excludes: + continue if ns.verbose: print(path) parse_file(path, verify=not ns.force) From 3337074b65f4bafd8031fc16b305bd7bf132deca Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 8 Aug 2023 12:25:37 +0200 Subject: [PATCH 2/7] Wording --- Doc/howto/clinic.rst | 2 +- Tools/clinic/clinic.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Doc/howto/clinic.rst b/Doc/howto/clinic.rst index 47ebcbbd7d5a85..0a576e9f0bcde5 100644 --- a/Doc/howto/clinic.rst +++ b/Doc/howto/clinic.rst @@ -188,7 +188,7 @@ The CLI supports the following options: The directory tree to walk in :option:`--make` mode. -.. option:: --exclude FILES +.. option:: --exclude EXCLUDES Comma-separated list of files to exclude in :option:`--make` mode. This option has no effect unless :option:`--make` is also given. diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 1b4afa250a75e0..233d69b4355cfc 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5832,8 +5832,8 @@ def create_cli() -> argparse.ArgumentParser: help="walk --srcdir to run over all relevant files") cmdline.add_argument("--srcdir", type=str, default=os.curdir, help="the directory tree to walk in --make mode") - cmdline.add_argument("--exclude", type=str, - help="a list of files to exclude --make mode") + cmdline.add_argument("--exclude", type=str, metavar="EXCLUDES", + help="a comma-separated list of files to exclude --make mode") cmdline.add_argument("filename", metavar="FILE", type=str, nargs="*", help="the list of files to process") return cmdline From 14fcaf080a678b025d34b3d71a27b4862a315849 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 8 Aug 2023 12:26:39 +0200 Subject: [PATCH 3/7] Wording --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 233d69b4355cfc..6048563edca3cc 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5833,7 +5833,7 @@ def create_cli() -> argparse.ArgumentParser: cmdline.add_argument("--srcdir", type=str, default=os.curdir, help="the directory tree to walk in --make mode") cmdline.add_argument("--exclude", type=str, metavar="EXCLUDES", - help="a comma-separated list of files to exclude --make mode") + help="a comma-separated list of files to exclude in --make mode") cmdline.add_argument("filename", metavar="FILE", type=str, nargs="*", help="the list of files to process") return cmdline From 59908be840ba38c30f8846c89969bfecafd32b52 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 8 Aug 2023 12:42:10 +0200 Subject: [PATCH 4/7] Default "" --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 6048563edca3cc..c5585cb8a398b9 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5832,7 +5832,7 @@ def create_cli() -> argparse.ArgumentParser: help="walk --srcdir to run over all relevant files") cmdline.add_argument("--srcdir", type=str, default=os.curdir, help="the directory tree to walk in --make mode") - cmdline.add_argument("--exclude", type=str, metavar="EXCLUDES", + cmdline.add_argument("--exclude", type=str, default="", metavar="EXCLUDES", help="a comma-separated list of files to exclude in --make mode") cmdline.add_argument("filename", metavar="FILE", type=str, nargs="*", help="the list of files to process") From a2787bce8dec10484cf441a9065f1c977908e00a Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 8 Aug 2023 12:59:05 +0200 Subject: [PATCH 5/7] Normalise paths, provide better UX, and add clinic-tests make target --- Doc/howto/clinic.rst | 6 +++--- Makefile.pre.in | 4 ++++ Tools/clinic/clinic.py | 13 ++++++++++--- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/Doc/howto/clinic.rst b/Doc/howto/clinic.rst index 0a576e9f0bcde5..2d89ccc203b6d6 100644 --- a/Doc/howto/clinic.rst +++ b/Doc/howto/clinic.rst @@ -188,10 +188,10 @@ The CLI supports the following options: The directory tree to walk in :option:`--make` mode. -.. option:: --exclude EXCLUDES +.. option:: --exclude EXCLUDE - Comma-separated list of files to exclude in :option:`--make` mode. - This option has no effect unless :option:`--make` is also given. + A file to exclude in :option:`--make` mode. + This option can be given multiple times. .. option:: FILE ... diff --git a/Makefile.pre.in b/Makefile.pre.in index 2a626504c7aef9..d8fdb34747011d 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -778,6 +778,10 @@ clinic: check-clean-src $(srcdir)/Modules/_blake2/blake2s_impl.c $(PYTHON_FOR_REGEN) $(srcdir)/Tools/clinic/clinic.py --make --exclude Lib/test/clinic.test.c --srcdir $(srcdir) $(PYTHON_FOR_REGEN) $(srcdir)/Tools/build/generate_global_objects.py +.PHONY: clinic-tests +clinic-tests: check-clean-src $(srcdir)/Lib/test/clinic.test.c + $(PYTHON_FOR_REGEN) $(srcdir)/Tools/clinic/clinic.py -f $(srcdir)/Lib/test/clinic.test.c + # Build the interpreter $(BUILDPYTHON): Programs/python.o $(LINK_PYTHON_DEPS) $(LINKCC) $(PY_CORE_LDFLAGS) $(LINKFORSHARED) -o $@ Programs/python.o $(LINK_PYTHON_OBJS) $(LIBS) $(MODLIBS) $(SYSLIBS) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index c5585cb8a398b9..bb7764d49e63d1 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5832,8 +5832,9 @@ def create_cli() -> argparse.ArgumentParser: help="walk --srcdir to run over all relevant files") cmdline.add_argument("--srcdir", type=str, default=os.curdir, help="the directory tree to walk in --make mode") - cmdline.add_argument("--exclude", type=str, default="", metavar="EXCLUDES", - help="a comma-separated list of files to exclude in --make mode") + cmdline.add_argument("--exclude", type=str, action="append", + help=("a file to exclude in --make mode; " + "can be given multiple times")) cmdline.add_argument("filename", metavar="FILE", type=str, nargs="*", help="the list of files to process") return cmdline @@ -5905,7 +5906,11 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: parser.error("can't use -o or filenames with --make") if not ns.srcdir: parser.error("--srcdir must not be empty with --make") - excludes = [os.path.join(ns.srcdir, f) for f in ns.exclude.split(",")] + if ns.exclude: + excludes = [os.path.join(ns.srcdir, f) for f in ns.exclude] + excludes = [os.path.normpath(f) for f in excludes] + else: + excludes = [] for root, dirs, files in os.walk(ns.srcdir): for rcs_dir in ('.svn', '.git', '.hg', 'build', 'externals'): if rcs_dir in dirs: @@ -5915,7 +5920,9 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: if not filename.endswith(('.c', '.cpp', '.h')): continue path = os.path.join(root, filename) + path = os.path.normpath(path) if path in excludes: + print("Excluding", path) continue if ns.verbose: print(path) From 352f30d89faa95cb314e599d10a8fa984b73ca62 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 8 Aug 2023 13:26:17 +0200 Subject: [PATCH 6/7] Add test --- Lib/test/test_clinic.py | 26 ++++++++++++++++++++++++++ Tools/clinic/clinic.py | 1 - 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index d13d8623f8093b..14e6bdeefa47e3 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -2136,6 +2136,32 @@ def create_files(files, srcdir, code): path = os.path.join(ext_path, filename) self.assertNotIn(path, out) + def test_cli_make_exclude(self): + code = dedent(""" + /*[clinic input] + [clinic start generated code]*/ + """) + with os_helper.temp_dir() as tmp_dir: + # add some folders, some C files and a Python file + for fn in "file1.c", "file2.c", "file3.c": + path = os.path.join(tmp_dir, fn) + with open(path, "w", encoding="utf-8") as f: + f.write(code) + + # Run clinic in verbose mode with --make on tmpdir. + # Exclude file2.c and file3.c. + out = self.expect_success( + "-v", "--make", "--srcdir", tmp_dir, + "--exclude", os.path.join(tmp_dir, "file2.c"), + # The added ./ should be normalised away. + "--exclude", os.path.join("./", tmp_dir, "file3.c"), + ) + + # expect verbose mode to only mention the C files in tmp_dir + self.assertIn("file1.c", out) + self.assertNotIn("file2.c", out) + self.assertNotIn("file3.c", out) + def test_cli_verbose(self): with os_helper.temp_dir() as tmp_dir: fn = os.path.join(tmp_dir, "test.c") diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index bb7764d49e63d1..b8e98f2652930d 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5922,7 +5922,6 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: path = os.path.join(root, filename) path = os.path.normpath(path) if path in excludes: - print("Excluding", path) continue if ns.verbose: print(path) From 8d837c04e7a6aa2f2c7e764fd58183d2e70ee6e0 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 8 Aug 2023 22:20:26 +0200 Subject: [PATCH 7/7] Address review --- Lib/test/test_clinic.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 46e1f2dfdc1d2b..8ed7e214742d50 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -2221,9 +2221,9 @@ def test_cli_make_exclude(self): /*[clinic input] [clinic start generated code]*/ """) - with os_helper.temp_dir() as tmp_dir: + with os_helper.temp_dir(quiet=False) as tmp_dir: # add some folders, some C files and a Python file - for fn in "file1.c", "file2.c", "file3.c": + for fn in "file1.c", "file2.c", "file3.c", "file4.c": path = os.path.join(tmp_dir, fn) with open(path, "w", encoding="utf-8") as f: f.write(code) @@ -2234,13 +2234,16 @@ def test_cli_make_exclude(self): "-v", "--make", "--srcdir", tmp_dir, "--exclude", os.path.join(tmp_dir, "file2.c"), # The added ./ should be normalised away. - "--exclude", os.path.join("./", tmp_dir, "file3.c"), + "--exclude", os.path.join(tmp_dir, "./file3.c"), + # Relative paths should also work. + "--exclude", "file4.c" ) # expect verbose mode to only mention the C files in tmp_dir self.assertIn("file1.c", out) self.assertNotIn("file2.c", out) self.assertNotIn("file3.c", out) + self.assertNotIn("file4.c", out) def test_cli_verbose(self): with os_helper.temp_dir() as tmp_dir: