Repository: pypa/pipx
Pull request #1357 (2024-04-19 07:02:01)

refactor: unify batch behavior of "install-all", "reinstall-all" and "upgrade-all"
<!-- add an 'x' in the brackets below -->

- [ ] I have added a news fragment under `changelog.d/` (if the patch affects the end users)

No changelog provided since it is refactor.

## Summary of changes

Per the discussion here https://github.com/pypa/pipx/pull/1348#discussion_r1570815169, this pull request unified the batch behavior of `install-all`, `reinstall-all` and `upgrade-all`.

Note that `uninstall-all` is not included in this pull request since the logic is totally different from others. I would suggest to use a separate pull request for `uninstall-all` just like #1348 since the behavior is totally changed.

## 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>P.S. Mark it as draft as depends on #1348</del>



Patch:
diff --git a/src/pipx/commands/install.py b/src/pipx/commands/install.py
index fb6620c..dca5144 100644
--- a/src/pipx/commands/install.py
+++ b/src/pipx/commands/install.py
@@ -232,7 +232,7 @@ def install_all(
         else:
             installed.append(venv_dir.name)
     if len(installed) == 0:
-        print(f"{sleep} No packages installed after running 'pipx install-all {spec_metadata_file}'")
+        print(f"No packages installed after running 'pipx install-all {spec_metadata_file}' {sleep}")
     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
diff --git a/src/pipx/commands/reinstall.py b/src/pipx/commands/reinstall.py
index 2bd1a2a..72011c5 100644
--- a/src/pipx/commands/reinstall.py
+++ b/src/pipx/commands/reinstall.py
@@ -111,6 +111,7 @@ def reinstall_all(
 ) -> ExitCode:
     """Returns pipx exit code."""
     failed: List[str] = []
+    reinstalled: List[str] = []
 
     # iterate on all packages and reinstall them
     # for the first one, we also trigger
@@ -120,7 +121,7 @@ def reinstall_all(
         if venv_dir.name in skip:
             continue
         try:
-            package_exit = reinstall(
+            reinstall(
                 venv_dir=venv_dir,
                 local_bin_dir=local_bin_dir,
                 local_man_dir=local_man_dir,
@@ -134,8 +135,9 @@ def reinstall_all(
             failed.append(venv_dir.name)
         else:
             first_reinstall = False
-            if package_exit != 0:
-                failed.append(venv_dir.name)
+            reinstalled.append(venv_dir.name)
+    if len(reinstalled) == 0:
+        print(f"No packages reinstalled after running 'pipx reinstall-all' {sleep}")
     if len(failed) > 0:
         raise PipxError(f"The following package(s) failed to reinstall: {', '.join(failed)}")
     # Any failure to install will raise PipxError, otherwise success
diff --git a/src/pipx/commands/upgrade.py b/src/pipx/commands/upgrade.py
index 98049ea..90cf7ac 100644
--- a/src/pipx/commands/upgrade.py
+++ b/src/pipx/commands/upgrade.py
@@ -1,5 +1,6 @@
 import logging
 import os
+import sys
 from pathlib import Path
 from typing import Dict, List, Optional, Sequence
 
@@ -220,15 +221,16 @@ def upgrade_all(
     python_flag_passed: bool = False,
 ) -> ExitCode:
     """Returns pipx exit code."""
-    venv_error = False
-    venvs_upgraded = 0
+    failed: List[str] = []
+    upgraded: List[str] = []
+
     for venv_dir in venv_container.iter_venv_dirs():
         venv = Venv(venv_dir, verbose=verbose)
         venv.check_upgrade_shared_libs(pip_args=pip_args, verbose=verbose)
         if venv_dir.name in skip or "--editable" in venv.pipx_metadata.main_package.pip_args:
             continue
         try:
-            venvs_upgraded += _upgrade_venv(
+            _upgrade_venv(
                 venv_dir,
                 venv.pipx_metadata.main_package.pip_args,
                 verbose=verbose,
@@ -237,18 +239,14 @@ def upgrade_all(
                 force=force,
                 python_flag_passed=python_flag_passed,
             )
-
         except PipxError as e:
-            venv_error = True
-            logger.error(f"Error encountered when upgrading {venv_dir.name}:")
-            logger.error(f"{e}\n")
-
-    if venvs_upgraded == 0:
-        print(f"Versions did not change after running 'pipx upgrade' for each package {sleep}")
-    if venv_error:
-        raise PipxError(
-            "\nSome packages encountered errors during upgrade.\n" "    See specific error messages above.",
-            wrap_message=False,
-        )
-
+            print(e, file=sys.stderr)
+            failed.append(venv_dir.name)
+        else:
+            upgraded.append(venv_dir.name)
+    if len(upgraded) == 0:
+        print(f"No packages upgraded after running 'pipx upgrade-all' {sleep}")
+    if len(failed) > 0:
+        raise PipxError(f"The following package(s) failed to upgrade: {','.join(failed)}")
+    # Any failure to install will raise PipxError, otherwise success
     return EXIT_CODE_OK


Test patch:
diff --git a/tests/test_reinstall_all.py b/tests/test_reinstall_all.py
index 5ac446b..d930c5e 100644
--- a/tests/test_reinstall_all.py
+++ b/tests/test_reinstall_all.py
@@ -11,6 +11,12 @@ def test_reinstall_all(pipx_temp_env, capsys):
     assert not run_pipx_cli(["reinstall-all", "--python", sys.executable])
 
 
+def test_reinstall_all_none(pipx_temp_env, capsys):
+    assert not run_pipx_cli(["reinstall-all"])
+    captured = capsys.readouterr()
+    assert "No packages reinstalled after running 'pipx reinstall-all'" in captured.out
+
+
 @pytest.mark.parametrize("metadata_version", PIPX_METADATA_LEGACY_VERSIONS)
 def test_reinstall_all_legacy_venv(pipx_temp_env, capsys, metadata_version):
     assert not run_pipx_cli(["install", "pycowsay"])
diff --git a/tests/test_upgrade_all.py b/tests/test_upgrade_all.py
index 6479b99..acdd06b 100644
--- a/tests/test_upgrade_all.py
+++ b/tests/test_upgrade_all.py
@@ -9,6 +9,12 @@ def test_upgrade_all(pipx_temp_env, capsys):
     assert not run_pipx_cli(["upgrade-all"])
 
 
+def test_upgrade_all_none(pipx_temp_env, capsys):
+    assert not run_pipx_cli(["upgrade-all"])
+    captured = capsys.readouterr()
+    assert "No packages upgraded after running 'pipx upgrade-all'" in captured.out
+
+
 @pytest.mark.parametrize("metadata_version", PIPX_METADATA_LEGACY_VERSIONS)
 def test_upgrade_all_legacy_venv(pipx_temp_env, capsys, caplog, metadata_version):
     assert run_pipx_cli(["upgrade", "pycowsay"])
@@ -17,6 +23,6 @@ def test_upgrade_all_legacy_venv(pipx_temp_env, capsys, caplog, metadata_version
     if metadata_version is None:
         capsys.readouterr()
         assert run_pipx_cli(["upgrade-all"])
-        assert "Error encountered when upgrading pycowsay" in caplog.text
+        assert "The following package(s) failed to upgrade: pycowsay" in caplog.text
     else:
         assert not run_pipx_cli(["upgrade-all"])