Repository: pipxproject/pipx
Pull request #228 (2019-09-26 06:29:08)

Change reinstall-all to have optional --python arg.
Use optional `--python` Instead of positional required python argument for `reinstall-all`.

This makes `reinstall-all` much more useful to use after a system python upgrade--instead of hunting for the proper location of the new system python, the user can just execute `pipx reinstall-all` and the system should find the proper new upgraded python executable.

This also makes `reinstall-all` consistent with `install` and `run`, subcommands which both use `--python` as their optional switch for specifying the python interpreter location, and this change uses the same default of `DEFAULT_PYTHON` as those subcommands.

cs01: The reason I made `python` positional was because I didn't really see any use in re-installing with the same Python version. Clearly you do though. It seems like this would be easy enough to do currently if you install a new system python though:
```
pipx reinstall-all python
```
Does anyone else have any opinions here on which is more useful? 

This will be a breaking API change and I don't really see a clear benefit to it.
itsayellow: One thing I was wondering was if just typing in python would yield the same python as `DEFAULT_PYTHON` in the code.  For instance with my pyenv setup, I have a pyenv shim symlink.  Would just typing `python` on the command-line link to that symlink, or to what that symlink was pointing to?  I'd rather have it as the object, not the shim.

Another idea for a non-API-breaking change would be to leave python as a positional argument, but just give it a default if it is absent.  Is this more amenable?
itsayellow: For reference, I had Issue #146 in mind with this change.
cs01: > Would just typing python on the command-line link to that symlink, or to what that symlink was pointing to?

I don't use pyenv, but it wouldn't be hard to test this out

> For reference, I had Issue #146 in mind with this change.

I haven't reproduced it since I don't use pyenv, but didn't you mention the cause of the error was when uninstalling? If so, then passing the path to the new python will still result in the same error since the venv is associated with the non-existent Python. 

A potentially really easy hack to fix #146 is to try to use the Python interpreter when uninstalling (as pipx currently does), and if that fails, then to fall back to `sys.executable`. 
itsayellow: I just tried it.  It appears that `pipx reinstall-all python` does indeed drill down through symlinks to find the actual file.  So `pipx reinstall-all python` is probably a good workaround.  I was just thinking it would be nice to save some keystrokes whenever my python is upgraded and I have to reinstall-all again.  (which is the only time I ever use `reinstall-all`)

The uninstall problem is just one stop on the road to getting `reinstall-all` to work for fixing all the broken venvs.   But the bottom line of Issue #146 is that whenever python is upgraded (deleting the old python path) we need to run `reinstall-all`.
cs01: A force push was recently done, so this PR will have to be rebased onto origin/master. Apologies for the inconvenience. See https://github.com/pipxproject/pipx/issues/270.
itsayellow: > A force push was recently done, so this PR will have to be rebased onto origin/master. Apologies for the inconvenience. See #270.

Done--rebased.
itsayellow: > In general it looks fine. The changelog should be updated.

The changelog top-line and version inside of pipx still say 0.14.0.0, even though 0.14.0.0 has already been released.

How should I specify the top of the changelog?  Is there a new version number to put, or should there be a placeholder instead of a version number?
cs01: > How should I specify the top of the changelog? Is there a new version number to put, or should there be a placeholder instead of a version number?

Add a new section at the top with the title "dev". You can see an example of this in https://github.com/pipxproject/pipx/pull/246.

Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 32fcc3f..a577aba 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -1,3 +1,7 @@
+dev
+
+- Change `reinstall-all` to use system python by default for apps.  Now use `--python` option to specify a different python version.
+
 0.14.0.0
 
 - Speed up operations by using shared venv for `pip`, `setuptools`, and `wheel`. You can see more detail in the 'how pipx works' section of the documentation. (#164, @pfmoore)
diff --git a/src/pipx/main.py b/src/pipx/main.py
index 21898a4..463da96 100644
--- a/src/pipx/main.py
+++ b/src/pipx/main.py
@@ -392,10 +392,10 @@ def _add_reinstall_all(subparsers):
     p = subparsers.add_parser(
         "reinstall-all",
         formatter_class=LineWrapRawTextHelpFormatter,
-        help="Reinstall all packages with a different Python executable",
+        help="Reinstall all packages",
         description=textwrap.dedent(
             """
-        Reinstalls all packages using a different version of Python.
+        Reinstalls all packages.
 
         Packages are uninstalled, then installed with pipx install PACKAGE.
         This is useful if you upgraded to a new version of Python and want
@@ -407,7 +407,11 @@ def _add_reinstall_all(subparsers):
         """
         ),
     )
-    p.add_argument("python")
+    p.add_argument(
+        "--python",
+        default=constants.DEFAULT_PYTHON,
+        help="The Python version to reinstall the package's CLI app with. Must be v3.5+.",
+    )
     add_include_dependencies(p)
     add_pip_venv_args(p)
     p.add_argument("--skip", nargs="+", default=[], help="skip these packages")


Test patch:
diff --git a/tests/test_reinstall_all.py b/tests/test_reinstall_all.py
index 03df2a0..2b3b8fd 100644
--- a/tests/test_reinstall_all.py
+++ b/tests/test_reinstall_all.py
@@ -6,4 +6,4 @@ from helpers import run_pipx_cli
 
 def test_reinstall_all(pipx_temp_env, capsys):
     assert not run_pipx_cli(["install", "pycowsay"])
-    assert not run_pipx_cli(["reinstall-all", sys.executable])
+    assert not run_pipx_cli(["reinstall-all", "--python", sys.executable])