diff --git a/esphome/writer.py b/esphome/writer.py index 3124e9e12..721db07f9 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -1,8 +1,12 @@ +from collections.abc import Callable import importlib import logging import os from pathlib import Path import re +import shutil +import stat +from types import TracebackType from esphome import loader from esphome.config import iter_component_configs, iter_components @@ -301,9 +305,24 @@ def clean_cmake_cache(): pioenvs_cmake_path.unlink() -def clean_build(clear_pio_cache: bool = True): - import shutil +def _rmtree_error_handler( + func: Callable[[str], object], + path: str, + exc_info: tuple[type[BaseException], BaseException, TracebackType | None], +) -> None: + """Error handler for shutil.rmtree to handle read-only files on Windows. + On Windows, git pack files and other files may be marked read-only, + causing shutil.rmtree to fail with "Access is denied". This handler + removes the read-only flag and retries the deletion. + """ + if os.access(path, os.W_OK): + raise exc_info[1].with_traceback(exc_info[2]) + os.chmod(path, stat.S_IWUSR | stat.S_IRUSR) + func(path) + + +def clean_build(clear_pio_cache: bool = True): # 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)") @@ -312,11 +331,11 @@ def clean_build(clear_pio_cache: bool = True): pioenvs = CORE.relative_pioenvs_path() if pioenvs.is_dir(): _LOGGER.info("Deleting %s", pioenvs) - shutil.rmtree(pioenvs) + shutil.rmtree(pioenvs, onerror=_rmtree_error_handler) piolibdeps = CORE.relative_piolibdeps_path() if piolibdeps.is_dir(): _LOGGER.info("Deleting %s", piolibdeps) - shutil.rmtree(piolibdeps) + shutil.rmtree(piolibdeps, onerror=_rmtree_error_handler) dependencies_lock = CORE.relative_build_path("dependencies.lock") if dependencies_lock.is_file(): _LOGGER.info("Deleting %s", dependencies_lock) @@ -337,12 +356,10 @@ def clean_build(clear_pio_cache: bool = True): cache_dir = Path(config.get("platformio", "cache_dir")) if cache_dir.is_dir(): _LOGGER.info("Deleting PlatformIO cache %s", cache_dir) - shutil.rmtree(cache_dir) + shutil.rmtree(cache_dir, onerror=_rmtree_error_handler) def clean_all(configuration: list[str]): - import shutil - data_dirs = [] for config in configuration: item = Path(config) @@ -364,7 +381,7 @@ def clean_all(configuration: list[str]): if item.is_file() and not item.name.endswith(".json"): item.unlink() elif item.is_dir() and item.name != "storage": - shutil.rmtree(item) + shutil.rmtree(item, onerror=_rmtree_error_handler) # Clean PlatformIO project files try: @@ -378,7 +395,7 @@ def clean_all(configuration: list[str]): path = Path(config.get("platformio", pio_dir)) if path.is_dir(): _LOGGER.info("Deleting PlatformIO %s %s", pio_dir, path) - shutil.rmtree(path) + shutil.rmtree(path, onerror=_rmtree_error_handler) GITIGNORE_CONTENT = """# Gitignore settings for ESPHome diff --git a/tests/unit_tests/test_writer.py b/tests/unit_tests/test_writer.py index a2a358f4d..9fa60c06e 100644 --- a/tests/unit_tests/test_writer.py +++ b/tests/unit_tests/test_writer.py @@ -1,7 +1,9 @@ """Test writer module functionality.""" from collections.abc import Callable +import os from pathlib import Path +import stat from typing import Any from unittest.mock import MagicMock, patch @@ -15,6 +17,7 @@ from esphome.writer import ( CPP_INCLUDE_BEGIN, CPP_INCLUDE_END, GITIGNORE_CONTENT, + clean_all, clean_build, clean_cmake_cache, storage_should_clean, @@ -1062,3 +1065,103 @@ def test_clean_all_preserves_json_files( # Verify logging mentions cleaning assert "Cleaning" in caplog.text assert str(build_dir) in caplog.text + + +@patch("esphome.writer.CORE") +def test_clean_build_handles_readonly_files( + mock_core: MagicMock, + tmp_path: Path, +) -> None: + """Test clean_build handles read-only files (e.g., git pack files on Windows).""" + # Create directory structure with read-only files + pioenvs_dir = tmp_path / ".pioenvs" + pioenvs_dir.mkdir() + git_dir = pioenvs_dir / ".git" / "objects" / "pack" + git_dir.mkdir(parents=True) + + # Create a read-only file (simulating git pack files on Windows) + readonly_file = git_dir / "pack-abc123.pack" + readonly_file.write_text("pack data") + os.chmod(readonly_file, stat.S_IRUSR) # Read-only + + # Setup mocks + mock_core.relative_pioenvs_path.return_value = pioenvs_dir + mock_core.relative_piolibdeps_path.return_value = tmp_path / ".piolibdeps" + mock_core.relative_build_path.return_value = tmp_path / "dependencies.lock" + + # Verify file is read-only + assert not os.access(readonly_file, os.W_OK) + + # Call the function - should not crash + clean_build() + + # Verify directory was removed despite read-only files + assert not pioenvs_dir.exists() + + +@patch("esphome.writer.CORE") +def test_clean_all_handles_readonly_files( + mock_core: MagicMock, + tmp_path: Path, +) -> None: + """Test clean_all handles read-only files.""" + # Create config directory + config_dir = tmp_path / "config" + config_dir.mkdir() + + build_dir = config_dir / ".esphome" + build_dir.mkdir() + + # Create a subdirectory with read-only files + subdir = build_dir / "subdir" + subdir.mkdir() + readonly_file = subdir / "readonly.txt" + readonly_file.write_text("content") + os.chmod(readonly_file, stat.S_IRUSR) # Read-only + + # Verify file is read-only + assert not os.access(readonly_file, os.W_OK) + + # Call the function - should not crash + clean_all([str(config_dir)]) + + # Verify directory was removed despite read-only files + assert not subdir.exists() + assert build_dir.exists() # .esphome dir itself is preserved + + +@patch("esphome.writer.CORE") +def test_clean_build_reraises_for_other_errors( + mock_core: MagicMock, + tmp_path: Path, +) -> None: + """Test clean_build re-raises errors that are not read-only permission issues.""" + # Create directory structure with a read-only subdirectory + # This prevents file deletion and triggers the error handler + pioenvs_dir = tmp_path / ".pioenvs" + pioenvs_dir.mkdir() + subdir = pioenvs_dir / "subdir" + subdir.mkdir() + test_file = subdir / "test.txt" + test_file.write_text("content") + + # Make subdir read-only so files inside can't be deleted + os.chmod(subdir, stat.S_IRUSR | stat.S_IXUSR) + + # Setup mocks + mock_core.relative_pioenvs_path.return_value = pioenvs_dir + mock_core.relative_piolibdeps_path.return_value = tmp_path / ".piolibdeps" + mock_core.relative_build_path.return_value = tmp_path / "dependencies.lock" + + try: + # Mock os.access in writer module to return True (writable) + # This simulates a case where the error is NOT due to read-only permissions + # so the error handler should re-raise instead of trying to fix permissions + with ( + patch("esphome.writer.os.access", return_value=True), + pytest.raises(PermissionError), + ): + clean_build() + finally: + # Cleanup - restore write permission so tmp_path cleanup works + os.chmod(subdir, stat.S_IRWXU)