Repository: pipxproject/pipx (python)

Issue:
Should installing pipx shared libs use `pip_args` from the latest package?
<!-- Thank you for filing a bug! Please feel free to answer as much or as little of this template as you can. -->

**Describe the bug**
<!-- Please be as detailed as possible! -->
Currently when installing or upgrading the pipx shared libs, the functions `_SharedLibs.upgrade()` and `_SharedLibs.create()` accept `pip_args` arguments. 
https://github.com/pipxproject/pipx/blob/08bd15610bc406dabe8bf0db4adaba4a3a8b89ea/src/pipx/shared_libs.py#L33
https://github.com/pipxproject/pipx/blob/08bd15610bc406dabe8bf0db4adaba4a3a8b89ea/src/pipx/shared_libs.py#L61

They are used if a normal `pipx install` needs a shared library create/upgrade, and the `pipx install` pip args are used to also create the shared libs.

They are NOT used when creating/upgrading the shared libs during a `pipx list` or any access of an existing venv.  (!!)

Recently this failed for me in the following situation:  In a Python 3.6 test I was trying to install a package for the first time using the pip arguments `--use-feature=2020-resolver`.  It caused pipx to fail with an error because before installing the package, it went to install the shared libs using pip argument `--use-feature=2020-resolver`.  Since the original Python 3.6 pip doesn't have that option, it couldn't even upgrade pip in the first place and crashed.

I'm wondering if we should just remove `pip_args` from `_SharedLibs.upgrade()` and `_SharedLibs.create()`.   It seems kind of arbitrary that only when upgrading the shared libs during a package install do they get the `pip_args` that happen to be used for that package, but not otherwise.

Is there a reason a user would need custom pip args to be used with the installation of the shared libs?  If so, maybe we need a different more reliable mechanism.

**How to reproduce**
<!-- If possible, include output of `pipx --verbose ...` -->

**Expected behavior**
<!-- What should have happened? -->


Pull request:
Make shared libraries create, upgrade not accept pip args
<!-- add an 'x' in the brackets below -->
* [x] I have added an entry to `docs/changelog.md`

## Summary of changes
Closes #544 

Changed the shared libs `create` and `upgrade` functions to no longer accept `pip_args`, which used to be applied inconsistently, sometimes taken from a recent `pipx install`, sometimes not at all.  Now they are never applied.  There are a lot of pip arguments that can even break the installation and/or upgrade of shared libraries (see example in #544).

It is assumed that `PIP_` environment variables can be used if pip behavior in pipx needs to be controlled consistently.

Also changed in `create()`:
```
pip install --upgrade --ignore-installed pip setuptools wheel
```
to
```
pip install --upgrade --force-reinstall pip setuptools wheel
```
As it seems safer to force a reinstall than just to ignore the old libraries.  This code is meant to force a fetch of a fresh pip from pypi to ignore any odd system custom pip version.

Incidental change: on the verbose display of shared libraries age in seconds, I removed the display of fractional seconds! 😄 

## 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
```


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 0caa8a8..d971f99 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -3,11 +3,12 @@ dev
 - Added reinstall command for reinstalling a single venv.
 - Changed `pipx run` on non-Windows systems to actually replace pipx process with the app process instead of running it as a subprocess.  (Now using python's `os.exec*`)
 - [bugfix] Fixed bug with reinstall-all command when package have been installed using a specifier. Now the initial specifier is used.
-- [bugfix] Override display of DEFAULT_PYTHON value when generating web documentation for `pipx install` #523
+- [bugfix] Override display of `PIPX_DEFAULT_PYTHON` value when generating web documentation for `pipx install` #523
 - [bugfix] Wrap help documentation for environment variables.
 - [bugfix] Fixed uninstall crash that could happen on Windows for certain packages
 - [feature] Venv package name arguments now do not have to match exactly as pipx has them stored, but can be specified in any python-package-name-equivalent way. (i.e. case does not matter, and `.`, `-`, `_` characters are interchangeable.)
 - [change] Venvs with a suffix: A suffix can contain any characters, but for purposes of uniqueness, python package name rules apply--upper- and lower-case letters are equivalent, and any number of `.`, `-`, or `_` characters in a row are equivalent.  (e.g. if you have a suffixed venv `pylint_1.0A` you could not add another suffixed venv called `pylint--1-0a`, as it would not be a unique name.)
+- [implementation detail] Pipx shared libraries (providing pip, setuptools, wheel to pipx) are no longer installed using pip arguments taken from the last regular pipx install.  If you need to apply pip arguments to pipx's use of pip for its internal shared libraries, use PIP_\* environment variables.
 
 0.15.6.0
 
@@ -16,7 +17,7 @@ dev
 - [bugfix] Fixed regression in list, inject, upgrade, reinstall-all commands when suffixed packages are used.
 - [bugfix] Do not reset package url during upgrade when main package is `pipx`
 - Updated help text to show description for `ensurepath` and `completions` help
-- Added support for user-defined default python interpreter via new PIPX_DEFAULT_PYTHON.  Helpful for use with pyenv among other uses.
+- Added support for user-defined default python interpreter via new `PIPX_DEFAULT_PYTHON`.  Helpful for use with pyenv among other uses.
 - [bugfix] Fixed bug where extras were ignored with a PEP 508 package specification with a URL.
 
 0.15.5.1
diff --git a/src/pipx/shared_libs.py b/src/pipx/shared_libs.py
index 504cb31..376c1d4 100644
--- a/src/pipx/shared_libs.py
+++ b/src/pipx/shared_libs.py
@@ -2,7 +2,7 @@ import datetime
 import logging
 import time
 from pathlib import Path
-from typing import List
+from typing import List, Optional
 
 from pipx import constants
 from pipx.animate import animate
@@ -30,13 +30,13 @@ class _SharedLibs:
 
         return self._site_packages
 
-    def create(self, pip_args: List[str], verbose: bool = False):
+    def create(self, verbose: bool = False):
         if not self.is_valid:
             with animate("creating shared libraries", not verbose):
                 run_verify([DEFAULT_PYTHON, "-m", "venv", "--clear", self.root])
             # ignore installed packages to ensure no unexpected patches from the OS vendor
             # are used
-            self.upgrade(["--ignore-installed"] + pip_args, verbose)
+            self.upgrade(pip_args=["--force-reinstall"], verbose=verbose)
 
     @property
     def is_valid(self):
@@ -53,16 +53,20 @@ class _SharedLibs:
         now = time.time()
         time_since_last_update_sec = now - self.pip_path.stat().st_mtime
         logging.info(
-            f"Time since last upgrade of shared libs, in seconds: {time_since_last_update_sec}. "
-            f"Upgrade will be run by pipx if greater than {SHARED_LIBS_MAX_AGE_SEC}."
+            f"Time since last upgrade of shared libs, in seconds: {time_since_last_update_sec:.0f}. "
+            f"Upgrade will be run by pipx if greater than {SHARED_LIBS_MAX_AGE_SEC:.0f}."
         )
         return time_since_last_update_sec > SHARED_LIBS_MAX_AGE_SEC
 
-    def upgrade(self, pip_args: List[str], verbose: bool = False):
+    def upgrade(self, *, pip_args: Optional[List[str]] = None, verbose: bool = False):
         # Don't try to upgrade multiple times per run
         if self.has_been_updated_this_run:
             logging.info(f"Already upgraded libraries in {self.root}")
             return
+
+        if pip_args is None:
+            pip_args = []
+
         logging.info(f"Upgrading shared libraries in {self.root}")
 
         ignored_args = ["--editable"]
diff --git a/src/pipx/venv.py b/src/pipx/venv.py
index ef61ebf..7af73cd 100644
--- a/src/pipx/venv.py
+++ b/src/pipx/venv.py
@@ -92,9 +92,9 @@ class Venv:
         if self._existing and self.uses_shared_libs:
             if shared_libs.is_valid:
                 if shared_libs.needs_upgrade:
-                    shared_libs.upgrade([], verbose)
+                    shared_libs.upgrade(verbose=verbose)
             else:
-                shared_libs.create([], verbose)
+                shared_libs.create(verbose)
 
             if not shared_libs.is_valid:
                 raise PipxError(
@@ -146,7 +146,7 @@ class Venv:
         with animate("creating virtual environment", self.do_animation):
             cmd = [self.python, "-m", "venv", "--without-pip"]
             run_verify(cmd + venv_args + [str(self.root)])
-        shared_libs.create(pip_args, self.verbose)
+        shared_libs.create(self.verbose)
         pipx_pth = get_site_packages(self.python_path) / PIPX_SHARED_PTH
         # write path pointing to the shared libs site-packages directory
         # example pipx_pth location:
@@ -176,7 +176,7 @@ class Venv:
 
     def upgrade_packaging_libraries(self, pip_args: List[str]) -> None:
         if self.uses_shared_libs:
-            shared_libs.upgrade(pip_args, self.verbose)
+            shared_libs.upgrade(verbose=self.verbose)
         else:
             # TODO: setuptools and wheel? Original code didn't bother
             # but shared libs code does.


Test patch:
diff --git a/tests/test_install.py b/tests/test_install.py
index d27c3a3..4967336 100644
--- a/tests/test_install.py
+++ b/tests/test_install.py
@@ -25,7 +25,10 @@ def install_package(capsys, pipx_temp_env, caplog, package, package_name=""):
     if not package_name:
         package_name = package
 
-    run_pipx_cli(["install", package, "--verbose"])
+    # TODO: remove "--feature=2020-resolver" when pip 20.3 is released
+    run_pipx_cli(
+        ["install", package, "--verbose", "--pip-args='--use-feature=2020-resolver'"]
+    )
     captured = capsys.readouterr()
     assert f"installed package {package_name}" in captured.out
     if not sys.platform.startswith("win"):
diff --git a/tests/test_run.py b/tests/test_run.py
index 546ddcf..f5bfecc 100644
--- a/tests/test_run.py
+++ b/tests/test_run.py
@@ -148,8 +148,17 @@ def test_package_determination(
 
     caplog.set_level(logging.INFO)
 
+    # TODO: remove "--feature=2020-resolver" when pip 20.3 is released
     run_pipx_cli_exit(
-        ["run", "--verbose", "--spec", package_or_url, "--"] + app_appargs
+        [
+            "run",
+            "--verbose",
+            "--pip-args='--use-feature=2020-resolver'",
+            "--spec",
+            package_or_url,
+            "--",
+        ]
+        + app_appargs
     )
 
     assert "Cannot determine package name" not in caplog.text
diff --git a/tests/test_shared_libs.py b/tests/test_shared_libs.py
index 9707ba7..ec064c6 100644
--- a/tests/test_shared_libs.py
+++ b/tests/test_shared_libs.py
@@ -15,7 +15,7 @@ from pipx import shared_libs
 )
 def test_auto_update_shared_libs(capsys, pipx_temp_env, mtime_minus_now, needs_upgrade):
     now = time.time()
-    shared_libs.shared_libs.create([], verbose=True)
+    shared_libs.shared_libs.create(verbose=True)
     shared_libs.shared_libs.has_been_updated_this_run = False
 
     access_time = now  # this can be anything
diff --git a/tests/test_uninstall.py b/tests/test_uninstall.py
index 8052506..2247a8e 100644
--- a/tests/test_uninstall.py
+++ b/tests/test_uninstall.py
@@ -10,7 +10,15 @@ def test_uninstall(pipx_temp_env, capsys):
 
 
 def test_uninstall_multiple_same_app(pipx_temp_env, capsys):
-    assert not run_pipx_cli(["install", "kaggle==1.5.9", "--include-deps"])
+    # TODO: remove --pip-args when 2020-resolver is default in new pip
+    assert not run_pipx_cli(
+        [
+            "install",
+            "kaggle==1.5.9",
+            "--include-deps",
+            "--pip-args='--use-feature=2020-resolver'",
+        ]
+    )
     assert not run_pipx_cli(["uninstall", "kaggle"])