Repository: beeware/briefcase
Pull request #774 (2022-06-27 23:05:22)

Docker usage tweaks
- Revise wording in messages to users about docker usage.
- Containers created by `docker run` are now automatically deleted via `--rm`.
- A pseudo-tty is no longer created by `docker run`.

Using the `--tty` flag for `docker run` was then telling `pip` and `apt` to run as though they were printing to a terminal. Since the docker output is being streamed, this terminal output could become corrupted. Removing the `--tty` flag causes these programs to avoid outputing dynamic content or terminal control characters.

The inspiration for the `--rm` stems from discovering _a lot_ of orphaned containers in my dev environment. Given the containers created by `docker run` are not re-used (and actually re-using them is rather difficult), the `--rm` flag tells docker to delete the container with `docker run` exits.

## PR Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
- [x] All new features have been tested
- [x] All new features have been documented
- [x] I have read the **CONTRIBUTING.md** file
- [x] I will abide by the code of conduct



Patch:
diff --git a/changes/774.feature.rst b/changes/774.feature.rst
new file mode 100644
index 00000000..246c2e4b
--- /dev/null
+++ b/changes/774.feature.rst
@@ -0,0 +1,1 @@
+Temporary docker containers are now cleaned up after use. The wording of Docker progress messages has also been improved.
diff --git a/src/briefcase/integrations/docker.py b/src/briefcase/integrations/docker.py
index a05cd19b..90a921f5 100644
--- a/src/briefcase/integrations/docker.py
+++ b/src/briefcase/integrations/docker.py
@@ -202,14 +202,15 @@ class Docker:
     def prepare(self):
         try:
             self.command.logger.info(
-                "Building Docker container image...", prefix=self.app.app_name
+                "Building Docker container image...",
+                prefix=self.app.app_name,
             )
             try:
                 system_requires = " ".join(self.app.system_requires)
             except AttributeError:
                 system_requires = ""
 
-            with self.command.input.wait_bar("Building container..."):
+            with self.command.input.wait_bar("Building Docker image..."):
                 self._subprocess.run(
                     [
                         "docker",
@@ -237,23 +238,24 @@ class Docker:
                 )
         except subprocess.CalledProcessError as e:
             raise BriefcaseCommandError(
-                f"Error building Docker container for {self.app.app_name}."
+                f"Error building Docker image for {self.app.app_name}."
             ) from e
 
     def run(self, args, env=None, **kwargs):
-        """Run a process inside the Docker container."""
-        # Set up the `docker run` invocation in interactive mode,
-        # with volume mounts for the platform and .briefcase directories.
-        # The :z suffix allows SELinux to modify the host mount; it is ignored
-        # on non-SELinux platforms.
+        """Run a process inside a Docker container."""
+        # Set up the `docker run` with volume mounts for the platform &
+        # .briefcase directories and to delete the temporary container
+        # after running the command.
+        # The :z suffix allows SELinux to modify the host mount; it is
+        # ignored on non-SELinux platforms.
         docker_args = [
             "docker",
             "run",
-            "--tty",
             "--volume",
             f"{self.command.platform_path}:/app:z",
             "--volume",
             f"{self.command.dot_briefcase_path}:/home/brutus/.briefcase:z",
+            "--rm",
         ]
 
         # If any environment variables have been defined, pass them in
@@ -262,10 +264,10 @@ class Docker:
             for key, value in env.items():
                 docker_args.extend(["--env", f"{key}={value}"])
 
-        # ... then the image name.
+        # ... then the image name to create the temporary container with
         docker_args.append(self.command.docker_image_tag(self.app))
 
-        # ... then add all the arguments
+        # ... then add the command (and its arguments) to run in the container
         for arg in args:
             arg = str(arg)
             if arg == sys.executable:


Test patch:
diff --git a/tests/integrations/docker/test_Docker__run.py b/tests/integrations/docker/test_Docker__run.py
index 3405acd5..253fff60 100644
--- a/tests/integrations/docker/test_Docker__run.py
+++ b/tests/integrations/docker/test_Docker__run.py
@@ -16,11 +16,11 @@ def test_simple_call(mock_docker, tmp_path, capsys):
         [
             "docker",
             "run",
-            "--tty",
             "--volume",
             f"{tmp_path / 'platform'}:/app:z",
             "--volume",
             f"{tmp_path / '.briefcase'}:/home/brutus/.briefcase:z",
+            "--rm",
             "briefcase/com.example.myapp:py3.X",
             "hello",
             "world",
@@ -40,11 +40,11 @@ def test_simple_call_with_arg(mock_docker, tmp_path, capsys):
         [
             "docker",
             "run",
-            "--tty",
             "--volume",
             f"{tmp_path / 'platform'}:/app:z",
             "--volume",
             f"{tmp_path / '.briefcase'}:/home/brutus/.briefcase:z",
+            "--rm",
             "briefcase/com.example.myapp:py3.X",
             "hello",
             "world",
@@ -64,11 +64,11 @@ def test_simple_call_with_path_arg(mock_docker, tmp_path, capsys):
         [
             "docker",
             "run",
-            "--tty",
             "--volume",
             f"{tmp_path / 'platform'}:/app:z",
             "--volume",
             f"{tmp_path / '.briefcase'}:/home/brutus/.briefcase:z",
+            "--rm",
             "briefcase/com.example.myapp:py3.X",
             "hello",
             os.fsdecode(tmp_path / "location"),
@@ -95,11 +95,11 @@ def test_simple_call_with_sys_executable_arg(
         [
             "docker",
             "run",
-            "--tty",
             "--volume",
             f"{tmp_path / 'platform'}:/app:z",
             "--volume",
             f"{tmp_path / '.briefcase'}:/home/brutus/.briefcase:z",
+            "--rm",
             "briefcase/com.example.myapp:py3.X",
             "hello",
             "python3.X",
@@ -124,11 +124,11 @@ def test_simple_verbose_call(mock_docker, tmp_path, capsys):
         [
             "docker",
             "run",
-            "--tty",
             "--volume",
             f"{tmp_path / 'platform'}:/app:z",
             "--volume",
             f"{tmp_path / '.briefcase'}:/home/brutus/.briefcase:z",
+            "--rm",
             "briefcase/com.example.myapp:py3.X",
             "hello",
             "world",
@@ -139,9 +139,10 @@ def test_simple_verbose_call(mock_docker, tmp_path, capsys):
     assert capsys.readouterr().out == (
         "\n"
         ">>> Running Command:\n"
-        ">>>     docker run --tty "
+        ">>>     docker run "
         f"--volume {tmp_path / 'platform'}:/app:z "
         f"--volume {tmp_path / '.briefcase'}:/home/brutus/.briefcase:z "
+        "--rm "
         "briefcase/com.example.myapp:py3.X "
         "hello world\n"
         ">>> Return code: 3\n"
diff --git a/tests/platforms/linux/appimage/test_build.py b/tests/platforms/linux/appimage/test_build.py
index b469d16c..022d6857 100644
--- a/tests/platforms/linux/appimage/test_build.py
+++ b/tests/platforms/linux/appimage/test_build.py
@@ -216,11 +216,11 @@ def test_build_appimage_with_docker(build_command, first_app, tmp_path):
         [
             "docker",
             "run",
-            "--tty",
             "--volume",
             f"{build_command.platform_path}:/app:z",
             "--volume",
             f"{build_command.dot_briefcase_path}:/home/brutus/.briefcase:z",
+            "--rm",
             "--env",
             "VERSION=0.0.1",
             f"briefcase/com.example.first-app:py3.{sys.version_info.minor}",