From 8046b2641cb577ea9937f6a85ad1319c0069d629 Mon Sep 17 00:00:00 2001 From: Stefan Pascu Date: Tue, 21 May 2024 13:57:13 -0400 Subject: [PATCH 1/8] removed unnecessary imports --- .gitignore | 1 + pyproject.toml | 5 ++- src/briefcase/commands/base.py | 2 + src/briefcase/integrations/__init__.py | 4 ++ src/briefcase/integrations/android_sdk.py | 5 ++- src/briefcase/integrations/base.py | 2 + src/briefcase/integrations/files.py | 30 +++++++++++++ src/briefcase/integrations/java.py | 5 ++- tests/integrations/conftest.py | 2 + tests/integrations/files/__init__.py | 0 .../integrations/files/test_Files__rename.py | 45 +++++++++++++++++++ .../integrations/files/test_Files__verify.py | 11 +++++ 12 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 src/briefcase/integrations/files.py create mode 100644 tests/integrations/files/__init__.py create mode 100644 tests/integrations/files/test_Files__rename.py create mode 100644 tests/integrations/files/test_Files__verify.py 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/pyproject.toml b/pyproject.toml index 35632c6dfe..c24719ea19 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,10 +90,11 @@ dependencies = [ "platformdirs >= 2.6, < 5.0", "psutil >= 5.9, < 8.0", "python-dateutil >= 2.9.0.post0", # transitive dependency (beeware/briefcase#1428) - "httpx >= 0.20, < 1.0", + "requests >= 2.28, < 3.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", + "tomli_w >= 1.0, < 2.0" ] [dependency-groups] diff --git a/src/briefcase/commands/base.py b/src/briefcase/commands/base.py index a45df168ec..0fc54dcbf0 100644 --- a/src/briefcase/commands/base.py +++ b/src/briefcase/commands/base.py @@ -45,6 +45,7 @@ ) from briefcase.integrations.base import ToolCache from briefcase.integrations.file import File +from briefcase.integrations.files import Files from briefcase.integrations.subprocess import Subprocess from briefcase.integrations.virtual_environment import VirtualEnvironment from briefcase.platforms import get_output_formats, get_platforms @@ -183,6 +184,7 @@ def __init__( Subprocess.verify(tools=self.tools) VirtualEnvironment.verify(tools=self.tools) File.verify(tools=self.tools) + Files.verify(tools=self.tools) if not is_clone: self.validate_locale() diff --git a/src/briefcase/integrations/__init__.py b/src/briefcase/integrations/__init__.py index 0c36d2ba0e..05b4c60cc6 100644 --- a/src/briefcase/integrations/__init__.py +++ b/src/briefcase/integrations/__init__.py @@ -3,6 +3,8 @@ cookiecutter, docker, file, + download, + files, flatpak, git, java, @@ -21,6 +23,8 @@ "cookiecutter", "docker", "file", + "download", + "files", "flatpak", "git", "java", diff --git a/src/briefcase/integrations/android_sdk.py b/src/briefcase/integrations/android_sdk.py index 38fd404a42..bf4bd2fcca 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.files.path_rename( + old_path=(self.cmdline_tools_path.parent / "cmdline-tools"), + new_path=self.cmdline_tools_path, ) # Zip file no longer needed once unpacked. diff --git a/src/briefcase/integrations/base.py b/src/briefcase/integrations/base.py index 53f8d80574..ea0f2df54f 100644 --- a/src/briefcase/integrations/base.py +++ b/src/briefcase/integrations/base.py @@ -32,6 +32,7 @@ from briefcase.integrations.android_sdk import AndroidSDK from briefcase.integrations.docker import Docker, DockerAppContext from briefcase.integrations.file import File + from briefcase.integrations.files import Files from briefcase.integrations.flatpak import Flatpak from briefcase.integrations.java import JDK from briefcase.integrations.linuxdeploy import LinuxDeploy @@ -151,6 +152,7 @@ class ToolCache(Mapping): app_context: Subprocess | DockerAppContext docker: Docker file: File + files: Files flatpak: Flatpak git: git_ java: JDK diff --git a/src/briefcase/integrations/files.py b/src/briefcase/integrations/files.py new file mode 100644 index 0000000000..0b85afece8 --- /dev/null +++ b/src/briefcase/integrations/files.py @@ -0,0 +1,30 @@ +from __future__ import annotations + +from pathlib import Path + +from tenacity import retry, stop_after_attempt, wait_fixed + +from briefcase.integrations.base import Tool, ToolCache + + +class Files(Tool): + name = "files" + full_name = "Files" + + @classmethod + def verify_install(cls, tools: ToolCache, **kwargs) -> Files: + """Make files available in tool cache.""" + # short circuit since already verified and available + if hasattr(tools, "files"): + return tools.files + + tools.files = Files(tools=tools) + return tools.files + + @retry(wait=wait_fixed(0.2), stop=stop_after_attempt(25)) + def path_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. + """ + old_path.rename(new_path) diff --git a/src/briefcase/integrations/java.py b/src/briefcase/integrations/java.py index 8dfa366c99..c27cb846e4 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.files.path_rename( + old_path=java_unpack_path, + new_path=self.tools.base_path / self.JDK_INSTALL_DIR_NAME, + ) def uninstall(self): """Uninstall a JDK.""" diff --git a/tests/integrations/conftest.py b/tests/integrations/conftest.py index 7047dfe4d6..caa6626c87 100644 --- a/tests/integrations/conftest.py +++ b/tests/integrations/conftest.py @@ -9,6 +9,7 @@ from briefcase.config import AppConfig from briefcase.integrations.base import ToolCache from briefcase.integrations.file import File +from briefcase.integrations.files import Files from briefcase.integrations.subprocess import Subprocess @@ -35,6 +36,7 @@ def mock_tools(dummy_console, tmp_path) -> ToolCache: # Make File and Subprocess always available File.verify(tools=mock_tools) + Files.verify(tools=mock_tools) Subprocess.verify(tools=mock_tools) return mock_tools diff --git a/tests/integrations/files/__init__.py b/tests/integrations/files/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integrations/files/test_Files__rename.py b/tests/integrations/files/test_Files__rename.py new file mode 100644 index 0000000000..c82269ec0c --- /dev/null +++ b/tests/integrations/files/test_Files__rename.py @@ -0,0 +1,45 @@ +import os +import sys +import threading +import time + +import pytest +import tenacity + + +def test_rename_path(mock_tools, tmp_path): + def openclose(filepath): + handler = filepath.open(encoding="UTF-8") + time.sleep(1) + 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,)) + rename_thread = threading.Thread( + target=mock_tools.files.path_rename, + args=(tmp_path / "orig-dir-1", tmp_path / "new-dir-1"), + ) + + file_access_thread.start() + rename_thread.start() + + rename_thread.join() + + assert "new-dir-1" in os.listdir(tmp_path) + + +@pytest.mark.xfail( + sys.platform == "win32", + raises=tenacity.RetryError, + reason="Windows can't rename folder in filepath when the file is open", +) +def test_rename_path_fail(mock_tools, tmp_path, monkeypatch): + (tmp_path / "orig-dir-2").mkdir() + (tmp_path / "orig-dir-2/orig-file").touch() + + with (tmp_path / "orig-dir-2/orig-file").open(encoding="UTF-8"): + + monkeypatch.setattr(tenacity.nap.time, "sleep", lambda x: True) + mock_tools.files.path_rename(tmp_path / "orig-dir-2", tmp_path / "new-dir-2") diff --git a/tests/integrations/files/test_Files__verify.py b/tests/integrations/files/test_Files__verify.py new file mode 100644 index 0000000000..5f8facfcb9 --- /dev/null +++ b/tests/integrations/files/test_Files__verify.py @@ -0,0 +1,11 @@ +from briefcase.integrations.files import Files + + +def test_short_circuit(mock_tools): + """Tool is not created if already cached.""" + mock_tools.files = "tool" + + tool = Files.verify(mock_tools) + + assert tool == "tool" + assert tool == mock_tools.files From 3a10d8bddd5ba845ffa3f6e8bef302edc59cf2e1 Mon Sep 17 00:00:00 2001 From: Stefan Pascu Date: Tue, 21 May 2024 14:11:36 -0400 Subject: [PATCH 2/8] Created 1780.bugfix.rst --- changes/1780.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/1780.bugfix.rst diff --git a/changes/1780.bugfix.rst b/changes/1780.bugfix.rst new file mode 100644 index 0000000000..8b176a17ca --- /dev/null +++ b/changes/1780.bugfix.rst @@ -0,0 +1 @@ +Added Files integration using tenacity for path dir rename retry From 1225f69bb97fdc555b53a5e39a5e5aaa74756722 Mon Sep 17 00:00:00 2001 From: Johanan Oppong Amoateng Date: Sun, 8 Mar 2026 13:25:53 +0000 Subject: [PATCH 3/8] address reviewer concerns --- src/briefcase/commands/base.py | 2 - src/briefcase/integrations/__init__.py | 6 +-- src/briefcase/integrations/android_sdk.py | 2 +- src/briefcase/integrations/base.py | 2 - src/briefcase/integrations/file.py | 15 +++++++ src/briefcase/integrations/files.py | 30 -------------- src/briefcase/integrations/java.py | 2 +- tests/integrations/conftest.py | 2 - .../integrations/files/test_Files__rename.py | 40 +++++++++---------- .../integrations/files/test_Files__verify.py | 8 ++-- 10 files changed, 42 insertions(+), 67 deletions(-) delete mode 100644 src/briefcase/integrations/files.py diff --git a/src/briefcase/commands/base.py b/src/briefcase/commands/base.py index 0fc54dcbf0..a45df168ec 100644 --- a/src/briefcase/commands/base.py +++ b/src/briefcase/commands/base.py @@ -45,7 +45,6 @@ ) from briefcase.integrations.base import ToolCache from briefcase.integrations.file import File -from briefcase.integrations.files import Files from briefcase.integrations.subprocess import Subprocess from briefcase.integrations.virtual_environment import VirtualEnvironment from briefcase.platforms import get_output_formats, get_platforms @@ -184,7 +183,6 @@ def __init__( Subprocess.verify(tools=self.tools) VirtualEnvironment.verify(tools=self.tools) File.verify(tools=self.tools) - Files.verify(tools=self.tools) if not is_clone: self.validate_locale() diff --git a/src/briefcase/integrations/__init__.py b/src/briefcase/integrations/__init__.py index 05b4c60cc6..670c670625 100644 --- a/src/briefcase/integrations/__init__.py +++ b/src/briefcase/integrations/__init__.py @@ -2,9 +2,8 @@ android_sdk, cookiecutter, docker, - file, download, - files, + file, flatpak, git, java, @@ -22,9 +21,8 @@ "android_sdk", "cookiecutter", "docker", - "file", "download", - "files", + "file", "flatpak", "git", "java", diff --git a/src/briefcase/integrations/android_sdk.py b/src/briefcase/integrations/android_sdk.py index bf4bd2fcca..0ed8ca0047 100644 --- a/src/briefcase/integrations/android_sdk.py +++ b/src/briefcase/integrations/android_sdk.py @@ -441,7 +441,7 @@ def install(self): self.tools.shutil.rmtree(self.cmdline_tools_path) # Rename the top level zip content to the final name - self.tools.files.path_rename( + self.tools.file.path_rename( old_path=(self.cmdline_tools_path.parent / "cmdline-tools"), new_path=self.cmdline_tools_path, ) diff --git a/src/briefcase/integrations/base.py b/src/briefcase/integrations/base.py index ea0f2df54f..53f8d80574 100644 --- a/src/briefcase/integrations/base.py +++ b/src/briefcase/integrations/base.py @@ -32,7 +32,6 @@ from briefcase.integrations.android_sdk import AndroidSDK from briefcase.integrations.docker import Docker, DockerAppContext from briefcase.integrations.file import File - from briefcase.integrations.files import Files from briefcase.integrations.flatpak import Flatpak from briefcase.integrations.java import JDK from briefcase.integrations.linuxdeploy import LinuxDeploy @@ -152,7 +151,6 @@ class ToolCache(Mapping): app_context: Subprocess | DockerAppContext docker: Docker file: File - files: Files flatpak: Flatpak git: git_ java: JDK diff --git a/src/briefcase/integrations/file.py b/src/briefcase/integrations/file.py index 20f342cd32..1ac3d21624 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 path_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/files.py b/src/briefcase/integrations/files.py deleted file mode 100644 index 0b85afece8..0000000000 --- a/src/briefcase/integrations/files.py +++ /dev/null @@ -1,30 +0,0 @@ -from __future__ import annotations - -from pathlib import Path - -from tenacity import retry, stop_after_attempt, wait_fixed - -from briefcase.integrations.base import Tool, ToolCache - - -class Files(Tool): - name = "files" - full_name = "Files" - - @classmethod - def verify_install(cls, tools: ToolCache, **kwargs) -> Files: - """Make files available in tool cache.""" - # short circuit since already verified and available - if hasattr(tools, "files"): - return tools.files - - tools.files = Files(tools=tools) - return tools.files - - @retry(wait=wait_fixed(0.2), stop=stop_after_attempt(25)) - def path_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. - """ - old_path.rename(new_path) diff --git a/src/briefcase/integrations/java.py b/src/briefcase/integrations/java.py index c27cb846e4..9a9dd992ce 100644 --- a/src/briefcase/integrations/java.py +++ b/src/briefcase/integrations/java.py @@ -310,7 +310,7 @@ def install(self): java_unpack_path = ( self.tools.base_path / f"jdk-{self.JDK_RELEASE}+{self.JDK_BUILD}" ) - self.tools.files.path_rename( + self.tools.file.path_rename( old_path=java_unpack_path, new_path=self.tools.base_path / self.JDK_INSTALL_DIR_NAME, ) diff --git a/tests/integrations/conftest.py b/tests/integrations/conftest.py index caa6626c87..7047dfe4d6 100644 --- a/tests/integrations/conftest.py +++ b/tests/integrations/conftest.py @@ -9,7 +9,6 @@ from briefcase.config import AppConfig from briefcase.integrations.base import ToolCache from briefcase.integrations.file import File -from briefcase.integrations.files import Files from briefcase.integrations.subprocess import Subprocess @@ -36,7 +35,6 @@ def mock_tools(dummy_console, tmp_path) -> ToolCache: # Make File and Subprocess always available File.verify(tools=mock_tools) - Files.verify(tools=mock_tools) Subprocess.verify(tools=mock_tools) return mock_tools diff --git a/tests/integrations/files/test_Files__rename.py b/tests/integrations/files/test_Files__rename.py index c82269ec0c..419600337b 100644 --- a/tests/integrations/files/test_Files__rename.py +++ b/tests/integrations/files/test_Files__rename.py @@ -1,7 +1,8 @@ import os -import sys import threading import time +from pathlib import Path +from unittest.mock import MagicMock import pytest import tenacity @@ -10,36 +11,33 @@ def test_rename_path(mock_tools, tmp_path): def openclose(filepath): handler = filepath.open(encoding="UTF-8") - time.sleep(1) - handler.close() + 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,)) - rename_thread = threading.Thread( - target=mock_tools.files.path_rename, - args=(tmp_path / "orig-dir-1", tmp_path / "new-dir-1"), - ) file_access_thread.start() - rename_thread.start() - - rename_thread.join() + mock_tools.file.path_rename(tmp_path / "orig-dir-1", tmp_path / "new-dir-1") + file_access_thread.join() assert "new-dir-1" in os.listdir(tmp_path) -@pytest.mark.xfail( - sys.platform == "win32", - raises=tenacity.RetryError, - reason="Windows can't rename folder in filepath when the file is open", -) -def test_rename_path_fail(mock_tools, tmp_path, monkeypatch): - (tmp_path / "orig-dir-2").mkdir() - (tmp_path / "orig-dir-2/orig-file").touch() +def test_rename_path_file_not_found(mock_tools, tmp_path): + """A FileNotFoundError is raised immediately without retrying.""" + with pytest.raises(FileNotFoundError): + mock_tools.file.path_rename(tmp_path / "does-not-exist", tmp_path / "new-name") - with (tmp_path / "orig-dir-2/orig-file").open(encoding="UTF-8"): - monkeypatch.setattr(tenacity.nap.time, "sleep", lambda x: True) - mock_tools.files.path_rename(tmp_path / "orig-dir-2", tmp_path / "new-dir-2") +def test_rename_path_fail(mock_tools, tmp_path, monkeypatch): + """Retries are exhausted when rename repeatedly raises PermissionError.""" + monkeypatch.setattr(tenacity.nap.time, "sleep", lambda x: True) + monkeypatch.setattr(Path, "rename", MagicMock(side_effect=PermissionError)) + + with pytest.raises(tenacity.RetryError): + mock_tools.file.path_rename(tmp_path / "orig-dir-2", tmp_path / "new-dir-2") diff --git a/tests/integrations/files/test_Files__verify.py b/tests/integrations/files/test_Files__verify.py index 5f8facfcb9..05dca89b76 100644 --- a/tests/integrations/files/test_Files__verify.py +++ b/tests/integrations/files/test_Files__verify.py @@ -1,11 +1,11 @@ -from briefcase.integrations.files import Files +from briefcase.integrations.files import File def test_short_circuit(mock_tools): """Tool is not created if already cached.""" - mock_tools.files = "tool" + mock_tools.file = "tool" - tool = Files.verify(mock_tools) + tool = File.verify(mock_tools) assert tool == "tool" - assert tool == mock_tools.files + assert tool == mock_tools.file From d1ae49b0578c4fe0daa78a22f9e13d38c91fe2d8 Mon Sep 17 00:00:00 2001 From: Johanan Oppong Amoateng Date: Sun, 8 Mar 2026 13:29:34 +0000 Subject: [PATCH 4/8] add requests --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index c24719ea19..1689951cfe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,6 +90,7 @@ dependencies = [ "platformdirs >= 2.6, < 5.0", "psutil >= 5.9, < 8.0", "python-dateutil >= 2.9.0.post0", # transitive dependency (beeware/briefcase#1428) + "httpx >= 0.20, < 1.0", "requests >= 2.28, < 3.0", "rich >= 12.6, < 15.0", "tenacity >= 8.0, < 9.0", From 9d9d970037400af614281c55cda519dc2bf2f94c Mon Sep 17 00:00:00 2001 From: Johanan Oppong Amoateng Date: Sun, 8 Mar 2026 13:40:35 +0000 Subject: [PATCH 5/8] fix ci --- src/briefcase/integrations/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/briefcase/integrations/__init__.py b/src/briefcase/integrations/__init__.py index 670c670625..0c36d2ba0e 100644 --- a/src/briefcase/integrations/__init__.py +++ b/src/briefcase/integrations/__init__.py @@ -2,7 +2,6 @@ android_sdk, cookiecutter, docker, - download, file, flatpak, git, @@ -21,7 +20,6 @@ "android_sdk", "cookiecutter", "docker", - "download", "file", "flatpak", "git", From 00370a6dcf109b80a3d8e8e8f550a1bd044fd14f Mon Sep 17 00:00:00 2001 From: Johanan Oppong Amoateng Date: Sun, 8 Mar 2026 13:44:15 +0000 Subject: [PATCH 6/8] fixes --- .../{files => file}/test_Files__rename.py | 0 tests/integrations/files/__init__.py | 0 tests/integrations/files/test_Files__verify.py | 11 ----------- 3 files changed, 11 deletions(-) rename tests/integrations/{files => file}/test_Files__rename.py (100%) delete mode 100644 tests/integrations/files/__init__.py delete mode 100644 tests/integrations/files/test_Files__verify.py diff --git a/tests/integrations/files/test_Files__rename.py b/tests/integrations/file/test_Files__rename.py similarity index 100% rename from tests/integrations/files/test_Files__rename.py rename to tests/integrations/file/test_Files__rename.py diff --git a/tests/integrations/files/__init__.py b/tests/integrations/files/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/integrations/files/test_Files__verify.py b/tests/integrations/files/test_Files__verify.py deleted file mode 100644 index 05dca89b76..0000000000 --- a/tests/integrations/files/test_Files__verify.py +++ /dev/null @@ -1,11 +0,0 @@ -from briefcase.integrations.files import File - - -def test_short_circuit(mock_tools): - """Tool is not created if already cached.""" - mock_tools.file = "tool" - - tool = File.verify(mock_tools) - - assert tool == "tool" - assert tool == mock_tools.file From 0eba7b71a362b80650154d00887b0f815fb51597 Mon Sep 17 00:00:00 2001 From: Johanan Oppong Amoateng Date: Wed, 11 Mar 2026 23:41:03 +0000 Subject: [PATCH 7/8] address pr reviews --- changes/1780.bugfix.rst | 2 +- pyproject.toml | 2 -- src/briefcase/integrations/android_sdk.py | 6 ++-- src/briefcase/integrations/file.py | 2 +- src/briefcase/integrations/java.py | 6 ++-- tests/integrations/file/test_Files__rename.py | 36 +++++++++++++++---- 6 files changed, 38 insertions(+), 16 deletions(-) diff --git a/changes/1780.bugfix.rst b/changes/1780.bugfix.rst index 8b176a17ca..a259c681e3 100644 --- a/changes/1780.bugfix.rst +++ b/changes/1780.bugfix.rst @@ -1 +1 @@ -Added Files integration using tenacity for path dir rename retry +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 0d16fe01e6..799daacf00 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -97,11 +97,9 @@ dependencies = [ "psutil >= 5.9, < 8.0", "python-dateutil >= 2.9.0.post0", # transitive dependency (beeware/briefcase#1428) "httpx >= 0.20, < 1.0", - "requests >= 2.28, < 3.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" ] [dependency-groups] diff --git a/src/briefcase/integrations/android_sdk.py b/src/briefcase/integrations/android_sdk.py index 0ed8ca0047..1eea218b7d 100644 --- a/src/briefcase/integrations/android_sdk.py +++ b/src/briefcase/integrations/android_sdk.py @@ -441,9 +441,9 @@ def install(self): self.tools.shutil.rmtree(self.cmdline_tools_path) # Rename the top level zip content to the final name - self.tools.file.path_rename( - old_path=(self.cmdline_tools_path.parent / "cmdline-tools"), - new_path=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 1ac3d21624..2909189414 100644 --- a/src/briefcase/integrations/file.py +++ b/src/briefcase/integrations/file.py @@ -344,7 +344,7 @@ def _fetch_and_write_content(self, response: httpx.Response, filename: Path): wait=wait_fixed(0.2), stop=stop_after_attempt(25), ) - def path_rename(self, old_path: Path, new_path: object): + 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 diff --git a/src/briefcase/integrations/java.py b/src/briefcase/integrations/java.py index 9a9dd992ce..b0942378e6 100644 --- a/src/briefcase/integrations/java.py +++ b/src/briefcase/integrations/java.py @@ -310,9 +310,9 @@ def install(self): java_unpack_path = ( self.tools.base_path / f"jdk-{self.JDK_RELEASE}+{self.JDK_BUILD}" ) - self.tools.file.path_rename( - old_path=java_unpack_path, - new_path=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): diff --git a/tests/integrations/file/test_Files__rename.py b/tests/integrations/file/test_Files__rename.py index 419600337b..ee74ea4a04 100644 --- a/tests/integrations/file/test_Files__rename.py +++ b/tests/integrations/file/test_Files__rename.py @@ -9,6 +9,14 @@ 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: @@ -22,22 +30,38 @@ def openclose(filepath): file_access_thread = threading.Thread(target=openclose, args=(tl,)) file_access_thread.start() - mock_tools.file.path_rename(tmp_path / "orig-dir-1", tmp_path / "new-dir-1") + # 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): +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.path_rename(tmp_path / "does-not-exist", tmp_path / "new-name") + 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.""" - monkeypatch.setattr(tenacity.nap.time, "sleep", lambda x: True) - monkeypatch.setattr(Path, "rename", MagicMock(side_effect=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.path_rename(tmp_path / "orig-dir-2", tmp_path / "new-dir-2") + 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) From 60b593902a7ba576d888fb4114844bffecbcb280 Mon Sep 17 00:00:00 2001 From: Johanan Oppong Amoateng Date: Wed, 11 Mar 2026 23:47:58 +0000 Subject: [PATCH 8/8] fix test --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 799daacf00..0a99da99b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -100,6 +100,7 @@ dependencies = [ "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", ] [dependency-groups]