From a7db031e736b626d068dc6fb8c3fd88b05c12919 Mon Sep 17 00:00:00 2001 From: David Montero Crespo Date: Thu, 20 Aug 2026 00:15:51 +0200 Subject: [PATCH] =?UTF-8?q?fix(espidf):=20one=20include=20root=20per=20lib?= =?UTF-8?q?rary=20=E2=80=94=20kill=20the=20header-shadow=20class?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merged component copied every library into ONE interleaved tree and then exported EVERY directory that held a file. A library's private internals therefore sat on the global -I and hijacked #includes issued by other libraries, by the arduino-esp32 core and by ESP-IDF itself. Each incident was patched by adding a name to _SHADOW_STD_HEADERS — chasing symptoms, since the denylist grows with every library that ships a generically-named internal header (three landed yesterday alone: FastLED's platforms/stub/Arduino.h, its fl/stl/asio/http/http_parser.h, and earlier LovyanGFX's lgfx/internal/limits.h). Adopt arduino-cli's model instead — the resolver every other Velxio board family already delegates to. It shares one global -I list too, but adds exactly ONE directory per library: /src for the recursive layout, for the flat legacy one, never a subdirectory; utility/ stays private to its owner. arduino-esp32 does the same as an IDF component. - Each library is copied into its own subtree of the component, so two libraries shipping the same relative path no longer collapse into one silently-picked copy. - INCLUDE_DIRS is one root per library, in resolution order. Internals stay copied and reachable through file-relative includes. - Legacy-layout utility/ dirs go in PRIV_INCLUDE_DIRS. - Root-level header collisions (the only shadowing still possible) are logged the way the Arduino IDE reports them, instead of being silent. - The transitive scan now walks only the library just copied. One component is deliberate: a component per library would swap this failure mode for REQUIRES-order shadowing (IDF exports component includes as plain -I, never -isystem) plus IDF component-name collisions, and buys no caching ccache does not already provide. Verified against the real FastLED+U8g2 project WITH the two denylist entries removed: green, byte-identical firmware size. The 3-library SSD1306+GFX+BusIO corpus is green too. VELXIO_PER_LIB_ROOTS=0 restores the legacy layout; the mode is folded into the build-variant hash so the two can never share a configured build dir. --- backend/app/services/espidf_compiler.py | 224 ++++++++++-- test/backend/unit/test_espidf_compiler.py | 40 ++- .../backend/unit/test_espidf_include_roots.py | 332 ++++++++++++++++++ 3 files changed, 557 insertions(+), 39 deletions(-) create mode 100644 test/backend/unit/test_espidf_include_roots.py diff --git a/backend/app/services/espidf_compiler.py b/backend/app/services/espidf_compiler.py index 60817444..f4775bca 100644 --- a/backend/app/services/espidf_compiler.py +++ b/backend/app/services/espidf_compiler.py @@ -60,6 +60,50 @@ _USE_PERSISTENT_DIR = ( ) +# ── Per-library include roots (Arduino-faithful layout) ────────────────────── +# The merged `user_libs_all` component used to copy every library's files into +# ONE interleaved tree and then put EVERY directory that held a file on the +# component's public INCLUDE_DIRS. That is the root cause of the header-shadow +# bug class: a library's private internals — FastLED's host-test +# `src/platforms/stub/Arduino.h`, its `src/fl/stl/asio/http/http_parser.h`, +# LovyanGFX's `src/lgfx/internal/limits.h` — landed on the global -I and +# hijacked `#include`s issued by OTHER libraries, by the arduino-esp32 core and +# by ESP-IDF itself. Every fix was another name in `_SHADOW_STD_HEADERS`, i.e. +# chasing symptoms: the denylist grows with every library that ships a +# generically-named internal header. +# +# arduino-cli — the resolver every other Velxio board family already delegates +# to — shares one global -I list too, but adds exactly ONE directory per +# library: `/src` for the recursive layout, `` for the legacy flat +# one, never a subdirectory (its header index is non-recursive, so +# `src/sub/foo.h` is only ever reachable as ``). `utility/` stays +# private to the library that owns it. arduino-esp32 does the same as an IDF +# component: one public include dir per bundled library. +# +# So: keep the single component — a component per library would trade this bug +# for REQUIRES-order shadowing (IDF exports component includes as plain -I, +# never -isystem) plus IDF-component name collisions — but give each library +# its own subtree and publish only its include root. +# +# Set VELXIO_PER_LIB_ROOTS=0 to fall back to the legacy interleaved layout +# without rebuilding the image (rollback hatch). +_PER_LIB_ROOTS = ( + os.environ.get('VELXIO_PER_LIB_ROOTS', '1') + not in ('0', 'false', 'False', '') +) + + +def _sanitise_lib_dirname(name: str) -> str: + """Component-relative directory name for a library. + + Cache folder names carry `@version-hash` suffixes (`fastled@3.10.5-017d…`) + and user libs can be named anything; keep the characters that are safe in a + path AND inside a quoted CMake string, fold the rest. + """ + safe = re.sub(r'[^A-Za-z0-9_.@-]', '_', name).strip('.') + return safe or 'lib' + + def _idf_version_signature() -> str: """Snapshot of the ESP-IDF + arduino-esp32 toolchain version. Used to invalidate persistent build dirs after an upstream submodule bump (the @@ -1488,10 +1532,15 @@ class ESPIDFCompiler: arduino-esp32 libs and bundled esp32_libs are always allowed (they are platform-provided, not user installs). - All library files are copied flat into one directory, so every header is - visible to every other header and source file without any cross-component - REQUIRES propagation — which is unreliable in ESP-IDF 4.x for deeply - nested transitive dependencies. + Layout (see the _PER_LIB_ROOTS note at module scope): every library is + copied into its OWN subtree of the component and exports exactly ONE + include root — `/src`, or `` for the flat legacy layout, with + `utility/` kept PRIVATE. That is arduino-cli's model, and the reason a + library's private internals can no longer hijack another library's (or + the core's, or ESP-IDF's) `#include`s. One component is deliberate: a + component per library would swap this failure mode for REQUIRES-order + shadowing plus IDF component-name collisions, and buys no caching that + ccache does not already provide. Search priority per header: 1. arduino_libs (user-installed via Library Manager) → merge into component @@ -1528,6 +1577,23 @@ class ESPIDFCompiler: header_to_comp: dict[str, str] = {} found_any = False + # Per-library bookkeeping for the Arduino-faithful layout + # (see _PER_LIB_ROOTS): + # lib_include_roots — ONE component-relative include root per library, + # in resolution order (arduino-cli's -I order). + # lib_priv_roots — legacy-layout `utility/` dirs. PRIVATE so they + # reach the library sources that need them without + # entering the sketch's namespace, exactly as + # arduino-cli scopes them to that library's units. + # root_header_owner — root header basename -> first library that + # published it, for collision reporting. + # lib_prefix_by_name— library dir name -> unique sanitised subdir. + lib_include_roots: list[str] = [] + lib_priv_roots: list[str] = [] + root_header_owner: dict[str, str] = {} + lib_prefix_by_name: dict[str, str] = {} + used_prefixes: set[str] = set() + headers_to_resolve: list[str] = list(ext_headers) # Headers the SKETCH itself includes (vs transitive pulls found by # re-scanning copied library headers). The architecture guard is @@ -1690,22 +1756,85 @@ class ESPIDFCompiler: # presumed to be buildable source. return True + # Destination subtree. Per-library roots keep each library in + # its OWN directory, so two libraries shipping the same + # relative path can no longer collapse into a single + # silently-picked copy; legacy mode interleaves them under the + # component root (first-resolved wins). + first_copy = lib_dir_name not in lib_prefix_by_name + if _PER_LIB_ROOTS: + if first_copy: + _prefix = _sanitise_lib_dirname(lib_dir_name) + if _prefix in used_prefixes: + _n = 2 + while f'{_prefix}_{_n}' in used_prefixes: + _n += 1 + _prefix = f'{_prefix}_{_n}' + used_prefixes.add(_prefix) + lib_prefix_by_name[lib_dir_name] = _prefix + lib_prefix = lib_prefix_by_name[lib_dir_name] + dest_root = comp_dir / lib_prefix + else: + lib_prefix = '' + lib_prefix_by_name.setdefault(lib_dir_name, '') + dest_root = comp_dir + for f in lib_root.rglob('*'): if not f.is_file(): continue rel_path = f.relative_to(lib_root) if not _should_include(rel_path): continue - # Track file by its relative path to preserve structure - file_key = str(rel_path).replace('\\', '/') + # Track file by its COMPONENT-relative path: it preserves + # structure, and in legacy mode it is also the cross-library + # dedup key. + rel_key = str(rel_path).replace('\\', '/') + file_key = f'{lib_prefix}/{rel_key}' if lib_prefix else rel_key if file_key not in seen_names: - dest = comp_dir / rel_path + dest = dest_root / rel_path dest.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(f, dest) seen_names.add(file_key) if f.suffix in ('.cpp', '.c') and file_key not in cpp_files: cpp_files.append(file_key) + if _PER_LIB_ROOTS and first_copy: + # arduino-cli's `SourceDir`: `/src` for the recursive + # layout, `` for the flat legacy one. Nothing else. + lib_include_roots.append( + f'{lib_prefix}/src' if has_src_layout else lib_prefix + ) + if not has_src_layout and (lib_root / 'utility').is_dir(): + lib_priv_roots.append(f'{lib_prefix}/utility') + # With one include root per library these root-level headers + # are the ONLY ones that can still shadow anything, so they + # must not be silent — the Arduino IDE reports the same + # thing as "Multiple libraries were found for X.h". + _src_dir = (lib_root / 'src') if has_src_layout else lib_root + try: + for _entry in sorted(_src_dir.iterdir()): + if not _entry.is_file(): + continue + if _entry.suffix.lower() not in ('.h', '.hpp', '.hh', '.hxx'): + continue + _owner = root_header_owner.setdefault(_entry.name, lib_dir_name) + if _owner != lib_dir_name: + logger.warning( + f'[espidf] header collision: <{_entry.name}> is ' + f'published by both "{_owner}" and ' + f'"{lib_dir_name}" — "{_owner}" wins (its ' + f'include root comes first)' + ) + if _entry.name.lower() in self._SHADOW_STD_HEADERS: + logger.warning( + f'[espidf] "{lib_dir_name}" publishes ' + f'{_entry.name} at its include root — that ' + f'shadows the toolchain/core header for every ' + f'library in this build' + ) + except OSError: + pass + # Scan newly copied headers for transitive includes. # Use rglob so libs with `src/` layout (e.g. GxEPD2, ArduinoJson) # are scanned recursively — otherwise their headers live under @@ -1713,7 +1842,10 @@ class ESPIDFCompiler: # ALL header extensions (.h/.hpp/.hh/.hxx/.inc): C++ libs like # M5Unified put the transitive `#include ` in a .hpp, # so a `*.h`-only scan silently drops that dependency. - for lib_file in comp_dir.rglob('*'): + # Per-library roots let this scan the library that was JUST + # copied instead of re-walking every library merged so far — + # same result (resolved_headers dedups), a fraction of the I/O. + for lib_file in (dest_root if _PER_LIB_ROOTS else comp_dir).rglob('*'): if lib_file.suffix.lower() not in ('.h', '.hpp', '.hh', '.hxx', '.inc'): continue try: @@ -1748,26 +1880,50 @@ class ESPIDFCompiler: # a stray `#include ` deep in FreeRTOS resolves to the # library's copy and breaks the build. The files stay copied so the # owning library's file-relative includes still resolve. - shadow_dirs: set[str] = set() - for file_key in seen_names: - if PurePosixPath(file_key).name.lower() in self._SHADOW_STD_HEADERS: - parent = str(PurePosixPath(file_key).parent) - if parent and parent != '.': - shadow_dirs.add(parent) - - include_dirs: set[str] = {'.'} - for file_key in seen_names: - parent = str(PurePosixPath(file_key).parent) - if parent and parent != '.' and parent not in shadow_dirs: - include_dirs.add(parent) - - if shadow_dirs: - logger.info( - f'[espidf] kept {len(shadow_dirs)} dir(s) off -I to avoid ' - f'shadowing standard headers: {sorted(shadow_dirs)}' + priv_include_dirs_line = '' + if _PER_LIB_ROOTS: + # ONE include root per library, in resolution order. This is the + # whole fix: a library's internals stay reachable by its own sources + # through file-relative includes, and by nobody else. No denylist + # needed for subdirectories because no subdirectory is ever exported. + include_dirs_line = 'INCLUDE_DIRS ' + ' '.join( + f'"{d}"' for d in ['.'] + lib_include_roots ) + if lib_priv_roots: + priv_include_dirs_line = 'PRIV_INCLUDE_DIRS ' + ' '.join( + f'"{d}"' for d in lib_priv_roots + ) + logger.info( + f'[espidf] include roots ({len(lib_include_roots)} librar' + f'{"y" if len(lib_include_roots) == 1 else "ies"}): ' + f'{lib_include_roots}' + + (f' | private: {lib_priv_roots}' if lib_priv_roots else '') + ) + else: + # Legacy interleaved layout: every directory holding a copied file + # is exported, minus the hand-maintained shadow denylist. + shadow_dirs: set[str] = set() + for file_key in seen_names: + if PurePosixPath(file_key).name.lower() in self._SHADOW_STD_HEADERS: + parent = str(PurePosixPath(file_key).parent) + if parent and parent != '.': + shadow_dirs.add(parent) - include_dirs_line = 'INCLUDE_DIRS ' + ' '.join(f'"{d}"' for d in sorted(include_dirs)) + include_dirs: set[str] = {'.'} + for file_key in seen_names: + parent = str(PurePosixPath(file_key).parent) + if parent and parent != '.' and parent not in shadow_dirs: + include_dirs.add(parent) + + if shadow_dirs: + logger.info( + f'[espidf] kept {len(shadow_dirs)} dir(s) off -I to avoid ' + f'shadowing standard headers: {sorted(shadow_dirs)}' + ) + + include_dirs_line = 'INCLUDE_DIRS ' + ' '.join( + f'"{d}"' for d in sorted(include_dirs) + ) # LovyanGFX's Bus_EPD.cpp (e-paper bus, compiled unconditionally as part # of the M5GFX merge) includes , which is provided by @@ -1850,12 +2006,20 @@ class ESPIDFCompiler: ' -Wno-error=uninitialized)\n' ) + _priv_cmake_line = ( + f' {priv_include_dirs_line}\n' if priv_include_dirs_line else '' + ) cmake_content = ( - '# Auto-generated by Velxio — all user libraries merged into one component.\n' - '# Directory structure preserved for libraries like ArduinoJson with src/ layout.\n' + '# Auto-generated by Velxio — user libraries as one IDF component.\n' + '# Each library keeps its own subtree and exports exactly ONE include\n' + '# root (/src, or for the flat legacy layout), matching\n' + "# arduino-cli's resolution semantics. Library internals stay\n" + '# reachable through file-relative includes and never enter another\n' + "# library's — or the core's — include path.\n" 'idf_component_register(\n' f' {srcs_line}\n' f' {include_dirs_line}\n' + f'{_priv_cmake_line}' f' REQUIRES {arduino_comp_name}{extra_requires}\n' ')\n' f'{defs_block}' @@ -3310,6 +3474,10 @@ class ESPIDFCompiler: # never share a configured build/ — the cmake cache and # every object file are tied to the IDF tree. + f'|idf:{5 if use_idf5 else 4}|ard:{int(arduino_mode)}' + # The user-libs layout changes every object path and the + # component's include list, so the two modes must never + # share a configured build/ (stale objects + cmake cache). + + f'|libroots:{int(_PER_LIB_ROOTS)}' + _lang_token ).encode() ).hexdigest()[:12] diff --git a/test/backend/unit/test_espidf_compiler.py b/test/backend/unit/test_espidf_compiler.py index 332b2d37..8fec601d 100644 --- a/test/backend/unit/test_espidf_compiler.py +++ b/test/backend/unit/test_espidf_compiler.py @@ -35,6 +35,24 @@ def make_compiler() -> ESPIDFCompiler: return comp +def merged_has(component_root: Path, *rel_suffixes: str) -> bool: + """True when every suffix exists somewhere under the merged component. + + The component's internal layout is a build-system detail that changed with + VELXIO_PER_LIB_ROOTS (each library now keeps its own subtree). These tests + care that a file WAS merged, not where — the exported include roots are + asserted directly in test_espidf_include_roots.py. + """ + found = { + str(p.relative_to(component_root)).replace('\\', '/') + for p in component_root.rglob('*') if p.is_file() + } + return all( + any(f == suffix or f.endswith('/' + suffix) for f in found) + for suffix in rel_suffixes + ) + + def make_library(libs_dir: Path, lib_name: str, headers: list[str], sources: list[str], use_src_subdir: bool = False) -> Path: lib_dir = libs_dir / lib_name @@ -283,7 +301,7 @@ class TestResolveLibraryComponents(unittest.TestCase): """Adafruit_GFX.h directly in sketch → copied into user_libs_all.""" self._resolve(['Adafruit_GFX.h']) self.assertTrue( - (self.user_libs / 'user_libs_all' / 'Adafruit_GFX.h').exists(), + merged_has(self.user_libs / 'user_libs_all', 'Adafruit_GFX.h'), 'Adafruit_GFX.h not copied into user_libs_all — will not be on include path', ) @@ -291,25 +309,25 @@ class TestResolveLibraryComponents(unittest.TestCase): """SSD1306 sketch: both GFX and SSD1306 files must be in user_libs_all.""" self._resolve(['Wire.h', 'Adafruit_GFX.h', 'Adafruit_SSD1306.h']) all_dir = self.user_libs / 'user_libs_all' - self.assertTrue((all_dir / 'Adafruit_GFX.h').exists(), + self.assertTrue(merged_has(all_dir, 'Adafruit_GFX.h'), 'Adafruit_GFX.h missing — SSD1306.cpp will fail to compile') - self.assertTrue((all_dir / 'Adafruit_SSD1306.h').exists(), + self.assertTrue(merged_has(all_dir, 'Adafruit_SSD1306.h'), 'Adafruit_SSD1306.h missing') - self.assertTrue((all_dir / 'Adafruit_GFX.cpp').exists()) - self.assertTrue((all_dir / 'Adafruit_SSD1306.cpp').exists()) + self.assertTrue(merged_has(all_dir, 'Adafruit_GFX.cpp')) + self.assertTrue(merged_has(all_dir, 'Adafruit_SSD1306.cpp')) def test_wire_files_not_in_merged_component(self): """Wire is a core esp32 lib — must NOT be copied into user_libs_all.""" self._resolve(['Wire.h', 'Adafruit_SSD1306.h']) all_dir = self.user_libs / 'user_libs_all' - self.assertFalse((all_dir / 'Wire.h').exists(), + self.assertFalse(merged_has(all_dir, 'Wire.h'), 'Wire.h was incorrectly copied — it is already in arduino-esp32') def test_transitive_gfx_discovered_via_ssd1306(self): """When sketch only includes SSD1306, GFX is discovered transitively.""" self._resolve(['Adafruit_SSD1306.h']) self.assertTrue( - (self.user_libs / 'user_libs_all' / 'Adafruit_GFX.h').exists(), + merged_has(self.user_libs / 'user_libs_all', 'Adafruit_GFX.h'), 'Transitive Adafruit_GFX.h not discovered — SSD1306.h includes it', ) @@ -560,15 +578,15 @@ class TestHeaderOnlyLibrary(unittest.TestCase): def test_header_only_library_still_exposes_its_header(self): cmake = self._cmake_for(['ArduinoJson.h']) self.assertIn('idf_component_register', cmake) - # The library's src/ layout is preserved, so the header lands at - # src/ArduinoJson.h and "src" has to be on the include path for the + # The library's src/ layout is preserved, and its include ROOT (the + # `src` dir, wherever the layout puts it) must be exported for the # sketch's #include to resolve. merged = self.user_libs / 'user_libs_all' self.assertTrue( - (merged / 'src' / 'ArduinoJson.h').exists(), + merged_has(merged, 'src/ArduinoJson.h'), 'ArduinoJson.h not copied — the sketch would fail on "No such file"', ) - self.assertIn('INCLUDE_DIRS "." "src"', cmake) + self.assertRegex(cmake, r'INCLUDE_DIRS "\." "(?:[^"]+/)?src"') def test_library_with_sources_keeps_the_progmem_definitions(self): cmake = self._cmake_for(['LiquidCrystal_I2C.h']) diff --git a/test/backend/unit/test_espidf_include_roots.py b/test/backend/unit/test_espidf_include_roots.py new file mode 100644 index 00000000..a0d93b12 --- /dev/null +++ b/test/backend/unit/test_espidf_include_roots.py @@ -0,0 +1,332 @@ +""" +Regression tests for the library include-root layout (VELXIO_PER_LIB_ROOTS). + +Every case here is a real incident. The merged component used to copy all +libraries into one interleaved tree and export EVERY directory that held a +file, so a library's private internals sat on the global -I and hijacked +`#include`s issued by other libraries, by the arduino-esp32 core and by +ESP-IDF itself: + + * FastLED ships a host-test stub `src/platforms/stub/Arduino.h`. With that + directory exported, U8g2's `#include ` resolved to the stub — + 'yield' was not declared, and fl::string polluted String overload + resolution inside the core's WString.h. + * FastLED ships `src/fl/stl/asio/http/http_parser.h`. Exported, it shadowed + the IDF http_parser component header that esp_http_server.h includes, and + every httpd_method_t use failed to compile. + * LovyanGFX ships `src/lgfx/internal/limits.h`. Exported, FreeRTOS's + portmacro.h picked it up inside an extern "C" block. + +The fix is arduino-cli's model: one subtree per library, exactly one exported +include root per library (`/src`, or `` for the flat legacy layout), +`utility/` private. These tests assert the property — no directory other than a +library's own include root is ever exported — rather than any single denylist +entry, so a new library shipping a new generically-named internal cannot +reintroduce the class. + +Run from the repo root: + python -m pytest test/backend/unit/test_espidf_include_roots.py -v +""" + +import re +import shutil +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).parent.parent.parent.parent / 'backend')) + +from app.services import espidf_compiler as espidf_mod +from app.services.espidf_compiler import ESPIDFCompiler + + +def make_compiler() -> ESPIDFCompiler: + comp = ESPIDFCompiler.__new__(ESPIDFCompiler) + comp.idf_path = '' + comp.arduino_path = '' + comp.has_arduino = False + comp.idf5_path = '' + comp.arduino5_path = '' + comp.has_arduino5 = False + return comp + + +def write(path: Path, text: str = '// x') -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding='utf-8') + + +class IncludeRootsBase(unittest.TestCase): + """Builds a libraries dir that mirrors the real offenders.""" + + def setUp(self): + self.tmp = Path(tempfile.mkdtemp()) + self.arduino_libs = self.tmp / 'libraries' + self.esp32_libs = self.tmp / 'esp32_libs' + self.user_libs = self.tmp / 'user_libs' + for d in (self.arduino_libs, self.esp32_libs, self.user_libs): + d.mkdir(parents=True) + self.comp = make_compiler() + + # FastLED: recursive layout, ships an Arduino.h stub and an + # http_parser.h deep inside src/, plus a unity build that reaches them + # through file-relative includes. + fl = self.arduino_libs / 'fastled@3.10.5-017d142ff4ff' + write(fl / 'src' / 'FastLED.h', '#include "platforms/stub/Arduino.h"') + write(fl / 'src' / 'FastLED.cpp') + write(fl / 'src' / 'platforms' / 'stub' / 'Arduino.h', '// host-test stub') + write(fl / 'src' / 'fl' / 'stl' / 'asio' / 'http' / 'http_parser.h') + write(fl / 'src' / 'fl' / 'stl' / 'string.h') + write(fl / 'library.properties', 'name=FastLED\narchitectures=*\n') + + # U8g2: recursive layout, includes expecting the CORE one. + u8 = self.arduino_libs / 'U8g2' + write(u8 / 'src' / 'U8g2lib.h', '#include ') + write(u8 / 'src' / 'U8g2lib.cpp', '#include ') + # A benign internal subdirectory: not on any denylist, so it is the + # clean way to show what each layout exports. + write(u8 / 'src' / 'fonts' / 'Font5x7.h') + write(u8 / 'library.properties', 'name=U8g2\narchitectures=*\n') + + # LovyanGFX-style internal limits.h. + lg = self.arduino_libs / 'M5GFX' + write(lg / 'src' / 'M5GFX.h') + write(lg / 'src' / 'M5GFX.cpp') + write(lg / 'src' / 'lgfx' / 'internal' / 'limits.h') + write(lg / 'library.properties', 'name=M5GFX\narchitectures=esp32\n') + + # Legacy flat layout with a utility/ dir (Arduino keeps it private). + legacy = self.arduino_libs / 'LegacyLib' + write(legacy / 'LegacyLib.h') + write(legacy / 'LegacyLib.cpp') + write(legacy / 'utility' / 'twi.h') + write(legacy / 'utility' / 'twi.c') + + def tearDown(self): + shutil.rmtree(self.tmp) + + def resolve(self, headers): + return self.comp._resolve_library_components( + headers, self.arduino_libs, self.esp32_libs, + 'arduino-esp32', self.user_libs, + ) + + def cmake_text(self) -> str: + return (self.user_libs / 'user_libs_all' / 'CMakeLists.txt').read_text() + + def exported_dirs(self) -> list[str]: + """Directories on the component's PUBLIC include path.""" + m = re.search(r'INCLUDE_DIRS ([^\n]*)', self.cmake_text()) + self.assertIsNotNone(m, 'component CMakeLists has no INCLUDE_DIRS') + return re.findall(r'"([^"]+)"', m.group(1)) + + def private_dirs(self) -> list[str]: + m = re.search(r'PRIV_INCLUDE_DIRS ([^\n]*)', self.cmake_text()) + return re.findall(r'"([^"]+)"', m.group(1)) if m else [] + + def merged_files(self) -> list[str]: + root = self.user_libs / 'user_libs_all' + return sorted( + str(p.relative_to(root)).replace('\\', '/') + for p in root.rglob('*') if p.is_file() + ) + + +class TestPerLibraryIncludeRoots(IncludeRootsBase): + """Default mode: one subtree and one exported include root per library.""" + + def setUp(self): + super().setUp() + self._saved = espidf_mod._PER_LIB_ROOTS + espidf_mod._PER_LIB_ROOTS = True + + def tearDown(self): + espidf_mod._PER_LIB_ROOTS = self._saved + super().tearDown() + + def test_only_library_roots_are_exported(self): + """THE property: nothing but '.' and one root per library is on -I.""" + self.resolve(['FastLED.h', 'U8g2lib.h', 'M5GFX.h']) + dirs = self.exported_dirs() + self.assertEqual(dirs[0], '.') + roots = dirs[1:] + self.assertEqual(len(roots), 3, f'expected one root per library, got {roots}') + for root in roots: + self.assertRegex( + root, r'^[^/]+/src$', + f'{root} is not a library include root — only /src may be exported', + ) + + def test_fastled_stub_arduino_h_is_not_exported(self): + """The 2026-08-19 incident: U8g2's must reach the core.""" + self.resolve(['FastLED.h', 'U8g2lib.h']) + for d in self.exported_dirs(): + self.assertFalse( + d.endswith('platforms/stub'), + f'{d} exported — FastLED\'s Arduino.h stub shadows the core again', + ) + + def test_fastled_http_parser_dir_is_not_exported(self): + """esp_http_server.h's must reach the IDF component.""" + self.resolve(['FastLED.h']) + for d in self.exported_dirs(): + self.assertFalse( + d.endswith('fl/stl/asio/http'), + f'{d} exported — FastLED\'s http_parser.h shadows the IDF header again', + ) + + def test_lovyangfx_internal_limits_dir_is_not_exported(self): + """FreeRTOS portmacro.h's must reach the toolchain.""" + self.resolve(['M5GFX.h']) + for d in self.exported_dirs(): + self.assertFalse( + d.endswith('lgfx/internal'), + f'{d} exported — LovyanGFX\'s limits.h shadows the toolchain again', + ) + + def test_benign_internal_subdirectory_is_not_exported_either(self): + """Not a denylist: NO subdirectory is exported, harmless or not.""" + self.resolve(['U8g2lib.h']) + self.assertFalse( + any(d.endswith('/fonts') for d in self.exported_dirs()), + 'a library internal subdir reached -I — the class is back', + ) + self.assertTrue( + any(f.endswith('src/fonts/Font5x7.h') for f in self.merged_files()), + 'the header must still be copied for file-relative includes', + ) + + def test_internals_are_still_copied(self): + """Exclusion is from -I only: file-relative includes must keep working.""" + files = self.merged_files() + self.resolve(['FastLED.h']) + files = self.merged_files() + self.assertTrue( + any(f.endswith('src/platforms/stub/Arduino.h') for f in files), + 'the stub was dropped — FastLED\'s own relative include breaks', + ) + self.assertTrue( + any(f.endswith('src/fl/stl/asio/http/http_parser.h') for f in files), + ) + + def test_each_library_keeps_its_own_subtree(self): + """No interleaving: a library's files live under its own directory.""" + self.resolve(['FastLED.h', 'U8g2lib.h']) + files = self.merged_files() + fastled = [f for f in files if f.endswith('FastLED.h')] + u8g2 = [f for f in files if f.endswith('U8g2lib.h')] + self.assertTrue(fastled and u8g2) + self.assertNotEqual( + fastled[0].split('/')[0], u8g2[0].split('/')[0], + 'libraries share a subtree — same-path files can collapse silently', + ) + + def test_same_relative_path_in_two_libraries_survives(self): + """Legacy dedup dropped one copy; per-library subtrees keep both.""" + a = self.arduino_libs / 'LibA' + write(a / 'src' / 'LibA.h') + write(a / 'src' / 'utility' / 'helper.h', '// A') + b = self.arduino_libs / 'LibB' + write(b / 'src' / 'LibB.h') + write(b / 'src' / 'utility' / 'helper.h', '// B') + self.resolve(['LibA.h', 'LibB.h']) + helpers = [f for f in self.merged_files() if f.endswith('utility/helper.h')] + self.assertEqual(len(helpers), 2, f'one copy was silently dropped: {helpers}') + + def test_legacy_utility_dir_is_private_not_public(self): + """arduino-cli scopes utility/ to the owning library, never globally.""" + self.resolve(['LegacyLib.h']) + self.assertFalse( + any(d.endswith('/utility') for d in self.exported_dirs()), + 'utility/ is on the public include path — a generic utility/debug.h ' + 'would shadow every other library\'s', + ) + self.assertTrue( + any(d.endswith('/utility') for d in self.private_dirs()), + 'utility/ must stay reachable privately for the library\'s own sources', + ) + + def test_flat_library_exports_its_root(self): + self.resolve(['LegacyLib.h']) + roots = [d for d in self.exported_dirs() if d != '.'] + self.assertTrue( + any(not r.endswith('/src') for r in roots), + f'flat-layout library must export its own root, got {roots}', + ) + + def test_sources_are_registered_with_library_prefixed_paths(self): + self.resolve(['FastLED.h']) + srcs = re.search(r'SRCS ([^\n]*)', self.cmake_text()).group(1) + self.assertIn('/src/FastLED.cpp', srcs) + + def test_versioned_cache_dirname_is_path_safe(self): + """Cache entries are `name@version-sha`; the subdir must stay valid.""" + self.resolve(['FastLED.h']) + roots = [d for d in self.exported_dirs() if d != '.'] + self.assertTrue(roots) + for r in roots: + self.assertNotRegex(r, r'[<>:"|?*\\\\]', f'unsafe path component: {r}') + + def test_transitive_dependency_still_resolves(self): + """Per-library scanning must not lose transitive pulls.""" + dep = self.arduino_libs / 'DepLib' + write(dep / 'src' / 'DepLib.h') + write(dep / 'src' / 'DepLib.cpp') + top = self.arduino_libs / 'TopLib' + write(top / 'src' / 'TopLib.h', '#include ') + write(top / 'src' / 'TopLib.cpp') + self.resolve(['TopLib.h']) + files = self.merged_files() + self.assertTrue(any(f.endswith('DepLib.h') for f in files), + 'transitive dependency was not merged') + + def test_still_one_component(self): + names, header_to_comp = self.resolve(['FastLED.h']) + self.assertEqual(names, ['user_libs_all']) + self.assertEqual(header_to_comp['FastLED.h'], 'user_libs_all') + + +class TestLegacyLayoutStillWorks(IncludeRootsBase): + """VELXIO_PER_LIB_ROOTS=0 must reproduce the old behaviour exactly.""" + + def setUp(self): + super().setUp() + self._saved = espidf_mod._PER_LIB_ROOTS + espidf_mod._PER_LIB_ROOTS = False + + def tearDown(self): + espidf_mod._PER_LIB_ROOTS = self._saved + super().tearDown() + + def test_files_are_interleaved_at_component_root(self): + self.resolve(['FastLED.h', 'U8g2lib.h']) + files = self.merged_files() + self.assertIn('src/FastLED.h', files) + self.assertIn('src/U8g2lib.h', files) + + def test_subdirectories_are_exported(self): + """The legacy mechanism: every dir holding a file is exported, and only + the hand-maintained denylist keeps the dangerous ones out. A benign + internal subdir therefore DOES reach -I here — which is exactly the + design the per-library layout replaces.""" + self.resolve(['U8g2lib.h']) + self.assertTrue( + any(d.endswith('fonts') for d in self.exported_dirs()), + 'legacy mode should still export internal subdirectories', + ) + + def test_denylisted_dirs_are_kept_off_the_include_path(self): + """The old safety net still works when the flag is off.""" + self.resolve(['FastLED.h']) + for d in self.exported_dirs(): + self.assertFalse(d.endswith('platforms/stub'), d) + self.assertFalse(d.endswith('fl/stl/asio/http'), d) + + def test_no_private_include_dirs(self): + self.resolve(['LegacyLib.h']) + self.assertEqual(self.private_dirs(), []) + + +if __name__ == '__main__': + unittest.main()