diff --git a/.gitignore b/.gitignore index 20dd4d44ef..09cb59993a 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,7 @@ venv* .eggs/ .tox/ /local +__pycache__/ .ruff_cache/ /pip-wheel-metadata diff --git a/changes/1780.bugfix.rst b/changes/1780.bugfix.rst new file mode 100644 index 0000000000..a259c681e3 --- /dev/null +++ b/changes/1780.bugfix.rst @@ -0,0 +1 @@ +Fixed a Windows issue where Briefcase could fail with a PermissionError when renaming recently created directories. diff --git a/pyproject.toml b/pyproject.toml index 667ed5e889..0a99da99b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -98,6 +98,7 @@ dependencies = [ "python-dateutil >= 2.9.0.post0", # transitive dependency (beeware/briefcase#1428) "httpx >= 0.20, < 1.0", "rich >= 12.6, < 15.0", + "tenacity >= 8.0, < 9.0", "tomli >= 2.0, < 3.0; python_version <= '3.10'", "tomli_w >= 1.0, < 2.0", ] diff --git a/src/briefcase/integrations/android_sdk.py b/src/briefcase/integrations/android_sdk.py index 38fd404a42..1eea218b7d 100644 --- a/src/briefcase/integrations/android_sdk.py +++ b/src/briefcase/integrations/android_sdk.py @@ -441,8 +441,9 @@ def install(self): self.tools.shutil.rmtree(self.cmdline_tools_path) # Rename the top level zip content to the final name - (self.cmdline_tools_path.parent / "cmdline-tools").rename( - self.cmdline_tools_path + self.tools.file.rename( + self.cmdline_tools_path.parent / "cmdline-tools", + self.cmdline_tools_path, ) # Zip file no longer needed once unpacked. diff --git a/src/briefcase/integrations/file.py b/src/briefcase/integrations/file.py index 20f342cd32..2909189414 100644 --- a/src/briefcase/integrations/file.py +++ b/src/briefcase/integrations/file.py @@ -13,6 +13,7 @@ import httpx import truststore +from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_fixed from briefcase.exceptions import ( BadNetworkResourceError, @@ -337,3 +338,17 @@ def _fetch_and_write_content(self, response: httpx.Response, filename: Path): # exist if the download fails or the user sends CTRL+C. with suppress(FileNotFoundError): self.tools.os.remove(temp_file.name) + + @retry( + retry=retry_if_exception_type(PermissionError), + wait=wait_fixed(0.2), + stop=stop_after_attempt(25), + ) + def rename(self, old_path: Path, new_path: object): + """Using tenacity for a retry policy on pathlib rename. + + Windows does not like renaming a dir in a path with an opened file, raising a + PermissionError. Only that error is retried; other errors (e.g. + FileNotFoundError) are surfaced immediately. + """ + old_path.rename(new_path) diff --git a/src/briefcase/integrations/java.py b/src/briefcase/integrations/java.py index 8dfa366c99..b0942378e6 100644 --- a/src/briefcase/integrations/java.py +++ b/src/briefcase/integrations/java.py @@ -310,7 +310,10 @@ def install(self): java_unpack_path = ( self.tools.base_path / f"jdk-{self.JDK_RELEASE}+{self.JDK_BUILD}" ) - java_unpack_path.rename(self.tools.base_path / self.JDK_INSTALL_DIR_NAME) + self.tools.file.rename( + java_unpack_path, + self.tools.base_path / self.JDK_INSTALL_DIR_NAME, + ) def uninstall(self): """Uninstall a JDK.""" diff --git a/tests/integrations/file/test_Files__rename.py b/tests/integrations/file/test_Files__rename.py new file mode 100644 index 0000000000..ee74ea4a04 --- /dev/null +++ b/tests/integrations/file/test_Files__rename.py @@ -0,0 +1,67 @@ +import os +import threading +import time +from pathlib import Path +from unittest.mock import MagicMock + +import pytest +import tenacity + + +def test_rename_path(mock_tools, tmp_path): + """Rename succeeds even when a file inside the directory is temporarily open. + + On Windows, renaming a directory while a file inside it is open raises a + PermissionError. This test simulates that scenario by opening a file in a background + thread, sleeping briefly, then closing it — verifying that the rename retries until + the file is released and ultimately succeeds. + """ + + def openclose(filepath): + handler = filepath.open(encoding="UTF-8") + try: + time.sleep(0.1) + finally: + handler.close() + + (tmp_path / "orig-dir-1").mkdir() + tl = tmp_path / "orig-dir-1/orig-file" + tl.touch() + file_access_thread = threading.Thread(target=openclose, args=(tl,)) + + file_access_thread.start() + # Sleep briefly so the background thread has time to open the file before + # the rename is attempted, ensuring the retry logic is exercised. + time.sleep(0.05) + mock_tools.file.rename(tmp_path / "orig-dir-1", tmp_path / "new-dir-1") + file_access_thread.join() + + assert "new-dir-1" in os.listdir(tmp_path) + + +def test_rename_path_file_not_found(mock_tools, tmp_path, monkeypatch): + """A FileNotFoundError is raised immediately without retrying.""" + mock_rename = MagicMock(side_effect=FileNotFoundError) + monkeypatch.setattr(Path, "rename", mock_rename) + + with pytest.raises(FileNotFoundError): + mock_tools.file.rename(tmp_path / "does-not-exist", tmp_path / "new-name") + + # rename should have been called exactly once — no retries + mock_rename.assert_called_once() + + +def test_rename_path_fail(mock_tools, tmp_path, monkeypatch): + """Retries are exhausted when rename repeatedly raises PermissionError.""" + mock_sleep = MagicMock() + monkeypatch.setattr(tenacity.nap.time, "sleep", mock_sleep) + mock_rename = MagicMock(side_effect=PermissionError) + monkeypatch.setattr(Path, "rename", mock_rename) + + with pytest.raises(tenacity.RetryError): + mock_tools.file.rename(tmp_path / "orig-dir-2", tmp_path / "new-dir-2") + + # 25 attempts total → 24 sleeps of 0.2 s each + assert mock_rename.call_count == 25 + assert mock_sleep.call_count == 24 + mock_sleep.assert_called_with(0.2)