Inject additional packages from text file (#1252)

* Naive `pipx inject <package> -r requirements.txt` Fixes #934 * Fix imports * Better combination of packages * Better help text for `inject -r` * Add unit test * Add changelog * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Rename changelog file * Fix pylint and mypy errors * Fix default for requirements * Use assignment operator since Python >= 3.8 * Update src/pipx/commands/inject.py Co-authored-by: chrysle <[email protected]> * Update src/pipx/main.py Co-authored-by: chrysle <[email protected]> * Update tests/test_inject.py Co-authored-by: chrysle <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update changelog.d/1252.feature.md Co-authored-by: chrysle <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update tests/test_inject.py Add comments to test file Co-authored-by: chrysle <[email protected]> * Update test_inject.py Add a blank line in inject-requirements file * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Logging at INFO level Also move after exception * Discard duplicated package specifications * Update 1252.feature.md * Update install-all command * Expand pipx inject example * Clarify changelog entry * Mention in main README * Check stdout and logs in test * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Better test for injected packages * Clarify ignoring of comments in example Co-authored-by: chrysle <[email protected]> * Clarify use of "requirement" file Co-authored-by: chrysle <[email protected]> * Update README.md Co-authored-by: chrysle <[email protected]> * Update README.md Co-authored-by: chrysle <[email protected]> * Check can inject each package independently * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix handling of tricky characters * Use logger where possible * More messages in logs * More debugging messages * Inject additional package that isn't already installed * Make inject order deterministic * Fix mypy error * tidy test_inject_single_package cases * Better comments on tests * Update 1252.feature.md Simplify news fragment * Update 1252.feature.md Fix new fragement * Update examples.md Be more explicit about the syntax for the "inject -r" files. * Fix examples.md * Update 1252.feature.md Fix markdown link * Update 1252.feature.md --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: chrysle <[email protected]> Co-authored-by: chrysle <[email protected]> Co-authored-by: Xuan (Sean) Hu <[email protected]>
pypa · May 14, 2024 · fca8a40 · fca8a40
1 parent 5589b69
commit fca8a40
Show file tree

Hide file tree

Showing 8 changed files with 190 additions and 39 deletions.
diff --git a/README.md b/README.md
@@ -246,6 +246,16 @@ If an application installed by pipx requires additional packages, you can add th
 pipx inject ipython matplotlib
 ```
 
+You can inject multiple packages by specifying them all on the command line,
+or by listing them in a text file, with one package per line,
+or a combination. For example:
+
+```
+pipx inject ipython matplotlib pandas
+# or:
+pipx inject ipython -r useful-packages.txt
+```
+
 ### Walkthrough: Running an Application in a Temporary Virtual Environment
 
 This is an alternative to `pipx install`.

diff --git a/changelog.d/1252.feature.md b/changelog.d/1252.feature.md
@@ -0,0 +1 @@
+Add `--requirement` option to `inject` command to read list of packages from a text file.
diff --git a/docs/examples.md b/docs/examples.md
@@ -98,13 +98,44 @@ Then you can run it as follows:
 One use of the inject command is setting up a REPL with some useful extra packages.
 
 ```
-pipx install ptpython
-pipx inject ptpython requests pendulum
+> pipx install ptpython
+> pipx inject ptpython requests pendulum
 ```
 
 After running the above commands, you will be able to import and use the `requests` and `pendulum` packages inside a
 `ptpython` repl.
 
+Equivalently, the extra packages can be listed in a text file (e.g. `useful-packages.txt`).
+Each line is a separate package specifier with the same syntax as the command line.
+Comments are supported with a `#` prefix.
+Hence, the syntax is a strict subset of the pip [requirements file format][pip-requirements] syntax.
+
+[pip-requirements]: https://pip.pypa.io/en/stable/reference/requirements-file-format/
+
+```
+# Additional packages
+requests
+
+pendulum # for easier datetimes
+```
+
+This file can then be given to `pipx inject` on the command line:
+
+```shell
+> pipx inject ptpython --requirement useful-packages.txt
+# or:
+> pipx inject ptpython -r useful-packages.txt
+```
+
+Note that these options can be repeated and used together, e.g.
+
+```
+> pipx inject ptpython package-1 -r extra-packages-1.txt -r extra-packages-2.txt package-2
+```
+
+If you require full pip functionality, then use the `runpip` command instead;
+however, the installed packages won't be recognised as "injected".
+
 ## `pipx list` example
 
 ```

diff --git a/src/pipx/commands/inject.py b/src/pipx/commands/inject.py
@@ -1,7 +1,9 @@
+import logging
 import os
+import re
 import sys
 from pathlib import Path
-from typing import List, Optional
+from typing import Generator, Iterable, List, Optional, Union
 
 from pipx import paths
 from pipx.colors import bold
@@ -11,6 +13,10 @@
 from pipx.util import PipxError, pipx_wrap
 from pipx.venv import Venv
 
+logger = logging.getLogger(__name__)
+
+COMMENT_RE = re.compile(r"(^|\s+)#.*$")
+
 
 def inject_dep(
     venv_dir: Path,
@@ -24,6 +30,8 @@ def inject_dep(
     force: bool,
     suffix: bool = False,
 ) -> bool:
+    logger.debug("Injecting package %s", package_spec)
+
     if not venv_dir.exists() or not next(venv_dir.iterdir()):
         raise PipxError(
             f"""
@@ -57,6 +65,7 @@ def inject_dep(
         )
 
     if not force and venv.has_package(package_name):
+        logger.info("Package %s has already been injected", package_name)
         print(
             pipx_wrap(
                 f"""
@@ -102,7 +111,8 @@ def inject_dep(
 def inject(
     venv_dir: Path,
     package_name: Optional[str],
-    package_specs: List[str],
+    package_specs: Iterable[str],
+    requirement_files: Iterable[str],
     pip_args: List[str],
     *,
     verbose: bool,
@@ -112,15 +122,28 @@ def inject(
     suffix: bool = False,
 ) -> ExitCode:
     """Returns pipx exit code."""
+    # Combined collection of package specifications
+    packages = list(package_specs)
+    for filename in requirement_files:
+        packages.extend(parse_requirements(filename))
+
+    # Remove duplicates and order deterministically
+    packages = sorted(set(packages))
+
+    if not packages:
+        raise PipxError("No packages have been specified.")
+    logger.info("Injecting packages: %r", packages)
+
+    # Inject packages
     if not include_apps and include_dependencies:
         include_apps = True
     all_success = True
-    for dep in package_specs:
+    for dep in packages:
         all_success &= inject_dep(
             venv_dir,
-            None,
-            dep,
-            pip_args,
+            package_name=None,
+            package_spec=dep,
+            pip_args=pip_args,
             verbose=verbose,
             include_apps=include_apps,
             include_dependencies=include_dependencies,
@@ -130,3 +153,17 @@ def inject(
 
     # Any failure to install will raise PipxError, otherwise success
     return EXIT_CODE_OK if all_success else EXIT_CODE_INJECT_ERROR
+
+
+def parse_requirements(filename: Union[str, os.PathLike]) -> Generator[str, None, None]:
+    """
+    Extract package specifications from requirements file.
+
+    Return all of the non-empty lines with comments removed.
+    """
+    # Based on https://github.com/pypa/pip/blob/main/src/pip/_internal/req/req_file.py
+    with open(filename) as f:
+        for line in f:
+            # Strip comments and filter empty lines
+            if pkgspec := COMMENT_RE.sub("", line).strip():
+                yield pkgspec
diff --git a/src/pipx/commands/install.py b/src/pipx/commands/install.py
@@ -215,10 +215,11 @@ def install_all(
             # 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,
+                    venv_dir=venv_dir,
+                    package_name=None,
+                    package_specs=[generate_package_spec(inject_package)],
+                    requirement_files=[],
+                    pip_args=pip_args,
                     verbose=verbose,
                     include_apps=inject_package.include_apps,
                     include_dependencies=inject_package.include_dependencies,

diff --git a/src/pipx/main.py b/src/pipx/main.py
@@ -297,6 +297,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar
             venv_dir,
             None,
             args.dependencies,
+            args.requirements,
             pip_args,
             verbose=verbose,
             include_apps=args.include_apps,
@@ -515,9 +516,22 @@ def _add_inject(subparsers, venv_completer: VenvCompleter, shared_parser: argpar
     ).completer = venv_completer
     p.add_argument(
         "dependencies",
-        nargs="+",
+        nargs="*",
         help="the packages to inject into the Virtual Environment--either package name or pip package spec",
     )
+    p.add_argument(
+        "-r",
+        "--requirement",
+        dest="requirements",
+        action="append",
+        default=[],
+        metavar="file",
+        help=(
+            "file containing the packages to inject into the Virtual Environment--"
+            "one package name or pip package spec per line. "
+            "May be specified multiple times."
+        ),
+    )
     p.add_argument(
         "--include-apps",
         action="store_true",

diff --git a/src/pipx/venv.py b/src/pipx/venv.py
@@ -159,6 +159,7 @@ def create_venv(self, venv_args: List[str], pip_args: List[str], override_shared
         """
         override_shared -- Override installing shared libraries to the pipx shared directory (default False)
         """
+        logger.info("Creating virtual environment")
         with animate("creating virtual environment", self.do_animation):
             cmd = [self.python, "-m", "venv"]
             if not override_shared:
@@ -213,11 +214,12 @@ def upgrade_packaging_libraries(self, pip_args: List[str]) -> None:
 
     def uninstall_package(self, package: str, was_injected: bool = False):
         try:
+            logger.info("Uninstalling %s", package)
             with animate(f"uninstalling {package}", self.do_animation):
                 cmd = ["uninstall", "-y"] + [package]
                 self._run_pip(cmd)
         except PipxError as e:
-            logging.info(e)
+            logger.info(e)
             raise PipxError(f"Error uninstalling {package}.") from None
 
         if was_injected:
@@ -240,10 +242,8 @@ def install_package(
         # check syntax and clean up spec and pip_args
         (package_or_url, pip_args) = parse_specifier_for_install(package_or_url, pip_args)
 
-        with animate(
-            f"installing {full_package_description(package_name, package_or_url)}",
-            self.do_animation,
-        ):
+        logger.info("Installing %s", package_descr := full_package_description(package_name, package_or_url))
+        with animate(f"installing {package_descr}", self.do_animation):
             # do not use -q with `pip install` so subprocess_post_check_pip_errors
             #   has more information to analyze in case of failure.
             cmd = [
@@ -287,7 +287,8 @@ def install_unmanaged_packages(self, requirements: List[str], pip_args: List[str
 
         # Note: We want to install everything at once, as that lets
         # pip resolve conflicts correctly.
-        with animate(f"installing {', '.join(requirements)}", self.do_animation):
+        logger.info("Installing %s", package_descr := ", ".join(requirements))
+        with animate(f"installing {package_descr}", self.do_animation):
             # do not use -q with `pip install` so subprocess_post_check_pip_errors
             #   has more information to analyze in case of failure.
             cmd = [
@@ -428,10 +429,8 @@ def has_package(self, package_name: str) -> bool:
         return bool(list(Distribution.discover(name=package_name, path=[str(get_site_packages(self.python_path))])))
 
     def upgrade_package_no_metadata(self, package_name: str, pip_args: List[str]) -> None:
-        with animate(
-            f"upgrading {full_package_description(package_name, package_name)}",
-            self.do_animation,
-        ):
+        logger.info("Upgrading %s", package_descr := full_package_description(package_name, package_name))
+        with animate(f"upgrading {package_descr}", self.do_animation):
             pip_process = self._run_pip(["--no-input", "install"] + pip_args + ["--upgrade", package_name])
         subprocess_post_check(pip_process)
 
@@ -445,10 +444,8 @@ def upgrade_package(
         is_main_package: bool,
         suffix: str = "",
     ) -> None:
-        with animate(
-            f"upgrading {full_package_description(package_name, package_or_url)}",
-            self.do_animation,
-        ):
+        logger.info("Upgrading %s", package_descr := full_package_description(package_name, package_or_url))
+        with animate(f"upgrading {package_descr}", self.do_animation):
             pip_process = self._run_pip(["--no-input", "install"] + pip_args + ["--upgrade", package_or_url])
         subprocess_post_check(pip_process)
 

diff --git a/tests/test_inject.py b/tests/test_inject.py
@@ -1,12 +1,36 @@
+import logging
+import re
+import textwrap
+
 import pytest  # type: ignore
 
 from helpers import PIPX_METADATA_LEGACY_VERSIONS, mock_legacy_venv, run_pipx_cli, skip_if_windows
 from package_info import PKG
 
 
-def test_inject_simple(pipx_temp_env, capsys):
+# Note that this also checks that packages used in other tests can be injected individually
+@pytest.mark.parametrize(
+    "pkg_spec,",
+    [
+        PKG["black"]["spec"],
+        PKG["nox"]["spec"],
+        PKG["pylint"]["spec"],
+        PKG["ipython"]["spec"],
+        "jaraco.clipboard==2.0.1",  # tricky character
+    ],
+)
+def test_inject_single_package(pipx_temp_env, capsys, caplog, pkg_spec):
     assert not run_pipx_cli(["install", "pycowsay"])
-    assert not run_pipx_cli(["inject", "pycowsay", PKG["black"]["spec"]])
+    assert not run_pipx_cli(["inject", "pycowsay", pkg_spec])
+
+    # Check arguments have been parsed correctly
+    assert f"Injecting packages: {[pkg_spec]!r}" in caplog.text
+
+    # Check it's actually being installed and into correct venv
+    captured = capsys.readouterr()
+    injected = re.findall(r"injected package (.+?) into venv pycowsay", captured.out)
+    pkg_name = pkg_spec.split("=", 1)[0].replace(".", "-")  # assuming spec is always of the form <name>==<version>
+    assert set(injected) == {pkg_name}
 
 
 @skip_if_windows
@@ -27,16 +51,6 @@ def test_inject_simple_legacy_venv(pipx_temp_env, capsys, metadata_version):
         assert "Please uninstall and install" in capsys.readouterr().err
 
 
-def test_inject_tricky_character(pipx_temp_env, capsys):
-    assert not run_pipx_cli(["install", "pycowsay"])
-    assert not run_pipx_cli(["inject", "pycowsay", "jaraco.clipboard==2.0.1"])
-
-
-def test_spec(pipx_temp_env, capsys):
-    assert not run_pipx_cli(["install", "pycowsay"])
-    assert not run_pipx_cli(["inject", "pycowsay", "pylint==3.0.4"])
-
-
 @pytest.mark.parametrize("with_suffix,", [(False,), (True,)])
 def test_inject_include_apps(pipx_temp_env, capsys, with_suffix):
     install_args = []
@@ -53,3 +67,49 @@ def test_inject_include_apps(pipx_temp_env, capsys, with_suffix):
         assert run_pipx_cli(["inject", "pycowsay", PKG["black"]["spec"], "--include-deps"])
 
     assert not run_pipx_cli(["inject", f"pycowsay{suffix}", PKG["black"]["spec"], "--include-deps"])
+
+
+@pytest.mark.parametrize(
+    "with_packages,",
+    [
+        (),  # no extra packages
+        ("black",),  # duplicate from requirements file
+        ("ipython",),  # additional package
+    ],
+)
+def test_inject_with_req_file(pipx_temp_env, capsys, caplog, tmp_path, with_packages):
+    caplog.set_level(logging.INFO)
+
+    req_file = tmp_path / "inject-requirements.txt"
+    req_file.write_text(
+        textwrap.dedent(
+            f"""
+                {PKG["black"]["spec"]} # a comment inline
+                {PKG["nox"]["spec"]}
+
+                {PKG["pylint"]["spec"]}
+                # comment on separate line
+            """
+        ).strip()
+    )
+    assert not run_pipx_cli(["install", "pycowsay"])
+
+    assert not run_pipx_cli(
+        ["inject", "pycowsay", *(PKG[pkg]["spec"] for pkg in with_packages), "--requirement", str(req_file)]
+    )
+
+    packages = [
+        ("black", PKG["black"]["spec"]),
+        ("nox", PKG["nox"]["spec"]),
+        ("pylint", PKG["pylint"]["spec"]),
+    ]
+    packages.extend((pkg, PKG[pkg]["spec"]) for pkg in with_packages)
+    packages = sorted(set(packages))
+
+    # Check arguments and files have been parsed correctly
+    assert f"Injecting packages: {[p for _, p in packages]!r}" in caplog.text
+
+    # Check they're actually being installed and into correct venv
+    captured = capsys.readouterr()
+    injected = re.findall(r"injected package (.+?) into venv pycowsay", captured.out)
+    assert set(injected) == {pkg for pkg, _ in packages}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Add `--requirement` option to `inject` command to read list of packages from a text file.