diff --git a/esphome/__main__.py b/esphome/__main__.py index 47dd8d273c..7c4028da44 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -695,6 +695,11 @@ def _wrap_to_code(name, comp, yaml_util): def write_cpp(config: ConfigType) -> int: from esphome import writer + # Refresh the storage sidecar and clean an incompatible previous build + # before regenerating any sources. This may full-wipe the build dir, so it + # has to run before write_cpp_file writes src/. + writer.update_storage_json() + if not get_bool_env(ENV_NOGITIGNORE): writer.write_gitignore() @@ -1631,7 +1636,7 @@ def command_clean(args: ArgsProtocol, config: ConfigType) -> int | None: from esphome import writer try: - writer.clean_build() + writer.clean_build(full=True) except OSError as err: _LOGGER.error("Error deleting build files: %s", err) return 1 diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 0b50f72382..9cc7a7ff12 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -7,7 +7,6 @@ from esphome.components.esp32 import get_esp32_variant, idf_version import esphome.config_validation as cv from esphome.core import CORE from esphome.helpers import mkdir_p, write_file_if_changed -from esphome.writer import update_storage_json def get_available_components() -> list[str] | None: @@ -213,11 +212,6 @@ target_link_options(${{COMPONENT_LIB}} PUBLIC def write_project(minimal: bool = False) -> None: """Write ESP-IDF project files.""" - # Refresh /storage/.yaml.json so the dashboard's - # /info and /downloads endpoints can locate the build (they 404 - # otherwise). This mirrors the PlatformIO build-gen path's call - # in build_gen/platformio.py:write_ini(). - update_storage_json() mkdir_p(CORE.build_path) mkdir_p(CORE.relative_src_path()) diff --git a/esphome/build_gen/platformio.py b/esphome/build_gen/platformio.py index 30dbb69d86..16c1597ccd 100644 --- a/esphome/build_gen/platformio.py +++ b/esphome/build_gen/platformio.py @@ -1,7 +1,7 @@ from esphome.const import __version__ from esphome.core import CORE from esphome.helpers import mkdir_p, read_file, write_file_if_changed -from esphome.writer import find_begin_end, update_storage_json +from esphome.writer import find_begin_end INI_AUTO_GENERATE_BEGIN = "; ========== AUTO GENERATED CODE BEGIN ===========" INI_AUTO_GENERATE_END = "; =========== AUTO GENERATED CODE END ============" @@ -58,7 +58,6 @@ def get_ini_content(): def write_ini(content): - update_storage_json() path = CORE.relative_build_path("platformio.ini") if path.is_file(): diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 6ef73a2199..2c520d0d2c 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -1005,7 +1005,9 @@ def _check_esphome_idf_framework_install( idf_tools_path = framework_path / "tools" / "idf_tools.py" _LOGGER.info("Checking ESP-IDF %s framework ...", version) # Logged every invocation (not just on install) so the user can verify the - # override. A changed URL needs ``esphome clean`` to force a re-download. + # override. A changed URL needs ``esphome clean-all`` to force a re-download + # (``esphome clean`` only wipes the build dir, not the extracted framework + # under /idf/frameworks/). if source_url: _LOGGER.info("Using framework source override: %s", source_url) diff --git a/esphome/writer.py b/esphome/writer.py index 84f2f8101a..b29b3c4b79 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -126,6 +126,13 @@ def storage_should_update_cmake_cache(old: StorageJSON, new: StorageJSON) -> boo def update_storage_json() -> None: + """Refresh the storage sidecar and clean an incompatible build. + + Runs at the start of ``write_cpp`` -- BEFORE any source/project files are + regenerated -- so the clean below can safely ``full``-wipe the whole build + directory (a switch of toolchain/framework/version also drops the stale + project scaffolding, not just the compiled objects). + """ path = storage_path() old = StorageJSON.load(path) new = StorageJSON.from_esphome_core(CORE, old) @@ -146,7 +153,7 @@ def update_storage_json() -> None: ) else: _LOGGER.info("Core config or version changed, cleaning build files...") - clean_build(clear_pio_cache=False) + clean_build(clear_pio_cache=False, full=True) elif storage_should_update_cmake_cache(old, new): _LOGGER.info("Integrations changed, cleaning cmake cache...") clean_cmake_cache() @@ -483,48 +490,89 @@ def write_cpp(code_s): def clean_cmake_cache(): - pioenvs = CORE.relative_pioenvs_path() - if pioenvs.is_dir(): - pioenvs_cmake_path = pioenvs / CORE.name / "CMakeCache.txt" - if pioenvs_cmake_path.is_file(): - _LOGGER.info("Deleting %s", pioenvs_cmake_path) - pioenvs_cmake_path.unlink() + # Drop the CMake cache so a component-set change forces a reconfigure. + # PlatformIO keeps it under .pioenvs//; the native ESP-IDF toolchain + # keeps it under build/ (where espidf's has_outdated_files() treats a + # missing CMakeCache.txt as stale). Only one exists for a given build. + cmake_cache_paths = ( + CORE.relative_pioenvs_path(CORE.name, "CMakeCache.txt"), + CORE.relative_build_path("build", "CMakeCache.txt"), + ) + for cmake_cache_path in cmake_cache_paths: + if cmake_cache_path.is_file(): + _LOGGER.info("Deleting %s", cmake_cache_path) + cmake_cache_path.unlink() -def clean_build(clear_pio_cache: bool = True): +def clean_build(clear_pio_cache: bool = True, *, full: bool = False): + """Remove build artifacts. + + By default only the compiled outputs are removed (``.pioenvs`` / + ``.piolibdeps`` / the native ESP-IDF ``build`` and ``managed_components`` + dirs) while the generated ``src/`` and project files are kept. This is what + in-build callers need: they regenerate a source/sdkconfig and then force a + rebuild without discarding the sources they just wrote. + + ``full=True`` wipes the entire build directory instead. Used by the + ``esphome clean`` command and by the pre-build clean in + ``update_storage_json`` (which runs before sources are regenerated) -- in + both cases nothing is mid-regeneration, so the next compile rebuilds from + scratch. It also drops stale project scaffolding the allow-list keeps (e.g. a + leftover platformio.ini / CMakeLists.txt from the other toolchain), making a + toolchain switch reliable. + """ # Allow skipping cache cleaning for integration tests if os.environ.get("ESPHOME_SKIP_CLEAN_BUILD"): _LOGGER.warning("Skipping build cleaning (ESPHOME_SKIP_CLEAN_BUILD set)") return - pioenvs = CORE.relative_pioenvs_path() - if pioenvs.is_dir(): - _LOGGER.info("Deleting %s", pioenvs) - rmtree(pioenvs) - piolibdeps = CORE.relative_piolibdeps_path() - if piolibdeps.is_dir(): - _LOGGER.info("Deleting %s", piolibdeps) - rmtree(piolibdeps) - dependencies_lock = CORE.relative_build_path("dependencies.lock") - if dependencies_lock.is_file(): - _LOGGER.info("Deleting %s", dependencies_lock) - dependencies_lock.unlink() + if full: + if CORE.build_path is not None: + build_path = Path(CORE.build_path) + if build_path.is_dir(): + _LOGGER.info("Deleting %s", build_path) + rmtree(build_path) + else: + pioenvs = CORE.relative_pioenvs_path() + if pioenvs.is_dir(): + _LOGGER.info("Deleting %s", pioenvs) + rmtree(pioenvs) + piolibdeps = CORE.relative_piolibdeps_path() + if piolibdeps.is_dir(): + _LOGGER.info("Deleting %s", piolibdeps) + rmtree(piolibdeps) + dependencies_lock = CORE.relative_build_path("dependencies.lock") + if dependencies_lock.is_file(): + _LOGGER.info("Deleting %s", dependencies_lock) + dependencies_lock.unlink() + # Native ESP-IDF toolchain artifacts: the IDF CMake/ninja build dir + # and the Component Manager's fetched managed components live under + # the project's build path, not under .pioenvs / .piolibdeps. + for name in ("build", "managed_components"): + idf_path = CORE.relative_build_path(name) + if idf_path.is_dir(): + _LOGGER.info("Deleting %s", idf_path) + rmtree(idf_path) + + # The idedata cache is derived from the build but lives under the data dir, + # not the build path, so it must be removed separately in both modes. idedata_cache = CORE.relative_internal_path("idedata", f"{CORE.name}.json") if idedata_cache.is_file(): _LOGGER.info("Deleting %s", idedata_cache) idedata_cache.unlink() - # Native ESP-IDF toolchain artifacts: the IDF CMake/ninja build dir - # and the Component Manager's fetched managed components live under - # the project's build path, not under .pioenvs / .piolibdeps. - for name in ("build", "managed_components"): - idf_path = CORE.relative_build_path(name) - if idf_path.is_dir(): - _LOGGER.info("Deleting %s", idf_path) - rmtree(idf_path) if not clear_pio_cache: return + # The native ESP-IDF toolchain caches PlatformIO libraries converted to IDF + # components under /pio_components, shared across builds and keyed + # by source hash (the analog of PlatformIO's global package cache). Drop it + # on an explicit clean so a corrupt/stale converted lib is re-fetched. + pio_components = CORE.relative_internal_path("pio_components") + if pio_components.is_dir(): + _LOGGER.info("Deleting %s", pio_components) + rmtree(pio_components) + # Clean PlatformIO cache to resolve CMake compiler detection issues # This helps when toolchain paths change or get corrupted try: diff --git a/tests/unit_tests/build_gen/test_platformio.py b/tests/unit_tests/build_gen/test_platformio.py index a124dbc128..da0010afa3 100644 --- a/tests/unit_tests/build_gen/test_platformio.py +++ b/tests/unit_tests/build_gen/test_platformio.py @@ -12,13 +12,6 @@ from esphome.build_gen import platformio from esphome.core import CORE -@pytest.fixture -def mock_update_storage_json() -> Generator[MagicMock]: - """Mock update_storage_json for all tests.""" - with patch("esphome.build_gen.platformio.update_storage_json") as mock: - yield mock - - @pytest.fixture def mock_write_file_if_changed() -> Generator[MagicMock]: """Mock write_file_if_changed for tests.""" @@ -26,9 +19,7 @@ def mock_write_file_if_changed() -> Generator[MagicMock]: yield mock -def test_write_ini_creates_new_file( - tmp_path: Path, mock_update_storage_json: MagicMock -) -> None: +def test_write_ini_creates_new_file(tmp_path: Path) -> None: """Test write_ini creates a new platformio.ini file.""" CORE.build_path = str(tmp_path) @@ -50,9 +41,7 @@ framework = arduino assert platformio.INI_AUTO_GENERATE_END in file_content -def test_write_ini_updates_existing_file( - tmp_path: Path, mock_update_storage_json: MagicMock -) -> None: +def test_write_ini_updates_existing_file(tmp_path: Path) -> None: """Test write_ini updates existing platformio.ini file.""" CORE.build_path = str(tmp_path) @@ -97,9 +86,7 @@ framework = arduino assert "platform = old" not in file_content -def test_write_ini_preserves_custom_sections( - tmp_path: Path, mock_update_storage_json: MagicMock -) -> None: +def test_write_ini_preserves_custom_sections(tmp_path: Path) -> None: """Test write_ini preserves custom sections outside auto-generate markers.""" CORE.build_path = str(tmp_path) @@ -148,7 +135,6 @@ monitor_speed = 115200 def test_write_ini_no_change_when_content_same( tmp_path: Path, - mock_update_storage_json: MagicMock, mock_write_file_if_changed: MagicMock, ) -> None: """Test write_ini doesn't rewrite file when content is unchanged.""" @@ -174,15 +160,3 @@ def test_write_ini_no_change_when_content_same( call_args = mock_write_file_if_changed.call_args[0] assert call_args[0] == ini_file assert content in call_args[1] - - -def test_write_ini_calls_update_storage_json( - tmp_path: Path, mock_update_storage_json: MagicMock -) -> None: - """Test write_ini calls update_storage_json.""" - CORE.build_path = str(tmp_path) - - content = "[env:test]\nplatform = esp32" - - platformio.write_ini(content) - mock_update_storage_json.assert_called_once() diff --git a/tests/unit_tests/test_writer.py b/tests/unit_tests/test_writer.py index 6f137fb351..1487517ca2 100644 --- a/tests/unit_tests/test_writer.py +++ b/tests/unit_tests/test_writer.py @@ -341,8 +341,8 @@ def test_update_storage_json_logging_when_old_is_none( with caplog.at_level("INFO"): update_storage_json() - # Verify clean_build was called - mock_clean_build.assert_called_once() + # Verify clean_build was called with a full wipe (runs before src is written) + mock_clean_build.assert_called_once_with(clear_pio_cache=False, full=True) # Verify the correct log message was used (not the component removal message) assert "Core config or version changed, cleaning build files..." in caplog.text @@ -392,60 +392,50 @@ def test_update_storage_json_logging_components_removed( new_storage.save.assert_called_once_with("/test/path") +def _mock_cmake_cache_paths(mock_core: MagicMock, tmp_path: Path) -> None: + """Wire relative_pioenvs_path/relative_build_path to tmp_path subtrees.""" + mock_core.name = "test_device" + mock_core.relative_pioenvs_path.side_effect = (tmp_path / ".pioenvs").joinpath + mock_core.relative_build_path.side_effect = tmp_path.joinpath + + @patch("esphome.writer.CORE") -def test_clean_cmake_cache( +def test_clean_cmake_cache_platformio( mock_core: MagicMock, tmp_path: Path, caplog: pytest.LogCaptureFixture, ) -> None: - """Test clean_cmake_cache removes CMakeCache.txt file.""" - # Create directory structure - pioenvs_dir = tmp_path / ".pioenvs" - pioenvs_dir.mkdir() - device_dir = pioenvs_dir / "test_device" - device_dir.mkdir() - cmake_cache_file = device_dir / "CMakeCache.txt" + """Test clean_cmake_cache removes the PlatformIO CMakeCache.txt.""" + _mock_cmake_cache_paths(mock_core, tmp_path) + cmake_cache_file = tmp_path / ".pioenvs" / "test_device" / "CMakeCache.txt" + cmake_cache_file.parent.mkdir(parents=True) cmake_cache_file.write_text("# CMake cache file") - # Setup mocks - mock_core.relative_pioenvs_path.return_value = pioenvs_dir - mock_core.name = "test_device" - - # Verify file exists before - assert cmake_cache_file.exists() - - # Call the function with caplog.at_level("INFO"): clean_cmake_cache() - # Verify file was removed assert not cmake_cache_file.exists() - - # Verify logging assert "Deleting" in caplog.text assert "CMakeCache.txt" in caplog.text @patch("esphome.writer.CORE") -def test_clean_cmake_cache_no_pioenvs_dir( +def test_clean_cmake_cache_esp_idf( mock_core: MagicMock, tmp_path: Path, + caplog: pytest.LogCaptureFixture, ) -> None: - """Test clean_cmake_cache when pioenvs directory doesn't exist.""" - # Setup non-existent directory path - pioenvs_dir = tmp_path / ".pioenvs" + """Test clean_cmake_cache removes the native ESP-IDF build/CMakeCache.txt.""" + _mock_cmake_cache_paths(mock_core, tmp_path) + cmake_cache_file = tmp_path / "build" / "CMakeCache.txt" + cmake_cache_file.parent.mkdir(parents=True) + cmake_cache_file.write_text("# CMake cache file") - # Setup mocks - mock_core.relative_pioenvs_path.return_value = pioenvs_dir + with caplog.at_level("INFO"): + clean_cmake_cache() - # Verify directory doesn't exist - assert not pioenvs_dir.exists() - - # Call the function - should not crash - clean_cmake_cache() - - # Verify directory still doesn't exist - assert not pioenvs_dir.exists() + assert not cmake_cache_file.exists() + assert str(cmake_cache_file) in caplog.text @patch("esphome.writer.CORE") @@ -453,27 +443,11 @@ def test_clean_cmake_cache_no_cmake_file( mock_core: MagicMock, tmp_path: Path, ) -> None: - """Test clean_cmake_cache when CMakeCache.txt doesn't exist.""" - # Create directory structure without CMakeCache.txt - pioenvs_dir = tmp_path / ".pioenvs" - pioenvs_dir.mkdir() - device_dir = pioenvs_dir / "test_device" - device_dir.mkdir() - cmake_cache_file = device_dir / "CMakeCache.txt" + """Test clean_cmake_cache when no CMakeCache.txt exists -- should not crash.""" + _mock_cmake_cache_paths(mock_core, tmp_path) - # Setup mocks - mock_core.relative_pioenvs_path.return_value = pioenvs_dir - mock_core.name = "test_device" - - # Verify file doesn't exist - assert not cmake_cache_file.exists() - - # Call the function - should not crash clean_cmake_cache() - # Verify file still doesn't exist - assert not cmake_cache_file.exists() - @patch("esphome.writer.CORE") def test_clean_build( @@ -507,6 +481,11 @@ def test_clean_build( managed_components_dir.mkdir() (managed_components_dir / "espressif__arduino-esp32").mkdir() + # Converted-PIO-library cache (native ESP-IDF), under the data dir. + pio_components_dir = tmp_path / "pio_components" + pio_components_dir.mkdir() + (pio_components_dir / "abc12345").mkdir() + # Create PlatformIO cache directory platformio_cache_dir = tmp_path / ".platformio" / ".cache" platformio_cache_dir.mkdir(parents=True) @@ -529,6 +508,7 @@ def test_clean_build( assert idedata_cache.exists() assert idf_build_dir.exists() assert managed_components_dir.exists() + assert pio_components_dir.exists() assert platformio_cache_dir.exists() # Mock PlatformIO's ProjectConfig cache_dir @@ -554,6 +534,7 @@ def test_clean_build( assert not idedata_cache.exists() assert not idf_build_dir.exists() assert not managed_components_dir.exists() + assert not pio_components_dir.exists() assert not platformio_cache_dir.exists() # Verify logging @@ -567,6 +548,41 @@ def test_clean_build( assert "PlatformIO cache" in caplog.text +@patch("esphome.writer.CORE") +def test_clean_build_full_wipes_build_dir( + mock_core: MagicMock, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """full=True wipes the whole build dir (incl. src/) but keeps siblings.""" + build_dir = tmp_path / "build" / "test" + (build_dir / "src").mkdir(parents=True) + (build_dir / "src" / "main.cpp").write_text("// generated") + (build_dir / "platformio.ini").write_text("[platformio]") + (build_dir / ".pioenvs").mkdir() + + idedata_cache = tmp_path / "idedata" / "test.json" + idedata_cache.parent.mkdir() + idedata_cache.write_text("{}") + + # A sibling of the build dir (under the data dir) must survive. + survivor = tmp_path / "keep_me.txt" + survivor.write_text("keep") + + # build_path may be a str (e.g. set from config); clean_build must coerce. + mock_core.build_path = str(build_dir) + mock_core.name = "test" + mock_core.relative_internal_path.side_effect = tmp_path.joinpath + + with caplog.at_level("INFO"): + clean_build(clear_pio_cache=False, full=True) + + assert not build_dir.exists() + assert not idedata_cache.exists() + assert survivor.exists() + assert str(build_dir) in caplog.text + + @patch("esphome.writer.CORE") def test_clean_build_partial_exists( mock_core: MagicMock, @@ -586,6 +602,7 @@ def test_clean_build_partial_exists( mock_core.relative_pioenvs_path.return_value = pioenvs_dir mock_core.relative_piolibdeps_path.return_value = piolibdeps_dir mock_core.relative_build_path.side_effect = lambda name: tmp_path / name + mock_core.relative_internal_path.side_effect = tmp_path.joinpath # Verify only pioenvs exists assert pioenvs_dir.exists() @@ -623,6 +640,7 @@ def test_clean_build_nothing_exists( mock_core.relative_pioenvs_path.return_value = pioenvs_dir mock_core.relative_piolibdeps_path.return_value = piolibdeps_dir mock_core.relative_build_path.side_effect = lambda name: tmp_path / name + mock_core.relative_internal_path.side_effect = tmp_path.joinpath # Verify nothing exists assert not pioenvs_dir.exists() @@ -659,6 +677,7 @@ def test_clean_build_platformio_not_available( mock_core.relative_pioenvs_path.return_value = pioenvs_dir mock_core.relative_piolibdeps_path.return_value = piolibdeps_dir mock_core.relative_build_path.side_effect = lambda name: tmp_path / name + mock_core.relative_internal_path.side_effect = tmp_path.joinpath # Verify all exist before assert pioenvs_dir.exists() @@ -697,6 +716,7 @@ def test_clean_build_empty_cache_dir( mock_core.relative_pioenvs_path.return_value = pioenvs_dir mock_core.relative_piolibdeps_path.return_value = tmp_path / ".piolibdeps" mock_core.relative_build_path.side_effect = lambda name: tmp_path / name + mock_core.relative_internal_path.side_effect = tmp_path.joinpath # Verify pioenvs exists before assert pioenvs_dir.exists() @@ -1425,6 +1445,7 @@ def test_clean_build_handles_readonly_files( mock_core.relative_pioenvs_path.return_value = pioenvs_dir mock_core.relative_piolibdeps_path.return_value = tmp_path / ".piolibdeps" mock_core.relative_build_path.side_effect = lambda name: tmp_path / name + mock_core.relative_internal_path.side_effect = tmp_path.joinpath # Verify file is read-only assert not os.access(readonly_file, os.W_OK) @@ -1489,6 +1510,7 @@ def test_clean_build_reraises_for_other_errors( mock_core.relative_pioenvs_path.return_value = pioenvs_dir mock_core.relative_piolibdeps_path.return_value = tmp_path / ".piolibdeps" mock_core.relative_build_path.side_effect = lambda name: tmp_path / name + mock_core.relative_internal_path.side_effect = tmp_path.joinpath try: # Mock os.access in writer module to return True (writable)