Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the rmtree callback Windows-only #1739

Merged
merged 6 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion git/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,9 @@ def handler(function: Callable, path: PathLike, _excinfo: Any) -> None:
raise SkipTest(f"FIXME: fails with: PermissionError\n {ex}") from ex
raise

if sys.version_info >= (3, 12):
if os.name != "nt":
shutil.rmtree(path)
elif sys.version_info >= (3, 12):
shutil.rmtree(path, onexc=handler)
else:
shutil.rmtree(path, onerror=handler)
Expand Down
63 changes: 47 additions & 16 deletions test/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,31 +109,66 @@ def test_deletes_dir_with_readonly_files(self, tmp_path):
sys.platform == "cygwin",
reason="Cygwin can't set the permissions that make the test meaningful.",
)
def test_wraps_perm_error_if_enabled(self, mocker, permission_error_tmpdir):
"""rmtree wraps PermissionError when HIDE_WINDOWS_KNOWN_ERRORS is true."""
def test_avoids_changing_permissions_outside_tree(self, tmp_path):
# Automatically works on Windows, but on Unix requires either special handling
# or refraining from attempting to fix PermissionError by making chmod calls.

dir1 = tmp_path / "dir1"
dir1.mkdir()
(dir1 / "file").write_bytes(b"")
(dir1 / "file").chmod(stat.S_IRUSR)
old_mode = (dir1 / "file").stat().st_mode

dir2 = tmp_path / "dir2"
dir2.mkdir()
(dir2 / "symlink").symlink_to(dir1 / "file")
dir2.chmod(stat.S_IRUSR | stat.S_IXUSR)

try:
rmtree(dir2)
except PermissionError:
pass # On Unix, dir2 is not writable, so dir2/symlink may not be deleted.
except SkipTest as ex:
self.fail(f"rmtree unexpectedly attempts skip: {ex!r}")

new_mode = (dir1 / "file").stat().st_mode
assert old_mode == new_mode, f"Should stay {old_mode:#o}, became {new_mode:#o}."

def _patch_for_wrapping_test(self, mocker, hide_windows_known_errors):
# Access the module through sys.modules so it is unambiguous which module's
# attribute we patch: the original git.util, not git.index.util even though
# git.index.util "replaces" git.util and is what "import git.util" gives us.
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", True)
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", hide_windows_known_errors)

# Disable common chmod functions so the callback can never fix the problem.
# Disable common chmod functions so the callback can't fix a PermissionError.
mocker.patch.object(os, "chmod")
mocker.patch.object(pathlib.Path, "chmod")

# Now we can see how an intractable PermissionError is treated.
@pytest.mark.skipif(
os.name != "nt",
reason="PermissionError is only ever wrapped on Windows",
)
def test_wraps_perm_error_if_enabled(self, mocker, permission_error_tmpdir):
"""rmtree wraps PermissionError on Windows when HIDE_WINDOWS_KNOWN_ERRORS is true."""
self._patch_for_wrapping_test(mocker, True)

with pytest.raises(SkipTest):
rmtree(permission_error_tmpdir)

@pytest.mark.skipif(
sys.platform == "cygwin",
reason="Cygwin can't set the permissions that make the test meaningful.",
)
def test_does_not_wrap_perm_error_unless_enabled(self, mocker, permission_error_tmpdir):
"""rmtree does not wrap PermissionError when HIDE_WINDOWS_KNOWN_ERRORS is false."""
# See comments in test_wraps_perm_error_if_enabled for details about patching.
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", False)
mocker.patch.object(os, "chmod")
mocker.patch.object(pathlib.Path, "chmod")
@pytest.mark.parametrize(
"hide_windows_known_errors",
[
pytest.param(False),
pytest.param(True, marks=pytest.mark.skipif(os.name == "nt", reason="We would wrap on Windows")),
],
)
def test_does_not_wrap_perm_error_unless_enabled(self, mocker, permission_error_tmpdir, hide_windows_known_errors):
"""rmtree does not wrap PermissionError on non-Windows systems or when HIDE_WINDOWS_KNOWN_ERRORS is false."""
self._patch_for_wrapping_test(mocker, hide_windows_known_errors)

with pytest.raises(PermissionError):
try:
Expand All @@ -144,11 +179,7 @@ def test_does_not_wrap_perm_error_unless_enabled(self, mocker, permission_error_
@pytest.mark.parametrize("hide_windows_known_errors", [False, True])
def test_does_not_wrap_other_errors(self, tmp_path, mocker, hide_windows_known_errors):
file_not_found_tmpdir = tmp_path / "testdir" # It is deliberately never created.

# See comments in test_wraps_perm_error_if_enabled for details about patching.
mocker.patch.object(sys.modules["git.util"], "HIDE_WINDOWS_KNOWN_ERRORS", hide_windows_known_errors)
mocker.patch.object(os, "chmod")
mocker.patch.object(pathlib.Path, "chmod")
self._patch_for_wrapping_test(mocker, hide_windows_known_errors)

with pytest.raises(FileNotFoundError):
try:
Expand Down
Loading