Change install_collection to avoid use of -p (#233)

As passing -p (target collection path destination) break ansible-galaxy
ability to find existing collection in other location, we avoid it
and use the environment variable approach instead.

Related: ansible/ansible-lint#3251

Author: ssbarnea

src/ansible_compat/runtime.py

@@ -253,14 +253,19 @@ def install_collection(
         if matches and Version(matches[1]).is_prerelease:
             cmd.append("--pre")
 
-        if destination:
-            cmd.extend(["-p", str(destination)])
+        cpaths: List[str] = self.config.collections_paths
+        if destination and str(destination) not in cpaths:
+            # we cannot use '-p' because it breaks galaxy ability to ignore already installed collections, so
+            # we hack ansible_collections_path instead and inject our own path there.
+            # pylint: disable=no-member
+            cpaths.insert(0, str(destination))
         cmd.append(f"{collection}")
 
         _logger.info("Running from %s : %s", os.getcwd(), " ".join(cmd))
         run = self.exec(
             cmd,
             retry=True,
+            env={**self.environ, ansible_collections_path(): ":".join(cpaths)},
         )
         if run.returncode != 0:
             msg = f"Command returned {run.returncode} code:\n{run.stdout}\n{run.stderr}"
@@ -351,15 +356,14 @@ def install_requirements(
                 )
             else:
                 cmd.extend(["-r", requirement])
-                cpaths = ansible_collections_path().split(":")
+                cpaths = self.config.collections_paths
                 if self.cache_dir:
                     # we cannot use '-p' because it breaks galaxy ability to ignore already installed collections, so
                     # we hack ansible_collections_path instead and inject our own path there.
                     dest_path = f"{self.cache_dir}/collections"
-                    cpaths = ansible_collections_path().split(":")
                     if dest_path not in cpaths:
+                        # pylint: disable=no-member
                         cpaths.insert(0, dest_path)
-                    # cmd.extend(["-p", f"{self.cache_dir}/collections"])
                 _logger.info("Running %s", " ".join(cmd))
                 result = self.exec(
                     cmd,

tox.ini

@@ -37,6 +37,8 @@ commands =
   # pytest users to run coverage when they just want to run a single test with `pytest -k test`
   coverage run -m pytest {posargs:}
   sh -c "coverage xml || true && coverage report"
+  # We fail if files are modified at the end
+  git diff --exit-code
 commands_pre =
   # safety measure to assure we do not accidentally run tests with broken dependencies
   {envpython} -m pip check
@@ -63,6 +65,7 @@ setenv =
   FORCE_COLOR = 1
 allowlist_externals =
   ansible
+  git
   sh
 
 [testenv:lint]