bpo-45020: Fix some corner cases for frozen module generation. (gh-28538)

This also includes some cleanup in preparation for a PR to make the "make all" output less noisy.

https://bugs.python.org/issue45020

5 files changed, 130 insertions, 69 deletions

diff --git a/Makefile.pre.in b/Makefile.pre.in
index 9ea359b70a..5564c1bae1 100644
--- a/Makefile.pre.in
+++ b/Makefile.pre.in
@@ -736,25 +736,54 @@ Programs/_testembed: Programs/_testembed.o $(LIBRARY_DEPS)
 ############################################################################
 # frozen modules (including importlib)
 
+# FROZEN_FILES_* are auto-generated by Tools/scripts/freeze_modules.py.
+FROZEN_FILES_IN = \
+		Lib/importlib/_bootstrap.py \
+		Lib/importlib/_bootstrap_external.py \
+		Lib/zipimport.py \
+		Lib/abc.py \
+		Lib/codecs.py \
+		Lib/io.py \
+		Lib/_collections_abc.py \
+		Lib/_sitebuiltins.py \
+		Lib/genericpath.py \
+		Lib/ntpath.py \
+		Lib/posixpath.py \
+		Lib/os.py \
+		Lib/site.py \
+		Lib/stat.py \
+		Lib/__hello__.py
+# End FROZEN_FILES_IN
+FROZEN_FILES_OUT = \
+		Python/frozen_modules/importlib._bootstrap.h \
+		Python/frozen_modules/importlib._bootstrap_external.h \
+		Python/frozen_modules/zipimport.h \
+		Python/frozen_modules/abc.h \
+		Python/frozen_modules/codecs.h \
+		Python/frozen_modules/io.h \
+		Python/frozen_modules/_collections_abc.h \
+		Python/frozen_modules/_sitebuiltins.h \
+		Python/frozen_modules/genericpath.h \
+		Python/frozen_modules/ntpath.h \
+		Python/frozen_modules/posixpath.h \
+		Python/frozen_modules/os.h \
+		Python/frozen_modules/site.h \
+		Python/frozen_modules/stat.h \
+		Python/frozen_modules/__hello__.h
+# End FROZEN_FILES_OUT
+
 Programs/_freeze_module.o: Programs/_freeze_module.c Makefile
 
 Programs/_freeze_module: Programs/_freeze_module.o $(LIBRARY_OBJS_OMIT_FROZEN)
 	$(LINKCC) $(PY_CORE_LDFLAGS) -o $@ Programs/_freeze_module.o $(LIBRARY_OBJS_OMIT_FROZEN) $(LIBS) $(MODLIBS) $(SYSLIBS)
 
-Tools/scripts/freeze_modules.py: Programs/_freeze_module
-
-.PHONY: regen-frozen
-regen-frozen: Tools/scripts/freeze_modules.py $(FROZEN_FILES)
-	$(PYTHON_FOR_REGEN) $(srcdir)/Tools/scripts/freeze_modules.py
-	@echo "The Makefile was updated, you may need to re-run make."
-
 # BEGIN: freezing modules
 
-Python/frozen_modules/importlib__bootstrap.h: Programs/_freeze_module Lib/importlib/_bootstrap.py
-	Programs/_freeze_module importlib._bootstrap $(srcdir)/Lib/importlib/_bootstrap.py $(srcdir)/Python/frozen_modules/importlib__bootstrap.h
+Python/frozen_modules/importlib._bootstrap.h: Programs/_freeze_module Lib/importlib/_bootstrap.py
+	Programs/_freeze_module importlib._bootstrap $(srcdir)/Lib/importlib/_bootstrap.py $(srcdir)/Python/frozen_modules/importlib._bootstrap.h
 
-Python/frozen_modules/importlib__bootstrap_external.h: Programs/_freeze_module Lib/importlib/_bootstrap_external.py
-	Programs/_freeze_module importlib._bootstrap_external $(srcdir)/Lib/importlib/_bootstrap_external.py $(srcdir)/Python/frozen_modules/importlib__bootstrap_external.h
+Python/frozen_modules/importlib._bootstrap_external.h: Programs/_freeze_module Lib/importlib/_bootstrap_external.py
+	Programs/_freeze_module importlib._bootstrap_external $(srcdir)/Lib/importlib/_bootstrap_external.py $(srcdir)/Python/frozen_modules/importlib._bootstrap_external.h
 
 Python/frozen_modules/zipimport.h: Programs/_freeze_module Lib/zipimport.py
 	Programs/_freeze_module zipimport $(srcdir)/Lib/zipimport.py $(srcdir)/Python/frozen_modules/zipimport.h
@@ -797,6 +826,13 @@ Python/frozen_modules/__hello__.h: Programs/_freeze_module Lib/__hello__.py
 
 # END: freezing modules
 
+Tools/scripts/freeze_modules.py: Programs/_freeze_module
+
+.PHONY: regen-frozen
+regen-frozen: Tools/scripts/freeze_modules.py $(FROZEN_FILES_IN)
+	$(PYTHON_FOR_REGEN) $(srcdir)/Tools/scripts/freeze_modules.py
+	@echo "The Makefile was updated, you may need to re-run make."
+
 # We keep this renamed target around for folks with muscle memory.
 .PHONY: regen-importlib
 regen-importlib: regen-frozen
@@ -1026,26 +1062,7 @@ regen-opcode-targets:
 Python/ceval.o: $(srcdir)/Python/opcode_targets.h $(srcdir)/Python/ceval_gil.h \
 		$(srcdir)/Python/condvar.h
 
-# FROZEN_FILES is auto-generated by Tools/scripts/freeze_modules.py.
-FROZEN_FILES = \
-		Python/frozen_modules/importlib__bootstrap.h \
-		Python/frozen_modules/importlib__bootstrap_external.h \
-		Python/frozen_modules/zipimport.h \
-		Python/frozen_modules/abc.h \
-		Python/frozen_modules/codecs.h \
-		Python/frozen_modules/io.h \
-		Python/frozen_modules/_collections_abc.h \
-		Python/frozen_modules/_sitebuiltins.h \
-		Python/frozen_modules/genericpath.h \
-		Python/frozen_modules/ntpath.h \
-		Python/frozen_modules/posixpath.h \
-		Python/frozen_modules/os.h \
-		Python/frozen_modules/site.h \
-		Python/frozen_modules/stat.h \
-		Python/frozen_modules/__hello__.h
-# End FROZEN_FILES
-
-Python/frozen.o: $(FROZEN_FILES)
+Python/frozen.o: $(FROZEN_FILES_OUT)
 
 # Generate DTrace probe macros, then rename them (PYTHON_ -> PyDTrace_) to
 # follow our naming conventions. dtrace(1) uses the output filename to generate
diff --git a/PCbuild/_freeze_module.vcxproj b/PCbuild/_freeze_module.vcxproj
index 382351ce23..ea6532d10d 100644
--- a/PCbuild/_freeze_module.vcxproj
+++ b/PCbuild/_freeze_module.vcxproj
@@ -232,13 +232,13 @@
     <!-- BEGIN frozen modules -->
     <None Include="..\Lib\importlib\_bootstrap.py">
       <ModName>importlib._bootstrap</ModName>
-      <IntFile>$(IntDir)importlib__bootstrap.g.h</IntFile>
-      <OutFile>$(PySourcePath)Python\frozen_modules\importlib__bootstrap.h</OutFile>
+      <IntFile>$(IntDir)importlib._bootstrap.g.h</IntFile>
+      <OutFile>$(PySourcePath)Python\frozen_modules\importlib._bootstrap.h</OutFile>
     </None>
     <None Include="..\Lib\importlib\_bootstrap_external.py">
       <ModName>importlib._bootstrap_external</ModName>
-      <IntFile>$(IntDir)importlib__bootstrap_external.g.h</IntFile>
-      <OutFile>$(PySourcePath)Python\frozen_modules\importlib__bootstrap_external.h</OutFile>
+      <IntFile>$(IntDir)importlib._bootstrap_external.g.h</IntFile>
+      <OutFile>$(PySourcePath)Python\frozen_modules\importlib._bootstrap_external.h</OutFile>
     </None>
     <None Include="..\Lib\zipimport.py">
       <ModName>zipimport</ModName>
diff --git a/Python/frozen.c b/Python/frozen.c
index 05b5281cce..f9ad07c0b4 100644
--- a/Python/frozen.c
+++ b/Python/frozen.c
@@ -38,8 +38,8 @@
 #include "Python.h"
 
 /* Includes for frozen modules: */
-#include "frozen_modules/importlib__bootstrap.h"
-#include "frozen_modules/importlib__bootstrap_external.h"
+#include "frozen_modules/importlib._bootstrap.h"
+#include "frozen_modules/importlib._bootstrap_external.h"
 #include "frozen_modules/zipimport.h"
 #include "frozen_modules/abc.h"
 #include "frozen_modules/codecs.h"
diff --git a/Tools/scripts/freeze_modules.py b/Tools/scripts/freeze_modules.py
index b3ae5c72a9..aa799d763a 100644
--- a/Tools/scripts/freeze_modules.py
+++ b/Tools/scripts/freeze_modules.py
@@ -11,8 +11,9 @@ import posixpath
 import subprocess
 import sys
 import textwrap
+import time
 
-from update_file import updating_file_with_tmpfile
+from update_file import updating_file_with_tmpfile, update_file_with_tmpfile
 
 
 ROOT_DIR = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
@@ -272,7 +273,7 @@ def resolve_frozen_file(frozenid, destdir=MODULES_DIR):
         except AttributeError:
             raise ValueError(f'unsupported frozenid {frozenid!r}')
     # We use a consistent naming convention for all frozen modules.
-    frozenfile = frozenid.replace('.', '_') + '.h'
+    frozenfile = f'{frozenid}.h'
     if not destdir:
         return frozenfile
     return os.path.join(destdir, frozenfile)
@@ -542,6 +543,7 @@ def regen_frozen(modules):
 
 
 def regen_makefile(modules):
+    pyfiles = []
     frozenfiles = []
     rules = ['']
     for src in _iter_sources(modules):
@@ -549,14 +551,16 @@ def regen_makefile(modules):
         frozenfiles.append(f'\t\t{header} \\')
 
         pyfile = relpath_for_posix_display(src.pyfile, ROOT_DIR)
-        # Note that we freeze the module to the target .h file
-        # instead of going through an intermediate file like we used to.
-        rules.append(f'{header}: Programs/_freeze_module {pyfile}')
-        rules.append(
-            (f'\tPrograms/_freeze_module {src.frozenid} '
-             f'$(srcdir)/{pyfile} $(srcdir)/{header}'))
-        rules.append('')
-
+        pyfiles.append(f'\t\t{pyfile} \\')
+
+        freeze = (f'Programs/_freeze_module {src.frozenid} '
+                  f'$(srcdir)/{pyfile} $(srcdir)/{header}')
+        rules.extend([
+            f'{header}: Programs/_freeze_module {pyfile}',
+            f'\t{freeze}',
+            '',
+        ])
+    pyfiles[-1] = pyfiles[-1].rstrip(" \\")
     frozenfiles[-1] = frozenfiles[-1].rstrip(" \\")
 
     print(f'# Updating {os.path.relpath(MAKEFILE)}')
@@ -564,8 +568,15 @@ def regen_makefile(modules):
         lines = infile.readlines()
         lines = replace_block(
             lines,
-            "FROZEN_FILES =",
-            "# End FROZEN_FILES",
+            "FROZEN_FILES_IN =",
+            "# End FROZEN_FILES_IN",
+            pyfiles,
+            MAKEFILE,
+        )
+        lines = replace_block(
+            lines,
+            "FROZEN_FILES_OUT =",
+            "# End FROZEN_FILES_OUT",
             frozenfiles,
             MAKEFILE,
         )
@@ -625,13 +636,15 @@ def regen_pcbuild(modules):
 
 def freeze_module(modname, pyfile=None, destdir=MODULES_DIR):
     """Generate the frozen module .h file for the given module."""
+    tmpsuffix = f'.{int(time.time())}'
     for modname, pyfile, ispkg in resolve_modules(modname, pyfile):
         frozenfile = resolve_frozen_file(modname, destdir)
-        _freeze_module(modname, pyfile, frozenfile)
+        _freeze_module(modname, pyfile, frozenfile, tmpsuffix)
 
 
-def _freeze_module(frozenid, pyfile, frozenfile):
-    tmpfile = frozenfile + '.new'
+def _freeze_module(frozenid, pyfile, frozenfile, tmpsuffix):
+    tmpfile = f'{frozenfile}.{int(time.time())}'
+    print(tmpfile)
 
     argv = [TOOL, frozenid, pyfile, tmpfile]
     print('#', '  '.join(os.path.relpath(a) for a in argv), flush=True)
@@ -642,7 +655,7 @@ def _freeze_module(frozenid, pyfile, frozenfile):
             sys.exit(f'ERROR: missing {TOOL}; you need to run "make regen-frozen"')
         raise  # re-raise
 
-    os.replace(tmpfile, frozenfile)
+    update_file_with_tmpfile(frozenfile, tmpfile, create=True)
 
 
 #######################################
@@ -652,15 +665,18 @@ def main():
     # Expand the raw specs, preserving order.
     modules = list(parse_frozen_specs(destdir=MODULES_DIR))
 
+    # Regen build-related files.
+    regen_makefile(modules)
+    regen_pcbuild(modules)
+
     # Freeze the target modules.
+    tmpsuffix = f'.{int(time.time())}'
     for src in _iter_sources(modules):
-        _freeze_module(src.frozenid, src.pyfile, src.frozenfile)
+        _freeze_module(src.frozenid, src.pyfile, src.frozenfile, tmpsuffix)
 
-    # Regen build-related files.
-    regen_manifest(modules)
+    # Regen files dependent of frozen file details.
     regen_frozen(modules)
-    regen_makefile(modules)
-    regen_pcbuild(modules)
+    regen_manifest(modules)
 
 
 if __name__ == '__main__':
diff --git a/Tools/scripts/update_file.py b/Tools/scripts/update_file.py
index 5e22486ec0..b4182c1d0c 100644
--- a/Tools/scripts/update_file.py
+++ b/Tools/scripts/update_file.py
@@ -46,19 +46,47 @@ def updating_file_with_tmpfile(filename, tmpfile=None):
     update_file_with_tmpfile(filename, tmpfile)
 
 
-def update_file_with_tmpfile(filename, tmpfile):
-    with open(filename, 'rb') as f:
-        old_contents = f.read()
-    with open(tmpfile, 'rb') as f:
-        new_contents = f.read()
-    if old_contents != new_contents:
+def update_file_with_tmpfile(filename, tmpfile, *, create=False):
+    try:
+        targetfile = open(filename, 'rb')
+    except FileNotFoundError:
+        if not create:
+            raise  # re-raise
+        outcome = 'created'
         os.replace(tmpfile, filename)
     else:
-        os.unlink(tmpfile)
+        with targetfile:
+            old_contents = targetfile.read()
+        with open(tmpfile, 'rb') as f:
+            new_contents = f.read()
+        # Now compare!
+        if old_contents != new_contents:
+            outcome = 'updated'
+            os.replace(tmpfile, filename)
+        else:
+            outcome = 'same'
+            os.unlink(tmpfile)
+    return outcome
 
 
 if __name__ == '__main__':
-    if len(sys.argv) != 3:
-        print("Usage: %s <path to be updated> <path with new contents>" % (sys.argv[0],))
-        sys.exit(1)
-    update_file_with_tmpfile(sys.argv[1], sys.argv[2])
+    import argparse
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--create', action='store_true')
+    parser.add_argument('--exitcode', action='store_true')
+    parser.add_argument('filename', help='path to be updated')
+    parser.add_argument('tmpfile', help='path with new contents')
+    args = parser.parse_args()
+    kwargs = vars(args)
+    setexitcode = kwargs.pop('exitcode')
+
+    outcome = update_file_with_tmpfile(**kwargs)
+    if setexitcode:
+        if outcome == 'same':
+            sys.exit(0)
+        elif outcome == 'updated':
+            sys.exit(1)
+        elif outcome == 'created':
+            sys.exit(2)
+        else:
+            raise NotImplementedError