[core] Fix clean all windows (#12217)
Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: J. Nick Koston <nick@home-assistant.io>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user