fix: handle read-only .git objects on Windows in shutil.rmtree calls

[ryanfk]

Summary

• On Windows, Git marks pack files and loose objects as read-only. When APM tries to remove a previously-cloned apm_modules directory or strip the .git folder after cloning, shutil.rmtree raises [WinError 5] Access is denied.
• Adds a _remove_readonly error handler that clears the read-only flag before retrying the delete.
• Wires the handler into all five shutil.rmtree calls in github_downloader.py, replacing both bare calls and ignore_errors=True (which silently left files behind on Windows).

Reproduction

apm install dev.azure.com/myorg/myproject/myrepo
# First install fails for any reason (e.g. wrong branch)
# Second install attempt hits:
# ❌ Failed to install myorg/myproject/myrepo: [WinError 5] Access is denied:
#    'C:\...\apm_modules\myorg\myproject\myrepo\.git\objects\02\...'

Fix

def _remove_readonly(func, path, _excinfo):
    """Clear read-only flag on Windows before retrying delete."""
    os.chmod(path, stat.S_IWRITE)
    func(path)

# Before:
shutil.rmtree(target_path)
# After:
shutil.rmtree(target_path, onerror=_remove_readonly)

Test plan

• Verified on Windows 11 — apm install now succeeds after a previously failed install leaves read-only .git objects in apm_modules/
• No-op on Linux/macOS — the onerror handler only triggers when the default delete fails, which doesn't happen on systems without read-only git objects

Fixes #90

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a Windows-specific issue where shutil.rmtree fails with [WinError 5] Access is denied when trying to delete .git directories. Git marks pack files and objects as read-only on Windows, which prevents standard deletion. The fix introduces a _remove_readonly error handler that clears read-only flags and handles file-in-use errors, then applies it to all five shutil.rmtree calls in github_downloader.py.

Changes:

• Added _remove_readonly error handler function that clears read-only flags (WinError 5) and handles file-in-use errors (WinError 32) with gc.collect() and retry logic
• Updated all 5 shutil.rmtree calls to use onerror=_remove_readonly instead of bare calls or ignore_errors=True
• Added explicit repo.close() call in resolve_git_reference method's finally block to release file handles before cleanup

[Copilot]

The new _remove_readonly error handler lacks test coverage. According to the project's testing principle ("When modifying existing code, add tests for the code paths you touch, on top of tests for the new functionality"), this new function should have tests that verify:

1. It correctly handles WinError 5 (read-only files) by clearing the read-only flag
2. It handles WinError 32 (file in use) with gc.collect() and retry logic
3. It works correctly on non-Windows systems (no-op when error handler isn't triggered)

Consider adding unit tests in tests/test_github_downloader.py that mock the error conditions and verify the handler's behavior.

[Copilot]

The repo object created by _clone_with_fallback is not explicitly closed before attempting to delete the .git directory at line 1199. This can cause WinError 32 (file in use) on Windows, as GitPython may hold file handles open. The same issue exists in resolve_git_reference (which you've addressed with repo.close() at lines 445-448), but it's missing here. Consider adding repo.close() before the shutil.rmtree(git_dir, ...) call, wrapped in a try-except to handle cases where repo might not be defined.

[danielmeppiel]

@ryanfk thank you for this fix - could you please address the review comments from Copilot? once that's done we are good to merge!