Repository: pypa/pipx
Pull request #1348 (2024-04-16 07:50:27)

feature: make install-all gather errors in batch
<!-- add an 'x' in the brackets below -->

- [x] I have added a news fragment under `changelog.d/` (if the patch affects the end users)

## Summary of changes

As discussed in https://github.com/pypa/pipx/issues/1132#issuecomment-2053680533, `install-all` needs a more robust strategy to deal with failed packages, so almost the same logic as `reinstall-all` is copied here. Besides, a test case for `install-all` is added.

## Test plan

<!-- provide evidence of testing, preferably with command(s) that can be copy+pasted by others -->

Tested by running

```
# command(s) to exercise these changes
nox -s tests
```

<del>BTW, this pull request is a refactor, so no news is added and there is also no referenced issue. Let me know if it is necessary.</del>

chrysle: Thanks for the quick submission; please add a changelog entry of type _feature_, as this is a fundamental change to the behaviour.
huxuan: > Thanks for the quick submission; please add a changelog entry of type _feature_, as this is a fundamental change to the behaviour.

Thanks for the response. So what the issue number should be? Or maybe I should create one for this?
huxuan: > Looks good to me! Ha, and I was actually mistaken with the "stylistic convention" – but doesn't matter I guess.

That is OK, we can follow up this in #1357. I will mark it as ready for review then.

Patch:
diff --git a/changelog.d/1348.feature.md b/changelog.d/1348.feature.md
new file mode 100644
index 0000000..8c763de
--- /dev/null
+++ b/changelog.d/1348.feature.md
@@ -0,0 +1,1 @@
+make `install-all` gather errors in batch
diff --git a/src/pipx/commands/install.py b/src/pipx/commands/install.py
index a4d35e4..fb6620c 100644
--- a/src/pipx/commands/install.py
+++ b/src/pipx/commands/install.py
@@ -1,4 +1,5 @@
 import json
+import sys
 from pathlib import Path
 from typing import Iterator, List, Optional
 
@@ -9,6 +10,7 @@ from pipx.constants import (
     EXIT_CODE_OK,
     ExitCode,
 )
+from pipx.emojis import sleep
 from pipx.interpreter import DEFAULT_PYTHON
 from pipx.pipx_metadata_file import PackageInfo, PipxMetadata, _json_decoder_object_hook
 from pipx.util import PipxError, pipx_wrap
@@ -186,41 +188,52 @@ def install_all(
 ) -> ExitCode:
     """Return pipx exit code."""
     venv_container = VenvContainer(paths.ctx.venvs)
+    failed: List[str] = []
+    installed: List[str] = []
 
     for venv_metadata in extract_venv_metadata(spec_metadata_file):
         # Install the main package
         main_package = venv_metadata.main_package
         venv_dir = venv_container.get_venv_dir(f"{main_package.package}{main_package.suffix}")
-        install(
-            venv_dir,
-            None,
-            [generate_package_spec(main_package)],
-            local_bin_dir,
-            local_man_dir,
-            python or get_python_interpreter(venv_metadata.source_interpreter),
-            pip_args,
-            venv_args,
-            verbose,
-            force=force,
-            reinstall=False,
-            include_dependencies=main_package.include_dependencies,
-            preinstall_packages=[],
-            suffix=main_package.suffix,
-        )
-
-        # Install the injected packages
-        for inject_package in venv_metadata.injected_packages.values():
-            commands.inject(
+        try:
+            install(
                 venv_dir,
                 None,
-                [generate_package_spec(inject_package)],
+                [generate_package_spec(main_package)],
+                local_bin_dir,
+                local_man_dir,
+                python or get_python_interpreter(venv_metadata.source_interpreter),
                 pip_args,
-                verbose=verbose,
-                include_apps=inject_package.include_apps,
-                include_dependencies=inject_package.include_dependencies,
+                venv_args,
+                verbose,
                 force=force,
-                suffix=inject_package.suffix == main_package.suffix,
+                reinstall=False,
+                include_dependencies=main_package.include_dependencies,
+                preinstall_packages=[],
+                suffix=main_package.suffix,
             )
 
+            # Install the injected packages
+            for inject_package in venv_metadata.injected_packages.values():
+                commands.inject(
+                    venv_dir,
+                    None,
+                    [generate_package_spec(inject_package)],
+                    pip_args,
+                    verbose=verbose,
+                    include_apps=inject_package.include_apps,
+                    include_dependencies=inject_package.include_dependencies,
+                    force=force,
+                    suffix=inject_package.suffix == main_package.suffix,
+                )
+        except PipxError as e:
+            print(e, file=sys.stderr)
+            failed.append(venv_dir.name)
+        else:
+            installed.append(venv_dir.name)
+    if len(installed) == 0:
+        print(f"{sleep} No packages installed after running 'pipx install-all {spec_metadata_file}'")
+    if len(failed) > 0:
+        raise PipxError(f"The following package(s) failed to install: {', '.join(failed)}")
     # Any failure to install will raise PipxError, otherwise success
     return EXIT_CODE_OK


Test patch:
diff --git a/testdata/pipx_metadata_multiple_errors.json b/testdata/pipx_metadata_multiple_errors.json
new file mode 100644
index 0000000..7c00f18
--- /dev/null
+++ b/testdata/pipx_metadata_multiple_errors.json
@@ -0,0 +1,65 @@
+{
+    "pipx_spec_version": "0.1",
+    "venvs": {
+        "dotenv": {
+            "metadata": {
+                "injected_packages": {},
+                "main_package": {
+                    "app_paths": [
+                    ],
+                    "app_paths_of_dependencies": {},
+                    "apps": [
+                    ],
+                    "apps_of_dependencies": [],
+                    "include_apps": true,
+                    "include_dependencies": false,
+                    "man_pages": [],
+                    "man_pages_of_dependencies": [],
+                    "man_paths": [],
+                    "man_paths_of_dependencies": {},
+                    "package": "dotenv",
+                    "package_or_url": "dotenv",
+                    "package_version": "0.0.5",
+                    "pip_args": [],
+                    "suffix": ""
+                },
+                "pipx_metadata_version": "0.4",
+                "python_version": "Python 3.10.12",
+                "source_interpreter": {
+                },
+                "venv_args": []
+            }
+        },
+        "weblate": {
+            "metadata": {
+                "injected_packages": {},
+                "main_package": {
+                    "app_paths": [
+                    ],
+                    "app_paths_of_dependencies": {},
+                    "apps": [
+                    ],
+                    "apps_of_dependencies": [],
+                    "include_apps": true,
+                    "include_dependencies": false,
+                    "man_pages": [
+                    ],
+                    "man_pages_of_dependencies": [],
+                    "man_paths": [
+                    ],
+                    "man_paths_of_dependencies": {},
+                    "package": "weblate",
+                    "package_or_url": "weblate",
+                    "package_version": "4.3.1",
+                    "pip_args": [],
+                    "suffix": ""
+                },
+                "pipx_metadata_version": "0.4",
+                "python_version": "Python 3.10.12",
+                "source_interpreter": {
+                },
+                "venv_args": []
+            }
+        }
+    }
+}
diff --git a/tests/test_install_all.py b/tests/test_install_all.py
new file mode 100644
index 0000000..d397508
--- /dev/null
+++ b/tests/test_install_all.py
@@ -0,0 +1,30 @@
+from pathlib import Path
+
+from helpers import run_pipx_cli
+
+
+def test_install_all(pipx_temp_env, tmp_path, capsys):
+    assert not run_pipx_cli(["install", "pycowsay"])
+    assert not run_pipx_cli(["install", "black"])
+    _ = capsys.readouterr()
+
+    assert not run_pipx_cli(["list", "--json"])
+    captured = capsys.readouterr()
+
+    pipx_list_path = Path(tmp_path) / "pipx_list.json"
+    with open(pipx_list_path, "w") as pipx_list_fh:
+        pipx_list_fh.write(captured.out)
+
+    assert not run_pipx_cli(["install-all", str(pipx_list_path)])
+
+    captured = capsys.readouterr()
+    assert "black" in captured.out
+    assert "pycowsay" in captured.out
+
+
+def test_install_all_multiple_errors(pipx_temp_env, root, capsys):
+    pipx_metadata_path = root / "testdata" / "pipx_metadata_multiple_errors.json"
+    assert run_pipx_cli(["install-all", str(pipx_metadata_path)])
+    captured = capsys.readouterr()
+    assert "The following package(s) failed to install: dotenv, weblate" in captured.err
+    assert f"No packages installed after running 'pipx install-all {pipx_metadata_path}'" in captured.out