-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
custom_usermod improvements #5403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b97b46a
3bfbbab
ac1a4df
1929267
210b4d8
05498f2
7f44396
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # Add a section to the linker script to store our dynamic arrays | ||
| # This is implemented as a pio post-script to ensure that we can | ||
| # place our linker script at the correct point in the command arguments. | ||
| Import("env") | ||
| from pathlib import Path | ||
|
|
||
| platform = env.get("PIOPLATFORM") | ||
| script_file = Path(f"tools/dynarray_{platform}.ld") | ||
| if script_file.is_file(): | ||
| linker_script = f"-T{script_file}" | ||
| if platform == "espressif32": | ||
| # For ESP32, the script must be added at the right point in the list | ||
| linkflags = env.get("LINKFLAGS", []) | ||
| idx = linkflags.index("memory.ld") | ||
| linkflags.insert(idx+1, linker_script) | ||
| env.Replace(LINKFLAGS=linkflags) | ||
| else: | ||
| # For other platforms, put it in last | ||
| env.Append(LINKFLAGS=[linker_script]) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| Import('env') | ||
| from collections import deque | ||
| from pathlib import Path # For OS-agnostic path manipulation | ||
| import re | ||
| from urllib.parse import urlparse | ||
| from click import secho | ||
| from SCons.Script import Exit | ||
| from platformio.builder.tools.piolib import LibBuilderBase | ||
|
|
@@ -25,25 +27,117 @@ def find_usermod(mod: str) -> Path: | |
| 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 | ||
| # Names of external/registry deps listed in custom_usermods. | ||
| # Populated during parsing below; read by is_wled_module() at configure time. | ||
| _custom_usermod_names: set[str] = set() | ||
|
|
||
| # Matches any RFC-valid URL scheme (http, https, git, git+https, symlink, file, hg+ssh, etc.) | ||
| _URL_SCHEME_RE = re.compile(r'^[a-zA-Z][a-zA-Z0-9+.-]*://') | ||
| # SSH git URL: user@host:path (e.g. git@github.com:user/repo.git#tag) | ||
| _SSH_URL_RE = re.compile(r'^[^@\s]+@[^@:\s]+:[^:\s]') | ||
| # Explicit custom name: "LibName = <spec>" (PlatformIO [<name>=]<spec> form) | ||
| _NAME_EQ_RE = re.compile(r'^([A-Za-z0-9_.-]+)\s*=\s*(\S.*)') | ||
|
|
||
|
|
||
| def _is_external_entry(line: str) -> bool: | ||
| """Return True if line is a lib_deps-style external/registry entry.""" | ||
| if _NAME_EQ_RE.match(line): # "LibName = <spec>" | ||
| return True | ||
| if _URL_SCHEME_RE.match(line): # https://, git://, symlink://, etc. | ||
| return True | ||
| if _SSH_URL_RE.match(line): # git@github.com:user/repo.git | ||
| return True | ||
| if '@' in line: # "owner/Name @ ^1.0.0" | ||
| return True | ||
| if re.match(r'^[^/\s]+/[^/\s]+$', line): # "owner/Name" | ||
| return True | ||
| return False | ||
|
|
||
|
|
||
| def _predict_dep_name(entry: str) -> str | None: | ||
| """Predict the library name PlatformIO will assign to this dep (best-effort). | ||
|
|
||
| Accuracy relies on the library's manifest "name" matching the repo/package | ||
| name in the spec. This holds for well-authored libraries; the libArchive | ||
| check (which requires library.json) provides an early-failure safety net. | ||
| """ | ||
| 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","") | ||
| entry = entry.strip() | ||
| # "LibName = <spec>" — name is given explicitly; always use it | ||
| m = _NAME_EQ_RE.match(entry) | ||
| if m: | ||
| return m.group(1).strip() | ||
| # URL scheme: extract name from path | ||
| if _URL_SCHEME_RE.match(entry): | ||
| parsed = urlparse(entry) | ||
| if parsed.netloc in ('github.com', 'gitlab.com', 'bitbucket.com'): | ||
| parts = [p for p in parsed.path.split('/') if p] | ||
| if len(parts) >= 2: | ||
| name = parts[1] | ||
| else: | ||
| name = Path(parsed.path.rstrip('/')).name.strip() | ||
| if name.endswith('.git'): | ||
| name = name[:-4] | ||
| return name or None | ||
|
Comment on lines
+70
to
+80
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: fd load_usermods.py -x wc -l {}Repository: wled/WLED Length of output: 87 🏁 Script executed: cat -n pio-scripts/load_usermods.py | head -150Repository: wled/WLED Length of output: 6777 🏁 Script executed: cat -n pio-scripts/load_usermods.py | sed -n '60,130p'Repository: wled/WLED Length of output: 3194 🏁 Script executed: python3 << 'EOF'
from urllib.parse import urlparse
from pathlib import Path
test_urls = [
"symlink:///path/to/usermod_name",
"file:///var/lib/repos/my_usermod",
"https://git.example.com/user/repo.git",
"git://self-hosted.org/usermods/cool-mod",
"https://github.com/user/my-usermod.git",
]
for url in test_urls:
parsed = urlparse(url)
# Current logic (lines 70-80) - only github/gitlab/bitbucket
if parsed.netloc in ('github.com', 'gitlab.com', 'bitbucket.com'):
parts = [p for p in parsed.path.split('/') if p]
if len(parts) >= 2:
current_name = parts[1]
else:
current_name = Path(parsed.path.rstrip('/')).name.strip()
else:
current_name = None
# Proposed logic (all schemes)
name = Path(parsed.path.rstrip('/')).name.strip()
if name.endswith('.git'):
name = name[:-4]
proposed_name = name or None
print(f"URL: {url}")
print(f" Current: {current_name}")
print(f" Proposed: {proposed_name}")
print()
EOFRepository: wled/WLED Length of output: 463 🏁 Script executed: cat -n pio-scripts/load_usermods.py | sed -n '140,210p'Repository: wled/WLED Length of output: 3592 🏁 Script executed: # Check if there's any validation or fallback that might handle unpredicted names
rg "library\.json" pio-scripts/ -A 3 -B 3Repository: wled/WLED Length of output: 2087 🏁 Script executed: # Check validate_modules.py to see how it uses WLED_MODULES
cat -n pio-scripts/validate_modules.py | head -60Repository: wled/WLED Length of output: 3076 🏁 Script executed: # Let's verify the proposed fix more carefully - ensure it handles all current code paths
cat -n pio-scripts/load_usermods.py | sed -n '70,93p'Repository: wled/WLED Length of output: 1197 🏁 Script executed: # Check if there are any tests or examples of custom_usermods usage
fd -t f "platformio.ini" -exec grep -l "custom_usermods" {} \;Repository: wled/WLED Length of output: 303 🏁 Script executed: fd platformio.ini -type fRepository: wled/WLED Length of output: 223 🏁 Script executed: # Check for any issues or PRs related to custom_usermods and name prediction
git log --oneline --all -- "pio-scripts/load_usermods.py" | head -20Repository: wled/WLED Length of output: 940 🏁 Script executed: # Let's verify the fix handles edge cases correctly
python3 << 'EOF'
from urllib.parse import urlparse
from pathlib import Path
test_cases = [
"symlink:///path/to/usermod_name",
"symlink:///path/to/usermod_name/",
"file:///var/lib/repos/my_usermod",
"file:///var/lib/repos/my_usermod.git",
"https://git.example.com/user/repo.git",
"git://self-hosted.org/usermods/cool-mod",
"https://github.com/user/my-usermod.git",
"symlink:///", # edge: empty path
"file://", # edge: no path
]
print("Testing proposed fix logic:")
for url in test_cases:
parsed = urlparse(url)
name = Path(parsed.path.rstrip('/')).name.strip()
if name.endswith('.git'):
name = name[:-4]
result = name or None
print(f"{url:50} → {result}")
EOFRepository: wled/WLED Length of output: 640 🏁 Script executed: # Let's check how PlatformIO resolves library names from deps to confirm behavior
rg "dep\.name" pio-scripts/load_usermods.py -B 2 -A 2Repository: wled/WLED Length of output: 195 🏁 Script executed: # Check if there's any fallback mechanism when a library can't be recognized by name
rg "library\.json.*name" pio-scripts/ -A 2 -B 2Repository: wled/WLED Length of output: 483 🏁 Script executed: # Verify the suggested fix doesn't have any syntax issues by testing it more thoroughly
python3 << 'EOF'
from urllib.parse import urlparse
from pathlib import Path
# Current code (lines 70-80)
def current_logic(entry):
parsed = urlparse(entry)
if parsed.netloc in ('github.com', 'gitlab.com', 'bitbucket.com'):
parts = [p for p in parsed.path.split('/') if p]
if len(parts) >= 2:
name = parts[1]
else:
name = Path(parsed.path.rstrip('/')).name.strip()
if name.endswith('.git'):
name = name[:-4]
return name or None
return None # Falls through for other schemes
# Proposed fix (restructured)
def proposed_logic(entry):
parsed = urlparse(entry)
if parsed.netloc in ('github.com', 'gitlab.com', 'bitbucket.com'):
parts = [p for p in parsed.path.split('/') if p]
if len(parts) >= 2:
name = parts[1]
else:
name = Path(parsed.path.rstrip('/')).name.strip()
else:
name = Path(parsed.path.rstrip('/')).name.strip()
if name.endswith('.git'):
name = name[:-4]
return name or None
test_cases = [
"https://github.com/user/my-usermod.git", # Should work in both
"symlink:///path/to/usermod", # Should fail in current, work in proposed
"file:///path/my_usermod.git", # Should fail in current, work in proposed
]
for url in test_cases:
curr = current_logic(url)
prop = proposed_logic(url)
status = "✓" if curr == prop or (curr is None and prop is not None) else "✗"
print(f"{status} {url:50} current={curr:15} proposed={prop}")
EOFRepository: wled/WLED Length of output: 307 🏁 Script executed: # Fix the test to handle None values
python3 << 'EOF'
from urllib.parse import urlparse
from pathlib import Path
# Current code (lines 70-80)
def current_logic(entry):
parsed = urlparse(entry)
if parsed.netloc in ('github.com', 'gitlab.com', 'bitbucket.com'):
parts = [p for p in parsed.path.split('/') if p]
if len(parts) >= 2:
name = parts[1]
else:
name = Path(parsed.path.rstrip('/')).name.strip()
if name.endswith('.git'):
name = name[:-4]
return name or None
return None # Falls through for other schemes
# Proposed fix (restructured)
def proposed_logic(entry):
parsed = urlparse(entry)
if parsed.netloc in ('github.com', 'gitlab.com', 'bitbucket.com'):
parts = [p for p in parsed.path.split('/') if p]
if len(parts) >= 2:
name = parts[1]
else:
name = Path(parsed.path.rstrip('/')).name.strip()
else:
name = Path(parsed.path.rstrip('/')).name.strip()
if name.endswith('.git'):
name = name[:-4]
return name or None
test_cases = [
"https://github.com/user/my-usermod.git",
"symlink:///path/to/usermod",
"file:///path/my_usermod.git",
]
print("Comparison of current vs proposed logic:")
for url in test_cases:
curr = current_logic(url)
prop = proposed_logic(url)
improved = "✓" if curr != prop else "="
print(f"{improved} {url:50}")
print(f" current: {str(curr):20} proposed: {str(prop)}")
EOFRepository: wled/WLED Length of output: 414 🏁 Script executed: # Now verify the impact: does is_wled_module rely on this name matching?
# Check how PlatformIO assigns names to library dependencies
rg "dep\.name" pio-scripts/ -B 3 -A 3Repository: wled/WLED Length of output: 492 Generic URL dependencies bypass WLED-module classification. When Extract basename for any URL scheme to ensure these external usermods receive the same WLED-specific treatment: Suggested fix if _URL_SCHEME_RE.match(entry):
parsed = urlparse(entry)
if parsed.netloc in ('github.com', 'gitlab.com', 'bitbucket.com'):
parts = [p for p in parsed.path.split('/') if p]
if len(parts) >= 2:
name = parts[1]
else:
name = Path(parsed.path.rstrip('/')).name.strip()
- if name.endswith('.git'):
- name = name[:-4]
- return name or None
+ else:
+ name = Path(parsed.path.rstrip('/')).name.strip()
+ if name.endswith('.git'):
+ name = name[:-4]
+ return name or NoneAlso applies to: 96-102, 114-125 🤖 Prompt for AI Agents |
||
| # SSH git URL: git@github.com:user/repo.git#tag → repo | ||
| if _SSH_URL_RE.match(entry): | ||
| path_part = entry.split(':', 1)[1].split('#')[0].rstrip('/') | ||
| name = Path(path_part).name | ||
| return (name[:-4] if name.endswith('.git') else name) or None | ||
| # Versioned registry: "owner/Name @ version" → Name | ||
| if '@' in entry: | ||
| name_part = entry.split('@')[0].strip() | ||
| return name_part.split('/')[-1].strip() if '/' in name_part else name_part | ||
| # Plain registry: "owner/Name" → Name | ||
| if re.match(r'^[^/\s]+/[^/\s]+$', entry): | ||
| return entry.split('/')[-1].strip() | ||
| return None | ||
|
|
||
| # 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 | ||
| symlinks = [f"symlink://{find_usermod(mod).resolve()}" for mod in usermods] | ||
| env.GetProjectConfig().set("env:" + env['PIOENV'], 'lib_deps', env.GetProjectOption('lib_deps') + symlinks) | ||
| 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-") | ||
| or dep.name in _custom_usermod_names | ||
| ) | ||
|
|
||
|
|
||
| ## Script starts here — parse custom_usermods | ||
| raw_usermods = env.GetProjectOption("custom_usermods", "") | ||
| usermods_libdeps: list[str] = [] | ||
|
|
||
| for line in raw_usermods.splitlines(): | ||
| line = line.strip() | ||
| if not line or line.startswith('#') or line.startswith(';'): | ||
| continue | ||
|
|
||
| if _is_external_entry(line): | ||
| # External URL or registry entry: pass through to lib_deps unchanged. | ||
| predicted = _predict_dep_name(line) | ||
| if predicted: | ||
| _custom_usermod_names.add(predicted) | ||
| else: | ||
| secho( | ||
| f"WARNING: Cannot determine library name for custom_usermods entry " | ||
| f"{line!r}. If it is not recognised as a WLED module at build time, " | ||
| f"ensure its library.json 'name' matches the repo name.", | ||
| fg="yellow", err=True) | ||
| usermods_libdeps.append(line) | ||
| else: | ||
| # Bare name(s): split on whitespace for backwards compatibility. | ||
| for token in line.split(): | ||
| if token == '*': | ||
| for mod_path in sorted(usermod_dir.iterdir()): | ||
| if mod_path.is_dir() and (mod_path / 'library.json').exists(): | ||
| _custom_usermod_names.add(mod_path.name) | ||
| usermods_libdeps.append(f"symlink://{mod_path.resolve()}") | ||
| else: | ||
| resolved = find_usermod(token) | ||
| _custom_usermod_names.add(resolved.name) | ||
| usermods_libdeps.append(f"symlink://{resolved.resolve()}") | ||
|
|
||
| if usermods_libdeps: | ||
| env.GetProjectConfig().set("env:" + env['PIOENV'], 'lib_deps', env.GetProjectOption('lib_deps') + usermods_libdeps) | ||
|
|
||
| # Utility function for assembling usermod include paths | ||
| def cached_add_includes(dep, dep_cache: set, includes: deque): | ||
|
|
@@ -86,16 +180,25 @@ def wrapped_ConfigureProjectLibBuilder(xenv): | |
| # Add WLED's own dependencies | ||
| for dir in extra_include_dirs: | ||
| dep.env.PrependUnique(CPPPATH=str(dir)) | ||
| # Ensure debug info is emitted for this module's source files. | ||
| # validate_modules.py uses `nm --defined-only -l` on the final ELF to check | ||
| # that each module has at least one symbol placed in the binary. The -l flag | ||
| # reads DWARF debug sections to map placed symbols back to their original source | ||
| # files; without -g those sections are absent and the check cannot attribute any | ||
| # symbol to a specific module. We scope this to usermods only — the main WLED | ||
| # build and other libraries are unaffected. | ||
| dep.env.AppendUnique(CCFLAGS=["-g"]) | ||
| # 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) | ||
| f"ERROR: libArchive=false is missing on usermod(s) {' '.join(broken_usermods)} -- " | ||
| f"modules will not compile in correctly. Add '\"build\": {{\"libArchive\": false}}' " | ||
| f"to their library.json.", | ||
| fg="red", err=True) | ||
| Exit(1) | ||
|
|
||
| # Save the depbuilders list for later validation | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,16 +1,10 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||
| import re | ||||||||||||||||||||||||||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||||||||||||||||||||||||||
| 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-") | ||||||||||||||||||||||||||||||||||||||||||||||||
| Import("env") | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| def read_lines(p: Path): | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -19,29 +13,71 @@ def read_lines(p: Path): | |||||||||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||
| def _get_nm_path(env) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||
| """ Derive the nm tool path from the build environment """ | ||||||||||||||||||||||||||||||||||||||||||||||||
| if "NM" in env: | ||||||||||||||||||||||||||||||||||||||||||||||||
| return env.subst("$NM") | ||||||||||||||||||||||||||||||||||||||||||||||||
| # Derive from the C compiler: xtensa-esp32-elf-gcc → xtensa-esp32-elf-nm | ||||||||||||||||||||||||||||||||||||||||||||||||
| cc = env.subst("$CC") | ||||||||||||||||||||||||||||||||||||||||||||||||
| nm = re.sub(r'(gcc|g\+\+)$', 'nm', os.path.basename(cc)) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return os.path.join(os.path.dirname(cc), nm) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| def check_elf_modules(elf_path: Path, env, module_lib_builders) -> set[str]: | ||||||||||||||||||||||||||||||||||||||||||||||||
| """ Check which modules have at least one defined symbol placed in the ELF. | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Returns the (sub)set of dirs that are found in the output ELF | ||||||||||||||||||||||||||||||||||||||||||||||||
| The map file is not a reliable source for this: with LTO, original object | ||||||||||||||||||||||||||||||||||||||||||||||||
| file paths are replaced by temporary ltrans.o partitions in all output | ||||||||||||||||||||||||||||||||||||||||||||||||
| sections, making per-module attribution impossible from the map alone. | ||||||||||||||||||||||||||||||||||||||||||||||||
| Instead we invoke nm --defined-only -l on the ELF, which uses DWARF debug | ||||||||||||||||||||||||||||||||||||||||||||||||
| info to attribute each placed symbol to its original source file. | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Requires usermod libraries to be compiled with -g so that DWARF sections | ||||||||||||||||||||||||||||||||||||||||||||||||
| are present in the ELF. load_usermods.py injects -g for all WLED modules | ||||||||||||||||||||||||||||||||||||||||||||||||
| via dep.env.AppendUnique(CCFLAGS=["-g"]). | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Returns the set of build_dir basenames for confirmed modules. | ||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||
| # 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") | ||||||||||||||||||||||||||||||||||||||||||||||||
| nm_path = _get_nm_path(env) | ||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||
| result = subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||
| [nm_path, "--defined-only", "-l", str(elf_path)], | ||||||||||||||||||||||||||||||||||||||||||||||||
| capture_output=True, text=True, errors="ignore", timeout=120, | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||
| nm_output = result.stdout | ||||||||||||||||||||||||||||||||||||||||||||||||
| except (subprocess.TimeoutExpired, FileNotFoundError, OSError) as e: | ||||||||||||||||||||||||||||||||||||||||||||||||
| secho(f"WARNING: nm failed ({e}); skipping per-module validation", fg="yellow", err=True) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return {Path(b.build_dir).name for b in module_lib_builders} # conservative pass | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+50
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -name "validate_modules.py" -type fRepository: wled/WLED Length of output: 86 🏁 Script executed: cat -n pio-scripts/validate_modules.py | head -100Repository: wled/WLED Length of output: 5057 🏁 Script executed: # Look for the check_elf_modules function
rg -n "def check_elf_modules" pio-scripts/validate_modules.py -A 50Repository: wled/WLED Length of output: 2785 🏁 Script executed: # Verify subprocess.run() behavior by checking Python documentation/behavior
python3 << 'EOF'
import subprocess
import sys
# Check: does subprocess.run() raise on non-zero exit by default?
print("subprocess.run() behavior with check=False (default):")
print("- Does NOT raise exception on non-zero exit code")
print("- Returns CompletedProcess object with returncode attribute")
print("- stdout/stderr available regardless of exit code")
print()
# Show what happens with an nm-like scenario
result = subprocess.run(['sh', '-c', 'exit 5'], capture_output=True, text=True)
print(f"Example: exit code 5 with check=False")
print(f" returncode: {result.returncode}")
print(f" exception raised: No")
print(f" stdout available: {repr(result.stdout)}")
EOFRepository: wled/WLED Length of output: 366 🏁 Script executed: # Check if there are other subprocess calls in the file for pattern comparison
rg -n "subprocess\." pio-scripts/validate_modules.pyRepository: wled/WLED Length of output: 164 Check
Suggested fix result = subprocess.run(
[nm_path, "--defined-only", "-l", str(elf_path)],
capture_output=True, text=True, errors="ignore", timeout=120,
)
+ if result.returncode != 0:
+ secho(
+ f"WARNING: nm failed with exit code {result.returncode}; skipping per-module validation",
+ fg="yellow",
+ err=True,
+ )
+ return {Path(b.build_dir).name for b in module_lib_builders}
nm_output = result.stdout📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.15.4)[error] 43-43: (S603) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Build a filtered set of lines that have a nonzero address. | ||||||||||||||||||||||||||||||||||||||||||||||||
| # nm --defined-only still includes debugging symbols (type 'N') such as the | ||||||||||||||||||||||||||||||||||||||||||||||||
| # per-CU markers GCC emits in .debug_info (e.g. "usermod_example_cpp_6734d48d"). | ||||||||||||||||||||||||||||||||||||||||||||||||
| # These live at address 0x00000000 in their debug section — not in any load | ||||||||||||||||||||||||||||||||||||||||||||||||
| # segment — so filtering them out leaves only genuinely placed symbols. | ||||||||||||||||||||||||||||||||||||||||||||||||
| placed_lines = [ | ||||||||||||||||||||||||||||||||||||||||||||||||
| line for line in nm_output.splitlines() | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (parts := line.split(None, 1)) and parts[0].lstrip('0') | ||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||
| placed_output = "\n".join(placed_lines) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| found = set() | ||||||||||||||||||||||||||||||||||||||||||||||||
| for line in map_file: | ||||||||||||||||||||||||||||||||||||||||||||||||
| matches = object_path_regex.findall(line) | ||||||||||||||||||||||||||||||||||||||||||||||||
| for m in matches: | ||||||||||||||||||||||||||||||||||||||||||||||||
| found.add(m) | ||||||||||||||||||||||||||||||||||||||||||||||||
| for builder in module_lib_builders: | ||||||||||||||||||||||||||||||||||||||||||||||||
| # builder.src_dir is the library source directory (used by is_wled_module() too) | ||||||||||||||||||||||||||||||||||||||||||||||||
| src_dir = str(builder.src_dir).rstrip("/\\") | ||||||||||||||||||||||||||||||||||||||||||||||||
| # Guard against prefix collisions (e.g. /path/to/mod vs /path/to/mod-extra) | ||||||||||||||||||||||||||||||||||||||||||||||||
| # by requiring a path separator immediately after the directory name. | ||||||||||||||||||||||||||||||||||||||||||||||||
| if re.search(re.escape(src_dir) + r'[/\\]', placed_output): | ||||||||||||||||||||||||||||||||||||||||||||||||
| found.add(Path(builder.build_dir).name) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return found | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| DYNARRAY_SECTION = ".dtors" if env.get("PIOPLATFORM") == "espressif8266" else ".dynarray" | ||||||||||||||||||||||||||||||||||||||||||||||||
| USERMODS_SECTION = f"{DYNARRAY_SECTION}.usermods.1" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| 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]) | ||||||||||||||||||||||||||||||||||||||||||||||||
| return len([x for x in map_file if USERMODS_SECTION in x]) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| def validate_map_file(source, target, env): | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -65,16 +101,17 @@ def validate_map_file(source, target, env): | |||||||||||||||||||||||||||||||||||||||||||||||
| 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()) | ||||||||||||||||||||||||||||||||||||||||||||||||
| elf_path = build_dir / env.subst("${PROGNAME}.elf") | ||||||||||||||||||||||||||||||||||||||||||||||||
| confirmed_modules = check_elf_modules(elf_path, env, module_lib_builders) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| 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!", | ||||||||||||||||||||||||||||||||||||||||||||||||
| f"ERROR: No symbols 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')) | ||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| /* ESP32 linker script fragment to add dynamic array section to binary */ | ||
| SECTIONS | ||
| { | ||
| .dynarray : | ||
| { | ||
| . = ALIGN(0x10); | ||
| KEEP(*(SORT_BY_INIT_PRIORITY(.dynarray.*))) | ||
| } > default_rodata_seg | ||
| } | ||
| INSERT AFTER .flash.rodata; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| /* dynarray.h | ||
|
|
||
| Macros for generating a "dynamic array", a static array of objects declared in different translation units | ||
|
|
||
| */ | ||
|
|
||
| #pragma once | ||
|
|
||
| // Declare the beginning and ending elements of a dynamic array of 'type'. | ||
| // This must be used in only one translation unit in your program for any given array. | ||
| #define DECLARE_DYNARRAY(type, array_name) \ | ||
| static type const DYNARRAY_BEGIN(array_name)[0] __attribute__((__section__(DYNARRAY_SECTION "." #array_name ".0"), unused)) = {}; \ | ||
| static type const DYNARRAY_END(array_name)[0] __attribute__((__section__(DYNARRAY_SECTION "." #array_name ".99999"), unused)) = {}; | ||
|
|
||
| // Declare an object that is a member of a dynamic array. "member name" must be unique; "array_section" is an integer for ordering items. | ||
| // It is legal to define multiple items with the same section name; the order of those items will be up to the linker. | ||
| #define DYNARRAY_MEMBER(type, array_name, member_name, array_section) type const member_name __attribute__((__section__(DYNARRAY_SECTION "." #array_name "." #array_section), used)) | ||
willmmiles marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| #define DYNARRAY_BEGIN(array_name) array_name##_begin | ||
| #define DYNARRAY_END(array_name) array_name##_end | ||
| #define DYNARRAY_LENGTH(array_name) (&DYNARRAY_END(array_name)[0] - &DYNARRAY_BEGIN(array_name)[0]) | ||
|
|
||
| #ifdef ESP8266 | ||
| // ESP8266 linker script cannot be extended with a unique section for dynamic arrays. | ||
| // We instead pack them in the ".dtors" section, as it's sorted and uploaded to the flash | ||
| // (but will never be used in the embedded system) | ||
| #define DYNARRAY_SECTION ".dtors" | ||
|
|
||
| #else /* ESP8266 */ | ||
|
|
||
| // Use a unique named section; the linker script must be extended to ensure it's correctly placed. | ||
| #define DYNARRAY_SECTION ".dynarray" | ||
|
|
||
| #endif | ||
Uh oh!
There was an error while loading. Please reload this page.