Repository: pipxproject/pipx
Pull request #498 (2020-09-18 20:23:00)

Suffix-related bugfixes and tests and some refactoring
<!-- add an 'x' in the brackets below -->
* [x] I have added an entry to `docs/changelog.md`
_(I'm always thankful for this checkbox as I often forget)_

## Summary of changes
This was my attempt to clean up the code completely for suffix operation, as well as fixing the fact that `reinstall-all` was broken.

Major Changes
* Fix reinstall-all to work with suffixed venvs
* Add many tests
  * Test `reinstall-all` with installed suffixed package
  * New helper function `mock_legacy_venv()` to convert existing venv to look like old venv with missing or previous-version metadata
  * Add tests that mock legacy venvs (no metadata or old metadata version) to all commands that access venvs
* Refactor code
  * eliminate some `package` arguments that were often `None` or not necessary anyway
  * try to eliminate ambiguity by renaming variables (e.g. many `package` renamed to `venv_dir.name`), now that it is possible for `main_package` != `package` != `venv_name`
  * Make new `venv.main_package_name` property which is a great convenience for code that accesses venvs
* Changed `inject` to exit with `PipxError` and display instructions to reinstall venv when trying to inject package on pre-0.15.0.0 venv with missing metadata (fixing an existing error condition found by new tests!)
* Changed `upgrade-all` to raise `PipxError` and hence non-zero exit code if any individual venv upgrade had an error.
* Added function `_convert_legacy_metadata` in `pipx_metadata_file.py` to centralize any adjustments needed for reading legacy metadata.
* New `venv.main_package_name` made `venv.get_package_metadata()` unnecessary and it was removed.
* Added help text to `--suffix` indicating that it is an experimental feature.
  
Minor / Incidental Changes
* Added `flake8-bugbear` to `.pre-commit-config.yaml` to make its use of `flake8` match our Lint tests
* Added example of `pipx install ./LOCAL_PATH` to `pipx install` help text

### TODO for later
`pipx` help text is now a little confusing, as it refers to `PACKAGE` when it could mean any of `VENV_WITH_SUFFIX`, `VENV`, `INSTALLED PACKAGE`, etc.  

I at least tried to make the internal code more self consistent, referring to `venv_dir.name`, `main_package`, `package` to clearly differentiate what used to be used interchangeably in the code as `package`.

## Test plan
<!-- provide evidence of testing, preferably with command(s) that can be copy+pasted by others -->
Tested by running

To test the most obvious problem, that `reinstall-all` fails currently on suffixed packages:
```
> pipx install --suffix 2 pycowsay 
  installed package pycowsay 0.0.0.1, Python 3.8.5
  These apps are now globally available
    - pycowsay2
done! ✨ 🌟 ✨
> pipx reinstall-all
Traceback (most recent call last):
  File "/usr/local/Cellar/pipx/0.15.5.1/libexec/bin/pipx", line 33, in <module>
    sys.exit(load_entry_point('pipx==0.15.5.1', 'console_scripts', 'pipx')())
  File "/usr/local/Cellar/pipx/0.15.5.1/libexec/lib/python3.8/site-packages/pipx/main.py", line 591, in cli
    return run_pipx_command(parsed_pipx_args)
  File "/usr/local/Cellar/pipx/0.15.5.1/libexec/lib/python3.8/site-packages/pipx/main.py", line 194, in run_pipx_command
    return commands.reinstall_all(
  File "/usr/local/Cellar/pipx/0.15.5.1/libexec/lib/python3.8/site-packages/pipx/commands/reinstall.py", line 75, in reinstall_all
    reinstall(
  File "/usr/local/Cellar/pipx/0.15.5.1/libexec/lib/python3.8/site-packages/pipx/commands/reinstall.py", line 15, in reinstall
    venv = Venv(venv_dir, verbose=verbose)
  File "/usr/local/Cellar/pipx/0.15.5.1/libexec/lib/python3.8/site-packages/pipx/venv.py", line 82, in __init__
    self.pipx_metadata = PipxMetadata(venv_dir=path)
  File "/usr/local/Cellar/pipx/0.15.5.1/libexec/lib/python3.8/site-packages/pipx/pipx_metadata_file.py", line 68, in __init__
    self.read()
  File "/usr/local/Cellar/pipx/0.15.5.1/libexec/lib/python3.8/site-packages/pipx/pipx_metadata_file.py", line 144, in read
    self.from_dict(
  File "/usr/local/Cellar/pipx/0.15.5.1/libexec/lib/python3.8/site-packages/pipx/pipx_metadata_file.py", line 103, in from_dict
    self.main_package = PackageInfo(**input_dict["main_package"])
TypeError: __new__() got an unexpected keyword argument 'suffix'
> 

```



itsayellow: P.S. Do I need a changelog entry for change in exit code in case of a problem with one or more venvs with `upgrade-all`?
itsayellow: Another question to consider:  It seems to me that since we have only one suffix per venv, the proper place for `suffix` in the metadata would be at the `PipxMetadata`-level of abstraction.  However, for implementation it seems much easier for it to be in the lower `PackageInfo` level.

Any opinions on this?  It would be good to resolve before the next release of pipx puts metadata v0.2 into the wild.

Patch:
diff --git a/.github/workflows/on-push.yml b/.github/workflows/on-push.yml
index 70e3a10..6b4ea54 100644
--- a/.github/workflows/on-push.yml
+++ b/.github/workflows/on-push.yml
@@ -77,18 +77,6 @@ jobs:
         key: ${{ runner.os }}-pip${{ matrix.python-version }}
     - name: Upgrade pip
       run: python -m pip install --upgrade pip
-    # macOS: Install wheel to allow wheel-building for nox and regex
-    # Build regex wheel into pip cache, so future pipx installs don't compile.
-    # This avoids future compile errors in nox tests because of missing
-    #   header files.
-    # Inside a pytest session, tests/conftest.py removes all existing paths from
-    #   the system PATH, which can cause C compilation to fail.  So here we
-    #   pre-build some packages' wheels outside of pytest.
-    - name: Install wheel and build wheel for regex to pip cache
-      if: ${{ runner.os == 'macOS' }}
-      run: |
-        python -m pip install wheel
-        python -m pip install regex
     - name: Install nox
       run: pip install nox
     - name: Execute Tests
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 9c0a344..14c9a66 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -25,6 +25,9 @@ repos:
   rev: 3.8.3
   hooks:
   - id: flake8
+    additional_dependencies: [
+        'flake8-bugbear==20.1.4'
+    ]
 - repo: https://github.com/pre-commit/mirrors-mypy
   rev: v0.782
   hooks:
diff --git a/docs/changelog.md b/docs/changelog.md
index d2d880b..0b72694 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -1,7 +1,7 @@
 dev
 
 - [docs] Update license
-- [bugfix] Fixed regression in list, inject and upgrade commands when suffixed packages are used.
+- [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`
 - Update help text to show description for `ensurepath` and `completions` help
 
diff --git a/noxfile.py b/noxfile.py
index 957a2ce..757deed 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -23,10 +23,26 @@ lint_dependencies = [
     "check-manifest",
     "packaging>=20.0",
 ]
+# Packages that need an intact system PATH to compile on macOS
+macos_prebuild_packages = ["argon2-cffi", "regex"]
+
+
+def prebuild_wheels(session, package_list):
+    wheel_dir = Path(session.virtualenv.location) / "prebuild_wheels"
+    wheel_dir.mkdir()
+    session.install("wheel")
+    for prebuild_package in package_list:
+        session.run(
+            "pip", "wheel", f"--wheel-dir={wheel_dir}", prebuild_package, silent=True,
+        )
 
 
 @nox.session(python=python)
 def tests(session):
+    if sys.platform == "darwin":
+        # For macOS, need /usr/bin in PATH to compile some packages, but pytest
+        #   setup clears PATH.  So pre-build some wheels to the pip cache.
+        prebuild_wheels(session, macos_prebuild_packages)
     session.install("-e", ".", "pytest", "pytest-cov")
     tests = session.posargs or ["tests"]
     session.run(
diff --git a/src/pipx/commands/common.py b/src/pipx/commands/common.py
index 7c90c19..8539304 100644
--- a/src/pipx/commands/common.py
+++ b/src/pipx/commands/common.py
@@ -21,19 +21,12 @@ from pipx.venv import Venv
 
 
 def expose_apps_globally(
-    local_bin_dir: Path,
-    app_paths: List[Path],
-    package: str,
-    *,
-    force: bool,
-    suffix: str = "",
+    local_bin_dir: Path, app_paths: List[Path], *, force: bool, suffix: str = "",
 ) -> None:
     if not _can_symlink(local_bin_dir):
-        _copy_package_apps(local_bin_dir, app_paths, package, suffix=suffix)
+        _copy_package_apps(local_bin_dir, app_paths, suffix=suffix)
     else:
-        _symlink_package_apps(
-            local_bin_dir, app_paths, package, force=force, suffix=suffix
-        )
+        _symlink_package_apps(local_bin_dir, app_paths, force=force, suffix=suffix)
 
 
 _can_symlink_cache: Dict[Path, bool] = {}
@@ -61,7 +54,7 @@ def _can_symlink(local_bin_dir: Path) -> bool:
 
 
 def _copy_package_apps(
-    local_bin_dir: Path, app_paths: List[Path], package: str, suffix: str = "",
+    local_bin_dir: Path, app_paths: List[Path], suffix: str = "",
 ) -> None:
     for src_unresolved in app_paths:
         src = src_unresolved.resolve()
@@ -77,12 +70,7 @@ def _copy_package_apps(
 
 
 def _symlink_package_apps(
-    local_bin_dir: Path,
-    app_paths: List[Path],
-    package: str,
-    *,
-    force: bool,
-    suffix: str = "",
+    local_bin_dir: Path, app_paths: List[Path], *, force: bool, suffix: str = "",
 ) -> None:
     for app_path in app_paths:
         app_name = app_path.name
@@ -129,30 +117,32 @@ def _symlink_package_apps(
 
 
 def get_package_summary(
-    path: Path,
+    venv_dir: Path,
     *,
     package: str = None,
     new_install: bool = False,
     include_injected: bool = False,
 ) -> str:
-    venv = Venv(path)
+    venv = Venv(venv_dir)
     python_path = venv.python_path.resolve()
 
-    package_metadata = venv.get_package_metadata(package)
-    package = package_metadata.package or venv.root.name
+    if package is None:
+        package = venv.main_package_name
 
     if not python_path.is_file():
-        return f"   package {red(bold(package))} has invalid interpreter {str(python_path)}"
+        return f"   package {red(bold(venv_dir.name))} has invalid interpreter {str(python_path)}"
     if not venv.package_metadata:
         return (
-            f"   package {red(bold(package))} has missing internal pipx metadata.\n"
+            f"   package {red(bold(venv_dir.name))} has missing internal pipx metadata.\n"
             f"       It was likely installed using a pipx version before 0.15.0.0.\n"
             f"       Please uninstall and install this package, or reinstall-all to fix."
         )
 
+    package_metadata = venv.package_metadata[package]
+
     if package_metadata.package_version is None:
         not_installed = red("is not installed")
-        return f"   package {bold(package)} {not_installed} in the venv {str(path)}"
+        return f"   package {bold(package)} {not_installed} in the venv {venv_dir.name}"
 
     apps = package_metadata.apps + package_metadata.apps_of_dependencies
     exposed_app_paths = _get_exposed_app_paths_for_package(
@@ -281,8 +271,8 @@ def run_post_install_actions(
     *,
     force: bool,
 ):
-    package_metadata = venv.get_package_metadata(package)
-    package = package_metadata.package or package
+    package_metadata = venv.package_metadata[package]
+
     display_name = f"{package}{package_metadata.suffix}"
 
     if not package_metadata.app_paths and not include_dependencies:
@@ -323,7 +313,7 @@ def run_post_install_actions(
         if venv.safe_to_remove():
             venv.remove_venv()
         raise PipxError(
-            f"No apps associated with package {display_name} or its dependencies."
+            f"No apps associated with package {display_name} or its dependencies. "
             "If you are attempting to install a library, pipx should not be used. "
             "Consider using pip or a similar tool instead."
         )
@@ -331,7 +321,6 @@ def run_post_install_actions(
     expose_apps_globally(
         local_bin_dir,
         package_metadata.app_paths,
-        package,
         force=force,
         suffix=package_metadata.suffix,
     )
@@ -339,11 +328,7 @@ def run_post_install_actions(
     if include_dependencies:
         for _, app_paths in package_metadata.app_paths_of_dependencies.items():
             expose_apps_globally(
-                local_bin_dir,
-                app_paths,
-                package,
-                force=force,
-                suffix=package_metadata.suffix,
+                local_bin_dir, app_paths, force=force, suffix=package_metadata.suffix,
             )
 
     print(get_package_summary(venv_dir, package=package, new_install=True,))
diff --git a/src/pipx/commands/inject.py b/src/pipx/commands/inject.py
index 84ead43..417b049 100644
--- a/src/pipx/commands/inject.py
+++ b/src/pipx/commands/inject.py
@@ -34,6 +34,14 @@ def inject(
 
     venv = Venv(venv_dir, verbose=verbose)
 
+    if not venv.package_metadata:
+        raise PipxError(
+            f"Can't inject {package_spec!r} into Virtual Environment {venv_dir.name!r}.\n"
+            f"    {venv_dir.name!r} has missing internal pipx metadata.\n"
+            "    It was likely installed using a pipx version before 0.15.0.0.\n"
+            f"    Please uninstall and install {venv_dir.name!r}, or reinstall-all to fix."
+        )
+
     # package_spec is anything pip-installable, including package_name, vcs spec,
     #   zip file, or tar.gz file.
     if package_name is None:
diff --git a/src/pipx/commands/install.py b/src/pipx/commands/install.py
index d7e62b5..c5d3319 100644
--- a/src/pipx/commands/install.py
+++ b/src/pipx/commands/install.py
@@ -18,18 +18,19 @@ def install(
     *,
     force: bool,
     include_dependencies: bool,
-    suffix: Optional[str] = None,
+    suffix: str = "",
 ):
     # package_spec is anything pip-installable, including package_name, vcs spec,
     #   zip file, or tar.gz file.
-    # Determine package_name to properly name venv directory.
-    if venv_dir is None or package_name is None:
+
+    if package_name is None:
         package_name = package_name_from_spec(
             package_spec, python, pip_args=pip_args, verbose=verbose
         )
+    if venv_dir is None:
         venv_container = VenvContainer(constants.PIPX_LOCAL_VENVS)
         venv_dir = venv_container.get_venv_dir(package_name)
-        if suffix is not None:
+        if suffix != "":
             venv_dir = venv_dir.parent / f"{venv_dir.stem}{suffix}"
 
     try:
@@ -42,7 +43,7 @@ def install(
             print(f"Installing to existing directory {str(venv_dir)!r}")
         else:
             print(
-                f"{package_name!r} already seems to be installed. "
+                f"{venv_dir.name!r} already seems to be installed. "
                 f"Not modifying existing installation in {str(venv_dir)!r}. "
                 "Pass '--force' to force installation."
             )
@@ -58,7 +59,7 @@ def install(
             include_dependencies=include_dependencies,
             include_apps=True,
             is_main_package=True,
-            suffix=suffix or "",
+            suffix=suffix,
         )
         run_post_install_actions(
             venv,
diff --git a/src/pipx/commands/reinstall.py b/src/pipx/commands/reinstall.py
index 4878387..c4064f0 100644
--- a/src/pipx/commands/reinstall.py
+++ b/src/pipx/commands/reinstall.py
@@ -1,6 +1,6 @@
+import sys
 from pathlib import Path
 from typing import List
-import sys
 
 from pipx.commands.inject import inject
 from pipx.commands.install import install
@@ -10,21 +10,21 @@ from pipx.venv import Venv, VenvContainer
 
 
 def reinstall(
-    *, venv_dir: Path, package: str, local_bin_dir: Path, python: str, verbose: bool,
+    *, venv_dir: Path, local_bin_dir: Path, python: str, verbose: bool,
 ):
     venv = Venv(venv_dir, verbose=verbose)
 
     if venv.pipx_metadata.main_package.package_or_url is not None:
         package_or_url = venv.pipx_metadata.main_package.package_or_url
     else:
-        package_or_url = package
+        package_or_url = venv.main_package_name
 
-    uninstall(venv_dir, package, local_bin_dir, verbose)
+    uninstall(venv_dir, local_bin_dir, verbose)
 
     # install main package first
     install(
         venv_dir,
-        package,
+        venv.main_package_name,
         package_or_url,
         local_bin_dir,
         python,
@@ -33,6 +33,7 @@ def reinstall(
         verbose,
         force=True,
         include_dependencies=venv.pipx_metadata.main_package.include_dependencies,
+        suffix=venv.pipx_metadata.main_package.suffix,
     )
 
     # now install injected packages
@@ -44,7 +45,7 @@ def reinstall(
             # This should never happen, but package_or_url is type
             #   Optional[str] so mypy thinks it could be None
             raise PipxError(
-                f"Internal Error injecting package {injected_package} into {package}"
+                f"Internal Error injecting package {injected_package} into {venv_dir.name}"
             )
         inject(
             venv_dir,
@@ -68,20 +69,18 @@ def reinstall_all(
 ):
     failed: List[str] = []
     for venv_dir in venv_container.iter_venv_dirs():
-        package = venv_dir.name
-        if package in skip:
+        if venv_dir.name in skip:
             continue
         try:
             reinstall(
                 venv_dir=venv_dir,
-                package=package,
                 local_bin_dir=local_bin_dir,
                 python=python,
                 verbose=verbose,
             )
         except PipxError as e:
             print(e, file=sys.stderr)
-            failed.append(package)
+            failed.append(venv_dir.name)
     if len(failed) > 0:
         raise PipxError(
             f"The following package(s) failed to reinstall: {', '.join(failed)}"
diff --git a/src/pipx/commands/uninstall.py b/src/pipx/commands/uninstall.py
index e76a2ff..bef0c96 100644
--- a/src/pipx/commands/uninstall.py
+++ b/src/pipx/commands/uninstall.py
@@ -2,21 +2,20 @@ import logging
 from pathlib import Path
 from shutil import which
 from typing import List
-from pipx.util import WINDOWS, rmdir
-
 
 from pipx import constants
-from pipx.emojies import hazard, stars, sleep
+from pipx.emojies import hazard, sleep, stars
+from pipx.util import WINDOWS, rmdir
 from pipx.venv import Venv, VenvContainer
 
 
-def uninstall(venv_dir: Path, package: str, local_bin_dir: Path, verbose: bool):
+def uninstall(venv_dir: Path, local_bin_dir: Path, verbose: bool):
     """Uninstall entire venv_dir, including main package and all injected
     packages.
     """
     if not venv_dir.exists():
-        print(f"Nothing to uninstall for {package} {sleep}")
-        app = which(package)
+        print(f"Nothing to uninstall for {venv_dir.name} {sleep}")
+        app = which(venv_dir.name)
         if app:
             print(
                 f"{hazard}  Note: '{app}' still exists on your system and is on your PATH"
@@ -35,7 +34,8 @@ def uninstall(venv_dir: Path, package: str, local_bin_dir: Path, verbose: bool):
         # fallback if not metadata from pipx_metadata.json
         if venv.python_path.is_file():
             # has a valid python interpreter and can get metadata about the package
-            metadata = venv.get_venv_metadata_for_package(package)
+            # In pre-metadata-pipx venv_dir.name is name of main package
+            metadata = venv.get_venv_metadata_for_package(venv_dir.name)
             app_paths = metadata.app_paths
             for dep_paths in metadata.app_paths_of_dependencies.values():
                 app_paths += dep_paths
@@ -43,7 +43,7 @@ def uninstall(venv_dir: Path, package: str, local_bin_dir: Path, verbose: bool):
             # Doesn't have a valid python interpreter. We'll take our best guess on what to uninstall
             # here based on symlink location. pipx doesn't use symlinks on windows, so this is for
             # non-windows only.
-            # The heuristic here is any symlink in ~/.local/bin pointing to .local/pipx/venvs/PACKAGE/bin
+            # The heuristic here is any symlink in ~/.local/bin pointing to .local/pipx/venvs/VENV_NAME/bin
             # should be uninstalled.
             if WINDOWS:
                 app_paths = []
@@ -68,10 +68,9 @@ def uninstall(venv_dir: Path, package: str, local_bin_dir: Path, verbose: bool):
                     symlink.unlink()
 
     rmdir(venv_dir)
-    print(f"uninstalled {package}! {stars}")
+    print(f"uninstalled {venv_dir.name}! {stars}")
 
 
 def uninstall_all(venv_container: VenvContainer, local_bin_dir: Path, verbose: bool):
     for venv_dir in venv_container.iter_venv_dirs():
-        package = venv_dir.name
-        uninstall(venv_dir, package, local_bin_dir, verbose)
+        uninstall(venv_dir, local_bin_dir, verbose)
diff --git a/src/pipx/commands/upgrade.py b/src/pipx/commands/upgrade.py
index 562ba26..65b1537 100644
--- a/src/pipx/commands/upgrade.py
+++ b/src/pipx/commands/upgrade.py
@@ -12,7 +12,6 @@ from pipx.venv import Venv, VenvContainer
 
 def upgrade(
     venv_dir: Path,
-    package: str,
     pip_args: List[str],
     verbose: bool,
     *,
@@ -28,6 +27,7 @@ def upgrade(
         )
 
     venv = Venv(venv_dir, verbose=verbose)
+    package = venv.main_package_name
 
     if not venv.package_metadata:
         print(
@@ -37,8 +37,7 @@ def upgrade(
         )
         return 0
 
-    package_metadata = venv.get_package_metadata(package)
-    package = package_metadata.package or package
+    package_metadata = venv.package_metadata[package]
 
     if package_metadata.package_or_url is None:
         raise PipxError(f"Internal Error: package {package} has corrupt pipx metadata.")
@@ -62,15 +61,14 @@ def upgrade(
     )
     # TODO 20191026: upgrade injected packages also (Issue #79)
 
-    package_metadata = venv.get_package_metadata(package)
-    package = package_metadata.package or package
+    package_metadata = venv.package_metadata[package]
+
     display_name = f"{package_metadata.package}{package_metadata.suffix}"
     new_version = package_metadata.package_version
 
     expose_apps_globally(
         constants.LOCAL_BIN_DIR,
         package_metadata.app_paths,
-        package,
         force=force,
         suffix=package_metadata.suffix,
     )
@@ -80,7 +78,6 @@ def upgrade(
             expose_apps_globally(
                 constants.LOCAL_BIN_DIR,
                 app_paths,
-                package,
                 force=force,
                 suffix=package_metadata.suffix,
             )
@@ -103,16 +100,18 @@ def upgrade(
 def upgrade_all(
     venv_container: VenvContainer, verbose: bool, *, skip: List[str], force: bool
 ):
-    packages_upgraded = 0
+    venv_error = False
+    venvs_upgraded = 0
     for venv_dir in venv_container.iter_venv_dirs():
-        package = venv_dir.name
         venv = Venv(venv_dir, verbose=verbose)
-        if package in skip or "--editable" in venv.pipx_metadata.main_package.pip_args:
+        if (
+            venv_dir.name in skip
+            or "--editable" in venv.pipx_metadata.main_package.pip_args
+        ):
             continue
         try:
-            packages_upgraded += upgrade(
+            venvs_upgraded += upgrade(
                 venv_dir,
-                package,
                 venv.pipx_metadata.main_package.pip_args,
                 verbose,
                 upgrading_all=True,
@@ -120,9 +119,15 @@ def upgrade_all(
             )
 
         except Exception:
-            logging.error(f"Error encountered when upgrading {package}")
+            venv_error = True
+            logging.error(f"Error encountered when upgrading {venv_dir.name}")
 
-    if packages_upgraded == 0:
+    if venvs_upgraded == 0:
         print(
             f"Versions did not change after running 'pip upgrade' for each package {sleep}"
         )
+    if venv_error:
+        raise PipxError(
+            "Some packages encountered errors during upgrade. "
+            "See specific error messages above."
+        )
diff --git a/src/pipx/main.py b/src/pipx/main.py
index f9ecf31..41a4898 100644
--- a/src/pipx/main.py
+++ b/src/pipx/main.py
@@ -66,6 +66,7 @@ packages. 'sudo' is not required to do this.
 pipx install PACKAGE_NAME
 pipx install --python PYTHON PACKAGE_NAME
 pipx install VCS_URL
+pipx install ./LOCAL_PATH
 pipx install ZIP_FILE
 pipx install TAR_GZ_FILE
 
@@ -179,12 +180,12 @@ def run_pipx_command(args: argparse.Namespace):  # noqa: C901
             )
     elif args.command == "upgrade":
         return commands.upgrade(
-            venv_dir, package, pip_args, verbose, upgrading_all=False, force=args.force
+            venv_dir, pip_args, verbose, upgrading_all=False, force=args.force
         )
     elif args.command == "list":
         return commands.list_packages(venv_container, args.include_injected)
     elif args.command == "uninstall":
-        return commands.uninstall(venv_dir, package, constants.LOCAL_BIN_DIR, verbose)
+        return commands.uninstall(venv_dir, constants.LOCAL_BIN_DIR, verbose)
     elif args.command == "uninstall-all":
         return commands.uninstall_all(venv_container, constants.LOCAL_BIN_DIR, verbose)
     elif args.command == "upgrade-all":
@@ -263,7 +264,12 @@ def _add_install(subparsers):
         help="Modify existing virtual environment and files in PIPX_BIN_DIR",
     )
     p.add_argument(
-        "--suffix", help="Optional suffix for virtual environment and executable names"
+        "--suffix",
+        default="",
+        help=(
+            "Optional suffix for virtual environment and executable names. "
+            "NOTE: The suffix feature is experimental and subject to change."
+        ),
     )
     p.add_argument(
         "--python",
@@ -327,8 +333,7 @@ def _add_upgrade(subparsers, autocomplete_list_of_installed_packages):
 def _add_upgrade_all(subparsers):
     p = subparsers.add_parser(
         "upgrade-all",
-        help="Upgrade all packages. "
-        "Runs `pip install -U <pkgname>` for each package.",
+        help="Upgrade all packages. Runs `pip install -U <pkgname>` for each package.",
         description="Upgrades all packages within their virtual environments by running 'pip install --upgrade PACKAGE'",
     )
 
diff --git a/src/pipx/pipx_metadata_file.py b/src/pipx/pipx_metadata_file.py
index 49fc9e1..eb7c984 100644
--- a/src/pipx/pipx_metadata_file.py
+++ b/src/pipx/pipx_metadata_file.py
@@ -78,15 +78,27 @@ class PipxMetadata:
             "pipx_metadata_version": self.__METADATA_VERSION__,
         }
 
-    def from_dict(self, input_dict: Dict[str, Any]) -> None:
-        main_package_data = input_dict["main_package"]
-        if main_package_data["package"] != self.venv_dir.name:
-            # handle older suffixed packages gracefully
-            main_package_data["suffix"] = self.venv_dir.name.replace(
-                main_package_data["package"], ""
+    def _convert_legacy_metadata(self, metadata_dict: Dict[str, Any]) -> Dict[str, Any]:
+        if metadata_dict["pipx_metadata_version"] == self.__METADATA_VERSION__:
+            return metadata_dict
+        elif metadata_dict["pipx_metadata_version"] == "0.1":
+            main_package_data = metadata_dict["main_package"]
+            if main_package_data["package"] != self.venv_dir.name:
+                # handle older suffixed packages gracefully
+                main_package_data["suffix"] = self.venv_dir.name.replace(
+                    main_package_data["package"], ""
+                )
+            return metadata_dict
+        else:
+            raise PipxError(
+                f"{self.venv_dir.name}: Unknown metadata version "
+                f"{metadata_dict['pipx_metadata_version']}. "
+                "Perhaps it was installed with a later version of pipx."
             )
 
-        self.main_package = PackageInfo(**main_package_data)
+    def from_dict(self, input_dict: Dict[str, Any]) -> None:
+        input_dict = self._convert_legacy_metadata(input_dict)
+        self.main_package = PackageInfo(**input_dict["main_package"])
         self.python_version = input_dict["python_version"]
         self.venv_args = input_dict["venv_args"]
         self.injected_packages = {
diff --git a/src/pipx/venv.py b/src/pipx/venv.py
index 377a10e..3bf1e6f 100644
--- a/src/pipx/venv.py
+++ b/src/pipx/venv.py
@@ -2,7 +2,7 @@ import json
 import logging
 import pkgutil
 from pathlib import Path
-from typing import Dict, Generator, List, NamedTuple, Optional, Set
+from typing import Dict, Generator, List, NamedTuple, Set
 
 from pipx.animate import animate
 from pipx.constants import PIPX_SHARED_PTH
@@ -121,17 +121,14 @@ class Venv:
             ] = self.pipx_metadata.main_package
         return return_dict
 
-    def get_package_metadata(self, package: Optional[str] = None) -> PackageInfo:
-        if package is None or package == self.root.name:
-            return self.pipx_metadata.main_package
-
-        try:
-            return self.package_metadata[package]
-        except KeyError:
-            raise PipxError(
-                f"Package is not installed. Expected to find package {package}, "
-                "but it does not exist."
-            )
+    @property
+    def main_package_name(self) -> str:
+        if self.pipx_metadata.main_package.package is None:
+            # This is OK, because if no metadata, we are pipx < v0.15.0.0 and
+            #   venv_name==main_package_name
+            return self.root.name
+        else:
+            return self.pipx_metadata.main_package.package
 
     def create_venv(self, venv_args: List[str], pip_args: List[str]) -> None:
         with animate("creating virtual environment", self.do_animation):
@@ -313,7 +310,7 @@ class Venv:
             apps_of_dependencies=venv_package_metadata.apps_of_dependencies,
             app_paths_of_dependencies=venv_package_metadata.app_paths_of_dependencies,
             package_version=venv_package_metadata.package_version,
-            suffix=suffix or "",
+            suffix=suffix,
         )
         if is_main_package:
             self.pipx_metadata.main_package = package_info
@@ -368,7 +365,7 @@ class Venv:
             include_dependencies=include_dependencies,
             include_apps=include_apps,
             is_main_package=is_main_package,
-            suffix=suffix or "",
+            suffix=suffix,
         )
 
     def _run_pip(self, cmd: List[str]) -> int:


Test patch:
diff --git a/tests/helpers.py b/tests/helpers.py
index 989ed47..1f7d011 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -1,18 +1,42 @@
+import json
+import os
 import subprocess
 import sys
+from pathlib import Path
 from shutil import which
-from typing import List
+from typing import Any, Dict, List, Optional
 from unittest import mock
 
-from pipx import main
+from pipx import constants, main, pipx_metadata_file
 
+MOCK_PIPXMETADATA_0_1: Dict[str, Any] = {
+    "main_package": None,
+    "python_version": None,
+    "venv_args": [],
+    "injected_packages": {},
+    "pipx_metadata_version": "0.1",
+}
 
-def run_pipx_cli(pipx_args: List[str]):
+MOCK_PACKAGE_INFO_0_1: Dict[str, Any] = {
+    "package": None,
+    "package_or_url": None,
+    "pip_args": [],
+    "include_dependencies": False,
+    "include_apps": True,
+    "apps": [],
+    "app_paths": [],
+    "apps_of_dependencies": [],
+    "app_paths_of_dependencies": {},
+    "package_version": "",
+}
+
+
+def run_pipx_cli(pipx_args: List[str]) -> int:
     with mock.patch.object(sys, "argv", ["pipx"] + pipx_args):
         return main.cli()
 
 
-def which_python(python_exe):
+def which_python(python_exe: str) -> Optional[str]:
     try:
         pyenv_which = subprocess.run(
             ["pyenv", "which", python_exe],
@@ -28,3 +52,70 @@ def which_python(python_exe):
     else:
         # pyenv on system, but pyenv has no path to python_exe
         return None
+
+
+def _mock_legacy_package_info(
+    modern_package_info: Dict[str, Any], metadata_version: str
+) -> Dict[str, Any]:
+    if metadata_version == "0.1":
+        mock_package_info_template = MOCK_PACKAGE_INFO_0_1
+    else:
+        raise Exception(
+            f"Internal Test Error: Unknown metadata_version={metadata_version}"
+        )
+
+    mock_package_info = {}
+    for key in mock_package_info_template:
+        mock_package_info[key] = modern_package_info[key]
+
+    return mock_package_info
+
+
+def mock_legacy_venv(venv_name: str, metadata_version: Optional[str] = None) -> None:
+    """Convert a venv installed with the most recent pipx to look like
+    one with a previous metadata version.
+    metadata_version=None refers to no metadata file (pipx pre-0.15.0.0)
+    """
+    venv_dir = Path(constants.PIPX_LOCAL_VENVS) / venv_name
+
+    if metadata_version is None:
+        os.remove(venv_dir / "pipx_metadata.json")
+        return
+
+    modern_metadata = pipx_metadata_file.PipxMetadata(venv_dir).to_dict()
+
+    if metadata_version == "0.1":
+        mock_pipx_metadata_template = MOCK_PIPXMETADATA_0_1
+    else:
+        raise Exception(
+            f"Internal Test Error: Unknown metadata_version={metadata_version}"
+        )
+
+    # Convert to mock old metadata
+    mock_pipx_metadata = {}
+    for key in mock_pipx_metadata_template:
+        if key == "main_package":
+            mock_pipx_metadata[key] = _mock_legacy_package_info(
+                modern_metadata[key], metadata_version=metadata_version
+            )
+        if key == "injected_packages":
+            mock_pipx_metadata[key] = {}
+            for injected in modern_metadata[key]:
+                mock_pipx_metadata[key][injected] = _mock_legacy_package_info(
+                    modern_metadata[key][injected], metadata_version=metadata_version
+                )
+        else:
+            mock_pipx_metadata[key] = modern_metadata[key]
+    mock_pipx_metadata["pipx_metadata_version"] = mock_pipx_metadata_template[
+        "pipx_metadata_version"
+    ]
+
+    # replicate pipx_metadata_file.PipxMetadata.write()
+    with open(venv_dir / "pipx_metadata.json", "w") as pipx_metadata_fh:
+        json.dump(
+            mock_pipx_metadata,
+            pipx_metadata_fh,
+            indent=4,
+            sort_keys=True,
+            cls=pipx_metadata_file.JsonEncoderHandlesPath,
+        )
diff --git a/tests/test_inject.py b/tests/test_inject.py
index 399eeb0..0afa067 100644
--- a/tests/test_inject.py
+++ b/tests/test_inject.py
@@ -1,20 +1,32 @@
 import pytest  # type: ignore
 
-from helpers import run_pipx_cli
+from helpers import mock_legacy_venv, run_pipx_cli
 
 
-def test_simple(pipx_temp_env, capsys):
+def test_inject_simple(pipx_temp_env, capsys):
     assert not run_pipx_cli(["install", "pycowsay"])
     assert not run_pipx_cli(["inject", "pycowsay", "black"])
 
 
+@pytest.mark.parametrize("metadata_version", [None, "0.1"])
+def test_inject_simple_legacy_venv(pipx_temp_env, capsys, metadata_version):
+    assert not run_pipx_cli(["install", "pycowsay"])
+    mock_legacy_venv("pycowsay", metadata_version=metadata_version)
+    if metadata_version is not None:
+        assert not run_pipx_cli(["inject", "pycowsay", "black"])
+    else:
+        # no metadata in venv should result in PipxError with message
+        assert run_pipx_cli(["inject", "pycowsay", "black"])
+        assert "Please uninstall and install" in capsys.readouterr().err
+
+
 def test_spec(pipx_temp_env, capsys):
     assert not run_pipx_cli(["install", "pycowsay"])
     assert not run_pipx_cli(["inject", "pycowsay", "pylint==2.3.1"])
 
 
 @pytest.mark.parametrize("with_suffix,", [(False,), (True,)])
-def test_include_apps(pipx_temp_env, capsys, with_suffix):
+def test_inject_include_apps(pipx_temp_env, capsys, with_suffix):
     install_args = []
     suffix = ""
 
diff --git a/tests/test_list.py b/tests/test_list.py
index 3096b00..159d3d3 100644
--- a/tests/test_list.py
+++ b/tests/test_list.py
@@ -1,4 +1,6 @@
-from helpers import run_pipx_cli
+import pytest  # type: ignore
+
+from helpers import mock_legacy_venv, run_pipx_cli
 from pipx import constants, util
 
 
@@ -27,7 +29,28 @@ def test_missing_interpreter(pipx_temp_env, monkeypatch, capsys):
 def test_list_suffix(pipx_temp_env, monkeypatch, capsys):
     suffix = "_x"
     assert not run_pipx_cli(["install", "pycowsay", f"--suffix={suffix}"])
+
     assert not run_pipx_cli(["list"])
+    captured = capsys.readouterr()
+    assert f"package pycowsay 0.0.0.1 (pycowsay{suffix})," in captured.out
 
+
+@pytest.mark.parametrize("metadata_version", [None, "0.1"])
+def test_list_legacy_venv(pipx_temp_env, monkeypatch, capsys, metadata_version):
+    assert not run_pipx_cli(["install", "pycowsay"])
+    mock_legacy_venv("pycowsay", metadata_version=metadata_version)
+
+    assert not run_pipx_cli(["list"])
+    captured = capsys.readouterr()
+    assert "package pycowsay 0.0.0.1," in captured.out
+
+
+@pytest.mark.parametrize("metadata_version", ["0.1"])
+def test_list_suffix_legacy_venv(pipx_temp_env, monkeypatch, capsys, metadata_version):
+    suffix = "_x"
+    assert not run_pipx_cli(["install", "pycowsay", f"--suffix={suffix}"])
+    mock_legacy_venv(f"pycowsay{suffix}", metadata_version=metadata_version)
+
+    assert not run_pipx_cli(["list"])
     captured = capsys.readouterr()
     assert f"package pycowsay 0.0.0.1 (pycowsay{suffix})," in captured.out
diff --git a/tests/test_reinstall_all.py b/tests/test_reinstall_all.py
index 15dea1b..c51f742 100644
--- a/tests/test_reinstall_all.py
+++ b/tests/test_reinstall_all.py
@@ -1,8 +1,34 @@
 import sys
 
-from helpers import run_pipx_cli
+import pytest  # type: ignore
+
+from helpers import mock_legacy_venv, 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", "--python", sys.executable])
+
+
+@pytest.mark.parametrize("metadata_version", [None, "0.1"])
+def test_reinstall_all_legacy_venv(pipx_temp_env, capsys, metadata_version):
+    assert not run_pipx_cli(["install", "pycowsay"])
+    mock_legacy_venv("pycowsay", metadata_version=metadata_version)
+
+    assert not run_pipx_cli(["reinstall-all", "--python", sys.executable])
+
+
+def test_reinstall_all_suffix(pipx_temp_env, capsys):
+    suffix = "_x"
+    assert not run_pipx_cli(["install", "pycowsay", f"--suffix={suffix}"])
+
+    assert not run_pipx_cli(["reinstall-all", "--python", sys.executable])
+
+
+@pytest.mark.parametrize("metadata_version", ["0.1"])
+def test_reinstall_all_suffix_legacy_venv(pipx_temp_env, capsys, metadata_version):
+    suffix = "_x"
+    assert not run_pipx_cli(["install", "pycowsay", f"--suffix={suffix}"])
+    mock_legacy_venv(f"pycowsay{suffix}", metadata_version=metadata_version)
+
+    assert not run_pipx_cli(["reinstall-all", "--python", sys.executable])
diff --git a/tests/test_uninstall.py b/tests/test_uninstall.py
index fea4f6e..57ae5f2 100644
--- a/tests/test_uninstall.py
+++ b/tests/test_uninstall.py
@@ -1,4 +1,6 @@
-from helpers import run_pipx_cli
+import pytest  # type: ignore
+
+from helpers import mock_legacy_venv, run_pipx_cli
 from pipx import constants, util
 
 
@@ -7,6 +9,13 @@ def test_uninstall(pipx_temp_env, capsys):
     assert not run_pipx_cli(["uninstall", "pycowsay"])
 
 
+@pytest.mark.parametrize("metadata_version", [None, "0.1"])
+def test_uninstall_legacy_venv(pipx_temp_env, capsys, metadata_version):
+    assert not run_pipx_cli(["install", "pycowsay"])
+    mock_legacy_venv("pycowsay", metadata_version=metadata_version)
+    assert not run_pipx_cli(["uninstall", "pycowsay"])
+
+
 def test_uninstall_suffix(pipx_temp_env, capsys):
     name = "pbr"
     suffix = "_a"
@@ -19,6 +28,20 @@ def test_uninstall_suffix(pipx_temp_env, capsys):
     assert not (constants.LOCAL_BIN_DIR / executable).exists()
 
 
+@pytest.mark.parametrize("metadata_version", ["0.1"])
+def test_uninstall_suffix_legacy_venv(pipx_temp_env, capsys, metadata_version):
+    name = "pbr"
+    suffix = "_a"
+    executable = f"{name}{suffix}{'.exe' if constants.WINDOWS else ''}"
+
+    assert not run_pipx_cli(["install", "pbr", f"--suffix={suffix}"])
+    mock_legacy_venv(f"{name}{suffix}", metadata_version=metadata_version)
+    assert (constants.LOCAL_BIN_DIR / executable).exists()
+
+    assert not run_pipx_cli(["uninstall", f"{name}{suffix}"])
+    assert not (constants.LOCAL_BIN_DIR / executable).exists()
+
+
 def test_uninstall_with_missing_interpreter(pipx_temp_env, capsys):
     assert not run_pipx_cli(["install", "pycowsay"])
 
diff --git a/tests/test_uninstall_all.py b/tests/test_uninstall_all.py
index 8f3a000..9715e4b 100644
--- a/tests/test_uninstall_all.py
+++ b/tests/test_uninstall_all.py
@@ -1,6 +1,15 @@
-from helpers import run_pipx_cli
+import pytest  # type: ignore
+
+from helpers import mock_legacy_venv, run_pipx_cli
 
 
 def test_uninstall_all(pipx_temp_env, capsys):
     assert not run_pipx_cli(["install", "pycowsay"])
     assert not run_pipx_cli(["uninstall-all"])
+
+
+@pytest.mark.parametrize("metadata_version", [None, "0.1"])
+def test_uninstall_all_legacy_venv(pipx_temp_env, capsys, metadata_version):
+    assert not run_pipx_cli(["install", "pycowsay"])
+    mock_legacy_venv("pycowsay", metadata_version=metadata_version)
+    assert not run_pipx_cli(["uninstall-all"])
diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py
index 1d4aea2..02ce34b 100644
--- a/tests/test_upgrade.py
+++ b/tests/test_upgrade.py
@@ -1,4 +1,6 @@
-from helpers import run_pipx_cli
+import pytest  # type: ignore
+
+from helpers import mock_legacy_venv, run_pipx_cli
 
 
 def test_upgrade(pipx_temp_env, capsys):
@@ -7,6 +9,14 @@ def test_upgrade(pipx_temp_env, capsys):
     assert not run_pipx_cli(["upgrade", "pycowsay"])
 
 
+@pytest.mark.parametrize("metadata_version", [None, "0.1"])
+def test_upgrade_legacy_venv(pipx_temp_env, capsys, metadata_version):
+    assert run_pipx_cli(["upgrade", "pycowsay"])
+    assert not run_pipx_cli(["install", "pycowsay"])
+    mock_legacy_venv("pycowsay", metadata_version=metadata_version)
+    assert not run_pipx_cli(["upgrade", "pycowsay"])
+
+
 def test_upgrade_suffix(pipx_temp_env, capsys):
     name = "pycowsay"
     suffix = "_a"
@@ -14,3 +24,14 @@ def test_upgrade_suffix(pipx_temp_env, capsys):
     assert not run_pipx_cli(["install", name, f"--suffix={suffix}"])
     assert run_pipx_cli(["upgrade", f"{name}"])
     assert not run_pipx_cli(["upgrade", f"{name}{suffix}"])
+
+
+@pytest.mark.parametrize("metadata_version", ["0.1"])
+def test_upgrade_suffix_legacy_venv(pipx_temp_env, capsys, metadata_version):
+    name = "pycowsay"
+    suffix = "_a"
+
+    assert not run_pipx_cli(["install", name, f"--suffix={suffix}"])
+    mock_legacy_venv(f"{name}{suffix}", metadata_version=metadata_version)
+    assert run_pipx_cli(["upgrade", f"{name}"])
+    assert not run_pipx_cli(["upgrade", f"{name}{suffix}"])
diff --git a/tests/test_upgrade_all.py b/tests/test_upgrade_all.py
index d70aa01..9dae190 100644
--- a/tests/test_upgrade_all.py
+++ b/tests/test_upgrade_all.py
@@ -1,7 +1,17 @@
-from helpers import run_pipx_cli
+import pytest  # type: ignore
+
+from helpers import mock_legacy_venv, run_pipx_cli
 
 
 def test_upgrade_all(pipx_temp_env, capsys):
     assert run_pipx_cli(["upgrade", "pycowsay"])
     assert not run_pipx_cli(["install", "pycowsay"])
     assert not run_pipx_cli(["upgrade-all"])
+
+
+@pytest.mark.parametrize("metadata_version", [None, "0.1"])
+def test_upgrade_all_legacy_venv(pipx_temp_env, capsys, metadata_version):
+    assert run_pipx_cli(["upgrade", "pycowsay"])
+    assert not run_pipx_cli(["install", "pycowsay"])
+    mock_legacy_venv("pycowsay", metadata_version=metadata_version)
+    assert not run_pipx_cli(["upgrade-all"])