Fix optional dependency installation (#322)

* Fix optional dependency installation

* Additional test

* Type fix

* Update join

Author: cidrblock

.config/constraints.txt

@@ -5,11 +5,12 @@
 #    pip-compile --all-extras --no-annotate --output-file=.config/constraints.txt --strip-extras .config/requirements.in pyproject.toml
 #
 ansible-builder==3.1.0
+argcomplete==3.6.2
 astroid==3.3.9
 attrs==25.3.0
 babel==2.17.0
 backrefs==5.8
-beautifulsoup4==4.13.3
+beautifulsoup4==4.13.4
 bindep==2.13.0
 build==1.2.2.post1
 cachetools==5.5.2
@@ -26,7 +27,7 @@ coverage==7.8.0
 csscompressor==0.9.5
 cssselect2==0.8.0
 defusedxml==0.7.1
-dill==0.3.9
+dill==0.4.0
 distlib==0.3.9
 distro==1.9.0
 dnspython==2.7.0
@@ -60,7 +61,7 @@ mkdocs-gen-files==0.5.0
 mkdocs-get-deps==0.2.0
 mkdocs-htmlproofer-plugin==1.3.0
 mkdocs-macros-plugin==1.3.7
-mkdocs-material==9.6.11
+mkdocs-material==9.6.12
 mkdocs-material-extensions==1.3.1
 mkdocs-minify-plugin==0.8.0
 mkdocs-monorepo-plugin==1.1.0
@@ -80,7 +81,7 @@ platformdirs==4.3.7
 pluggy==1.5.0
 pre-commit==4.2.0
 pycparser==2.22
-pydoclint==0.6.5
+pydoclint==0.6.6
 pygments==2.19.1
 pylint==3.3.6
 pymdown-extensions==10.14.3

.config/pydoclint-baseline.txt

@@ -5,6 +5,3 @@ tests/unit/test_treemaker.py
     DOC101: Method `SafeEnvBuilder.__init__`: Docstring contains fewer arguments than in function signature.
     DOC103: Method `SafeEnvBuilder.__init__`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [clear: bool, prompt: str | None, symlinks: bool, system_site_packages: bool, upgrade: bool, upgrade_deps: bool, with_pip: bool].
 --------------------
-tests/unit/test_utils.py
-    DOC502: Function `test_builder_found` has a "Raises" section in the docstring, but there are not "raise" statements in the body
---------------------

.pre-commit-config.yaml

@@ -53,7 +53,7 @@ repos:
       - id: tox-ini-fmt
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.11.4
+    rev: v0.11.5
     hooks:
       - id: ruff
         entry: sh -c 'ruff check --fix --force-exclude && ruff format --force-exclude'
@@ -66,7 +66,7 @@ repos:
         name: Spell check with cspell
 
   - repo: https://github.com/jsh9/pydoclint
-    rev: "0.6.5"
+    rev: "0.6.6"
     hooks:
       - id: pydoclint
         # This allows automatic reduction of the baseline file when needed.

src/ansible_dev_environment/subcommands/installer.py

@@ -16,6 +16,7 @@
 from ansible_dev_environment.utils import (
     builder_introspect,
     collections_from_requirements,
+    opt_deps_to_files,
     oxford_join,
     subprocess_run,
 )
@@ -77,6 +78,8 @@ def run(self) -> None:
 
         if self._config.args.requirement or self._config.args.cpi:
             self._install_galaxy_requirements()
+
+        opt_dep_paths = None
         if self._config.args.collection_specifier:
             collections = [
                 parse_collection_request(
@@ -98,7 +101,15 @@ def run(self) -> None:
                     self._output.critical(msg)
                 self._install_galaxy_collections(collections=distant_collections)
 
-        builder_introspect(config=self._config, output=self._output)
+            opt_dep_paths = [
+                path
+                for collection in collections
+                for path in opt_deps_to_files(collection, self._output)
+            ]
+            msg = f"Optional dependencies found: {oxford_join(opt_dep_paths)}"
+            self._output.info(msg)
+
+        builder_introspect(config=self._config, opt_dep_paths=opt_dep_paths, output=self._output)
         self._pip_install()
         Checker(config=self._config, output=self._output).system_deps()
 

src/ansible_dev_environment/utils.py

@@ -18,12 +18,15 @@
 
 
 if TYPE_CHECKING:
+    from collections.abc import Sequence
     from pathlib import Path
     from types import TracebackType
 
+    from .collection import Collection
     from .config import Config
     from .output import Output
 
+
 from typing import Any
 
 
@@ -156,50 +159,60 @@ def subprocess_run(  # pylint: disable=too-many-positional-arguments
         )
 
 
-def oxford_join(words: list[str]) -> str:
+def oxford_join(words: Sequence[str | Path]) -> str:
     """Join a list of words with commas and an oxford comma.
 
     Args:
         words: A list of words to join
     Returns:
         A string of words joined with commas and an oxford comma
     """
-    words.sort()
-    if not words:
-        return ""
-    if len(words) == 1:
-        return words[0]
-    if len(words) == 2:  # noqa: PLR2004
-        return " and ".join(words)
-    return ", ".join(words[:-1]) + ", and " + words[-1]
-
-
-def opt_deps_to_files(collection_path: Path, dep_str: str) -> list[Path]:
+    _words = sorted([str(word) for word in words])
+    match _words:
+        case [word]:
+            return word
+        case [word_one, word_two]:
+            return f"{word_one} and {word_two}"
+        case [*first_words, last_word]:
+            return f"{', '.join(first_words)}, and {last_word}"
+        case _:
+            return ""
+
+
+def opt_deps_to_files(collection: Collection, output: Output) -> list[Path]:
     """Convert a string of optional dependencies to a list of files.
 
     Args:
-        collection_path: The path to the collection
-        dep_str: A string of optional dependencies
+        collection: The collection object
+        output: The output object
     Returns:
-        A list of files
+        A list of paths
     """
-    deps = dep_str.split(",")
+    if not collection.opt_deps:
+        msg = "No optional dependencies specified."
+        output.debug(msg)
+        return []
+
+    deps = collection.opt_deps.split(",")
     files = []
     for dep in deps:
         _dep = dep.strip()
-        variant1 = collection_path / f"{_dep}-requirements.txt"
+        variant1 = collection.path / f"{_dep}-requirements.txt"
         if variant1.exists():
             files.append(variant1)
             continue
-        variant2 = collection_path / f"requirements-{_dep}.txt"
+        variant2 = collection.path / f"requirements-{_dep}.txt"
         if variant2.exists():
             files.append(variant2)
             continue
         msg = (
             f"Failed to find optional dependency file for '{_dep}'."
             f" Checked for '{variant1.name}' and '{variant2.name}'. Skipping."
         )
-        logger.error(msg)
+        output.error(msg)
+    count = len(files)
+    msg = f"Found {count} optional dependency file{'s' * (count > 1)}. {oxford_join(files)}"
+    output.debug(msg)
     return files
 
 
@@ -277,7 +290,11 @@ def collect_manifests(  # noqa: C901
     return sort_dict(collections)
 
 
-def builder_introspect(config: Config, output: Output) -> None:
+def builder_introspect(
+    config: Config,
+    output: Output,
+    opt_dep_paths: list[Path] | None = None,
+) -> None:
     """Introspect a collection.
 
     Use the sys executable to run builder, since it is a direct dependency
@@ -286,29 +303,20 @@ def builder_introspect(config: Config, output: Output) -> None:
     Args:
         config: The configuration object.
         output: The output object.
+        opt_dep_paths: A list of optional dependency paths.
     """
     command = (
         f"{sys.executable} -m ansible_builder introspect {config.site_pkg_path}"
         f" --write-pip {config.discovered_python_reqs}"
         f" --write-bindep {config.discovered_bindep_reqs}"
         " --sanitize"
     )
-    if (
-        hasattr(config.args, "collection_specifier")
-        and hasattr(config, "collection")
-        and config.collection.opt_deps
-        and config.collection.path
-    ):
-        dep_paths = opt_deps_to_files(
-            collection_path=config.collection.path,
-            dep_str=config.collection.opt_deps,
-        )
-        for dep_path in dep_paths:
-            command += f" --user-pip {dep_path}"
+    for opt_dep_path in opt_dep_paths or []:
+        command += f" --user-pip {opt_dep_path}"
     msg = f"Writing discovered python requirements to: {config.discovered_python_reqs}"
-    logger.debug(msg)
+    output.debug(msg)
     msg = f"Writing discovered system requirements to: {config.discovered_bindep_reqs}"
-    logger.debug(msg)
+    output.debug(msg)
     work = "Persisting requirements to file system"
     try:
         subprocess_run(

tests/integration/test_basic.py

@@ -26,6 +26,8 @@ def test_venv(
 ) -> None:
     """Basic smoke test.
 
+    Test for a local collection install with optional dependencies
+
     Args:
         capsys: Capture stdout and stderr
         tmp_path: Temporary directory
@@ -56,17 +58,19 @@ def test_venv(
         [
             "ade",
             "install",
-            str(tmp_path / "cisco.nxos"),
+            str(tmp_path / "cisco.nxos[test]"),
             "--venv=venv",
             "--no-ansi",
+            "-vvv",
         ],
     )
     with pytest.raises(SystemExit):
         main()
-    string = "Installed collections include: ansible.netcommon, ansible.utils,"
-    captured = capsys.readouterr()
 
-    assert string in captured.out
+    captured = capsys.readouterr()
+    assert "Installed collections include: ansible.netcommon, ansible.utils," in captured.out
+    assert "Optional dependencies found" in captured.out
+    assert "'pytest-xdist  # from collection user'" in captured.out
 
     monkeypatch.setattr(
         "sys.argv",

tests/unit/test_utils.py

@@ -13,7 +13,12 @@
 )
 from ansible_dev_environment.config import Config
 from ansible_dev_environment.output import Output
-from ansible_dev_environment.utils import TermFeatures, builder_introspect, str_to_bool
+from ansible_dev_environment.utils import (
+    TermFeatures,
+    builder_introspect,
+    opt_deps_to_files,
+    str_to_bool,
+)
 
 
 term_features = TermFeatures(color=False, links=False)
@@ -137,8 +142,6 @@ def test_builder_found(
         monkeypatch: The pytest Monkeypatch fixture
         session_venv: The session venv
 
-    Raises:
-        AssertionError: if either file is not found
     """
 
     @property  # type: ignore[misc]
@@ -196,3 +199,37 @@ def test_str_to_bool() -> None:
     assert str_to_bool("off") is False
 
     assert str_to_bool("anything else") is None
+
+
+def test_opt_deps_to_files(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None:
+    """Test the opt_deps_to_files function.
+
+    Args:
+        tmp_path: A temporary path
+        capsys: The pytest LogCaptureFixture
+    """
+    # Create a temporary file with some content
+    f1 = tmp_path / "test-requirements.txt"
+    f1.touch()
+    f2 = tmp_path / "requirements-dev.txt"
+    f2.touch()
+
+    collection = Collection(
+        config=config,
+        cname="cname",
+        cnamespace="cnamespace",
+        local=True,
+        path=tmp_path,
+        specifier="",
+        original=str(tmp_path),
+        opt_deps="test,dev,foo",
+        csource=[],
+    )
+
+    result = opt_deps_to_files(collection, output)
+
+    captured = capsys.readouterr()
+
+    assert result[0] == f1
+    assert result[1] == f2
+    assert "Error: Failed to find optional dependency file for 'foo'." in captured.err