Skip to content

Fix bugs affecting exception wrapping in rmtree callback #1700

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

Merged
merged 19 commits into from
Oct 10, 2023
Merged
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
Prev Previous commit
Next Next commit
Fix onerror callback type hinting, improve style
The onerror parameter of shutil.rmtree receives a 3-tuple like what
sys.exc_info() gives, not a string. Since we are not using that
parameter anyway, I've fixed it in the onerror function defined
in git.util.rmtree by changing it to Any rather than hinting it
narrowly.

I've also renamed the parameters so the names are based on those
that are documented in the shutil.rmtree documentation. The names
are not part of the function's interface (this follows both from
the documentation and the typeshed hints) but using those names may
make it easier to understand the function.

- func is renamed to function.

- path remains path.

- exc_info is renamed to _excinfo. This parameter is documented as
  excinfo, but I've prepended an underscore to signifity that it is
  unused.

These changes are to a local function and non-breaking.

Finally, although not all type checkers will flag this as an error
automatically, the git.util.rmtree function, like the shutil.rmtree
function it calls, is conceptually void, so it should not have any
return statements with operands. Because the return statement
appeared at the end, I've removed "return".
  • Loading branch information
EliahKagan committed Oct 9, 2023
commit ccbb2732efcfa265568f1a535a8b746ed07ed82a
6 changes: 3 additions & 3 deletions git/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,20 +182,20 @@ def rmtree(path: PathLike) -> None:
:note: we use shutil rmtree but adjust its behaviour to see whether files that
couldn't be deleted are read-only. Windows will not remove them in that case"""

def onerror(func: Callable, path: PathLike, exc_info: str) -> None:
def onerror(function: Callable, path: PathLike, _excinfo: Any) -> None:
# Is the error an access error ?
os.chmod(path, stat.S_IWUSR)

try:
func(path) # Will scream if still not possible to delete.
function(path) # Will scream if still not possible to delete.
except PermissionError as ex:
if HIDE_WINDOWS_KNOWN_ERRORS:
from unittest import SkipTest

raise SkipTest(f"FIXME: fails with: PermissionError\n {ex}") from ex
raise

return shutil.rmtree(path, False, onerror)
shutil.rmtree(path, False, onerror)


def rmfile(path: PathLike) -> None:
Expand Down