From 3afb66f665f813bbd59202f917a4f997576b1d7a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 1 Sep 2023 01:21:48 +0200 Subject: [PATCH 1/6] gh-108740: Fix "make regen-all" race condition Fix a race condition in "make regen-all". The deepfreeze.c source and files generated by Argument Clinic are now generated or updated before generating "global objects". Previously, some identifiers may miss depending on the order in which these files were generated. * "make regen-global-objects": Make sure that deepfreeze.c is generated and up to date, and always run "make clinic". * "make clinic" no longer runs generate_global_objects.py script anymore. * "make regen-deepfreeze" now only updates deepfreeze.c, it doesn't build deepfreeze.o anymore. * Remove misleading messages in "make regen-global-objects" and "make clinic". They are outdated, these commands are now safe to use. * Document generates files in Doc/using/configure.rst --- Doc/using/configure.rst | 23 +++++++++++++++++++ Makefile.pre.in | 17 +++++++------- ...-09-01-01-39-26.gh-issue-108740.JHExAQ.rst | 4 ++++ 3 files changed, 36 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Build/2023-09-01-01-39-26.gh-issue-108740.JHExAQ.rst diff --git a/Doc/using/configure.rst b/Doc/using/configure.rst index ad58255c33b33b..2007c7ddb6f730 100644 --- a/Doc/using/configure.rst +++ b/Doc/using/configure.rst @@ -60,6 +60,29 @@ See also :pep:`7` "Style Guide for C Code" and :pep:`11` "CPython platform support". +Generated files +=============== + +To reduce build dependencies, Python source code contains multiple generated +files. Commands to regenerate all generated files:: + + make regen-all + make regen-stdlib-module-names + make regen-limited-abi + make regen-configure + +The ``Makefile.pre.in`` documents generated files, their inputs, and tools used +to regenerate them. Search for ``regen-*`` make targets. + +The ``make regen-configure`` command runs `tiran/cpython_autoconf +`_ container for reproducible build; +see container ``entry.sh`` script. The container is optional, the following +command can be run locally, the generated files depend on autoconf and aclocal +versions:: + + autoreconf -ivf -Werror + + .. _configure-options: Configure Options diff --git a/Makefile.pre.in b/Makefile.pre.in index cf77cbd13ad4d9..d443348c6918d0 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -490,6 +490,7 @@ OBJECT_OBJS= \ Objects/weakrefobject.o \ @PERF_TRAMPOLINE_OBJ@ +DEEPFREEZE_C = Python/deepfreeze/deepfreeze.c DEEPFREEZE_OBJS = Python/deepfreeze/deepfreeze.o ########################################################################## @@ -777,7 +778,6 @@ coverage-report: regen-token regen-frozen .PHONY: clinic 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 @@ -1252,7 +1252,7 @@ regen-frozen: Tools/build/freeze_modules.py $(FROZEN_FILES_IN) # Deepfreeze targets .PHONY: regen-deepfreeze -regen-deepfreeze: $(DEEPFREEZE_OBJS) +regen-deepfreeze: $(DEEPFREEZE_C) DEEPFREEZE_DEPS=$(srcdir)/Tools/build/deepfreeze.py Include/internal/pycore_global_strings.h $(FREEZE_MODULE_DEPS) $(FROZEN_FILES_OUT) @@ -1284,8 +1284,6 @@ Python/deepfreeze/deepfreeze.c: $(DEEPFREEZE_DEPS) Python/frozen_modules/frozen_only.h:frozen_only \ -o Python/deepfreeze/deepfreeze.c # END: deepfreeze modules - @echo "Note: Deepfreeze may have added some global objects," - @echo " so run 'make regen-global-objects' if necessary." # We keep this renamed target around for folks with muscle memory. .PHONY: regen-importlib @@ -1295,10 +1293,11 @@ regen-importlib: regen-frozen # Global objects .PHONY: regen-global-objects -regen-global-objects: $(srcdir)/Tools/build/generate_global_objects.py +# Dependencies which can add and/or remove _Py_ID() identifiers: +# - deepfreeze.c +# - "make clinic" +regen-global-objects: $(srcdir)/Tools/build/generate_global_objects.py $(DEEPFREEZE_C) clinic $(PYTHON_FOR_REGEN) $(srcdir)/Tools/build/generate_global_objects.py - @echo "Note: Global objects can be added or removed by other tools (e.g. deepfreeze), " - @echo " so be sure to re-run regen-global-objects after those tools." ############################################################################ # ABI @@ -1321,8 +1320,9 @@ regen-limited-abi: all # Regenerate all generated files .PHONY: regen-all +# "regen-global-objects" runs "clinic". regen-all: regen-cases regen-typeslots \ - regen-token regen-ast regen-keyword regen-sre regen-frozen clinic \ + regen-token regen-ast regen-keyword regen-sre regen-frozen \ regen-pegen-metaparser regen-pegen regen-test-frozenmain \ regen-test-levenshtein regen-global-objects @echo @@ -2597,6 +2597,7 @@ recheck: autoconf: (cd $(srcdir); autoreconf -ivf -Werror) +# See https://github.com/tiran/cpython_autoconf container .PHONY: regen-configure regen-configure: @if command -v podman >/dev/null; then RUNTIME="podman"; else RUNTIME="docker"; fi; \ diff --git a/Misc/NEWS.d/next/Build/2023-09-01-01-39-26.gh-issue-108740.JHExAQ.rst b/Misc/NEWS.d/next/Build/2023-09-01-01-39-26.gh-issue-108740.JHExAQ.rst new file mode 100644 index 00000000000000..190d50387f339e --- /dev/null +++ b/Misc/NEWS.d/next/Build/2023-09-01-01-39-26.gh-issue-108740.JHExAQ.rst @@ -0,0 +1,4 @@ +Fix a race condition in ``make regen-all``. The ``deepfreeze.c`` source and +files generated by Argument Clinic are now generated or updated before +generating "global objects". Previously, some identifiers may miss depending +on the order in which these files were generated. Patch by Victor Stinner. From 6a909ddd77c99b40ac588256affefd2baaedbe00 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sat, 2 Sep 2023 20:37:26 +0200 Subject: [PATCH 2/6] Update Doc/using/configure.rst --- Doc/using/configure.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/using/configure.rst b/Doc/using/configure.rst index 2007c7ddb6f730..ba1898e35b5fbc 100644 --- a/Doc/using/configure.rst +++ b/Doc/using/configure.rst @@ -71,7 +71,7 @@ files. Commands to regenerate all generated files:: make regen-limited-abi make regen-configure -The ``Makefile.pre.in`` documents generated files, their inputs, and tools used +The ``Makefile.pre.in`` file documents generated files, their inputs, and tools used to regenerate them. Search for ``regen-*`` make targets. The ``make regen-configure`` command runs `tiran/cpython_autoconf From af96ee6541e9c017e453de9ac99f0479b9498e2d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 6 Sep 2023 09:57:11 +0200 Subject: [PATCH 3/6] Style - place coments above .PHONY targets - reword comment about implicit clinic regen --- Makefile.pre.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.pre.in b/Makefile.pre.in index d443348c6918d0..8ad9f1344a7ba1 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1292,10 +1292,10 @@ regen-importlib: regen-frozen ############################################################################ # Global objects -.PHONY: regen-global-objects # Dependencies which can add and/or remove _Py_ID() identifiers: # - deepfreeze.c # - "make clinic" +.PHONY: regen-global-objects regen-global-objects: $(srcdir)/Tools/build/generate_global_objects.py $(DEEPFREEZE_C) clinic $(PYTHON_FOR_REGEN) $(srcdir)/Tools/build/generate_global_objects.py @@ -1319,8 +1319,8 @@ regen-limited-abi: all ############################################################################ # Regenerate all generated files +# "clinic" is regenerated implicitly via "regen-global-objects". .PHONY: regen-all -# "regen-global-objects" runs "clinic". regen-all: regen-cases regen-typeslots \ regen-token regen-ast regen-keyword regen-sre regen-frozen \ regen-pegen-metaparser regen-pegen regen-test-frozenmain \ From a760e70fefebac28021e75b20d6ca5c698b7ee87 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 6 Sep 2023 10:09:08 +0200 Subject: [PATCH 4/6] Use DEEPFREEZE_C in target rule as well --- Makefile.pre.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.pre.in b/Makefile.pre.in index 8ad9f1344a7ba1..aa3eaabc7559f6 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1257,7 +1257,7 @@ regen-deepfreeze: $(DEEPFREEZE_C) DEEPFREEZE_DEPS=$(srcdir)/Tools/build/deepfreeze.py Include/internal/pycore_global_strings.h $(FREEZE_MODULE_DEPS) $(FROZEN_FILES_OUT) # BEGIN: deepfreeze modules -Python/deepfreeze/deepfreeze.c: $(DEEPFREEZE_DEPS) +$(DEEPFREEZE_C): $(DEEPFREEZE_DEPS) $(PYTHON_FOR_FREEZE) $(srcdir)/Tools/build/deepfreeze.py \ Python/frozen_modules/importlib._bootstrap.h:importlib._bootstrap \ Python/frozen_modules/importlib._bootstrap_external.h:importlib._bootstrap_external \ From 418d72e970774b6676d0fc8691e83297af35b6f9 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 6 Sep 2023 11:04:55 +0200 Subject: [PATCH 5/6] Revert "Use DEEPFREEZE_C in target rule as well" This reverts commit 4803f4e9a9adf17d08701fa44218fce0af70b058. We must keep the previous rule because of the Makefile patching (!) that the generate-* scripts does. --- Makefile.pre.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.pre.in b/Makefile.pre.in index aa3eaabc7559f6..8ad9f1344a7ba1 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1257,7 +1257,7 @@ regen-deepfreeze: $(DEEPFREEZE_C) DEEPFREEZE_DEPS=$(srcdir)/Tools/build/deepfreeze.py Include/internal/pycore_global_strings.h $(FREEZE_MODULE_DEPS) $(FROZEN_FILES_OUT) # BEGIN: deepfreeze modules -$(DEEPFREEZE_C): $(DEEPFREEZE_DEPS) +Python/deepfreeze/deepfreeze.c: $(DEEPFREEZE_DEPS) $(PYTHON_FOR_FREEZE) $(srcdir)/Tools/build/deepfreeze.py \ Python/frozen_modules/importlib._bootstrap.h:importlib._bootstrap \ Python/frozen_modules/importlib._bootstrap_external.h:importlib._bootstrap_external \ From 66e8d7013c39111118ad0e3c8a75202d38e8e8a2 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 6 Sep 2023 18:54:16 +0200 Subject: [PATCH 6/6] Use $(DEEPFREEZE_C) --- Makefile.pre.in | 2 +- Tools/build/freeze_modules.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.pre.in b/Makefile.pre.in index 8ad9f1344a7ba1..aa3eaabc7559f6 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1257,7 +1257,7 @@ regen-deepfreeze: $(DEEPFREEZE_C) DEEPFREEZE_DEPS=$(srcdir)/Tools/build/deepfreeze.py Include/internal/pycore_global_strings.h $(FREEZE_MODULE_DEPS) $(FROZEN_FILES_OUT) # BEGIN: deepfreeze modules -Python/deepfreeze/deepfreeze.c: $(DEEPFREEZE_DEPS) +$(DEEPFREEZE_C): $(DEEPFREEZE_DEPS) $(PYTHON_FOR_FREEZE) $(srcdir)/Tools/build/deepfreeze.py \ Python/frozen_modules/importlib._bootstrap.h:importlib._bootstrap \ Python/frozen_modules/importlib._bootstrap_external.h:importlib._bootstrap_external \ diff --git a/Tools/build/freeze_modules.py b/Tools/build/freeze_modules.py index ee4dd2f8682ba4..12200979fa4bc7 100644 --- a/Tools/build/freeze_modules.py +++ b/Tools/build/freeze_modules.py @@ -585,7 +585,7 @@ def regen_makefile(modules): pyfiles = [] frozenfiles = [] rules = [''] - deepfreezerules = ["Python/deepfreeze/deepfreeze.c: $(DEEPFREEZE_DEPS)", + deepfreezerules = ["$(DEEPFREEZE_C): $(DEEPFREEZE_DEPS)", "\t$(PYTHON_FOR_FREEZE) $(srcdir)/Tools/build/deepfreeze.py \\"] for src in _iter_sources(modules): frozen_header = relpath_for_posix_display(src.frozenfile, ROOT_DIR)