diff --git a/changes/991.feature.rst b/changes/991.feature.rst new file mode 100644 index 000000000..86ee11316 --- /dev/null +++ b/changes/991.feature.rst @@ -0,0 +1 @@ +``briefcase open linux appimage`` now starts a shell session in the Docker context, rather than opening the project folder. diff --git a/changes/992.bugfix.rst b/changes/992.bugfix.rst new file mode 100644 index 000000000..d05d60ac8 --- /dev/null +++ b/changes/992.bugfix.rst @@ -0,0 +1 @@ +Local file references in requirements no longer break AppImage builds. diff --git a/src/briefcase/commands/create.py b/src/briefcase/commands/create.py index da378095e..65bf55f87 100644 --- a/src/briefcase/commands/create.py +++ b/src/briefcase/commands/create.py @@ -427,10 +427,6 @@ def _write_requirements_file( :param requirements_path: The full path to a requirements.txt file that will be written. """ - # Windows allows both / and \ as a path separator in requirements. - separators = [os.sep] - if os.altsep: - separators.append(os.altsep) with self.input.wait_bar("Writing requirements file..."): with requirements_path.open("w", encoding="utf-8") as f: @@ -439,14 +435,21 @@ def _write_requirements_file( # If the requirement is a local path, convert it to # absolute, because Flatpak moves the requirements file # to a different place before using it. - if any(sep in requirement for sep in separators) and ( - not _has_url(requirement) - ): + if _is_local_requirement(requirement): # We use os.path.abspath() rather than Path.resolve() # because we *don't* want Path's symlink resolving behavior. requirement = os.path.abspath(self.base_path / requirement) f.write(f"{requirement}\n") + def _pip_requires(self, requires: List[str]): + """Convert the list of requirements to be passed to pip into its final + form. + + :param requires: The user-specified list of app requirements + :returns: The final list of requirement arguments to pass to pip + """ + return requires + def _extra_pip_args(self, app: BaseConfig): """Any additional arguments that must be passed to pip when installing packages. @@ -456,6 +459,26 @@ def _extra_pip_args(self, app: BaseConfig): """ return [] + def _pip_kwargs(self, app: BaseConfig): + """Generate the kwargs to pass when invoking pip. + + :param app: The app configuration + :returns: The kwargs to pass to the pip call + """ + # If there is a support package provided, add the cross-platform + # folder of the support package to the PYTHONPATH. This allows + # a support package to specify a sitecustomize.py that will make + # pip behave as if it was being run on the target platform. + pip_kwargs = {} + try: + pip_kwargs["env"] = { + "PYTHONPATH": str(self.support_path(app) / "platform-site"), + } + except KeyError: + pass + + return pip_kwargs + def _install_app_requirements( self, app: BaseConfig, @@ -477,18 +500,6 @@ def _install_app_requirements( # Install requirements if requires: with self.input.wait_bar("Installing app requirements..."): - # If there is a support package provided, add the cross-platform - # folder of the support package to the PYTHONPATH. This allows - # a support package to specify a sitecustomize.py that will make - # pip behave as if it was being run on the target platform. - pip_kwargs = {} - try: - pip_kwargs["env"] = { - "PYTHONPATH": str(self.support_path(app) / "platform-site"), - } - except KeyError: - pass - try: self.tools[app].app_context.run( [ @@ -502,9 +513,9 @@ def _install_app_requirements( f"--target={app_packages_path}", ] + self._extra_pip_args(app) - + requires, + + self._pip_requires(requires), check=True, - **pip_kwargs, + **self._pip_kwargs(app), ) except subprocess.CalledProcessError as e: raise RequirementsInstallError() from e @@ -828,9 +839,15 @@ def __call__(self, app: Optional[BaseConfig] = None, **options): return state -# Detects any of the URL schemes supported by pip -# (https://pip.pypa.io/en/stable/topics/vcs-support/). 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 ( @@ -841,3 +858,17 @@ def _has_url(requirement): + ["bzr+http", "bzr+https", "bzr+ssh", "bzr+sftp", "bzr+ftp", "bzr+lp"] ) ) + + +def _is_local_requirement(requirement): + """Determine if the requirement is a local file path. + + :param requirement: The requirement to check + :returns: True if the requirement is a local file path + """ + # Windows allows both / and \ as a path separator in requirements. + separators = [os.sep] + if os.altsep: + separators.append(os.altsep) + + return any(sep in requirement for sep in separators) and (not _has_url(requirement)) diff --git a/src/briefcase/commands/open.py b/src/briefcase/commands/open.py index 72f144e78..fc642ac2b 100644 --- a/src/briefcase/commands/open.py +++ b/src/briefcase/commands/open.py @@ -8,13 +8,13 @@ class OpenCommand(BaseCommand): command = "open" - def open_project(self, project_path): + def _open_app(self, app: BaseConfig): if self.tools.host_os == "Windows": - self.tools.os.startfile(project_path) + self.tools.os.startfile(self.project_path(app)) elif self.tools.host_os == "Darwin": - self.tools.subprocess.Popen(["open", project_path]) + self.tools.subprocess.Popen(["open", self.project_path(app)]) else: - self.tools.subprocess.Popen(["xdg-open", project_path]) + self.tools.subprocess.Popen(["xdg-open", self.project_path(app)]) def open_app(self, app: BaseConfig, **options): """Open the project for an app. @@ -33,7 +33,7 @@ def open_app(self, app: BaseConfig, **options): f"Opening {self.project_path(app).relative_to(self.base_path)}...", prefix=app.app_name, ) - self.open_project(project_path) + self._open_app(app) return state diff --git a/src/briefcase/integrations/docker.py b/src/briefcase/integrations/docker.py index e28ac2bed..a761a7c48 100644 --- a/src/briefcase/integrations/docker.py +++ b/src/briefcase/integrations/docker.py @@ -315,7 +315,7 @@ def _dockerize_path(self, arg: str): return arg - def _dockerize_args(self, args, env=None): + def _dockerize_args(self, args, interactive=False, mounts=None, env=None): """Convert arguments and environment into a Docker-compatible form. Convert an argument and environment specification into a form that can be used as arguments to invoke Docker. This involves: @@ -330,17 +330,30 @@ def _dockerize_args(self, args, env=None): :returns: A list of arguments that can be used to invoke the command inside a docker container. """ + docker_args = ["docker", "run", "--rm"] + + # Add "-it" if in interactive mode + if interactive: + docker_args.append("-it") + + # Add default volume mounts for the app folder, plus the Briefcase data + # path. + # # The :z suffix on volume mounts allows SELinux to modify the host # mount; it is ignored on non-SELinux platforms. - docker_args = [ - "docker", - "run", - "--volume", - f"{self.host_platform_path}:/app:z", - "--volume", - f"{self.host_data_path}:{self.docker_data_path}:z", - "--rm", - ] + docker_args.extend( + [ + "--volume", + f"{self.host_platform_path}:/app:z", + "--volume", + f"{self.host_data_path}:{self.docker_data_path}:z", + ] + ) + + # Add any extra volume mounts + if mounts: + for source, target in mounts: + docker_args.extend(["--volume", f"{source}:{target}:z"]) # If any environment variables have been defined, pass them in # as --env arguments to Docker. @@ -356,24 +369,32 @@ def _dockerize_args(self, args, env=None): return docker_args - def run(self, args, env=None, **kwargs): + def run(self, args, env=None, interactive=False, mounts=None, **kwargs): """Run a process inside a Docker container.""" # Any exceptions from running the process are *not* caught. # This ensures that "docker.run()" behaves as closely to # "subprocess.run()" as possible. self.tools.logger.info("Entering Docker context...", prefix=self.app.app_name) + if interactive: + kwargs["stream_output"] = False + self.tools.subprocess.run( - self._dockerize_args(args, env=env), + self._dockerize_args( + args, + interactive=interactive, + mounts=mounts, + env=env, + ), **kwargs, ) self.tools.logger.info("Leaving Docker context", prefix=self.app.app_name) - def check_output(self, args, env=None, **kwargs): + def check_output(self, args, env=None, mounts=None, **kwargs): """Run a process inside a Docker container, capturing output.""" # Any exceptions from running the process are *not* caught. # This ensures that "docker.check_output()" behaves as closely to # "subprocess.check_output()" as possible. return self.tools.subprocess.check_output( - self._dockerize_args(args, env=env), + self._dockerize_args(args, mounts=mounts, env=env), **kwargs, ) diff --git a/src/briefcase/platforms/linux/appimage.py b/src/briefcase/platforms/linux/appimage.py index 2d36656c1..041bcb823 100644 --- a/src/briefcase/platforms/linux/appimage.py +++ b/src/briefcase/platforms/linux/appimage.py @@ -1,5 +1,7 @@ import os import subprocess +from pathlib import Path +from typing import List from briefcase.commands import ( BuildCommand, @@ -10,6 +12,7 @@ RunCommand, UpdateCommand, ) +from briefcase.commands.create import _is_local_requirement from briefcase.config import AppConfig from briefcase.exceptions import BriefcaseCommandError from briefcase.integrations.docker import Docker, DockerAppContext @@ -62,7 +65,7 @@ def clone_options(self, command): self.use_docker = command.use_docker -class LinuxAppImageMixin(LinuxAppImagePassiveMixin): +class LinuxAppImageMostlyPassiveMixin(LinuxAppImagePassiveMixin): def docker_image_tag(self, app): """The Docker image tag for an app.""" return ( @@ -70,8 +73,7 @@ def docker_image_tag(self, app): ) def verify_tools(self): - """Verify that Docker is available; and if it isn't that we're on - Linux.""" + """If we're using docker, verify that it is available.""" super().verify_tools() if self.use_docker: if self.tools.host_os == "Windows": @@ -80,10 +82,6 @@ def verify_tools(self): ) else: Docker.verify(tools=self.tools) - elif self.tools.host_os != "Linux": - raise BriefcaseCommandError( - "Linux AppImages can only be generated on Linux without Docker." - ) def verify_app_tools(self, app: AppConfig): """Verify App environment is prepared and available. @@ -110,6 +108,39 @@ def verify_app_tools(self, app: AppConfig): # Establish Docker as app context before letting super set subprocess super().verify_app_tools(app) + def _requirements_mounts(self, app: AppConfig): + """Generate the list of Docker volume mounts needed to support local + requirements. + + This list of mounts will include *all* possible requirements (i.e., + `requires` *and* `test_requires`) + + :param app: The app for which requirement mounts are needed + :returns: A list of (source, target) pairs; source is a fully resolved + local filesystem path; target is a path on the Docker filesystem. + """ + requires = app.requires.copy() if app.requires else [] + if app.test_requires: + requires.extend(app.test_requires) + + mounts = [] + for requirement in requires: + if _is_local_requirement(requirement): + source = Path(requirement).resolve() + mounts.append((str(source), f"/requirements/{source.name}")) + + return mounts + + +class LinuxAppImageMixin(LinuxAppImageMostlyPassiveMixin): + def verify_tools(self): + """If we're *not* using Docker, verify that we're actually on Linux.""" + super().verify_tools() + if not self.use_docker and self.tools.host_os != "Linux": + raise BriefcaseCommandError( + "Linux AppImages can only be generated on Linux without Docker." + ) + class LinuxAppImageCreateCommand(LinuxAppImageMixin, CreateCommand): description = "Create and populate a Linux AppImage." @@ -126,14 +157,74 @@ def support_package_url(self, support_revision): + self.support_package_filename(support_revision) ) + def _pip_requires(self, requires: List[str]): + """Convert the requirements list to an AppImage compatible format. + + If running in no-docker mode, this returns the requirements as specified + by the user. + + If running in Docker mode, any local file requirements are converted into + a reference in the /requirements mount folder. + + :param requires: The user-specified list of app requirements + :returns: The final list of requirement arguments to pass to pip + """ + original = super()._pip_requires(requires) + if not self.use_docker: + return original + + final = [] + for requirement in original: + if _is_local_requirement(requirement): + source = Path(requirement).resolve() + final.append(f"/requirements/{source.name}") + else: + # Use the requirement as-specified + final.append(requirement) + + return final + + def _pip_kwargs(self, app: AppConfig): + """Generate the kwargs to pass when invoking pip. + + Adds any mounts needed to support local file requirements. + + :param app: The app configuration + :returns: The kwargs to pass to the pip call + """ + kwargs = super()._pip_kwargs(app) + + if self.use_docker: + mounts = self._requirements_mounts(app) + if mounts: + kwargs["mounts"] = mounts + + return kwargs + class LinuxAppImageUpdateCommand(LinuxAppImageCreateCommand, UpdateCommand): description = "Update an existing Linux AppImage." -class LinuxAppImageOpenCommand(LinuxAppImagePassiveMixin, OpenCommand): +class LinuxAppImageOpenCommand(LinuxAppImageMostlyPassiveMixin, OpenCommand): description = "Open the folder containing an existing Linux AppImage project." + def _open_app(self, app: AppConfig): + # If we're using Docker, open an interactive shell in the container + if self.use_docker: + kwargs = {} + mounts = self._requirements_mounts(app) + if mounts: + kwargs["mounts"] = mounts + + self.tools[app].app_context.run( + ["/bin/bash"], + interactive=True, + **kwargs, + ) + else: + super()._open_app(app) + class LinuxAppImageBuildCommand(LinuxAppImageMixin, BuildCommand): description = "Build a Linux AppImage." diff --git a/tests/commands/open/conftest.py b/tests/commands/open/conftest.py index f599df195..343a0be81 100644 --- a/tests/commands/open/conftest.py +++ b/tests/commands/open/conftest.py @@ -54,9 +54,9 @@ def verify_app_tools(self, app): super().verify_app_tools(app=app) self.actions.append(("verify-app-tools", app.app_name)) - def open_project(self, project_path): - super().open_project(project_path) - self.actions.append(("open", project_path)) + def _open_app(self, app): + super()._open_app(app) + self.actions.append(("open", app.app_name)) # These commands override the default behavior, simply tracking that # they were invoked, rather than instantiating a Create command. diff --git a/tests/commands/open/test_open_project.py b/tests/commands/open/test__open_app.py similarity index 100% rename from tests/commands/open/test_open_project.py rename to tests/commands/open/test__open_app.py diff --git a/tests/commands/open/test_call.py b/tests/commands/open/test_call.py index b309de54a..c73b62c7c 100644 --- a/tests/commands/open/test_call.py +++ b/tests/commands/open/test_call.py @@ -10,10 +10,10 @@ def test_open(open_command, first_app, second_app): ("verify",), ("verify-app-tools", "first"), # open the first app - ("open", first_app), + ("open", "first"), ("verify-app-tools", "second"), # open the second app - ("open", second_app), + ("open", "second"), ] @@ -29,7 +29,7 @@ def test_open_single(open_command, first_app): ("verify",), ("verify-app-tools", "first"), # open the first app - ("open", first_app), + ("open", "first"), ] @@ -46,5 +46,5 @@ def test_create_before_open(open_command, tmp_path): # create, then open the first app ("create", "first", {}), ("verify-app-tools", "first"), - ("open", tmp_path / "tester" / "dummy" / "first" / "first.project"), + ("open", "first"), ] diff --git a/tests/integrations/docker/conftest.py b/tests/integrations/docker/conftest.py index 8f63701d9..d71a7a797 100644 --- a/tests/integrations/docker/conftest.py +++ b/tests/integrations/docker/conftest.py @@ -62,4 +62,7 @@ def mock_docker_app_context(tmp_path, my_app, mock_tools) -> DockerAppContext: python_version="3.X", ) + # Reset the mock so that the prepare call doesn't appear in test results. + mock_docker_app_context.tools.subprocess._subprocess.Popen.reset_mock() + return mock_docker_app_context diff --git a/tests/integrations/docker/test_DockerAppContext__check_output.py b/tests/integrations/docker/test_DockerAppContext__check_output.py index beed96770..a37e69aba 100644 --- a/tests/integrations/docker/test_DockerAppContext__check_output.py +++ b/tests/integrations/docker/test_DockerAppContext__check_output.py @@ -10,15 +10,51 @@ def test_simple_call(mock_docker_app_context, tmp_path, capsys): """A simple call will be invoked.""" assert mock_docker_app_context.check_output(["hello", "world"]) == "goodbye\n" - mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_with( + mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_once_with( [ "docker", "run", + "--rm", "--volume", f"{tmp_path / 'platform'}:/app:z", "--volume", f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", + "briefcase/com.example.myapp:py3.X", + "hello", + "world", + ], + text=True, + encoding=ANY, + ) + assert capsys.readouterr().out == "" + + +def test_extra_mounts(mock_docker_app_context, tmp_path, capsys): + """A call can request additional mounts.""" + assert ( + mock_docker_app_context.check_output( + ["hello", "world"], + mounts=[ + ("/path/to/first", "/container/first"), + ("/path/to/second", "/container/second"), + ], + ) + == "goodbye\n" + ) + + mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_once_with( + [ + "docker", + "run", "--rm", + "--volume", + f"{tmp_path / 'platform'}:/app:z", + "--volume", + f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", + "--volume", + "/path/to/first:/container/first:z", + "--volume", + "/path/to/second:/container/second:z", "briefcase/com.example.myapp:py3.X", "hello", "world", @@ -42,15 +78,15 @@ def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): ) assert output == "goodbye\n" - mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_with( + mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_once_with( [ "docker", "run", + "--rm", "--volume", f"{tmp_path / 'platform'}:/app:z", "--volume", f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", - "--rm", "--env", "MAGIC=True", "--env", @@ -82,15 +118,15 @@ def test_call_with_path_arg_and_env(mock_docker_app_context, tmp_path, capsys): ) assert output == "goodbye\n" - mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_with( + mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_once_with( [ "docker", "run", + "--rm", "--volume", f"{tmp_path / 'platform'}:/app:z", "--volume", f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", - "--rm", "--env", "MAGIC=True", "--env", @@ -115,15 +151,15 @@ def test_simple_verbose_call(mock_docker_app_context, tmp_path, capsys): assert mock_docker_app_context.check_output(["hello", "world"]) == "goodbye\n" - mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_with( + mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_once_with( [ "docker", "run", + "--rm", "--volume", f"{tmp_path / 'platform'}:/app:z", "--volume", f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", - "--rm", "briefcase/com.example.myapp:py3.X", "hello", "world", @@ -135,9 +171,9 @@ def test_simple_verbose_call(mock_docker_app_context, tmp_path, capsys): "\n" ">>> Running Command:\n" ">>> docker run " + "--rm " f"--volume {tmp_path / 'platform'}:/app:z " f"--volume {tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z " - "--rm " "briefcase/com.example.myapp:py3.X " "hello world\n" ">>> Working Directory:\n" diff --git a/tests/integrations/docker/test_DockerAppContext__run.py b/tests/integrations/docker/test_DockerAppContext__run.py index 885cce959..693beed61 100644 --- a/tests/integrations/docker/test_DockerAppContext__run.py +++ b/tests/integrations/docker/test_DockerAppContext__run.py @@ -12,15 +12,87 @@ def test_simple_call(mock_docker_app_context, tmp_path, capsys): mock_docker_app_context.run(["hello", "world"]) - mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_with( + mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_once_with( [ "docker", "run", + "--rm", "--volume", f"{tmp_path / 'platform'}:/app:z", "--volume", f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", + "briefcase/com.example.myapp:py3.X", + "hello", + "world", + ], + text=True, + encoding=ANY, + bufsize=1, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + ) + assert capsys.readouterr().out == ( + "\n" + "[myapp] Entering Docker context...\n" + "\n" + "[myapp] Leaving Docker context\n" + ) + + +def test_interactive(mock_docker_app_context, tmp_path, capsys): + """Docker can be invoked in interactive mode.""" + mock_docker_app_context.run(["hello", "world"], interactive=True) + + # Interactive means the call to run is direct, rather than going through Popen + mock_docker_app_context.tools.subprocess._subprocess.run.assert_called_once_with( + [ + "docker", + "run", "--rm", + "-it", + "--volume", + f"{tmp_path / 'platform'}:/app:z", + "--volume", + f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", + "briefcase/com.example.myapp:py3.X", + "hello", + "world", + ], + text=True, + encoding=ANY, + ) + assert capsys.readouterr().out == ( + "\n" + "[myapp] Entering Docker context...\n" + "\n" + "[myapp] Leaving Docker context\n" + ) + + +def test_extra_mounts(mock_docker_app_context, tmp_path, capsys): + """A subprocess call can be augmented with mounts.""" + + mock_docker_app_context.run( + ["hello", "world"], + mounts=[ + ("/path/to/first", "/container/first"), + ("/path/to/second", "/container/second"), + ], + ) + + mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_once_with( + [ + "docker", + "run", + "--rm", + "--volume", + f"{tmp_path / 'platform'}:/app:z", + "--volume", + f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", + "--volume", + "/path/to/first:/container/first:z", + "--volume", + "/path/to/second:/container/second:z", "briefcase/com.example.myapp:py3.X", "hello", "world", @@ -52,15 +124,15 @@ def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): universal_newlines=True, ) - mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_with( + mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_once_with( [ "docker", "run", + "--rm", "--volume", f"{tmp_path / 'platform'}:/app:z", "--volume", f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", - "--rm", "--env", "MAGIC=True", "--env", @@ -99,15 +171,15 @@ def test_call_with_path_arg_and_env(mock_docker_app_context, tmp_path, capsys): cwd=tmp_path / "cwd", ) - mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_with( + mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_once_with( [ "docker", "run", + "--rm", "--volume", f"{tmp_path / 'platform'}:/app:z", "--volume", f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", - "--rm", "--env", "MAGIC=True", "--env", @@ -131,6 +203,62 @@ def test_call_with_path_arg_and_env(mock_docker_app_context, tmp_path, capsys): ) +@pytest.mark.skipif( + sys.platform == "win32", reason="Windows paths aren't converted in Docker context" +) +def test_interactive_with_path_arg_and_env_and_mounts( + mock_docker_app_context, tmp_path, capsys +): + """Docker can be invoked in interactive mode with all the extras.""" + mock_docker_app_context.run( + ["hello", tmp_path / "location"], + env={ + "MAGIC": "True", + "PATH": f"/somewhere/safe:{tmp_path / 'briefcase' / 'tools'}:{tmp_path / 'platform' / 'location'}", + }, + cwd=tmp_path / "cwd", + interactive=True, + mounts=[ + ("/path/to/first", "/container/first"), + ("/path/to/second", "/container/second"), + ], + ) + + # Interactive means the call to run is direct, rather than going through Popen + mock_docker_app_context.tools.subprocess._subprocess.run.assert_called_once_with( + [ + "docker", + "run", + "--rm", + "-it", + "--volume", + f"{tmp_path / 'platform'}:/app:z", + "--volume", + f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", + "--volume", + "/path/to/first:/container/first:z", + "--volume", + "/path/to/second:/container/second:z", + "--env", + "MAGIC=True", + "--env", + "PATH=/somewhere/safe:/home/brutus/.cache/briefcase/tools:/app/location", + "briefcase/com.example.myapp:py3.X", + "hello", + os.fsdecode(tmp_path / "location"), + ], + cwd=os.fsdecode(tmp_path / "cwd"), + text=True, + encoding=ANY, + ) + assert capsys.readouterr().out == ( + "\n" + "[myapp] Entering Docker context...\n" + "\n" + "[myapp] Leaving Docker context\n" + ) + + @pytest.mark.skipif( sys.platform == "win32", reason="Windows paths aren't converted in Docker context" ) @@ -140,15 +268,15 @@ def test_simple_verbose_call(mock_docker_app_context, tmp_path, capsys): mock_docker_app_context.run(["hello", "world"]) - mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_with( + mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_once_with( [ "docker", "run", + "--rm", "--volume", f"{tmp_path / 'platform'}:/app:z", "--volume", f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", - "--rm", "briefcase/com.example.myapp:py3.X", "hello", "world", @@ -165,9 +293,9 @@ def test_simple_verbose_call(mock_docker_app_context, tmp_path, capsys): "\n" ">>> Running Command:\n" ">>> docker run " + "--rm " f"--volume {tmp_path / 'platform'}:/app:z " f"--volume {tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z " - "--rm " "briefcase/com.example.myapp:py3.X " "hello world\n" ">>> Working Directory:\n" diff --git a/tests/platforms/conftest.py b/tests/platforms/conftest.py index 0f8309ec8..64fd6bb84 100644 --- a/tests/platforms/conftest.py +++ b/tests/platforms/conftest.py @@ -14,6 +14,8 @@ def first_app_config(): version="0.0.1", description="The first simple app \\ demonstration", sources=["src/first_app"], + requires=["foo==1.2.3", "bar>=4.5"], + test_requires=["pytest"], ) diff --git a/tests/platforms/linux/appimage/test__requirements_mounts.py b/tests/platforms/linux/appimage/test__requirements_mounts.py new file mode 100644 index 000000000..67ed0f414 --- /dev/null +++ b/tests/platforms/linux/appimage/test__requirements_mounts.py @@ -0,0 +1,84 @@ +import sys + +import pytest + +from briefcase.platforms.linux.appimage import LinuxAppImageMostlyPassiveMixin + + +@pytest.mark.skipif( + sys.platform == "win32", reason="Windows paths aren't converted in Docker context" +) +@pytest.mark.parametrize( + "requires, test_requires, mounts", + [ + # No requirements or test requirements + (None, None, []), + (None, [], []), + ([], None, []), + ([], [], []), + # No local requirements + # Requirements, but not test requirements + (["first", "second"], None, []), + # Test Requirements, but no requirements + (None, ["test_first", "test_second"], []), + # Requirements, and test requirements + (["first", "second"], ["test_first", "test_second"], []), + # Local requirements in the mix + # Requirements, but not test requirements + ( + [ + "first", + "/path/to/local1", + "/path/to/local2", + "second", + ], + None, + [ + ("/path/to/local1", "/requirements/local1"), + ("/path/to/local2", "/requirements/local2"), + ], + ), + # Test Requirements, but no requirements + ( + None, + [ + "test_first", + "/path/to/test_local1", + "/path/to/test_local2", + "test_second", + ], + [ + ("/path/to/test_local1", "/requirements/test_local1"), + ("/path/to/test_local2", "/requirements/test_local2"), + ], + ), + # Requirements, and test requirements + ( + [ + "first", + "/path/to/local1", + "/path/to/local2", + "second", + ], + [ + "test_first", + "/path/to/test_local1", + "/path/to/test_local2", + "test_second", + ], + [ + ("/path/to/local1", "/requirements/local1"), + ("/path/to/local2", "/requirements/local2"), + ("/path/to/test_local1", "/requirements/test_local1"), + ("/path/to/test_local2", "/requirements/test_local2"), + ], + ), + ], +) +def test_requirements_mounts(first_app_config, requires, test_requires, mounts): + command = LinuxAppImageMostlyPassiveMixin() + + first_app_config.requires = requires + first_app_config.test_requires = test_requires + + assert command._requirements_mounts(first_app_config) == mounts diff --git a/tests/platforms/linux/appimage/test_build.py b/tests/platforms/linux/appimage/test_build.py index b4c650545..32e632604 100644 --- a/tests/platforms/linux/appimage/test_build.py +++ b/tests/platforms/linux/appimage/test_build.py @@ -363,11 +363,11 @@ def test_build_appimage_in_docker(build_command, first_app, tmp_path, monkeypatc [ "docker", "run", + "--rm", "--volume", f"{build_command.platform_path}:/app:z", "--volume", f"{build_command.data_path}:/home/brutus/.cache/briefcase:z", - "--rm", "--env", "VERSION=0.0.1", "--env", @@ -464,11 +464,11 @@ def test_build_appimage_with_plugins_in_docker(build_command, first_app, tmp_pat [ "docker", "run", + "--rm", "--volume", f"{build_command.platform_path}:/app:z", "--volume", f"{build_command.data_path}:/home/brutus/.cache/briefcase:z", - "--rm", "--env", "DEPLOY_GTK_VERSION=3", "--env", diff --git a/tests/platforms/linux/appimage/test_create.py b/tests/platforms/linux/appimage/test_create.py index e89c83ef5..bedcd3486 100644 --- a/tests/platforms/linux/appimage/test_create.py +++ b/tests/platforms/linux/appimage/test_create.py @@ -32,7 +32,6 @@ def test_support_package_url(tmp_path): ) def test_install_app_requirements_in_docker(first_app_config, tmp_path): """If Docker is in use, a docker context is used to invoke pip.""" - first_app_config.requires = ["foo==1.2.3", "bar>=4.5"] command = LinuxAppImageCreateCommand( logger=Log(), @@ -75,11 +74,85 @@ def test_install_app_requirements_in_docker(first_app_config, tmp_path): [ "docker", "run", + "--rm", "--volume", f"{tmp_path / 'base_path' / 'linux'}:/app:z", "--volume", f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", + "briefcase/com.example.first-app:py3.X", + "python3.X", + "-u", + "-m", + "pip", + "install", + "--upgrade", + "--no-user", + "--target=/app/appimage/First App/path/to/app_packages", + "foo==1.2.3", + "bar>=4.5", + ], + check=True, + ) + + +@pytest.mark.skipif( + sys.platform == "win32", reason="Windows paths aren't converted in Docker context" +) +def test_install_app_requirements_in_docker_with_locals(first_app_config, tmp_path): + """If the app has local requirements, the docker container has extra + mounts.""" + + # Add a local requirement + first_app_config.requires.append("/path/to/local") + + command = LinuxAppImageCreateCommand( + logger=Log(), + console=Console(), + base_path=tmp_path / "base_path", + data_path=tmp_path / "briefcase", + ) + command.use_docker = True + + # Mock the existence of Docker. + command.tools.subprocess = MagicMock(spec_set=Subprocess) + + command._path_index = { + first_app_config: {"app_packages_path": "path/to/app_packages"} + } + + # Provide Docker app context + command.tools[first_app_config].app_context = DockerAppContext( + tools=command.tools, + app=first_app_config, + ) + command.tools[first_app_config].app_context.prepare( + image_tag="briefcase/com.example.first-app:py3.X", + dockerfile_path=tmp_path + / "base_path" + / "linux" + / "appimage" + / "First App" + / "Dockerfile", + app_base_path=tmp_path / "base_path", + host_platform_path=tmp_path / "base_path" / "linux", + host_data_path=tmp_path / "briefcase", + python_version="3.X", + ) + + command.install_app_requirements(first_app_config, test_mode=False) + + # pip was invoked inside docker. + command.tools.subprocess.run.assert_called_with( + [ + "docker", + "run", "--rm", + "--volume", + f"{tmp_path / 'base_path' / 'linux'}:/app:z", + "--volume", + f"{tmp_path / 'briefcase'}:/home/brutus/.cache/briefcase:z", + "--volume", + "/path/to/local:/requirements/local:z", "briefcase/com.example.first-app:py3.X", "python3.X", "-u", @@ -91,6 +164,7 @@ def test_install_app_requirements_in_docker(first_app_config, tmp_path): "--target=/app/appimage/First App/path/to/app_packages", "foo==1.2.3", "bar>=4.5", + "/requirements/local", ], check=True, ) @@ -101,7 +175,6 @@ def test_install_app_requirements_in_docker(first_app_config, tmp_path): ) def test_install_app_requirements_no_docker(first_app_config, tmp_path): """If docker is *not* in use, calls are made on raw subprocess.""" - first_app_config.requires = ["foo==1.2.3", "bar>=4.5"] command = LinuxAppImageCreateCommand( logger=Log(), diff --git a/tests/platforms/linux/appimage/test_open.py b/tests/platforms/linux/appimage/test_open.py index ff37eb65a..69d701fdc 100644 --- a/tests/platforms/linux/appimage/test_open.py +++ b/tests/platforms/linux/appimage/test_open.py @@ -5,6 +5,7 @@ import pytest from briefcase.console import Console, Log +from briefcase.integrations.docker import DockerAppContext from briefcase.integrations.subprocess import Subprocess from briefcase.platforms.linux.appimage import LinuxAppImageOpenCommand @@ -20,14 +21,144 @@ def open_command(tmp_path, first_app_config): data_path=tmp_path / "briefcase", ) command.tools.os = MagicMock(spec_set=os) - command.tools.subprocess = MagicMock(spec_set=Subprocess) + + # Store the underlying subprocess instance + command._subprocess = MagicMock(spec_set=Subprocess) + command.tools.subprocess = command._subprocess return command +@pytest.mark.skipif( + sys.platform == "win32", reason="Windows paths aren't converted in Docker context" +) +def test_open_docker(open_command, first_app_config, tmp_path): + """Open starts a docker session by default.""" + + # Enable docker + open_command.use_docker = True + + # Provide Docker app context + open_command.tools[first_app_config].app_context = DockerAppContext( + tools=open_command.tools, + app=first_app_config, + ) + open_command.tools[first_app_config].app_context.prepare( + image_tag=f"briefcase/com.example.first-app:py3.{sys.version_info.minor}", + dockerfile_path=tmp_path + / "base_path" + / "linux" + / "appimage" + / "First App" + / "Dockerfile", + app_base_path=tmp_path / "base_path", + host_platform_path=tmp_path / "base_path" / "linux", + host_data_path=tmp_path / "briefcase", + python_version=f"3.{sys.version_info.minor}", + ) + # Clear out any calls recorded during preparation + open_command._subprocess.run.reset_mock() + + # Create the desktop file that would be in the project folder. + create_file( + open_command.project_path(first_app_config) + / "First App.AppDir" + / "com.example.firstapp.desktop", + "FreeDesktop file", + ) + + # Open the app + open_command.open_app(first_app_config) + + # The docker session was started + open_command._subprocess.run.assert_called_once_with( + [ + "docker", + "run", + "--rm", + "-it", + "--volume", + f"{open_command.platform_path}:/app:z", + "--volume", + f"{open_command.data_path}:/home/brutus/.cache/briefcase:z", + f"briefcase/com.example.first-app:py3.{sys.version_info.minor}", + "/bin/bash", + ], + stream_output=False, + ) + + +@pytest.mark.skipif( + sys.platform == "win32", reason="Windows paths aren't converted in Docker context" +) +def test_open_docker_with_mounts(open_command, first_app_config, tmp_path): + """Extra mounts will be added to the docker session if there are local + requirements.""" + + first_app_config.requires.append("/path/to/local") + first_app_config.test_requires.append("/path/to/test_local") + + # Enable docker + open_command.use_docker = True + + # Provide Docker app context + open_command.tools[first_app_config].app_context = DockerAppContext( + tools=open_command.tools, + app=first_app_config, + ) + open_command.tools[first_app_config].app_context.prepare( + image_tag=f"briefcase/com.example.first-app:py3.{sys.version_info.minor}", + dockerfile_path=tmp_path + / "base_path" + / "linux" + / "appimage" + / "First App" + / "Dockerfile", + app_base_path=tmp_path / "base_path", + host_platform_path=tmp_path / "base_path" / "linux", + host_data_path=tmp_path / "briefcase", + python_version=f"3.{sys.version_info.minor}", + ) + # Clear out any calls recorded during preparation + open_command._subprocess.run.reset_mock() + + # Create the desktop file that would be in the project folder. + create_file( + open_command.project_path(first_app_config) + / "First App.AppDir" + / "com.example.firstapp.desktop", + "FreeDesktop file", + ) + + # Open the app + open_command.open_app(first_app_config) + + # The docker session was started + open_command._subprocess.run.assert_called_once_with( + [ + "docker", + "run", + "--rm", + "-it", + "--volume", + f"{open_command.platform_path}:/app:z", + "--volume", + f"{open_command.data_path}:/home/brutus/.cache/briefcase:z", + "--volume", + "/path/to/local:/requirements/local:z", + "--volume", + "/path/to/test_local:/requirements/test_local:z", + f"briefcase/com.example.first-app:py3.{sys.version_info.minor}", + "/bin/bash", + ], + stream_output=False, + ) + + @pytest.mark.skipif(sys.platform != "linux", reason="Linux specific test") -def test_open(open_command, first_app_config, tmp_path): - """On Linux, Open runs `xdg-open` on the project folder.""" +def test_open_no_docker_linux(open_command, first_app_config, tmp_path): + """On Linux, Open runs `xdg-open` on the project folder if we specify --no- + docker.""" # Create the desktop file that would be in the project folder. create_file( open_command.project_path(first_app_config) @@ -36,6 +167,9 @@ def test_open(open_command, first_app_config, tmp_path): "FreeDesktop file", ) + # Disable docker + open_command.use_docker = False + open_command(first_app_config) open_command.tools.subprocess.Popen.assert_called_once_with( @@ -44,3 +178,28 @@ def test_open(open_command, first_app_config, tmp_path): tmp_path / "base_path" / "linux" / "appimage" / "First App", ] ) + + +@pytest.mark.skipif(sys.platform != "darwin", reason="macOS specific test") +def test_open_no_docker_macOS(open_command, first_app_config, tmp_path): + """On macOS, Open runs `open` on the project folder if we specify --no- + docker.""" + # Create the desktop file that would be in the project folder. + create_file( + open_command.project_path(first_app_config) + / "First App.AppDir" + / "com.example.firstapp.desktop", + "FreeDesktop file", + ) + + # Disable docker + open_command.use_docker = False + + open_command(first_app_config) + + open_command.tools.subprocess.Popen.assert_called_once_with( + [ + "open", + tmp_path / "base_path" / "linux" / "appimage" / "First App", + ] + )