py: Change makemoduledefs process so it uses output of qstr extraction.

This cleans up the parsing of MP_REGISTER_MODULE() and generation of genhdr/moduledefs.h so that it uses the same process as compressed error string messages, using the output of qstr extraction. This makes sure all MP_REGISTER_MODULE()'s that are part of the build are correctly picked up. Previously the extraction would miss some (eg if you had a mod.c file in the board directory for an stm32 board). Build speed is more or less unchanged. Thanks to @stinos for the ports/windows/msvc/genhdr.targets changes. Signed-off-by: Damien George <damien@micropython.org>
micropython · Jun 2, 2022 · 47f6343 · 47f6343
1 parent 340872c
commit 47f6343
Show file tree

Hide file tree

Showing 9 changed files with 81 additions and 66 deletions.
diff --git a/ports/cc3200/bootmgr/bootloader.mk b/ports/cc3200/bootmgr/bootloader.mk
@@ -130,3 +130,7 @@ $(HEADER_BUILD)/qstrdefs.generated.h: | $(HEADER_BUILD)
 # Create an empty "mpversion.h" needed by py/mkrules.mk
 $(HEADER_BUILD)/mpversion.h: | $(HEADER_BUILD)
 	touch $@
+
+# Create an empty "moduledefs.h" needed by py/mkrules.mk
+$(HEADER_BUILD)/moduledefs.h: | $(HEADER_BUILD)
+	touch $@
diff --git a/ports/windows/msvc/genhdr.targets b/ports/windows/msvc/genhdr.targets
@@ -14,6 +14,7 @@
     <PyQstrDefs>$(PySrcDir)qstrdefs.h</PyQstrDefs>
     <QstrDefsCollected>$(DestDir)qstrdefscollected.h</QstrDefsCollected>
     <QstrGen>$(DestDir)qstrdefs.generated.h</QstrGen>
+    <ModuleDefsCollected>$(DestDir)/moduledefs.collected</ModuleDefsCollected>
     <PyPython Condition="'$(PyPython)' == ''">$(MICROPY_CPYTHON3)</PyPython>
     <PyPython Condition="'$(PyPython)' == ''">python</PyPython>
     <CLToolExe Condition="'$(CLToolExe)' == ''">cl.exe</CLToolExe>
@@ -99,17 +100,17 @@ using(var outFile = System.IO.File.CreateText(OutputFile)) {
     <Exec Command="$(PyPython) $(PySrcDir)makeqstrdefs.py cat qstr _ $(DestDir)qstr $(QstrDefsCollected)"/>
   </Target>
 
-  <Target Name="MakeModuleDefs" DependsOnTargets="MakeDestDir">
+  <Target Name="CollectModuleDefs" DependsOnTargets="MakeQstrDefs" Inputs="$(DestDir)qstr.i.last" Outputs="$(ModuleDefsCollected)">
+    <Exec Command="$(PyPython) $(PySrcDir)makeqstrdefs.py split module $(DestDir)qstr.i.last $(DestDir)module _"/>
+    <Exec Command="$(PyPython) $(PySrcDir)makeqstrdefs.py cat module _ $(DestDir)module $(ModuleDefsCollected)"/>
+  </Target>
+
+  <Target Name="MakeModuleDefs" DependsOnTargets="CollectModuleDefs" Inputs="$(ModuleDefsCollected)" Outputs="$(DestDir)moduledefs.h">
     <PropertyGroup>
       <DestFile>$(DestDir)moduledefs.h</DestFile>
       <TmpFile>$(DestFile).tmp</TmpFile>
     </PropertyGroup>
-    <ItemGroup>
-      <PyUserModuleFiles Include="@(ClCompile)">
-        <Path>$([System.String]::new('%(FullPath)').Replace('$(PyBaseDir)', ''))</Path>
-      </PyUserModuleFiles>
-    </ItemGroup>
-    <Exec Command="$(PyPython) $(PySrcDir)makemoduledefs.py --vpath=&quot;., $(PyBaseDir), $(PyUserCModules)&quot; @(PyUserModuleFiles->'%(Path)', ' ') > $(TmpFile)"/>
+    <Exec Command="$(PyPython) $(PySrcDir)makemoduledefs.py $(ModuleDefsCollected) > $(TmpFile)"/>
     <MSBuild Projects="$(MSBuildThisFileFullPath)" Targets="CopyFileIfDifferent" Properties="SourceFile=$(TmpFile);DestFile=$(DestFile)"/>
   </Target>
 

diff --git a/py/makemoduledefs.py b/py/makemoduledefs.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 
-# This pre-processor parses provided objects' c files for
+# This pre-processor parses a single file containing a list of
 # MP_REGISTER_MODULE(module_name, obj_module, enabled_define)
 # These are used to generate a header with the required entries for
 # "mp_rom_map_elem_t mp_builtin_module_table[]" in py/objmodule.c
@@ -9,45 +9,21 @@
 
 import re
 import io
-import os
 import argparse
 
 
-pattern = re.compile(r"[\n;]\s*MP_REGISTER_MODULE\((.*?),\s*(.*?),\s*(.*?)\);", flags=re.DOTALL)
+pattern = re.compile(r"\s*MP_REGISTER_MODULE\((.*?),\s*(.*?),\s*(.*?)\);", flags=re.DOTALL)
 
 
-def find_c_file(obj_file, vpath):
-    """Search vpaths for the c file that matches the provided object_file.
+def find_module_registrations(filename):
+    """Find any MP_REGISTER_MODULE definitions in the provided file.
 
-    :param str obj_file: object file to find the matching c file for
-    :param List[str] vpath: List of base paths, similar to gcc vpath
-    :return: str path to c file or None
-    """
-    c_file = None
-    relative_c_file = os.path.splitext(obj_file)[0] + ".c"
-    relative_c_file = relative_c_file.lstrip("/\\")
-    for p in vpath:
-        possible_c_file = os.path.join(p, relative_c_file)
-        if os.path.exists(possible_c_file):
-            c_file = possible_c_file
-            break
-
-    return c_file
-
-
-def find_module_registrations(c_file):
-    """Find any MP_REGISTER_MODULE definitions in the provided c file.
-
-    :param str c_file: path to c file to check
+    :param str filename: path to file to check
     :return: List[(module_name, obj_module, enabled_define)]
     """
     global pattern
 
-    if c_file is None:
-        # No c file to match the object file, skip
-        return set()
-
-    with io.open(c_file, encoding="utf-8") as c_file_obj:
+    with io.open(filename, encoding="utf-8") as c_file_obj:
         return set(re.findall(pattern, c_file_obj.read()))
 
 
@@ -93,19 +69,10 @@ def generate_module_table_header(modules):
 
 def main():
     parser = argparse.ArgumentParser()
-    parser.add_argument(
-        "--vpath", default=".", help="comma separated list of folders to search for c files in"
-    )
-    parser.add_argument("files", nargs="*", help="list of c files to search")
+    parser.add_argument("file", nargs=1, help="file with MP_REGISTER_MODULE definitions")
     args = parser.parse_args()
 
-    vpath = [p.strip() for p in args.vpath.split(",")]
-
-    modules = set()
-    for obj_file in args.files:
-        c_file = find_c_file(obj_file, vpath)
-        modules |= find_module_registrations(c_file)
-
+    modules = find_module_registrations(args.file[0])
     generate_module_table_header(sorted(modules))
 
 

diff --git a/py/makeqstrdefs.py b/py/makeqstrdefs.py
@@ -21,6 +21,9 @@
 # Extract MP_COMPRESSED_ROM_TEXT("") macros.  (Which come from MP_ERROR_TEXT)
 _MODE_COMPRESS = "compress"
 
+# Extract MP_REGISTER_MODULE(...) macros.
+_MODE_MODULE = "module"
+
 
 def is_c_source(fname):
     return os.path.splitext(fname)[1] in [".c"]
@@ -85,6 +88,8 @@ def process_file(f):
         re_match = re.compile(r"MP_QSTR_[_a-zA-Z0-9]+")
     elif args.mode == _MODE_COMPRESS:
         re_match = re.compile(r'MP_COMPRESSED_ROM_TEXT\("([^"]*)"\)')
+    elif args.mode == _MODE_MODULE:
+        re_match = re.compile(r"MP_REGISTER_MODULE\(.*?,\s*.*?,\s*.*?\);")
     output = []
     last_fname = None
     for line in f:
@@ -106,7 +111,7 @@ def process_file(f):
             if args.mode == _MODE_QSTR:
                 name = match.replace("MP_QSTR_", "")
                 output.append("Q(" + name + ")")
-            elif args.mode == _MODE_COMPRESS:
+            elif args.mode in (_MODE_COMPRESS, _MODE_MODULE):
                 output.append(match)
 
     if last_fname:
@@ -141,6 +146,8 @@ def cat_together():
     mode_full = "QSTR"
     if args.mode == _MODE_COMPRESS:
         mode_full = "Compressed data"
+    elif args.mode == _MODE_MODULE:
+        mode_full = "Module registrations"
     if old_hash != new_hash:
         print(mode_full, "updated")
         try:
@@ -201,7 +208,7 @@ class Args:
     args.output_dir = sys.argv[4]
     args.output_file = None if len(sys.argv) == 5 else sys.argv[5]  # Unused for command=split
 
-    if args.mode not in (_MODE_QSTR, _MODE_COMPRESS):
+    if args.mode not in (_MODE_QSTR, _MODE_COMPRESS, _MODE_MODULE):
         print("error: mode %s unrecognised" % sys.argv[2])
         sys.exit(2)
 

diff --git a/py/mkrules.cmake b/py/mkrules.cmake
@@ -2,13 +2,15 @@
 
 set(MICROPY_GENHDR_DIR "${CMAKE_BINARY_DIR}/genhdr")
 set(MICROPY_MPVERSION "${MICROPY_GENHDR_DIR}/mpversion.h")
-set(MICROPY_MODULEDEFS "${MICROPY_GENHDR_DIR}/moduledefs.h")
 set(MICROPY_QSTRDEFS_PY "${MICROPY_PY_DIR}/qstrdefs.h")
 set(MICROPY_QSTRDEFS_LAST "${MICROPY_GENHDR_DIR}/qstr.i.last")
 set(MICROPY_QSTRDEFS_SPLIT "${MICROPY_GENHDR_DIR}/qstr.split")
 set(MICROPY_QSTRDEFS_COLLECTED "${MICROPY_GENHDR_DIR}/qstrdefs.collected.h")
 set(MICROPY_QSTRDEFS_PREPROCESSED "${MICROPY_GENHDR_DIR}/qstrdefs.preprocessed.h")
 set(MICROPY_QSTRDEFS_GENERATED "${MICROPY_GENHDR_DIR}/qstrdefs.generated.h")
+set(MICROPY_MODULEDEFS_SPLIT "${MICROPY_GENHDR_DIR}/moduledefs.split")
+set(MICROPY_MODULEDEFS_COLLECTED "${MICROPY_GENHDR_DIR}/moduledefs.collected")
+set(MICROPY_MODULEDEFS "${MICROPY_GENHDR_DIR}/moduledefs.h")
 
 # Need to do this before extracting MICROPY_CPP_DEF below. Rest of frozen
 # manifest handling is at the end of this file.
@@ -43,6 +45,7 @@ find_package(Python3 REQUIRED COMPONENTS Interpreter)
 target_sources(${MICROPY_TARGET} PRIVATE
     ${MICROPY_MPVERSION}
     ${MICROPY_QSTRDEFS_GENERATED}
+    ${MICROPY_MODULEDEFS}
 )
 
 # Command to force the build of another command
@@ -62,15 +65,6 @@ add_custom_command(
     DEPENDS MICROPY_FORCE_BUILD
 )
 
-# Generate moduledefs.h
-
-add_custom_command(
-    OUTPUT ${MICROPY_MODULEDEFS}
-    COMMAND ${Python3_EXECUTABLE} ${MICROPY_PY_DIR}/makemoduledefs.py --vpath="/" ${MICROPY_SOURCE_QSTR} > ${MICROPY_MODULEDEFS}
-    DEPENDS ${MICROPY_MPVERSION}
-        ${MICROPY_SOURCE_QSTR}
-)
-
 # Generate qstrs
 
 # If any of the dependencies in this rule change then the C-preprocessor step must be run.
@@ -79,7 +73,7 @@ add_custom_command(
 add_custom_command(
     OUTPUT ${MICROPY_QSTRDEFS_LAST}
     COMMAND ${Python3_EXECUTABLE} ${MICROPY_PY_DIR}/makeqstrdefs.py pp ${CMAKE_C_COMPILER} -E output ${MICROPY_GENHDR_DIR}/qstr.i.last cflags ${MICROPY_CPP_FLAGS} -DNO_QSTR cxxflags ${MICROPY_CPP_FLAGS} -DNO_QSTR sources ${MICROPY_SOURCE_QSTR}
-    DEPENDS ${MICROPY_MODULEDEFS}
+    DEPENDS ${MICROPY_MPVERSION}
         ${MICROPY_SOURCE_QSTR}
     VERBATIM
     COMMAND_EXPAND_LISTS
@@ -120,6 +114,31 @@ add_custom_command(
     COMMAND_EXPAND_LISTS
 )
 
+# Generate moduledefs.h
+
+add_custom_command(
+    OUTPUT ${MICROPY_MODULEDEFS_SPLIT}
+    COMMAND ${Python3_EXECUTABLE} ${MICROPY_PY_DIR}/makeqstrdefs.py split module ${MICROPY_GENHDR_DIR}/qstr.i.last ${MICROPY_GENHDR_DIR}/module _
+    COMMAND touch ${MICROPY_MODULEDEFS_SPLIT}
+    DEPENDS ${MICROPY_QSTRDEFS_LAST}
+    VERBATIM
+    COMMAND_EXPAND_LISTS
+)
+
+add_custom_command(
+    OUTPUT ${MICROPY_MODULEDEFS_COLLECTED}
+    COMMAND ${Python3_EXECUTABLE} ${MICROPY_PY_DIR}/makeqstrdefs.py cat module _ ${MICROPY_GENHDR_DIR}/module ${MICROPY_MODULEDEFS_COLLECTED}
+    DEPENDS ${MICROPY_MODULEDEFS_SPLIT}
+    VERBATIM
+    COMMAND_EXPAND_LISTS
+)
+
+add_custom_command(
+    OUTPUT ${MICROPY_MODULEDEFS}
+    COMMAND ${Python3_EXECUTABLE} ${MICROPY_PY_DIR}/makemoduledefs.py ${MICROPY_MODULEDEFS_COLLECTED} > ${MICROPY_MODULEDEFS}
+    DEPENDS ${MICROPY_MODULEDEFS_COLLECTED}
+)
+
 # Build frozen code if enabled
 
 if(MICROPY_FROZEN_MANIFEST)

diff --git a/py/mkrules.mk b/py/mkrules.mk
@@ -7,6 +7,9 @@ endif
 # Extra deps that need to happen before object compilation.
 OBJ_EXTRA_ORDER_DEPS =
 
+# Generate moduledefs.h.
+OBJ_EXTRA_ORDER_DEPS += $(HEADER_BUILD)/moduledefs.h
+
 ifeq ($(MICROPY_ROM_TEXT_COMPRESSION),1)
 # If compression is enabled, trigger the build of compressed.data.h...
 OBJ_EXTRA_ORDER_DEPS += $(HEADER_BUILD)/compressed.data.h
@@ -100,7 +103,7 @@ $(OBJ): | $(HEADER_BUILD)/qstrdefs.generated.h $(HEADER_BUILD)/mpversion.h $(OBJ
 # - else, if list of newer prerequisites ($?) is not empty, then process just these ($?)
 # - else, process all source files ($^) [this covers "make -B" which can set $? to empty]
 # See more information about this process in docs/develop/qstr.rst.
-$(HEADER_BUILD)/qstr.i.last: $(SRC_QSTR) $(QSTR_GLOBAL_DEPENDENCIES) $(HEADER_BUILD)/moduledefs.h | $(QSTR_GLOBAL_REQUIREMENTS)
+$(HEADER_BUILD)/qstr.i.last: $(SRC_QSTR) $(QSTR_GLOBAL_DEPENDENCIES) | $(QSTR_GLOBAL_REQUIREMENTS)
 	$(ECHO) "GEN $@"
 	$(Q)$(PYTHON) $(PY_SRC)/makeqstrdefs.py pp $(CPP) output $(HEADER_BUILD)/qstr.i.last cflags $(QSTR_GEN_CFLAGS) cxxflags $(QSTR_GEN_CXXFLAGS) sources $^ dependencies $(QSTR_GLOBAL_DEPENDENCIES) changed_sources $?
 
@@ -113,6 +116,16 @@ $(QSTR_DEFS_COLLECTED): $(HEADER_BUILD)/qstr.split
 	$(ECHO) "GEN $@"
 	$(Q)$(PYTHON) $(PY_SRC)/makeqstrdefs.py cat qstr _ $(HEADER_BUILD)/qstr $@
 
+# Module definitions via MP_REGISTER_MODULE.
+$(HEADER_BUILD)/moduledefs.split: $(HEADER_BUILD)/qstr.i.last
+	$(ECHO) "GEN $@"
+	$(Q)$(PYTHON) $(PY_SRC)/makeqstrdefs.py split module $< $(HEADER_BUILD)/module _
+	$(Q)$(TOUCH) $@
+
+$(HEADER_BUILD)/moduledefs.collected: $(HEADER_BUILD)/moduledefs.split
+	$(ECHO) "GEN $@"
+	$(Q)$(PYTHON) $(PY_SRC)/makeqstrdefs.py cat module _ $(HEADER_BUILD)/module $@
+
 # Compressed error strings.
 $(HEADER_BUILD)/compressed.split: $(HEADER_BUILD)/qstr.i.last
 	$(ECHO) "GEN $@"

diff --git a/py/obj.h b/py/obj.h
@@ -421,7 +421,9 @@ typedef struct _mp_rom_obj_t { mp_const_obj_t o; } mp_rom_obj_t;
 // param obj_module: mp_obj_module_t instance
 // prarm enabled_define: used as `#if (enabled_define) around entry`
 
+#ifndef NO_QSTR
 #define MP_REGISTER_MODULE(module_name, obj_module, enabled_define)
+#endif
 
 // Underlying map/hash table implementation (not dict object or map function)
 

diff --git a/py/objmodule.c b/py/objmodule.c
@@ -34,7 +34,11 @@
 #include "py/runtime.h"
 #include "py/builtin.h"
 
+#ifndef NO_QSTR
+// Only include module definitions when not doing qstr extraction, because the
+// qstr extraction stage also generates this module definition header file.
 #include "genhdr/moduledefs.h"
+#endif
 
 #if MICROPY_MODULE_BUILTIN_INIT
 STATIC void mp_module_call_init(mp_obj_t module_name, mp_obj_t module_obj);
@@ -161,10 +165,8 @@ mp_obj_t mp_obj_new_module(qstr module_name) {
 // Global module table and related functions
 
 STATIC const mp_rom_map_elem_t mp_builtin_module_table[] = {
-    #ifdef MICROPY_REGISTERED_MODULES
     // builtin modules declared with MP_REGISTER_MODULE()
     MICROPY_REGISTERED_MODULES
-    #endif
 };
 
 MP_DEFINE_CONST_MAP(mp_builtin_module_map, mp_builtin_module_table);

diff --git a/py/py.mk b/py/py.mk
@@ -259,9 +259,9 @@ $(HEADER_BUILD)/compressed.data.h: $(HEADER_BUILD)/compressed.collected
 	$(Q)$(PYTHON) $(PY_SRC)/makecompresseddata.py $< > $@
 
 # build a list of registered modules for py/objmodule.c.
-$(HEADER_BUILD)/moduledefs.h: $(SRC_QSTR) $(QSTR_GLOBAL_DEPENDENCIES) | $(HEADER_BUILD)/mpversion.h
+$(HEADER_BUILD)/moduledefs.h: $(HEADER_BUILD)/moduledefs.collected
 	@$(ECHO) "GEN $@"
-	$(Q)$(PYTHON) $(PY_SRC)/makemoduledefs.py --vpath="., $(TOP), $(USER_C_MODULES)" $(SRC_QSTR) > $@
+	$(Q)$(PYTHON) $(PY_SRC)/makemoduledefs.py $< > $@
 
 # Standard C functions like memset need to be compiled with special flags so
 # the compiler does not optimise these functions in terms of themselves.