Skip to content

Commit

Permalink
feat: adds error handler to change file permissions if delete fails (#…
Browse files Browse the repository at this point in the history
…2474)

Co-authored-by: antazoey <[email protected]>
  • Loading branch information
bitwise-constructs and antazoey authored Jan 27, 2025
1 parent a87fba6 commit 7bd2810
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 3 deletions.
9 changes: 9 additions & 0 deletions src/ape/utils/os.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
import os
import re
import stat
import sys
import tarfile
import zipfile
Expand Down Expand Up @@ -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
Expand Down
11 changes: 8 additions & 3 deletions src/ape_pm/dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand Down
19 changes: 19 additions & 0 deletions tests/functional/test_dependencies.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import os
import shutil
from pathlib import Path

Expand Down Expand Up @@ -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"))
Expand Down

0 comments on commit 7bd2810

Please sign in to comment.