fix(espidf): no build-config header may drag in a library
Generalises the lv_conf.h fix, because it was never an LVGL problem. One live compile resolved SIX config headers against the shared library dir in a single build (sdkconfig.h, lv_conf.h, lv_conf_kconfig.h, lv_rt_thread_conf.h, User_Config.h, zconf.h): six chances to merge a library nobody asked for, and one of them did exactly that. The rule is now about the shape of the name (*conf.h, *config.h, *_setup.h, plus the known ones) and applies only to headers reached TRANSITIVELY -- a config header the sketch includes itself is intent and still resolves. Why skipping the lookup is safe rather than merely convenient: once a library is merged its own headers are on the include path, so a config header shipped BY the library that asks for it resolves at compile time with no lookup at all. The lookup can only ever pull in a DIFFERENT library, which for a config header is never what the project meant. A genuinely missing header now fails where it should -- named, in the compiler output -- instead of inside the sources of a library the sketch never mentioned. Tests cover all six shapes, the direct-include exception and an ordinary transitive dependency (Adafruit GFX -> BusIO) that must keep merging.
This commit is contained in:
parent
467cf108ff
commit
a92c59ee50
|
|
@ -1538,9 +1538,36 @@ class ESPIDFCompiler:
|
||||||
# are copied into the build's own source dir and found there, not through
|
# are copied into the build's own source dir and found there, not through
|
||||||
# this library scan.
|
# this library scan.
|
||||||
_PROJECT_SUPPLIED_HEADERS: frozenset[str] = frozenset({
|
_PROJECT_SUPPLIED_HEADERS: frozenset[str] = frozenset({
|
||||||
'lv_conf.h', 'lv_drv_conf.h',
|
'lv_conf.h', 'lv_drv_conf.h', 'user_config.h', 'user_setup.h',
|
||||||
|
'sdkconfig.h', 'zconf.h',
|
||||||
})
|
})
|
||||||
|
|
||||||
|
# ...and the general shape of one. Config headers are named by convention
|
||||||
|
# across the whole Arduino ecosystem, and one live build showed SIX of
|
||||||
|
# them being resolved against the shared library dir in a single compile
|
||||||
|
# (sdkconfig.h, lv_conf.h, lv_conf_kconfig.h, lv_rt_thread_conf.h,
|
||||||
|
# User_Config.h, zconf.h) — six chances to merge a library nobody asked
|
||||||
|
# for.
|
||||||
|
_CONFIG_HEADER_SUFFIXES: tuple[str, ...] = ('conf.h', 'config.h', '_setup.h')
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def _is_project_config_header(cls, header: str) -> bool:
|
||||||
|
"""Is this a build-configuration header rather than a library API?
|
||||||
|
|
||||||
|
Why skipping the library scan for these is SAFE rather than merely
|
||||||
|
convenient: once a library is merged, its own headers are on the
|
||||||
|
include path, so a config header shipped BY the library that asks for
|
||||||
|
it resolves at compile time without any lookup. The lookup can
|
||||||
|
therefore only ever pull in a DIFFERENT library — which for a config
|
||||||
|
header is never what the project meant. If the header really is
|
||||||
|
missing the compiler says so, naming it, instead of dying inside the
|
||||||
|
sources of a library the sketch never mentioned.
|
||||||
|
"""
|
||||||
|
name = Path(header).name.lower()
|
||||||
|
return name in cls._PROJECT_SUPPLIED_HEADERS or name.endswith(
|
||||||
|
cls._CONFIG_HEADER_SUFFIXES
|
||||||
|
)
|
||||||
|
|
||||||
# Basenames of C/C++ standard headers. A user library that ships a private
|
# Basenames of C/C++ standard headers. A user library that ships a private
|
||||||
# header with one of these names (e.g. LovyanGFX's src/lgfx/internal/
|
# header with one of these names (e.g. LovyanGFX's src/lgfx/internal/
|
||||||
# limits.h, algorithm.h, memory.h, alloca.h) must NOT put that directory on
|
# limits.h, algorithm.h, memory.h, alloca.h) must NOT put that directory on
|
||||||
|
|
@ -1849,6 +1876,10 @@ class ESPIDFCompiler:
|
||||||
used_prefixes: set[str] = set()
|
used_prefixes: set[str] = set()
|
||||||
|
|
||||||
headers_to_resolve: list[str] = list(ext_headers)
|
headers_to_resolve: list[str] = list(ext_headers)
|
||||||
|
# Headers the SKETCH itself included. The config-header guard below
|
||||||
|
# applies only to transitively-discovered ones: a direct include is an
|
||||||
|
# explicit statement of intent and stays resolvable.
|
||||||
|
direct_headers: set[str] = set(ext_headers)
|
||||||
# Headers the SKETCH itself includes (vs transitive pulls found by
|
# Headers the SKETCH itself includes (vs transitive pulls found by
|
||||||
# re-scanning copied library headers). The architecture guard is
|
# re-scanning copied library headers). The architecture guard is
|
||||||
# advisory for these: the user explicitly asked for the library, and
|
# advisory for these: the user explicitly asked for the library, and
|
||||||
|
|
@ -1879,13 +1910,15 @@ class ESPIDFCompiler:
|
||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Project-supplied config headers (lv_conf.h ...). See
|
# Build-configuration headers pulled in TRANSITIVELY. See
|
||||||
# _PROJECT_SUPPLIED_HEADERS: resolving these globally drags in
|
# _is_project_config_header: resolving these against the shared
|
||||||
# whatever unrelated library happens to vendor a copy.
|
# library dir drags in whatever unrelated library happens to
|
||||||
if header in self._PROJECT_SUPPLIED_HEADERS:
|
# vendor a copy, and that library's sources then fail on their own
|
||||||
|
# API in a build that never asked for them.
|
||||||
|
if header not in direct_headers and self._is_project_config_header(header):
|
||||||
logger.info(
|
logger.info(
|
||||||
f'[espidf] <{header}> is a project-supplied config header '
|
f'[espidf] <{header}> is a build-config header pulled in '
|
||||||
f'— never resolving against user libraries'
|
f'transitively — not resolving it against user libraries'
|
||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -200,6 +200,54 @@ class TestProjectSuppliedHeaders(unittest.TestCase):
|
||||||
"a sketch that only includes <lvgl.h> must not merge the glue library",
|
"a sketch that only includes <lvgl.h> must not merge the glue library",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_the_guard_is_generic_not_an_lvgl_special_case(self):
|
||||||
|
"""Every config-header shape, not a hardcoded list of two names.
|
||||||
|
|
||||||
|
One live compile resolved six of these against the shared library dir
|
||||||
|
in a single build, so the rule has to be about the shape of the name.
|
||||||
|
"""
|
||||||
|
for cfg in (
|
||||||
|
"lv_conf.h", "lv_conf_kconfig.h", "lv_rt_thread_conf.h",
|
||||||
|
"User_Config.h", "sdkconfig.h", "zconf.h", "User_Setup.h",
|
||||||
|
):
|
||||||
|
with self.subTest(header=cfg):
|
||||||
|
shutil.rmtree(self.libs, ignore_errors=True)
|
||||||
|
_mk(self.libs / "toplib" / "src" / "Top.h", f'#include "{cfg}"\n')
|
||||||
|
_mk(self.libs / "toplib" / "library.properties", "name=TopLib\n")
|
||||||
|
# An unrelated library that happens to vendor that config header.
|
||||||
|
_mk(self.libs / "zzz-stray" / "src" / cfg)
|
||||||
|
_mk(self.libs / "zzz-stray" / "library.properties", "name=Stray\n")
|
||||||
|
merged: dict[str, str] = {}
|
||||||
|
self._resolve(["Top.h"], merged)
|
||||||
|
self.assertEqual(merged.get("Top.h"), "TopLib")
|
||||||
|
self.assertNotIn(
|
||||||
|
"Stray", merged.values(),
|
||||||
|
f"<{cfg}> must not drag in an unrelated library",
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_a_direct_sketch_include_is_still_resolved(self):
|
||||||
|
"""The guard is for transitive pulls only.
|
||||||
|
|
||||||
|
A sketch that includes a config header itself is stating intent, and
|
||||||
|
that path keeps working exactly as before.
|
||||||
|
"""
|
||||||
|
_mk(self.libs / "somelib" / "src" / "my_config.h")
|
||||||
|
_mk(self.libs / "somelib" / "library.properties", "name=SomeLib\n")
|
||||||
|
merged: dict[str, str] = {}
|
||||||
|
self._resolve(["my_config.h"], merged)
|
||||||
|
self.assertEqual(merged.get("my_config.h"), "SomeLib")
|
||||||
|
|
||||||
|
def test_ordinary_transitive_dependencies_still_merge(self):
|
||||||
|
"""The guard must not touch normal library-to-library deps."""
|
||||||
|
_mk(self.libs / "adafruitgfx" / "src" / "Adafruit_GFX.h",
|
||||||
|
'#include <Adafruit_I2CDevice.h>\n')
|
||||||
|
_mk(self.libs / "adafruitgfx" / "library.properties", "name=Adafruit GFX Library\n")
|
||||||
|
_mk(self.libs / "adafruitbusio" / "src" / "Adafruit_I2CDevice.h")
|
||||||
|
_mk(self.libs / "adafruitbusio" / "library.properties", "name=Adafruit BusIO\n")
|
||||||
|
merged: dict[str, str] = {}
|
||||||
|
self._resolve(["Adafruit_GFX.h"], merged)
|
||||||
|
self.assertEqual(merged.get("Adafruit_I2CDevice.h"), "Adafruit BusIO")
|
||||||
|
|
||||||
def test_a_library_that_owns_lv_conf_is_still_reachable_by_its_own_header(self):
|
def test_a_library_that_owns_lv_conf_is_still_reachable_by_its_own_header(self):
|
||||||
# The guard is scoped to the config header, not to the library: a
|
# The guard is scoped to the config header, not to the library: a
|
||||||
# project that really wants the glue can still include its header.
|
# project that really wants the glue can still include its header.
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue