diff --git a/src/ape/utils/os.py b/src/ape/utils/os.py index 6a3a4213df..07419df681 100644 --- a/src/ape/utils/os.py +++ b/src/ape/utils/os.py @@ -1,6 +1,7 @@ import json import os import re +import stat import sys import tarfile import zipfile @@ -372,6 +373,14 @@ def extract_archive(archive_file: Path, destination: Optional[Path] = None): raise ValueError(f"Unsupported zip format: '{archive_file.suffix}'.") +def _remove_readonly(func, path, excinfo): + """ + Error handler for shutil.rmtree that handles removing read-only files. + """ + os.chmod(path, stat.S_IWRITE) + func(path) + + class CacheDirectory: """ A directory for caching data where each data item is named diff --git a/src/ape_pm/dependency.py b/src/ape_pm/dependency.py index 6e0828c90f..657a6441c0 100644 --- a/src/ape_pm/dependency.py +++ b/src/ape_pm/dependency.py @@ -16,7 +16,7 @@ from ape.managers.project import _version_to_options from ape.utils._github import _GithubClient, github_client from ape.utils.basemodel import ManagerAccessMixin -from ape.utils.os import clean_path, extract_archive, get_package_path, in_tempdir +from ape.utils.os import _remove_readonly, clean_path, extract_archive, get_package_path, in_tempdir def _fetch_local(src: Path, destination: Path, config_override: Optional[dict] = None): @@ -204,8 +204,12 @@ def __repr__(self) -> str: def fetch(self, destination: Path): destination.parent.mkdir(exist_ok=True, parents=True) if ref := self.ref: + # NOTE: destination path should not exist at this point, + # so delete it in case it's left over from a failure. + if destination.is_dir(): + shutil.rmtree(destination) + # Fetch using git-clone approach (by git-reference). - # NOTE: destination path does not exist at this point. self._fetch_ref(ref, destination) else: # Fetch using Version API from GitHub. @@ -222,7 +226,8 @@ def fetch(self, destination: Path): # NOTE: When using ref-from-a-version, ensure # it didn't create the destination along the way; # else, the ref is cloned in the wrong spot. - shutil.rmtree(destination, ignore_errors=True) + if destination.is_dir(): + shutil.rmtree(destination, onerror=_remove_readonly) try: self._fetch_ref(version, destination) except Exception: diff --git a/tests/functional/test_dependencies.py b/tests/functional/test_dependencies.py index a89a7451e9..58f7987ceb 100644 --- a/tests/functional/test_dependencies.py +++ b/tests/functional/test_dependencies.py @@ -1,4 +1,5 @@ import json +import os import shutil from pathlib import Path @@ -611,6 +612,24 @@ def test_fetch_ref(self, mock_client): "ApeWorX", "ApeNotAThing", path, branch="3.0.0" ) + def test_fetch_existing_destination_with_read_only_files(self, mock_client): + """ + Show it handles when the destination contains read-only files already + """ + dependency = GithubDependency(github="ApeWorX/ApeNotAThing", ref="3.0.0", name="apetestdep") + dependency._github_client = mock_client + + with create_tempdir() as path: + readonly_file = path / "readme.txt" + readonly_file.write_text("readme!") + + # NOTE: This only makes a difference on Windows. If using a UNIX system, + # rmtree still deletes readonly files regardless. Windows is more restrictive. + os.chmod(readonly_file, 0o444) # Read-only permissions + + dependency.fetch(path) + assert not readonly_file.is_file() + class TestPythonDependency: @pytest.fixture(scope="class", params=("site_package", "python", "pypi"))