Address review: quote ninja command paths, refuse unverified downloads, extend lib_ignore and build_unflags parity

This commit is contained in:
J. Nick Koston
2026-08-20 04:23:14 -05:00
parent 005c01e1e2
commit c579a2792f
6 changed files with 128 additions and 25 deletions
+22 -3
View File
@@ -64,6 +64,11 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary:
src_dir = build.get("srcDir") or next(
(d for d in ("src", "Src") if (read_path / d).is_dir()), "."
)
if "srcDir" in build and not (read_path / src_dir).is_dir():
# A silently empty source set would surface as link errors instead
_LOGGER.warning(
"Library %s declares srcDir %s which does not exist", name, src_dir
)
src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER))
# PlatformIO shell-lexes each build.flags entry
@@ -100,9 +105,9 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary:
for d in [include_dir, src_dir, *include_flags]:
if (path := (read_path / d)).is_dir():
lib.include_dirs.append(path.resolve())
elif d in include_flags:
# The includeDir/srcDir defaults are probes; an explicit -I that
# does not resolve is a manifest or packaging error worth naming
elif d in include_flags or (d == include_dir and "includeDir" in build):
# The includeDir/srcDir defaults are probes; an explicitly
# declared path that does not resolve is a manifest error
_LOGGER.warning(
"Library %s declares include dir %s which does not exist", name, d
)
@@ -127,7 +132,15 @@ def resolve_libraries(framework_path: Path) -> list[ArduinoLibrary]:
"""Resolve every ``cg.add_library()`` entry into an :class:`ArduinoLibrary`."""
bundled: list[ArduinoLibrary] = []
external: list[Library] = []
# PlatformIO's lib_ignore covers framework-bundled libraries too; the
# shared converter only filters the registry/git ones.
lib_ignore = {
ignore.split("/")[-1].lower()
for ignore in CORE.platformio_options.get("lib_ignore", [])
}
for library in CORE.platformio_libraries.values():
if library.name and library.name.split("/")[-1].lower() in lib_ignore:
continue
# A version pin means a registry package ("pngle@1.1.0"), never a
# framework-bundled library.
if (
@@ -161,9 +174,15 @@ def resolve_libraries(framework_path: Path) -> list[ArduinoLibrary]:
or not (framework_path / "libraries" / name).is_dir()
):
continue
if name.lower() in lib_ignore:
continue
try:
check_library_data(dep, ESP8266_PLATFORM, "arduino")
except InvalidLibrary as err:
# check_library_data's only raise is the platform filter, and
# rejecting another platform's dependency of a cross-platform
# manifest is routine (every ESPAsyncWebServer build hits it);
# a warning here would be noise, and the reason is in the log.
_LOGGER.debug("Skipping bundled dependency %s: %s", name, err)
continue
bundled_names.add(name)
+9 -5
View File
@@ -139,11 +139,15 @@ def _registry_download(
# ensure_list: a bare string would make ``in`` a substring test
systems = ensure_list(file.get("system") or "*")
if "*" in systems or system in systems:
return (
file["download_url"],
(file.get("checksum") or {}).get("sha256"),
file.get("size"),
)
sha256 = (file.get("checksum") or {}).get("sha256")
if not sha256:
# Never extract an unverified archive; the registry
# publishes a checksum for every package file.
raise EsphomeError(
f"The package registry returned no sha256 for "
f"{package} {version}; refusing the unverified download"
)
return (file["download_url"], sha256, file.get("size"))
raise EsphomeError(f"No {package} {version} build for this platform ({system})")
raise EsphomeError(f"{package} {version} not found in the package registry")
+13 -11
View File
@@ -489,14 +489,16 @@ def write_project(paths: dict[str, Path]) -> bool:
)
asflags = _ASFLAGS + defines + includes + project_compile_flags
# build_unflags applies to the framework flag sets too, as it does under
# PlatformIO (a silently ignored ``build_unflags: -Os`` would diverge).
if unflags := set(CORE.build_unflags):
# build_unflags applies to the framework flag sets too (compile and link),
# as under PlatformIO (a silently ignored ``build_unflags: -Os`` would
# diverge between the toolchains).
unflags = set(CORE.build_unflags)
if unflags:
cflags = [f for f in cflags if f not in unflags]
cxxflags = [f for f in cxxflags if f not in unflags]
asflags = [f for f in asflags if f not in unflags]
link_flags = list(_LINKFLAGS)
link_flags = [f for f in _LINKFLAGS if f not in unflags]
if esp8266_data.get(KEY_SCANF_FLOAT):
link_flags += ["-u", "_scanf_float"]
link_flags += project_link_flags
@@ -530,35 +532,35 @@ def write_project(paths: dict[str, Path]) -> bool:
f"ccache = {_q(ccache) if ccache else ''}",
"",
"rule cc",
" command = $ccache $cc -MMD -MF $out.d $cflags $flags -c $in -o $out",
' command = $ccache $cc -MMD -MF "$out.d" $cflags $flags -c "$in" -o "$out"',
" depfile = $out.d",
" deps = gcc",
" description = CC $out",
"rule cxx",
" command = $ccache $cxx -MMD -MF $out.d $cxxflags $flags -c $in -o $out",
' command = $ccache $cxx -MMD -MF "$out.d" $cxxflags $flags -c "$in" -o "$out"',
" depfile = $out.d",
" deps = gcc",
" description = CXX $out",
"rule asm",
" command = $ccache $cc -MMD -MF $out.d -x assembler-with-cpp $asflags $flags -c $in -o $out",
' command = $ccache $cc -MMD -MF "$out.d" -x assembler-with-cpp $asflags $flags -c "$in" -o "$out"',
" depfile = $out.d",
" deps = gcc",
" description = AS $out",
"rule ar",
f" command = $python $buildtool ar {_q(toolchain_bin / 'xtensa-lx106-elf-ar')} $out $out.rsp",
f' command = $python $buildtool ar {_q(toolchain_bin / "xtensa-lx106-elf-ar")} "$out" "$out.rsp"',
" rspfile = $out.rsp",
" rspfile_content = $in_newline",
" description = AR $out",
"rule link",
" command = $cxx -o $out $linkflags @$out.rsp $libdirflags -Wl,--start-group $archives $libflags -Wl,--end-group",
' command = $cxx -o "$out" $linkflags @"$out.rsp" $libdirflags -Wl,--start-group $archives $libflags -Wl,--end-group',
" rspfile = $out.rsp",
" rspfile_content = $in_newline",
" description = LINK $out",
"rule elf2bin",
f" command = $python {_q(framework / 'tools' / 'elf2bin.py')} --eboot {_q(framework / 'bootloaders' / 'eboot' / 'eboot.elf')} --app $in --flash_mode {esp8266_data[KEY_FLASH_MODE]} --flash_freq 40 --flash_size {_flash_size_str(flash_ld_name)} --path {_q(toolchain_bin)} --out $out",
f' command = $python {_q(framework / "tools" / "elf2bin.py")} --eboot {_q(framework / "bootloaders" / "eboot" / "eboot.elf")} --app "$in" --flash_mode {esp8266_data[KEY_FLASH_MODE]} --flash_freq 40 --flash_size {_flash_size_str(flash_ld_name)} --path {_q(toolchain_bin)} --out "$out"',
" description = BIN $out",
"rule copy",
" command = $python $buildtool copy $in $out",
' command = $python $buildtool copy "$in" "$out"',
" description = COPY $out",
"",
f"cflags = {' '.join(cflags)}",
@@ -249,6 +249,9 @@ def test_write_project_link_line_and_exclusions(tmp_path: Path) -> None:
assert "-T eagle.flash.4m.ld" in content
# scanf float disabled: the forced-link flag must not appear
assert "_scanf_float" not in content
# Compile inputs and outputs are quoted (space-containing cache paths)
assert '-c "$in" -o "$out"' in content
assert '--app "$in"' in content
# -L/-l from esphome build_flags reach the link line, not the compiles
assert '-L"/opt/blobs"' in content
assert "-luser_blob" in content
@@ -545,8 +548,11 @@ def test_write_project_build_unflags_apply_to_framework_flags(tmp_path: Path) ->
"""build_unflags removes flags from the framework sets, as PlatformIO does."""
paths = _make_framework(tmp_path)
_set_flags()
CORE.build_unflags = {"-fipa-pta"}
CORE.build_unflags = {"-fipa-pta", "-Wl,--gc-sections"}
content = _write_ninja(paths)
for line in content.splitlines():
if line.split(" = ")[0] in ("cflags", "cxxflags", "asflags"):
key = line.split(" = ")[0]
if key in ("cflags", "cxxflags", "asflags"):
assert "-fipa-pta" not in line
if key == "linkflags":
assert "-Wl,--gc-sections" not in line
@@ -222,3 +222,52 @@ def test_library_info_missing_explicit_include_warns(
lib = component._library_info("x", read_path, {"build": {"flags": ["-Inope"]}})
assert lib.include_dirs == [(read_path / "src").resolve()]
assert "include dir nope which does not exist" in caplog.text
def test_library_info_missing_declared_dirs_warn(
tmp_path: Path, caplog: pytest.LogCaptureFixture
) -> None:
"""Explicitly declared srcDir/includeDir that do not exist warn by name."""
read_path = tmp_path / "lib"
read_path.mkdir()
component._library_info(
"x", read_path, {"build": {"srcDir": "nosrc", "includeDir": "noinc"}}
)
assert "srcDir nosrc which does not exist" in caplog.text
assert "include dir noinc which does not exist" in caplog.text
def test_resolve_libraries_lib_ignore_covers_bundled(tmp_path: Path) -> None:
"""lib_ignore applies to framework-bundled libraries, as under PlatformIO."""
framework = _make_framework(tmp_path)
_add_library("ESP8266WiFi", None)
_add_library("Wire", None)
CORE.platformio_options = {"lib_ignore": ["Wire"]}
libs = component.resolve_libraries(framework)
assert [lib.name for lib in libs] == ["ESP8266WiFi"]
def test_resolve_libraries_lib_ignore_covers_bundled_dependencies(
tmp_path: Path,
) -> None:
framework = _make_framework(tmp_path)
_add_library("Some/External", "1.0.0")
CORE.platformio_options = {"lib_ignore": ["Wire"]}
lib_dir = tmp_path / "converted" / "external"
lib_dir.mkdir(parents=True)
converted = _converted(
"some__External", lib_dir, {"dependencies": [{"name": "Wire"}]}
)
def fake_convert(libraries: list, backend: LibraryBackend) -> list:
backend.emit(converted)
return [converted]
with (
patch.object(component, "convert_libraries", side_effect=fake_convert),
patch.object(component, "apply_extra_script"),
):
libs = component.resolve_libraries(framework)
assert [lib.name for lib in libs] == ["some__External"]
+27 -4
View File
@@ -92,7 +92,11 @@ def test_registry_download_bare_string_system() -> None:
resp = _registry_response(
[
{"system": "linux_x86", "download_url": "http://x/x86"},
{"system": "linux_x86_64", "download_url": "http://x/x86_64"},
{
"system": "linux_x86_64",
"download_url": "http://x/x86_64",
"checksum": {"sha256": "abc"},
},
]
)
with (
@@ -103,15 +107,34 @@ def test_registry_download_bare_string_system() -> None:
def test_registry_download_wildcard_system() -> None:
resp = _registry_response([{"system": "*", "download_url": "http://x/any"}])
resp = _registry_response(
[
{
"system": "*",
"download_url": "http://x/any",
"checksum": {"sha256": "abc"},
"size": 7,
}
]
)
with patch("requests.get", return_value=resp):
assert framework._registry_download("pkg", "1.0.0") == (
"http://x/any",
None,
None,
"abc",
7,
)
def test_registry_download_missing_checksum_raises() -> None:
"""An unverifiable archive is refused, never silently extracted."""
resp = _registry_response([{"system": "*", "download_url": "http://x/any"}])
with (
patch("requests.get", return_value=resp),
pytest.raises(EsphomeError, match="no sha256"),
):
framework._registry_download("pkg", "1.0.0")
def test_registry_download_no_system_match() -> None:
resp = _registry_response(
[{"system": ["windows_amd64"], "download_url": "http://x/win"}]