Change cache_dir location to prefer venv and project_directory (#439)

Author: ssbarnea

.config/pydoclint-baseline.txt

@@ -51,11 +51,6 @@ src/ansible_compat/loaders.py
     DOC501: Function `colpath_from_path` has "raise" statements, but the docstring does not have a "Raises" section
     DOC503: Function `colpath_from_path` exceptions in the "Raises" section in the docstring do not match those in the function body Raises values in the docstring: []. Raised exceptions in the body: ['InvalidPrerequisiteError'].
 --------------------
-src/ansible_compat/prerun.py
-    DOC101: Function `get_cache_dir`: Docstring contains fewer arguments than in function signature.
-    DOC103: Function `get_cache_dir`: 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: [project_dir: Path].
-    DOC201: Function `get_cache_dir` does not have a return section in docstring
---------------------
 src/ansible_compat/runtime.py
     DOC601: Class `Collection`: Class docstring contains fewer class attributes than actual class attributes.  (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
     DOC603: Class `Collection`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [name: str, path: Path, version: str]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
@@ -69,7 +64,7 @@ src/ansible_compat/runtime.py
     DOC501: Method `Plugins.__getattribute__` has "raise" statements, but the docstring does not have a "Raises" section
     DOC503: Method `Plugins.__getattribute__` exceptions in the "Raises" section in the docstring do not match those in the function body Raises values in the docstring: []. Raised exceptions in the body: ['AnsibleCompatError'].
     DOC601: Class `Runtime`: Class docstring contains fewer class attributes than actual class attributes.  (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
-    DOC603: Class `Runtime`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [_has_playbook_cache: dict[tuple[str, Path | None], bool], _version: Version | None, cache_dir: Path | None, collections: OrderedDict[str, Collection], initialized: bool, plugins: Plugins, require_module: bool]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
+    DOC603: Class `Runtime`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [_has_playbook_cache: dict[tuple[str, Path | None], bool], _version: Version | None, cache_dir: Path, collections: OrderedDict[str, Collection], initialized: bool, plugins: Plugins, require_module: bool]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
     DOC101: Method `Runtime.__init__`: Docstring contains fewer arguments than in function signature.
     DOC103: Method `Runtime.__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: [environ: dict[str, str] | None, isolated: bool, max_retries: int, min_required_version: str | None, project_dir: Path | None, require_module: bool, verbosity: int].
     DOC501: Method `Runtime.__init__` has "raise" statements, but the docstring does not have a "Raises" section
@@ -110,8 +105,6 @@ src/ansible_compat/runtime.py
     DOC501: Method `Runtime._prepare_ansible_paths` has "raise" statements, but the docstring does not have a "Raises" section
     DOC503: Method `Runtime._prepare_ansible_paths` exceptions in the "Raises" section in the docstring do not match those in the function body Raises values in the docstring: []. Raised exceptions in the body: ['RuntimeError'].
     DOC201: Method `Runtime._get_roles_path` does not have a return section in docstring
-    DOC101: Method `Runtime._install_galaxy_role`: Docstring contains fewer arguments than in function signature.
-    DOC103: Method `Runtime._install_galaxy_role`: 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: [project_dir: Path].
     DOC501: Method `Runtime._install_galaxy_role` has "raise" statements, but the docstring does not have a "Raises" section
     DOC503: Method `Runtime._install_galaxy_role` exceptions in the "Raises" section in the docstring do not match those in the function body Raises values in the docstring: []. Raised exceptions in the body: ['InvalidPrerequisiteError'].
     DOC101: Method `Runtime._update_env`: Docstring contains fewer arguments than in function signature.
@@ -285,13 +278,6 @@ test/test_runtime.py
     DOC101: Function `test_prepare_environment_symlink`: Docstring contains fewer arguments than in function signature.
     DOC103: Function `test_prepare_environment_symlink`: 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: [caplog: pytest.LogCaptureFixture, dest: str | Path, message: str].
 --------------------
-test/test_runtime_scan_path.py
-    DOC601: Class `ScanSysPath`: Class docstring contains fewer class attributes than actual class attributes.  (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
-    DOC603: Class `ScanSysPath`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [raises_not_found: bool, scan: bool]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
-    DOC201: Method `ScanSysPath.__str__` does not have a return section in docstring
-    DOC101: Function `test_scan_sys_path`: Docstring contains fewer arguments than in function signature.
-    DOC103: Function `test_scan_sys_path`: 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: [monkeypatch: MonkeyPatch, param: ScanSysPath, runtime_tmp: Runtime, tmp_path: Path, venv_module: VirtualEnvironment].
---------------------
 test/test_schema.py
     DOC101: Function `json_from_asset`: Docstring contains fewer arguments than in function signature.
     DOC103: Function `json_from_asset`: 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: [file_name: str].

.taplo.toml

@@ -0,0 +1,9 @@
+[formatting]
+# compatibility between toml-sort-fix pre-commit hook and panekj.even-betterer-toml extension
+align_comments = false
+array_trailing_comma = false
+compact_arrays = true
+compact_entries = false
+compact_inline_tables = true
+inline_table_expand = false
+reorder_keys = true

.vscode/settings.json

@@ -1,4 +1,7 @@
 {
+  "[json]": {
+    "editor.defaultFormatter": "esbenp.prettier-vscode"
+  },
   "[jsonc]": {
     "editor.defaultFormatter": "esbenp.prettier-vscode"
   },
@@ -14,8 +17,6 @@
     "editor.formatOnSave": true
   },
   "editor.formatOnSave": true,
-  "evenBetterToml.formatter.alignComments": false,
-  "evenBetterToml.formatter.allowedBlankLines": 2,
   "files.exclude": {
     "*.egg-info": true,
     ".pytest_cache": true,
@@ -37,8 +38,14 @@
   "python.testing.pytestArgs": ["tests"],
   "python.testing.pytestEnabled": true,
   "python.testing.unittestEnabled": false,
+  "sortLines.filterBlankLines": true,
   "yaml.completion": true,
   "yaml.customTags": ["!encrypted/pkcs1-oaep scalar", "!vault scalar"],
   "yaml.format.enable": false,
-  "yaml.validate": true
+  "yaml.validate": true,
+  "evenBetterToml.formatter.alignComments": false,
+  "evenBetterToml.formatter.arrayTrailingComma": true,
+  "[toml]": {
+    "editor.defaultFormatter": "panekj.even-betterer-toml"
+  }
 }

pyproject.toml

@@ -397,7 +397,16 @@ known-first-party = ["ansible_compat"]
 known-third-party = ["packaging"]
 
 [tool.ruff.lint.per-file-ignores]
-"test/**/*.py" = ["DOC402", "DOC501", "SLF001", "S101", "S404", "FBT001", "PLC2701"]
+"test/**/*.py" = [
+  "DOC402",
+  "DOC501",
+  "FBT001",
+  "PLC2701",
+  "PLR0917",
+  "S101",
+  "S404",
+  "SLF001"
+]
 
 [tool.ruff.lint.pydocstyle]
 convention = "google"
@@ -430,4 +439,10 @@ sort_table_keys = true
 [tool.uv.pip]
 annotation-style = "line"
 custom-compile-command = "tox run deps"
-no-emit-package = ["ansible-core", "pip", "resolvelib", "typing_extensions", "uv"]
+no-emit-package = [
+  "ansible-core",
+  "pip",
+  "resolvelib",
+  "typing_extensions",
+  "uv"
+]

src/ansible_compat/prerun.py

@@ -1,22 +1,29 @@
 """Utilities for configuring ansible runtime environment."""
 
-import hashlib
 import os
 from pathlib import Path
 
 
-def get_cache_dir(project_dir: Path) -> Path:
-    """Compute cache directory to be used based on project path."""
-    # we only use the basename instead of the full path in order to ensure that
-    # we would use the same key regardless the location of the user home
-    # directory or where the project is clones (as long the project folder uses
-    # the same name).
-    basename = project_dir.resolve().name.encode(encoding="utf-8")
-    # 6 chars of entropy should be enough
-    cache_key = hashlib.sha256(basename).hexdigest()[:6]
-    cache_dir = (
-        Path(os.getenv("XDG_CACHE_HOME", "~/.cache")).expanduser()
-        / "ansible-compat"
-        / cache_key
-    )
+def get_cache_dir(project_dir: Path, *, isolated: bool = True) -> Path:
+    """Compute cache directory to be used based on project path.
+
+    Args:
+        project_dir: Path to the project directory.
+        isolated: Whether to use isolated cache directory.
+
+    Returns:
+        Cache directory path.
+    """
+    if "VIRTUAL_ENV" in os.environ:
+        cache_dir = Path(os.environ["VIRTUAL_ENV"]) / ".ansible"
+    elif isolated:
+        cache_dir = project_dir / ".ansible"
+    else:
+        cache_dir = Path(os.environ.get("ANSIBLE_HOME", "~/.ansible")).expanduser()
+
+    # Ensure basic folder structure exists so `ansible-galaxy list` does not
+    # fail with: None of the provided paths were usable. Please specify a valid path with
+    for name in ("roles", "collections"):  # pragma: no cover
+        (cache_dir / name).mkdir(parents=True, exist_ok=True)
+
     return cache_dir

src/ansible_compat/runtime.py

@@ -150,7 +150,7 @@ class Runtime:
 
     _version: Version | None = None
     collections: OrderedDict[str, Collection] = OrderedDict()
-    cache_dir: Path | None = None
+    cache_dir: Path
     # Used to track if we have already initialized the Ansible runtime as attempts
     # to do it multiple tilmes will cause runtime warnings from within ansible-core
     initialized: bool = False
@@ -209,8 +209,8 @@ def __init__(
         if "PYTHONWARNINGS" not in self.environ:  # pragma: no cover
             self.environ["PYTHONWARNINGS"] = "ignore:Blowfish has been deprecated"
 
-        if isolated:
-            self.cache_dir = get_cache_dir(self.project_dir)
+        self.cache_dir = get_cache_dir(self.project_dir, isolated=self.isolated)
+
         self.config = AnsibleConfig(cache_dir=self.cache_dir)
 
         # Add the sys.path to the collection paths if not isolated
@@ -356,8 +356,7 @@ def _ensure_module_available(self) -> None:
 
     def clean(self) -> None:
         """Remove content of cache_dir."""
-        if self.cache_dir:
-            shutil.rmtree(self.cache_dir, ignore_errors=True)
+        shutil.rmtree(self.cache_dir, ignore_errors=True)
 
     def run(  # ruff: disable=PLR0913
         self,
@@ -567,8 +566,7 @@ def install_requirements(  # noqa: C901
             ]
             if self.verbosity > 0:
                 cmd.extend(["-" + ("v" * self.verbosity)])
-            if self.cache_dir:
-                cmd.extend(["--roles-path", f"{self.cache_dir}/roles"])
+            cmd.extend(["--roles-path", f"{self.cache_dir}/roles"])
 
             if offline:
                 _logger.warning(
@@ -668,8 +666,7 @@ def prepare_environment(  # noqa: C901
                             destination=destination,
                         )
 
-        if self.cache_dir:
-            destination = self.cache_dir / "collections"
+        destination = self.cache_dir / "collections"
         for name, min_version in required_collections.items():
             self.install_collection(
                 f"{name}:>={min_version}",
@@ -837,10 +834,7 @@ def _get_roles_path(self) -> Path:
         not mentioned or set to `False`, it returns the first path in
         `default_roles_path`.
         """
-        if self.cache_dir:
-            path = Path(f"{self.cache_dir}/roles")
-        else:
-            path = Path(self.config.default_roles_path[0]).expanduser()
+        path = Path(f"{self.cache_dir}/roles")
         return path
 
     def _install_galaxy_role(

test/test_prerun.py

@@ -1,6 +1,12 @@
 """Tests for ansible_compat.prerun module."""
 
+from __future__ import annotations
+
 from pathlib import Path
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from _pytest.monkeypatch import MonkeyPatch
 
 from ansible_compat.prerun import get_cache_dir
 
@@ -10,3 +16,25 @@ def test_get_cache_dir_relative() -> None:
     relative_path = Path()
     abs_path = relative_path.resolve()
     assert get_cache_dir(relative_path) == get_cache_dir(abs_path)
+
+
+def test_get_cache_dir_no_isolation_no_venv(monkeypatch: MonkeyPatch) -> None:
+    """Test behaviors of get_cache_dir.
+
+    Args:
+        monkeypatch: Pytest fixture for monkeypatching
+    """
+    monkeypatch.delenv("VIRTUAL_ENV", raising=False)
+    monkeypatch.delenv("ANSIBLE_HOME", raising=False)
+    assert get_cache_dir(Path(), isolated=False) == Path("~/.ansible").expanduser()
+
+
+def test_get_cache_dir_isolation_no_venv(monkeypatch: MonkeyPatch) -> None:
+    """Test behaviors of get_cache_dir.
+
+    Args:
+        monkeypatch: Pytest fixture for monkeypatching
+    """
+    monkeypatch.delenv("VIRTUAL_ENV", raising=False)
+    monkeypatch.delenv("ANSIBLE_HOME", raising=False)
+    assert get_cache_dir(Path(), isolated=True) == Path() / ".ansible"

test/test_runtime.py

@@ -175,11 +175,6 @@ def test_runtime_install_role(
             f"{Path(runtime.config.default_roles_path[0]).expanduser()}/{role_name}",
         ).is_symlink()
     runtime.clean()
-    # also test that clean does not break when cache_dir is missing
-    tmp_dir = runtime.cache_dir
-    runtime.cache_dir = None
-    runtime.clean()
-    runtime.cache_dir = tmp_dir
 
 
 def test_prepare_environment_with_collections(runtime_tmp: Runtime) -> None:
@@ -654,10 +649,9 @@ def test_upgrade_collection(runtime_tmp: Runtime) -> None:
     runtime_tmp.require_collection("community.molecule", "0.1.0")
 
 
-def test_require_collection_no_cache_dir() -> None:
+def test_require_collection_not_isolated() -> None:
     """Check require_collection without a cache directory."""
-    runtime = Runtime()
-    assert not runtime.cache_dir
+    runtime = Runtime(isolated=False)
     runtime.require_collection("community.molecule", "0.1.0", install=True)
 
 
@@ -1024,6 +1018,11 @@ def test_runtime_has_playbook() -> None:
     """Tests has_playbook method."""
     runtime = Runtime(require_module=True)
 
+    runtime.prepare_environment(
+        required_collections={"community.molecule": "0.1.0"},
+        install_local=True,
+    )
+
     assert not runtime.has_playbook("this-does-not-exist.yml")
     # call twice to ensure cache is used:
     assert not runtime.has_playbook("this-does-not-exist.yml")

test/test_runtime_scan_path.py

@@ -2,7 +2,6 @@
 
 import json
 import textwrap
-from dataclasses import dataclass, fields
 from pathlib import Path
 
 import pytest
@@ -19,26 +18,11 @@
 V2_COLLECTION_FULL_NAME = f"{V2_COLLECTION_NAMESPACE}.{V2_COLLECTION_NAME}"
 
 
-@dataclass
-class ScanSysPath:
-    """Parameters for scan tests."""
-
-    scan: bool
-    raises_not_found: bool
-
-    def __str__(self) -> str:
-        """Return a string representation of the object."""
-        parts = [
-            f"{field.name}{str(getattr(self, field.name))[0]}" for field in fields(self)
-        ]
-        return "-".join(parts)
-
-
 @pytest.mark.parametrize(
-    ("param"),
+    ("scan", "raises_not_found"),
     (
-        ScanSysPath(scan=False, raises_not_found=True),
-        ScanSysPath(scan=True, raises_not_found=False),
+        pytest.param(False, True, id="0"),
+        pytest.param(True, False, id="1"),
     ),
     ids=str,
 )
@@ -47,16 +31,23 @@ def test_scan_sys_path(
     monkeypatch: MonkeyPatch,
     runtime_tmp: Runtime,
     tmp_path: Path,
-    param: ScanSysPath,
+    scan: bool,
+    raises_not_found: bool,
 ) -> None:
     """Confirm sys path is scanned for collections.
 
-    :param venv_module: Fixture for a virtual environment
-    :param monkeypatch: Fixture for monkeypatching
-    :param runtime_tmp: Fixture for a Runtime object
-    :param tmp_dir: Fixture for a temporary directory
-    :param param: The parameters for the test
+    Args:
+        venv_module: Fixture for a virtual environment
+        monkeypatch: Fixture for monkeypatching
+        runtime_tmp: Fixture for a Runtime object
+        tmp_path: Fixture for a temporary directory
+        scan: Whether to scan the sys path
+        raises_not_found: Whether the collection is expected to be found
     """
+    # Isolated the test from the others, so ansible will not find collections
+    # that might be installed by other tests.
+    monkeypatch.setenv("VIRTUAL_ENV", venv_module.project.as_posix())
+    monkeypatch.setenv("ANSIBLE_HOME", tmp_path.as_posix())
     first_site_package_dir = venv_module.site_package_dirs()[0]
 
     installed_to = (
@@ -76,7 +67,7 @@ def test_scan_sys_path(
     # Confirm the collection is installed
     assert installed_to.exists()
     # Set the sys scan path environment variable
-    monkeypatch.setenv("ANSIBLE_COLLECTIONS_SCAN_SYS_PATH", str(param.scan))
+    monkeypatch.setenv("ANSIBLE_COLLECTIONS_SCAN_SYS_PATH", str(scan))
     # Set the ansible collections paths to avoid bleed from other tests
     monkeypatch.setenv("ANSIBLE_COLLECTIONS_PATH", str(tmp_path))
 
@@ -91,7 +82,7 @@ def test_scan_sys_path(
     )
 
     proc = venv_module.python_script_run(script)
-    if param.raises_not_found:
+    if raises_not_found:
         assert proc.returncode != 0, (proc.stdout, proc.stderr)
         assert "InvalidPrerequisiteError" in proc.stderr
         assert "'community.molecule' not found" in proc.stderr

tox.ini

@@ -74,6 +74,7 @@ passenv =
   LANG
   LC_*
 setenv =
+  ANSIBLE_HOME = {envdir}/.ansible
   ANSIBLE_DEVEL_WARNING='false'
   COVERAGE_FILE = {env:COVERAGE_FILE:{envdir}/.coverage.{envname}}
   COVERAGE_PROCESS_START={toxinidir}/pyproject.toml