diff --git a/changes/2956.misc.md b/changes/2956.misc.md new file mode 100644 index 0000000000..2ae8003267 --- /dev/null +++ b/changes/2956.misc.md @@ -0,0 +1 @@ +Some internal file testing tools were refactored. diff --git a/src/briefcase/commands/create.py b/src/briefcase/commands/create.py index b62a7ce1ea..b0ca97243c 100644 --- a/src/briefcase/commands/create.py +++ b/src/briefcase/commands/create.py @@ -4,7 +4,6 @@ import hashlib import os import platform -import re import shutil import subprocess import sys @@ -30,8 +29,6 @@ from .base import BaseCommand, full_options -relative_path_matcher = re.compile(r"^\.{1,2}[\\/]") - def cookiecutter_cache_path(template): """Determine the cookiecutter template cache directory given a template URL. @@ -569,7 +566,12 @@ def _write_requirements_file( f.write(f"{requirement}\n") if requirement_installer_args_path: - pip_args = "\n".join(self._extra_pip_args(app)) + pip_args = "\n".join( + self.tools.file.resolve_relative_args( + app.requirement_installer_args, + self.base_path, + ) + ) requirement_installer_args_path.write_text( f"{pip_args}\n", encoding="utf-8" ) @@ -589,19 +591,10 @@ def _extra_pip_args(self, app: FinalizedAppConfig): :param app: The app configuration :returns: A list of additional arguments """ - args: list[str] = [] - for argument in app.requirement_installer_args: - to_append = argument - if relative_path_matcher.match(argument) and self.tools.file.is_local_path( - argument - ): - abs_path = os.path.abspath(self.base_path / argument) - if Path(abs_path).exists(): - to_append = abs_path - - args.append(to_append) - - return args + return self.tools.file.resolve_relative_args( + app.requirement_installer_args, + self.base_path, + ) def _pip_install( self, diff --git a/src/briefcase/integrations/file.py b/src/briefcase/integrations/file.py index b8475ab162..256ae957ae 100644 --- a/src/briefcase/integrations/file.py +++ b/src/briefcase/integrations/file.py @@ -2,6 +2,7 @@ import itertools import os +import re import shutil import ssl import sys @@ -22,47 +23,7 @@ ) from briefcase.integrations.base import Tool, ToolCache - -def _has_url(requirement): - """Determine if the requirement is defined as a URL. - - Detects any of the URL schemes supported by pip - (https://pip.pypa.io/en/stable/topics/vcs-support/). - - :param requirement: The requirement to check - :returns: True if the requirement is a URL supported by pip. - """ - return any( - f"{scheme}:" in requirement - for scheme in ( - "http", - "https", - "file", - "ftp", - "git+file", - "git+https", - "git+ssh", - "git+http", - "git+git", - "git", - "hg+file", - "hg+http", - "hg+https", - "hg+ssh", - "hg+static-http", - "svn", - "svn+svn", - "svn+http", - "svn+https", - "svn+ssh", - "bzr+http", - "bzr+https", - "bzr+ssh", - "bzr+sftp", - "bzr+ftp", - "bzr+lp", - ) - ) +RELATIVE_PATH_RE = re.compile(r"^\.{1,2}[\\/]") class File(Tool): @@ -148,6 +109,47 @@ def ssl_context(self): return self._ssl_context + def is_scm_url(self, path): + """Determine if the requirement is defined as a URL. + + Detects any of the URL schemes supported by pip + (https://pip.pypa.io/en/stable/topics/vcs-support/). + + :param requirement: The requirement to check + :returns: True if the requirement is a URL supported by pip. + """ + return any( + f"{scheme}:" in path + for scheme in ( + "http", + "https", + "file", + "ftp", + "git+file", + "git+https", + "git+ssh", + "git+http", + "git+git", + "git", + "hg+file", + "hg+http", + "hg+https", + "hg+ssh", + "hg+static-http", + "svn", + "svn+svn", + "svn+http", + "svn+https", + "svn+ssh", + "bzr+http", + "bzr+https", + "bzr+ssh", + "bzr+sftp", + "bzr+ftp", + "bzr+lp", + ) + ) + def is_local_path(self, reference: str | os.PathLike) -> bool: """Determine if the reference is a local file path. @@ -159,7 +161,9 @@ def is_local_path(self, reference: str | os.PathLike) -> bool: if os.altsep: separators.append(os.altsep) - return any(sep in reference for sep in separators) and (not _has_url(reference)) + return any(sep in reference for sep in separators) and ( + not self.is_scm_url(reference) + ) def is_archive(self, filename: str | os.PathLike) -> bool: """Can a file be unpacked via `shutil.unpack_archive()`? @@ -182,6 +186,28 @@ def is_archive(self, filename: str | os.PathLike) -> bool: } return not file_extensions.isdisjoint(self.supported_archive_extensions) + def resolve_relative_args(self, args: list[str], base_path: Path) -> list[str]: + """Convert a list of arguments so that all relative file path references are + converted into resolved paths relative to the base path. + + An argument is only converted if: + 1. It "looks" like a relative path (i.e., it starts with a . or ..) + 2. The path referenced actually exists + + :param args: The initial list of arguments + :param base_path: The base path against which file references should be resolved + :returns: The resolved list of arguments. + """ + resolved_args: list[str] = [] + for arg in args: + if RELATIVE_PATH_RE.match(arg) and self.tools.file.is_local_path(arg): + abs_path = (base_path / arg).resolve() + if abs_path.exists(): + arg = abs_path + + resolved_args.append(arg) + return resolved_args + @property def supported_archive_extensions(self) -> set[str]: return { diff --git a/tests/commands/create/conftest.py b/tests/commands/create/conftest.py index f5b1a8c0a4..b7aaadcbcb 100644 --- a/tests/commands/create/conftest.py +++ b/tests/commands/create/conftest.py @@ -13,7 +13,7 @@ from briefcase.integrations.base import Tool from briefcase.integrations.subprocess import Subprocess -from ...utils import create_file +from ...utils import create_file, create_toml_file @pytest.fixture @@ -274,36 +274,39 @@ def bundle_path(myapp, tmp_path): @pytest.fixture def app_packages_path_index(bundle_path): (bundle_path / "path/to/app_packages").mkdir(parents=True, exist_ok=True) - with (bundle_path / "briefcase.toml").open("wb") as f: - index = { + create_toml_file( + bundle_path / "briefcase.toml", + { "paths": { "app_path": "path/to/app", "app_packages_path": "path/to/app_packages", "support_path": "path/to/support", "support_revision": 37, } - } - tomli_w.dump(index, f) + }, + ) @pytest.fixture def app_requirements_path_index(bundle_path): - with (bundle_path / "briefcase.toml").open("wb") as f: - index = { + create_toml_file( + bundle_path / "briefcase.toml", + { "paths": { "app_path": "path/to/app", "app_requirements_path": "path/to/requirements.txt", "support_path": "path/to/support", "support_revision": 37, } - } - tomli_w.dump(index, f) + }, + ) @pytest.fixture def app_requirement_installer_args_path_index(bundle_path): - with (bundle_path / "briefcase.toml").open("wb") as f: - index = { + create_toml_file( + bundle_path / "briefcase.toml", + { "paths": { "app_path": "path/to/app", "app_requirements_path": "path/to/requirements.txt", @@ -311,33 +314,35 @@ def app_requirement_installer_args_path_index(bundle_path): "support_path": "path/to/support", "support_revision": 37, } - } - tomli_w.dump(index, f) + }, + ) @pytest.fixture def no_support_revision_index(bundle_path): - with (bundle_path / "briefcase.toml").open("wb") as f: - index = { + create_toml_file( + bundle_path / "briefcase.toml", + { "paths": { "app_path": "path/to/app", "app_requirements_path": "path/to/requirements.txt", "support_path": "path/to/support", } - } - tomli_w.dump(index, f) + }, + ) @pytest.fixture def no_support_path_index(bundle_path): - with (bundle_path / "briefcase.toml").open("wb") as f: - index = { + create_toml_file( + bundle_path / "briefcase.toml", + { "paths": { "app_path": "path/to/app", "app_requirements_path": "path/to/requirements.txt", } - } - tomli_w.dump(index, f) + }, + ) @pytest.fixture diff --git a/tests/commands/create/test_install_app_requirements.py b/tests/commands/create/test_install_app_requirements.py index bd9e7a066c..51fb424b6a 100644 --- a/tests/commands/create/test_install_app_requirements.py +++ b/tests/commands/create/test_install_app_requirements.py @@ -230,7 +230,7 @@ def test_app_packages_requirement_installer_args_path_transformed( "--no-user", f"--target={app_packages_path}", "--extra-index-url", - os.path.abspath(create_command.base_path / "packages"), + (create_command.base_path / "packages").resolve(), "package", ], check=True, diff --git a/tests/conftest.py b/tests/conftest.py index 3494c3583e..ddf2639a9a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,12 +1,18 @@ import inspect import os +import platform +import shutil import subprocess +import sys import time from unittest.mock import ANY, MagicMock import pytest from briefcase.config import DraftAppConfig +from briefcase.integrations.base import ToolCache +from briefcase.integrations.file import File +from briefcase.integrations.subprocess import Subprocess from .utils import DummyConsole, create_file @@ -132,3 +138,31 @@ def first_app(first_app_unbuilt, tmp_path): ) return first_app_unbuilt + + +@pytest.fixture +def mock_tools(dummy_console, tmp_path) -> ToolCache: + mock_tools = ToolCache( + console=dummy_console, + base_path=tmp_path / "tools", + home_path=tmp_path / "home", + ) + + # Mock stdlib tools + mock_tools.os = MagicMock(spec_set=os) + mock_tools.platform = MagicMock(spec_set=platform) + mock_tools.shutil = MagicMock(spec_set=shutil) + mock_tools.sys = MagicMock(spec_set=sys) + + # Mock an empty environment + mock_tools.os.environ = {} + + # Create base directories + mock_tools.base_path.mkdir(parents=True) + mock_tools.home_path.mkdir(parents=True) + + # Make File and Subprocess always available + File.verify(tools=mock_tools) + Subprocess.verify(tools=mock_tools) + + return mock_tools diff --git a/tests/integrations/conftest.py b/tests/integrations/conftest.py index 567a314258..2669baf616 100644 --- a/tests/integrations/conftest.py +++ b/tests/integrations/conftest.py @@ -1,47 +1,10 @@ -import os -import platform -import shutil -import sys -from unittest.mock import MagicMock - import pytest from briefcase.config import DraftAppConfig -from briefcase.integrations.base import ToolCache -from briefcase.integrations.file import File -from briefcase.integrations.subprocess import Subprocess from ..utils import create_file -@pytest.fixture -def mock_tools(dummy_console, tmp_path) -> ToolCache: - mock_tools = ToolCache( - console=dummy_console, - base_path=tmp_path / "tools", - home_path=tmp_path / "home", - ) - - # Mock stdlib tools - mock_tools.os = MagicMock(spec_set=os) - mock_tools.platform = MagicMock(spec_set=platform) - mock_tools.shutil = MagicMock(spec_set=shutil) - mock_tools.sys = MagicMock(spec_set=sys) - - # Mock an empty environment - mock_tools.os.environ = {} - - # Create base directories - mock_tools.base_path.mkdir(parents=True) - mock_tools.home_path.mkdir(parents=True) - - # Make File and Subprocess always available - File.verify(tools=mock_tools) - Subprocess.verify(tools=mock_tools) - - return mock_tools - - @pytest.fixture def first_app_config(tmp_path): create_file(tmp_path / "base_path" / "LICENSE", "MIT License") diff --git a/tests/integrations/file/test_File__is_scm_url.py b/tests/integrations/file/test_File__is_scm_url.py new file mode 100644 index 0000000000..9113964bbb --- /dev/null +++ b/tests/integrations/file/test_File__is_scm_url.py @@ -0,0 +1,44 @@ +import pytest + + +@pytest.mark.parametrize( + ("path", "is_url"), + [ + # Files and file-like things + ("/path/to/file.txt", False), + ("path/to/file.txt", False), + ("file.txt", False), + # URLs + ("http://example.com", True), + ("https://example.com", True), + ("http://github.com/example/repo", True), + ("https://github.com/example/repo", True), + ("file://github.com/example/repo", True), + ("ftp://github.com/example/repo", True), + ("git+file://github.com/example/repo", True), + ("git+https://github.com/example/repo", True), + ("git+ssh://github.com/example/repo", True), + ("git+http://github.com/example/repo", True), + ("git+git://github.com/example/repo", True), + ("git://github.com/example/repo", True), + ("hg+file://github.com/example/repo", True), + ("hg+http://github.com/example/repo", True), + ("hg+https://github.com/example/repo", True), + ("hg+ssh://github.com/example/repo", True), + ("hg+static-http://github.com/example/repo", True), + ("svn://github.com/example/repo", True), + ("svn+svn://github.com/example/repo", True), + ("svn+http://github.com/example/repo", True), + ("svn+https://github.com/example/repo", True), + ("svn+ssh://github.com/example/repo", True), + ("bzr+http://github.com/example/repo", True), + ("bzr+https://github.com/example/repo", True), + ("bzr+ssh://github.com/example/repo", True), + ("bzr+sftp://github.com/example/repo", True), + ("bzr+ftp://github.com/example/repo", True), + ("bzr+lp://github.com/example/repo", True), + ], +) +def test_is_scm_url(mock_tools, path, is_url): + """URLs can be identified.""" + assert mock_tools.file.is_scm_url(path) == is_url diff --git a/tests/integrations/file/test_File__resolve_relative_args.py b/tests/integrations/file/test_File__resolve_relative_args.py new file mode 100644 index 0000000000..6be64a2e9f --- /dev/null +++ b/tests/integrations/file/test_File__resolve_relative_args.py @@ -0,0 +1,57 @@ +import pytest + +from ...utils import create_file + + +@pytest.mark.parametrize( + "args", + [ + # Empty list of args + [], + # Non-file references + ["--foo", "bar"], + ["--foo", ".file"], + # Non-existent relative references + ["--foo", "./does-not-exist"], + ["--foo", "../does-not-exist"], + ["--foo", "./somewhere/does-not-exist"], + ["--foo", "../somewhere/does-not-exist"], + ], +) +def test_no_op(mock_tools, args, tmp_path): + """File paths that aren't existent relative references are unchanged.""" + assert mock_tools.file.resolve_relative_args(args, tmp_path / "deep/other") == args + + +@pytest.mark.parametrize( + "args", + [ + ["--foo", "bar"], + ["--foo", ".file"], + ], +) +def test_no_op_existing(mock_tools, args, tmp_path): + """Non-file references that do match filenames aren't transformed.""" + create_file((tmp_path / "tools" / args[1]).resolve(), "content") + + assert mock_tools.file.resolve_relative_args(args, tmp_path / "deep/other") == args + + +@pytest.mark.parametrize( + "args", + [ + # Existent relative references + ["--foo", "./does-not-exist"], + ["--foo", "../does-not-exist"], + ["--foo", "./somewhere/does-not-exist"], + ["--foo", "../somewhere/does-not-exist"], + ], +) +def test_transformed(mock_tools, args, tmp_path): + """File paths that are existent relative references are transformed.""" + create_file((tmp_path / "deep/other" / args[1]).resolve(), "content") + + assert mock_tools.file.resolve_relative_args(args, tmp_path / "deep/other") == [ + args[0], + (tmp_path / "deep/other" / args[1]).resolve(), + ] diff --git a/tests/utils.py b/tests/utils.py index 8fffd67385..56345b7cc1 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -10,6 +10,7 @@ from pathlib import Path import httpx +import tomli_w from httpx_retries import Retry, RetryTransport from rich.markup import escape @@ -72,6 +73,23 @@ def create_file(filepath, content, mode="w", chmod=None): return filepath +def create_toml_file(tomlpath, content): + """A test utility to create a TOML file with known content. + + Ensures that the directory for the file exists, and writes a TOML file with specific + content. + + :param tomlpath: The path for the TOML file to create. + :param content: A dictionary of content that tomllib can use to create the file. + :returns: The path to the file that was created. + """ + tomlpath.parent.mkdir(parents=True, exist_ok=True) + with tomlpath.open("wb") as f: + tomli_w.dump(content, f) + + return tomlpath + + def create_plist_file(plistpath, content): """A test utility to create a plist file with known content.