mirror of
https://github.com/esphome/esphome.git
synced 2026-08-22 22:26:21 +00:00
Non-destructive platform_version check, honest compdb errors, missing-binutils refusal
The platform_version pop defaults to the schema spec so a second validation pass over an already-validated dict cannot warn about a key the user never set. An unparsable compile database now fails naming the parse error and the offending output instead of blaming renamed ninja rules. analyze-memory validates the native objdump/readelf exist and fails by tool name rather than silently analyzing with host binutils. The shared smoke-test helper is renamed _toolchain_components_to_test (it serves the esp32 PlatformIO job too), and the decode-dedup state is cleared by an autouse fixture instead of by hand.
This commit is contained in:
@@ -2022,6 +2022,19 @@ def command_analyze_memory(args: ArgsProtocol, config: ConfigType) -> int:
|
|||||||
# Get idedata for analysis
|
# Get idedata for analysis
|
||||||
idedata = None
|
idedata = None
|
||||||
if native_toolchain is not None:
|
if native_toolchain is not None:
|
||||||
|
for tool in (
|
||||||
|
native_toolchain.get_objdump_path(),
|
||||||
|
native_toolchain.get_readelf_path(),
|
||||||
|
):
|
||||||
|
if not tool.is_file():
|
||||||
|
# The analyzer would silently fall back to host binutils,
|
||||||
|
# which cannot read the target ELF
|
||||||
|
_LOGGER.error(
|
||||||
|
"%s is missing; the toolchain install may be incomplete "
|
||||||
|
"(run 'esphome clean-all')",
|
||||||
|
tool,
|
||||||
|
)
|
||||||
|
return 1
|
||||||
objdump_path = str(native_toolchain.get_objdump_path())
|
objdump_path = str(native_toolchain.get_objdump_path())
|
||||||
readelf_path = str(native_toolchain.get_readelf_path())
|
readelf_path = str(native_toolchain.get_readelf_path())
|
||||||
|
|
||||||
|
|||||||
@@ -151,8 +151,12 @@ def _write_compile_commands(
|
|||||||
raise EsphomeError(f"Could not generate compile_commands.json: {result.stderr}")
|
raise EsphomeError(f"Could not generate compile_commands.json: {result.stderr}")
|
||||||
try:
|
try:
|
||||||
entries = json.loads(result.stdout)
|
entries = json.loads(result.stdout)
|
||||||
except ValueError:
|
except ValueError as err:
|
||||||
entries = None
|
(build_dir / "compile_commands.json").unlink(missing_ok=True)
|
||||||
|
raise EsphomeError(
|
||||||
|
f"ninja produced an unparsable compile database: {err} "
|
||||||
|
f"(output starts {result.stdout[:120]!r})"
|
||||||
|
) from err
|
||||||
if not entries:
|
if not entries:
|
||||||
# compdb exits 0 with [] for unknown rule names; a renamed compile
|
# compdb exits 0 with [] for unknown rule names; a renamed compile
|
||||||
# rule must fail the build, not silently strand every consumer
|
# rule must fail the build, not silently strand every consumer
|
||||||
|
|||||||
@@ -131,7 +131,10 @@ def _validate_native_toolchain(config: ConfigType) -> ConfigType:
|
|||||||
# platform_version is a PlatformIO concept; drop it (as esp32's native
|
# platform_version is a PlatformIO concept; drop it (as esp32's native
|
||||||
# toolchain does), warning when a custom pin is discarded. The floor
|
# toolchain does), warning when a custom pin is discarded. The floor
|
||||||
# above guarantees the schema-derived default is the ARDUINO_4 spec.
|
# above guarantees the schema-derived default is the ARDUINO_4 spec.
|
||||||
if conf.pop(CONF_PLATFORM_VERSION, None) != _ARDUINO_4_PLATFORM_SPEC:
|
if (
|
||||||
|
conf.pop(CONF_PLATFORM_VERSION, _ARDUINO_4_PLATFORM_SPEC)
|
||||||
|
!= _ARDUINO_4_PLATFORM_SPEC
|
||||||
|
):
|
||||||
_LOGGER.warning(
|
_LOGGER.warning(
|
||||||
"'platform_version' is ignored by 'toolchain: arduino'; the native "
|
"'platform_version' is ignored by 'toolchain: arduino'; the native "
|
||||||
"toolchain downloads the framework and compiler directly"
|
"toolchain downloads the framework and compiler directly"
|
||||||
|
|||||||
@@ -596,12 +596,12 @@ def esp32_platformio_components_to_test(branch: str | None = None) -> list[str]:
|
|||||||
Returns:
|
Returns:
|
||||||
Sorted list of component names to compile.
|
Sorted list of component names to compile.
|
||||||
"""
|
"""
|
||||||
return _native_components_to_test(
|
return _toolchain_components_to_test(
|
||||||
branch, ESP32_PLATFORMIO_TEST_COMPONENTS, _esp32_platformio_path_or_file_trigger
|
branch, ESP32_PLATFORMIO_TEST_COMPONENTS, _esp32_platformio_path_or_file_trigger
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def _native_components_to_test(
|
def _toolchain_components_to_test(
|
||||||
branch: str | None,
|
branch: str | None,
|
||||||
test_set: frozenset[str] | set[str],
|
test_set: frozenset[str] | set[str],
|
||||||
infra_trigger: Callable[[list[str]], bool],
|
infra_trigger: Callable[[list[str]], bool],
|
||||||
@@ -690,7 +690,7 @@ def esp8266_native_components_to_test(branch: str | None = None) -> list[str]:
|
|||||||
list on core or infrastructure changes, otherwise the intersection with
|
list on core or infrastructure changes, otherwise the intersection with
|
||||||
the changed-component dependency closure (empty list skips the job).
|
the changed-component dependency closure (empty list skips the job).
|
||||||
"""
|
"""
|
||||||
return _native_components_to_test(
|
return _toolchain_components_to_test(
|
||||||
branch, ESP8266_NATIVE_TEST_COMPONENTS, _esp8266_native_path_or_file_trigger
|
branch, ESP8266_NATIVE_TEST_COMPONENTS, _esp8266_native_path_or_file_trigger
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from collections.abc import Generator
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from types import SimpleNamespace
|
from types import SimpleNamespace
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
@@ -29,9 +30,12 @@ from esphome.types import ConfigType
|
|||||||
|
|
||||||
|
|
||||||
@pytest.fixture(autouse=True)
|
@pytest.fixture(autouse=True)
|
||||||
def _arduino_toolchain() -> None:
|
def _arduino_toolchain() -> Generator[None]:
|
||||||
# The suite-wide reset_core fixture clears CORE.toolchain after each test
|
# The suite-wide reset_core fixture clears CORE.toolchain after each test
|
||||||
CORE.toolchain = Toolchain.ARDUINO
|
CORE.toolchain = Toolchain.ARDUINO
|
||||||
|
esp8266._DECODE_WARNED_AT.clear()
|
||||||
|
yield
|
||||||
|
esp8266._DECODE_WARNED_AT.clear()
|
||||||
|
|
||||||
|
|
||||||
def _config(
|
def _config(
|
||||||
@@ -123,7 +127,6 @@ def test_decode_pc_native_missing_tools_warns_once(
|
|||||||
) -> None:
|
) -> None:
|
||||||
"""A stack dump of many addresses produces one missing-tool warning."""
|
"""A stack dump of many addresses produces one missing-tool warning."""
|
||||||
|
|
||||||
esp8266._DECODE_WARNED_AT.clear()
|
|
||||||
with (
|
with (
|
||||||
patch(
|
patch(
|
||||||
"esphome.arduino8266.toolchain.get_addr2line_path",
|
"esphome.arduino8266.toolchain.get_addr2line_path",
|
||||||
@@ -137,7 +140,6 @@ def test_decode_pc_native_missing_tools_warns_once(
|
|||||||
esp8266._decode_pc({}, "40201234")
|
esp8266._decode_pc({}, "40201234")
|
||||||
esp8266._decode_pc({}, "40201238")
|
esp8266._decode_pc({}, "40201238")
|
||||||
assert caplog.text.count("Cannot decode crash addresses") == 1
|
assert caplog.text.count("Cannot decode crash addresses") == 1
|
||||||
esp8266._DECODE_WARNED_AT.clear()
|
|
||||||
|
|
||||||
|
|
||||||
def test_decode_pc_platformio_missing_tools_warns_once(
|
def test_decode_pc_platformio_missing_tools_warns_once(
|
||||||
@@ -147,14 +149,12 @@ def test_decode_pc_platformio_missing_tools_warns_once(
|
|||||||
warning level as the native one; raw undecoded addresses with no
|
warning level as the native one; raw undecoded addresses with no
|
||||||
stated reason are undiagnosable at default log level."""
|
stated reason are undiagnosable at default log level."""
|
||||||
|
|
||||||
esp8266._DECODE_WARNED_AT.clear()
|
|
||||||
CORE.toolchain = Toolchain.PLATFORMIO
|
CORE.toolchain = Toolchain.PLATFORMIO
|
||||||
idedata = SimpleNamespace(addr2line_path=None, firmware_elf_path=None)
|
idedata = SimpleNamespace(addr2line_path=None, firmware_elf_path=None)
|
||||||
with patch("esphome.platformio.toolchain.get_idedata", return_value=idedata):
|
with patch("esphome.platformio.toolchain.get_idedata", return_value=idedata):
|
||||||
esp8266._decode_pc({}, "40201234")
|
esp8266._decode_pc({}, "40201234")
|
||||||
esp8266._decode_pc({}, "40201238")
|
esp8266._decode_pc({}, "40201238")
|
||||||
assert caplog.text.count("Cannot decode crash addresses") == 1
|
assert caplog.text.count("Cannot decode crash addresses") == 1
|
||||||
esp8266._DECODE_WARNED_AT.clear()
|
|
||||||
|
|
||||||
|
|
||||||
def test_resolve_toolchain_rejects_unsupported() -> None:
|
def test_resolve_toolchain_rejects_unsupported() -> None:
|
||||||
|
|||||||
@@ -158,10 +158,19 @@ def test_write_compile_commands(tmp_path: Path) -> None:
|
|||||||
assert (build_dir / "compile_commands.json").read_text() == entries
|
assert (build_dir / "compile_commands.json").read_text() == entries
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("stdout", ["[]\n", "not json"])
|
@pytest.mark.parametrize(
|
||||||
def test_write_compile_commands_empty_db_raises(tmp_path: Path, stdout: str) -> None:
|
("stdout", "match"),
|
||||||
"""An empty compile database (compdb exits 0 with [] for unknown rule
|
[
|
||||||
names) must fail the build and drop any stale database."""
|
("[]\n", "empty compile database"),
|
||||||
|
# A parse failure names its cause, not the rule-name story
|
||||||
|
("not json", "unparsable compile database.*not json"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_write_compile_commands_bad_db_raises(
|
||||||
|
tmp_path: Path, stdout: str, match: str
|
||||||
|
) -> None:
|
||||||
|
"""An empty or unparsable compile database fails the build with its
|
||||||
|
actual cause and drops any stale database."""
|
||||||
build_dir = tmp_path / "build"
|
build_dir = tmp_path / "build"
|
||||||
build_dir.mkdir()
|
build_dir.mkdir()
|
||||||
(build_dir / "compile_commands.json").write_text("[stale]")
|
(build_dir / "compile_commands.json").write_text("[stale]")
|
||||||
@@ -171,7 +180,7 @@ def test_write_compile_commands_empty_db_raises(tmp_path: Path, stdout: str) ->
|
|||||||
"run",
|
"run",
|
||||||
return_value=MagicMock(returncode=0, stdout=stdout),
|
return_value=MagicMock(returncode=0, stdout=stdout),
|
||||||
),
|
),
|
||||||
pytest.raises(EsphomeError, match="empty compile database"),
|
pytest.raises(EsphomeError, match=match),
|
||||||
):
|
):
|
||||||
toolchain._write_compile_commands(tmp_path / "ninja", build_dir, {})
|
toolchain._write_compile_commands(tmp_path / "ninja", build_dir, {})
|
||||||
assert not (build_dir / "compile_commands.json").exists()
|
assert not (build_dir / "compile_commands.json").exists()
|
||||||
|
|||||||
@@ -7223,9 +7223,15 @@ def test_command_analyze_memory_native_toolchains(
|
|||||||
CORE.toolchain = toolchain
|
CORE.toolchain = toolchain
|
||||||
|
|
||||||
config = {CONF_ESPHOME: {CONF_NAME: "test_device"}}
|
config = {CONF_ESPHOME: {CONF_NAME: "test_device"}}
|
||||||
|
# The tools must exist: a missing binutils now fails by name instead of
|
||||||
|
# silently falling back to host tools
|
||||||
|
objdump = tmp_path / "objdump"
|
||||||
|
readelf = tmp_path / "readelf"
|
||||||
|
objdump.write_text("")
|
||||||
|
readelf.write_text("")
|
||||||
with (
|
with (
|
||||||
patch(f"{module}.get_objdump_path", return_value=Path("/tc/objdump")),
|
patch(f"{module}.get_objdump_path", return_value=objdump),
|
||||||
patch(f"{module}.get_readelf_path", return_value=Path("/tc/readelf")),
|
patch(f"{module}.get_readelf_path", return_value=readelf),
|
||||||
patch(f"{module}.get_elf_path", return_value=Path("/build/firmware.elf")),
|
patch(f"{module}.get_elf_path", return_value=Path("/build/firmware.elf")),
|
||||||
):
|
):
|
||||||
result = command_analyze_memory(MockArgs(), config)
|
result = command_analyze_memory(MockArgs(), config)
|
||||||
@@ -7234,13 +7240,33 @@ def test_command_analyze_memory_native_toolchains(
|
|||||||
# str(Path(...)) so the expectation matches the platform's separators
|
# str(Path(...)) so the expectation matches the platform's separators
|
||||||
mock_memory_analyzer_cli.assert_called_once_with(
|
mock_memory_analyzer_cli.assert_called_once_with(
|
||||||
str(Path("/build/firmware.elf")),
|
str(Path("/build/firmware.elf")),
|
||||||
str(Path("/tc/objdump")),
|
str(objdump),
|
||||||
str(Path("/tc/readelf")),
|
str(readelf),
|
||||||
set(),
|
set(),
|
||||||
idedata=None,
|
idedata=None,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_command_analyze_memory_missing_binutils_fails_by_name(
|
||||||
|
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||||
|
) -> None:
|
||||||
|
"""A truncated toolchain install fails naming the missing tool instead
|
||||||
|
of silently analyzing with host binutils."""
|
||||||
|
setup_core(platform="esp8266", tmp_path=tmp_path, name="test_device")
|
||||||
|
CORE.toolchain = Toolchain.ARDUINO
|
||||||
|
config = {CONF_ESPHOME: {CONF_NAME: "test_device"}}
|
||||||
|
module = "esphome.arduino8266.toolchain"
|
||||||
|
with (
|
||||||
|
patch(f"{module}.get_objdump_path", return_value=tmp_path / "missing-objdump"),
|
||||||
|
patch(f"{module}.get_readelf_path", return_value=tmp_path / "readelf"),
|
||||||
|
patch("esphome.__main__.write_cpp", return_value=0),
|
||||||
|
patch("esphome.__main__.compile_program", return_value=0),
|
||||||
|
):
|
||||||
|
assert command_analyze_memory(MockArgs(), config) == 1
|
||||||
|
assert "missing-objdump" in caplog.text
|
||||||
|
assert "toolchain install may be incomplete" in caplog.text
|
||||||
|
|
||||||
|
|
||||||
def test_command_idedata_incompatible_toolchain(tmp_path: Path) -> None:
|
def test_command_idedata_incompatible_toolchain(tmp_path: Path) -> None:
|
||||||
"""A non-native, non-platformio toolchain errors out cleanly."""
|
"""A non-native, non-platformio toolchain errors out cleanly."""
|
||||||
setup_core(platform=PLATFORM_ESP32, tmp_path=tmp_path)
|
setup_core(platform=PLATFORM_ESP32, tmp_path=tmp_path)
|
||||||
|
|||||||
Reference in New Issue
Block a user