Merge pull request #4669 from willmmiles/4597-usermods-not-building

Correct issues with usermods not being linked.
- Explicitly set libArchive: false in usermod library.json files
- Fix up symlink path generation on Windows
- Add validation script to report usermod linkage in resulting binary

Author: willmmiles

pio-scripts/load_usermods.py

@@ -1,17 +1,14 @@
 Import('env')
-import os.path
 from collections import deque
 from pathlib import Path   # For OS-agnostic path manipulation
-from platformio.package.manager.library import LibraryPackageManager
+from click import secho
+from SCons.Script import Exit
+from platformio.builder.tools.piolib import LibBuilderBase
 
-usermod_dir = Path(env["PROJECT_DIR"]) / "usermods"
-all_usermods = [f for f in usermod_dir.iterdir() if f.is_dir() and f.joinpath('library.json').exists()]
+usermod_dir = Path(env["PROJECT_DIR"]).resolve() / "usermods"
 
-if env['PIOENV'] == "usermods":
-   # Add all usermods
-   env.GetProjectConfig().set(f"env:usermods", 'custom_usermods', " ".join([f.name for f in all_usermods]))
-
-def find_usermod(mod: str):
+# Utility functions
+def find_usermod(mod: str) -> Path:
   """Locate this library in the usermods folder.
      We do this to avoid needing to rename a bunch of folders;
      this could be removed later
@@ -22,51 +19,36 @@ def find_usermod(mod: str):
     return mp
   mp = usermod_dir / f"{mod}_v2"
   if mp.exists():
-    return mp  
+    return mp
   mp = usermod_dir / f"usermod_v2_{mod}"
   if mp.exists():
     return mp
   raise RuntimeError(f"Couldn't locate module {mod} in usermods directory!")
 
+def is_wled_module(dep: LibBuilderBase) -> bool:
+  """Returns true if the specified library is a wled module
+  """
+  return usermod_dir in Path(dep.src_dir).parents or str(dep.name).startswith("wled-")
+
+## Script starts here
+# Process usermod option
 usermods = env.GetProjectOption("custom_usermods","")
+
+# Handle "all usermods" case
+if usermods == '*':
+  usermods = [f.name for f in usermod_dir.iterdir() if f.is_dir() and f.joinpath('library.json').exists()]
+else:
+  usermods = usermods.split()
+
 if usermods:
   # Inject usermods in to project lib_deps
-  proj = env.GetProjectConfig()
-  deps = env.GetProjectOption('lib_deps')
-  src_dir = proj.get("platformio", "src_dir")
-  src_dir = src_dir.replace('\\','/')
-  mod_paths = {mod: find_usermod(mod) for mod in usermods.split()}
-  usermods = [f"{mod} = symlink://{path}" for mod, path in mod_paths.items()]
-  proj.set("env:" + env['PIOENV'], 'lib_deps', deps + usermods)
-  # Force usermods to be installed in to the environment build state before the LDF runs
-  # Otherwise we won't be able to see them until it's too late to change their paths for LDF
-  # Logic is largely borrowed from PlaformIO internals
-  not_found_specs = []
-  for spec in usermods:
-    found = False
-    for storage_dir in env.GetLibSourceDirs():
-      #print(f"Checking {storage_dir} for {spec}")
-      lm = LibraryPackageManager(storage_dir)
-      if lm.get_package(spec):
-          #print("Found!")
-          found = True
-          break
-    if not found:
-        #print("Missing!")
-        not_found_specs.append(spec)
-  if not_found_specs:
-      lm = LibraryPackageManager(
-          env.subst(os.path.join("$PROJECT_LIBDEPS_DIR", "$PIOENV"))
-      )
-      for spec in not_found_specs:
-        #print(f"LU: forcing install of {spec}")
-        lm.install(spec)
-
+  symlinks = [f"symlink://{find_usermod(mod).resolve()}" for mod in usermods]
+  env.GetProjectConfig().set("env:" + env['PIOENV'], 'lib_deps', env.GetProjectOption('lib_deps') + symlinks)
 
 # Utility function for assembling usermod include paths
 def cached_add_includes(dep, dep_cache: set, includes: deque):
   """ Add dep's include paths to includes if it's not in the cache """
-  if dep not in dep_cache:   
+  if dep not in dep_cache:
     dep_cache.add(dep)
     for include in dep.get_include_dirs():
       if include not in includes:
@@ -82,13 +64,6 @@ def cached_add_includes(dep, dep_cache: set, includes: deque):
 
 # Our new wrapper
 def wrapped_ConfigureProjectLibBuilder(xenv):
-  # Update usermod properties
-  # Set libArchive before build actions are added
-  for um in (um for um in xenv.GetLibBuilders() if usermod_dir in Path(um.src_dir).parents):
-    build = um._manifest.get("build", {})
-    build["libArchive"] = False
-    um._manifest["build"] = build
-
   # Call the wrapped function
   result = old_ConfigureProjectLibBuilder.clone(xenv)()
 
@@ -102,12 +77,25 @@ def wrapped_ConfigureProjectLibBuilder(xenv):
   for dep in result.depbuilders:
      cached_add_includes(dep, processed_deps, extra_include_dirs)
 
-  for um in [dep for dep in result.depbuilders if usermod_dir in Path(dep.src_dir).parents]:
-    # Add the wled folder to the include path
-    um.env.PrependUnique(CPPPATH=wled_dir)
-    # Add WLED's own dependencies
-    for dir in extra_include_dirs:
-      um.env.PrependUnique(CPPPATH=dir)
+  broken_usermods = []
+  for dep in result.depbuilders:
+    if is_wled_module(dep):
+      # Add the wled folder to the include path
+      dep.env.PrependUnique(CPPPATH=str(wled_dir))
+      # Add WLED's own dependencies
+      for dir in extra_include_dirs:
+        dep.env.PrependUnique(CPPPATH=str(dir))
+      # Enforce that libArchive is not set; we must link them directly to the executable
+      if dep.lib_archive:
+        broken_usermods.append(dep)
+
+  if broken_usermods:
+    broken_usermods = [usermod.name for usermod in broken_usermods]
+    secho(
+      f"ERROR: libArchive=false is missing on usermod(s) {' '.join(broken_usermods)} -- modules will not compile in correctly",
+      fg="red",
+      err=True)
+    Exit(1)
 
   return result
 

pio-scripts/validate_modules.py

@@ -0,0 +1,92 @@
+import re
+from pathlib import Path   # For OS-agnostic path manipulation
+from typing import Iterable
+from click import secho
+from SCons.Script import Action, Exit
+from platformio.builder.tools.piolib import LibBuilderBase
+
+
+def is_wled_module(env, dep: LibBuilderBase) -> bool:
+  """Returns true if the specified library is a wled module
+  """
+  usermod_dir = Path(env["PROJECT_DIR"]).resolve() / "usermods"
+  return usermod_dir in Path(dep.src_dir).parents or str(dep.name).startswith("wled-")
+
+
+def read_lines(p: Path):
+    """ Read in the contents of a file for analysis """
+    with p.open("r", encoding="utf-8", errors="ignore") as f:
+        return f.readlines()
+
+
+def check_map_file_objects(map_file: list[str], dirs: Iterable[str]) -> set[str]:
+    """ Identify which dirs contributed to the final build
+
+        Returns the (sub)set of dirs that are found in the output ELF
+    """
+    # Pattern to match symbols in object directories
+    # Join directories into alternation
+    usermod_dir_regex = "|".join([re.escape(dir) for dir in dirs])
+    # Matches nonzero address, any size, and any path in a matching directory
+    object_path_regex = re.compile(r"0x0*[1-9a-f][0-9a-f]*\s+0x[0-9a-f]+\s+\S+[/\\](" + usermod_dir_regex + r")[/\\]\S+\.o")
+
+    found = set()
+    for line in map_file:
+        matches = object_path_regex.findall(line)
+        for m in matches:
+            found.add(m)
+    return found
+
+
+def count_usermod_objects(map_file: list[str]) -> int:
+    """ Returns the number of usermod objects in the usermod list """
+    # Count the number of entries in the usermods table section
+    return len([x for x in map_file if ".dtors.tbl.usermods.1" in x])
+
+
+def validate_map_file(source, target, env):
+    """ Validate that all modules appear in the output build """
+    build_dir = Path(env.subst("$BUILD_DIR"))
+    map_file_path = build_dir /  env.subst("${PROGNAME}.map")
+
+    if not map_file_path.exists():
+        secho(f"ERROR: Map file not found: {map_file_path}", fg="red", err=True)
+        Exit(1)
+
+    # Identify the WLED module source directories
+    module_lib_builders = [builder for builder in env.GetLibBuilders() if is_wled_module(env, builder)]
+
+    if env.GetProjectOption("custom_usermods","") == "*":
+        # All usermods build; filter non-platform-OK modules
+        module_lib_builders = [builder for builder in module_lib_builders if env.IsCompatibleLibBuilder(builder)]
+    else:
+        incompatible_builders = [builder for builder in module_lib_builders if not env.IsCompatibleLibBuilder(builder)]
+        if incompatible_builders:
+            secho(
+                f"ERROR: Modules {[b.name for b in incompatible_builders]} are not compatible with this platform!",
+                fg="red",
+                err=True)
+            Exit(1)
+
+    # Extract the values we care about
+    modules = {Path(builder.build_dir).name: builder.name for builder in module_lib_builders}
+    secho(f"INFO: {len(modules)} libraries linked as WLED optional/user modules")
+
+    # Now parse the map file
+    map_file_contents = read_lines(map_file_path)
+    usermod_object_count = count_usermod_objects(map_file_contents)
+    secho(f"INFO: {usermod_object_count} usermod object entries")
+
+    confirmed_modules = check_map_file_objects(map_file_contents, modules.keys())
+    missing_modules = [modname for mdir, modname in modules.items() if mdir not in confirmed_modules]
+    if missing_modules:
+        secho(
+            f"ERROR: No object files from {missing_modules} found in linked output!",
+            fg="red",
+            err=True)
+        Exit(1)
+    return None
+
+Import("env")
+env.Append(LINKFLAGS=[env.subst("-Wl,--Map=${BUILD_DIR}/${PROGNAME}.map")])
+env.AddPostAction("$BUILD_DIR/${PROGNAME}.elf", Action(validate_map_file, cmdstr='Checking linked optional modules (usermods) in map file'))

platformio.ini

@@ -116,6 +116,7 @@ extra_scripts =
   pre:pio-scripts/user_config_copy.py
   pre:pio-scripts/load_usermods.py
   pre:pio-scripts/build_ui.py
+  post:pio-scripts/validate_modules.py  ;; double-check the build output usermods
   ; post:pio-scripts/obj-dump.py  ;; convenience script to create a disassembly dump of the firmware (hardcore debugging)
 
 # ------------------------------------------------------------------------------
@@ -659,5 +660,5 @@ build_flags = ${common.build_flags} ${esp32_idf_V4.build_flags} -D WLED_RELEASE_
 lib_deps = ${esp32_idf_V4.lib_deps}
 monitor_filters = esp32_exception_decoder
 board_build.flash_mode = dio
-; custom_usermods = *every folder with library.json* -- injected by pio-scripts/load_usermods.py
+custom_usermods = *   ; Expands to all usermods in usermods folder
 board_build.partitions = ${esp32.extreme_partitions}  ; We're gonna need a bigger boat

usermods/ADS1115_v2/library.json

@@ -1,5 +1,6 @@
 {
   "name": "ADS1115_v2",
+  "build": { "libArchive": false },
   "dependencies": {
     "Adafruit BusIO": "https://github.com/adafruit/Adafruit_BusIO#1.13.2",
     "Adafruit ADS1X15": "https://github.com/adafruit/Adafruit_ADS1X15#2.4.0"

usermods/AHT10_v2/library.json

@@ -1,5 +1,6 @@
 {
   "name": "AHT10_v2",
+  "build": { "libArchive": false },  
   "dependencies": {
     "enjoyneering/AHT10":"~1.1.0"
   }

usermods/Analog_Clock/library.json

@@ -1,3 +1,4 @@
 {
-  "name": "Analog_Clock"
+  "name": "Analog_Clock",
+  "build": { "libArchive": false }
 }

usermods/Animated_Staircase/library.json

@@ -1,3 +1,4 @@
 {
-  "name": "Animated_Staircase"
+  "name": "Animated_Staircase",
+  "build": { "libArchive": false }
 }

usermods/BH1750_v2/library.json

@@ -1,5 +1,6 @@
 {
   "name": "BH1750_v2",
+  "build": { "libArchive": false },
   "dependencies": {
     "claws/BH1750":"^1.2.0"
   }

usermods/BME280_v2/library.json

@@ -1,5 +1,6 @@
 {
   "name": "BME280_v2",
+  "build": { "libArchive": false },
   "dependencies": {
     "finitespace/BME280":"~3.0.0"
   }

usermods/BME68X_v2/library.json

@@ -1,5 +1,6 @@
 {
   "name": "BME68X",
+  "build": { "libArchive": false },
   "dependencies": {
     "boschsensortec/BSEC Software Library":"^1.8.1492"
   }