From 2801dbbc3565e628c62b262c79b1544a7e704c0e Mon Sep 17 00:00:00 2001
From: Kyle Verhoog <kyle@verhoog.ca>
Date: Sat, 15 May 2021 03:44:56 -0400
Subject: [PATCH 1/8] Refactor interpreter evaluation

Before, interpreter evaluation was done at run-time when the interpreter
was first used.

With this patch interpreters are looked up when the environments are
generated.

Also, more precise interpreter specification is possible. Before,
specifying "3" in a riotfile would result in a "python3" executable
being looked up. So riot would fail if Python was installed as "python"
and no "python3" executable was available on the path.

Now interpreters can be specified with a file path, executable name
or a version (which will be looked up on the PATH).
---
 .github/workflows/main.yml    |   3 -
 riot/cli.py                   |   3 +-
 riot/riot.py                  | 251 +++++++++++++++++++++++++---------
 riotfile.py                   |   6 +-
 setup.py                      |   1 +
 tests/data/simple_riotfile.py |   2 +-
 tests/test_cli.py             |  24 +---
 tests/test_unit.py            |  28 ----
 8 files changed, 202 insertions(+), 116 deletions(-)
 delete mode 100644 tests/test_unit.py

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 0a86760..cd6ae30 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -1,9 +1,6 @@
 name: CI
 on:
-  pull_request:
   push:
-    branches:
-      - master
 jobs:
   black:
     runs-on: ubuntu-latest
diff --git a/riot/cli.py b/riot/cli.py
index ba4a421..dce3375 100644
--- a/riot/cli.py
+++ b/riot/cli.py
@@ -22,7 +22,8 @@ class InterpreterParamType(click.ParamType):
     name = "interpreter"
 
     def convert(self, value, param, ctx):
-        return Interpreter(value)
+        interp = Interpreter.resolve(value)
+        return interp
 
 
 PATTERN_ARG = click.argument("pattern", envvar="RIOT_PATTERN", default=r".*")
diff --git a/riot/riot.py b/riot/riot.py
index 2643f6c..58c9ac6 100644
--- a/riot/riot.py
+++ b/riot/riot.py
@@ -5,6 +5,7 @@
 import itertools
 import logging
 import os
+import re
 import shutil
 import subprocess
 import sys
@@ -12,6 +13,9 @@
 import typing as t
 
 import click
+from packaging.version import InvalidVersion
+from packaging.version import Version
+from packaging.version import VERSION_PATTERN
 
 logger = logging.getLogger(__name__)
 
@@ -62,60 +66,158 @@ def to_list(x: t.Union[_K, t.List[_K]]) -> t.List[_K]:
 _T_stdio = t.Union[None, int, t.IO[t.Any]]
 
 
-@dataclasses.dataclass(unsafe_hash=True, eq=True)
+@dataclasses.dataclass(eq=True)
 class Interpreter:
-    _T_hint = t.Union[float, int, str]
+    _path: str = dataclasses.field()
+    _version: Version = dataclasses.field(init=False)
 
-    hint: dataclasses.InitVar[_T_hint]
-    _hint: str = dataclasses.field(init=False)
+    def __post_init__(self):
+        """Get the version of the interpreter."""
+        output = subprocess.check_output(
+            [self._path, "--version"], stderr=subprocess.STDOUT
+        )
+        version = Version(output.decode().strip().split(" ")[1])
+        self._version = version
 
-    def __post_init__(self, hint: _T_hint) -> None:
-        """Normalize the data."""
-        self._hint = str(hint)
+    def __hash__(self) -> int:
+        """Return the hash of this interpreter."""
+        return hash(self._path)
+
+    def __repr__(self) -> str:
+        """Return the repr containing the path and version of this interpreter."""
+        return f"{self.__class__.__name__}('{self.version}', '{self._path}')"
 
     def __str__(self) -> str:
-        """Return the path of the interpreter executable."""
+        """Return the repr of this interpreter."""
         return repr(self)
 
+    @staticmethod
+    def _matches(v1: Version, v2: Version) -> bool:
+        """Return if v2 matches v1.
+
+        >>> Interpreter._matches(Version("3.8.2"), Version("3"))
+        True
+        >>> Interpreter._matches(Version("3.8.2"), Version("3.8"))
+        True
+        >>> Interpreter._matches(Version("3.8.2"), Version("3.8.2"))
+        True
+        >>> Interpreter._matches(Version("3.8.2"), Version("3.8.3"))
+        False
+        >>> Interpreter._matches(Version("3.8.2"), Version("3.6"))
+        False
+        >>> Interpreter._matches(Version("3.8.2"), Version("3.7"))
+        False
+        >>> Interpreter._matches(Version("3.8.2"), Version("2"))
+        False
+        >>> Interpreter._matches(Version("3.8.2-dev"), Version("3.8.2"))
+        True
+        >>> Interpreter._matches(Version("3.8.2-alpha"), Version("3.8.2"))
+        True
+
+        FIXME: unfortunately Version will set undefined segments to 0 so
+        we cannot distinguish between "3.0.0" and "3".
+        >>> Interpreter._matches(Version("3.8.2"), Version("3.0.0"))
+        True
+        >>> Interpreter._matches(Version("3.8.2"), Version("3.8.0"))
+        True
+        """
+        if v2 == v1:
+            return True
+        elif v2.micro != v1.micro and v2.micro != 0:
+            return False
+        elif v2.minor != v1.minor and v2.minor != 0:
+            return False
+        elif v2.major != v1.major and v2.major != 0:
+            return False
+        return True
+
+    @classmethod
     @functools.lru_cache()
-    def version(self) -> str:
-        path = self.path()
+    def find(cls) -> t.List["Interpreter"]:
+        """Find all Python interpreters discoverable on the PATH."""
+        ex_dirs = os.environ.get("PATH", "").split(":")
+        # Use a list because order matters (need the first match).
+        interp_paths: t.List[str] = []
+        # packaging says that the regular expression needs to be compiled
+        # with the following flags
+        expr = re.compile(f"python{VERSION_PATTERN}$", re.IGNORECASE | re.VERBOSE)
+        for d in ex_dirs:
+            if not os.path.isdir(d):
+                continue
+            for f in os.listdir(d):
+                if expr.match(f):
+                    interp_paths.append(os.path.join(d, f))
 
-        # Redirect stderr to stdout because Python 2 prints version on stderr
-        output = subprocess.check_output([path, "--version"], stderr=subprocess.STDOUT)
-        version = output.decode().strip().split(" ")[1]
-        return version
+        interps: t.List[Interpreter] = []
+        for i in interp_paths:
+            try:
+                interp = cls(i)
+            except InvalidVersion:
+                logger.warning(
+                    "Failed to parse version for interpreter %r.", i, exc_info=True
+                )
+            else:
+                interps.append(interp)
 
-    @functools.lru_cache()
-    def path(self) -> str:
-        """Return the Python interpreter path or raise.
+        return interps
+
+    @classmethod
+    def find_match(cls, v: Version) -> "Interpreter":
+        """Return a matching interpreter for the given version.
+
+        If one is not found then raises a FileNotFoundError.
+        """
+        interps = [i for i in cls.find() if i.matches(v)]
+        if len(interps):
+            return interps[0]
+        raise FileNotFoundError("No interpreter matching %r", v)
+
+    @classmethod
+    def resolve(cls, s: str) -> "Interpreter":
+        """Resolve a string into an interpreter.
 
-        This defers the error until the interpeter is actually required. This is
-        desirable for cases where a user might not require all the mentioned
-        interpreters to be installed for their usage.
+        Supports file paths, executables on the PATH and specific versions
+        (to be looked up from the PATH). Raises FileNotFoundError if no
+        resolution is found.
         """
-        py_ex = shutil.which(self._hint)
+        if os.path.exists(s):
+            return cls(s)
 
-        if not py_ex:
-            py_ex = shutil.which(f"python{self._hint}")
+        ex = shutil.which(s)
+        if ex:
+            return cls(ex)
 
-        if py_ex:
-            return py_ex
+        try:
+            version = Version(s)
+        except InvalidVersion as e:
+            raise FileNotFoundError("No interpreter found for %r" % s) from e
+        else:
+            return cls.find_match(version)
+
+    @property
+    def version(self) -> Version:
+        return self._version
 
-        raise FileNotFoundError(f"Python interpreter {self._hint} not found")
+    def matches(self, version: Version) -> bool:
+        """Return whether the given version matches the version of the interpreter."""
+        return self._matches(self.version, version)
+
+    def path(self) -> str:
+        """Return the path to this interpreter."""
+        return self._path
 
     def venv_path(self) -> str:
         """Return the path to the virtual environment for this interpreter."""
-        version = self.version().replace(".", "")
+        version = str(self.version).replace(".", "")
         return f".riot/venv_py{version}"
 
     def create_venv(self, recreate: bool) -> str:
-        """Attempt to create a virtual environment for this intepreter."""
+        """Attempt to create a virtual environment for this interpreter."""
         path: str = self.venv_path()
 
         if os.path.isdir(path) and not recreate:
             logger.info(
-                "Skipping creation of virtualenv '%s' as it already exists.", path
+                "Skipping creation of virtualenv %r as it already exists.", path
             )
             return path
 
@@ -136,7 +238,7 @@ class Venv:
     Example::
 
         Venv(
-          pys=[3.9],
+          pys=["3.9"],
           venvs=[
               Venv(
                   name="mypy",
@@ -158,13 +260,14 @@ class Venv:
     Args:
         name (str): Name of the instance. Overrides parent value.
         command (str): Command to run in the virtual environment. Overrides parent value.
-        pys  (List[float]): Python versions. Overrides parent value.
+        pys  (List[str]): Python version(s) to use. Can be a file path to an interpreter, an executable name (locatable
+            on the PATH or a version number of an interpreter that can be found on the PATH. Overrides parent value.
         pkgs (Dict[str, Union[str, List[str]]]): Packages and version(s) to install into the virtual env. Merges and overrides parent values.
         env  (Dict[str, Union[str, List[str]]]): Environment variables to define in the virtual env. Merges and overrides parent values.
         venvs (List[Venv]): List of Venvs that inherit the properties of this Venv (unless they are overridden).
     """
 
-    pys: dataclasses.InitVar[t.List[Interpreter._T_hint]] = None
+    pys: dataclasses.InitVar[t.Optional[t.List[str]]] = None
     pkgs: dataclasses.InitVar[t.Dict[str, t.List[str]]] = None
     env: dataclasses.InitVar[t.Dict[str, t.List[str]]] = None
     name: t.Optional[str] = None
@@ -173,7 +276,7 @@ class Venv:
 
     def __post_init__(self, pys, pkgs, env):
         """Normalize the data."""
-        self.pys = [Interpreter(py) for py in to_list(pys)] if pys is not None else []
+        self.pys = to_list(pys) if pys is not None else []
         self.pkgs = rm_singletons(pkgs) if pkgs else {}
         self.env = rm_singletons(env) if env else {}
 
@@ -217,11 +320,24 @@ def instances(
             # Expand out the instances for the venv.
             for env in expand_specs(resolved.env):
                 for py in resolved.pys:
+                    interpreter = None
+                    try:
+                        interpreter = Interpreter.resolve(str(py))
+                    except FileNotFoundError:
+                        logger.debug("Failed to find interpreter for %r", py)
+                    else:
+                        logger.debug(
+                            "Using %r for requirement %r",
+                            interpreter,
+                            py,
+                        )
+
                     for pkgs in expand_specs(resolved.pkgs):
                         yield VenvInstance(
                             name=resolved.name,
                             command=resolved.command,
-                            py=py,
+                            interpreter_version=py,
+                            interpreter=interpreter,
                             env=env,
                             pkgs=pkgs,
                         )
@@ -233,11 +349,17 @@ class VenvInstance:
     env: t.Tuple[t.Tuple[str, str]]
     name: t.Optional[str]
     pkgs: t.Tuple[t.Tuple[str, str]]
-    py: Interpreter
+    interpreter_version: str
+    """A null interpreter means that it was not found."""
+    interpreter: t.Optional[Interpreter]
 
     def venv_path(self) -> str:
         """Return path to directory of the instance."""
-        base_path = self.py.venv_path()
+        if not self.interpreter:
+            raise FileNotFoundError(
+                "No interpreter found for %r" % self.interpreter_version
+            )
+        base_path = self.interpreter.venv_path()
         venv_postfix = "_".join([f"{n}{rmchars('<=>.,', v)}" for n, v in self.pkgs])
         return f"{base_path}_{venv_postfix}"
 
@@ -262,6 +384,7 @@ def __init__(self, msg, completed_proc):
 @dataclasses.dataclass
 class Session:
     venv: Venv
+    interpreters: t.List[Interpreter]
     warnings = (
         "deprecated",
         "deprecation",
@@ -301,7 +424,9 @@ def from_config_file(cls, path: str) -> "Session":
             ) from e
         else:
             venv = getattr(config, "venv", Venv())
-            return cls(venv=venv)
+            interpreters = Interpreter.find()
+            logger.debug("Found interpreters %s.", ", ".join(map(str, interpreters)))
+            return cls(venv=venv, interpreters=interpreters)
 
     def is_warning(self, output):
         if output is None:
@@ -332,22 +457,23 @@ def run(
         )
 
         for inst in self.venv.instances(pattern=pattern):
-            if pythons and inst.py not in pythons:
-                logger.debug(
-                    "Skipping venv instance %s due to interpreter mismatch", inst
-                )
+            if not inst.interpreter:
+                logger.warning("Skipping %s due to missing interpreter", inst)
+                continue
+            if pythons and inst.interpreter not in pythons:
+                logger.debug("Skipping %s due to interpreter mismatch", inst)
                 continue
 
             try:
-                base_venv_path: str = inst.py.venv_path()
+                base_venv_path: str = inst.interpreter.venv_path()
             except FileNotFoundError:
                 if skip_missing:
-                    logger.warning("Skipping missing interpreter %s", inst.py)
+                    logger.warning("Skipping missing interpreter %s", inst.interpreter)
                     continue
                 else:
                     raise
 
-            logger.info("Running with %s", inst.py)
+            logger.info("Running with %s", inst.interpreter)
 
             # Resolve the packages required for this instance.
             pkgs: t.Dict[str, str] = {
@@ -457,7 +583,7 @@ def run(
         for r in results:
             failed = r.code != 0
             env_str = env_to_str(r.instance.env)
-            s = f"{r.instance.name}: {env_str} python{r.instance.py} {r.pkgstr}"
+            s = f"{r.instance.name}: {env_str} {r.instance.interpreter} {r.pkgstr}"
 
             if failed:
                 num_failed += 1
@@ -479,19 +605,18 @@ def run(
         if any(True for r in results if r.code != 0):
             sys.exit(1)
 
-    def list_venvs(self, pattern, venv_pattern, pythons=None, out=sys.stdout):
+    def list_venvs(self, pattern, venv_pattern, pythons=None):
         for inst in self.venv.instances(pattern=pattern):
-            if pythons and inst.py not in pythons:
+            if pythons and inst.interpreter not in pythons:
                 continue
-
-            if not venv_pattern.search(inst.venv_path()):
+            if not inst.interpreter or not venv_pattern.search(inst.venv_path()):
                 continue
+
             pkgs_str = " ".join(
                 f"'{get_pep_dep(name, version)}'" for name, version in inst.pkgs
             )
             env_str = env_to_str(inst.env)
-            py_str = f"Python {inst.py}"
-            click.echo(f"{inst.name} {env_str} {py_str} {pkgs_str}")
+            click.echo(f"{inst.name} {env_str} {inst.interpreter} {pkgs_str}")
 
     def generate_base_venvs(
         self,
@@ -502,25 +627,27 @@ def generate_base_venvs(
     ) -> None:
         """Generate all the required base venvs."""
         # Find all the python interpreters used.
-        required_pys: t.Set[Interpreter] = set(
-            [inst.py for inst in self.venv.instances(pattern=pattern)]
-        )
-        # Apply Python filters.
-        if pythons:
-            required_pys = required_pys.intersection(pythons)
+        required_pys = []
+
+        for inst in self.venv.instances(pattern=pattern):
+            if inst.interpreter is None:
+                logger.warning(
+                    "Interpreter for %r not found.", inst.interpreter_version
+                )
+                continue
+            if not pythons or inst.interpreter in pythons:
+                required_pys.append(inst.interpreter)
 
         logger.info(
-            "Generating virtual environments for interpreters %s",
+            "Generating base virtual environments for interpreters: %s.",
             ",".join(str(s) for s in required_pys),
         )
 
-        for py in required_pys:
+        for interpreter in required_pys:
             try:
-                venv_path = py.create_venv(recreate)
+                venv_path = interpreter.create_venv(recreate)
             except CmdFailure as e:
                 logger.error("Failed to create virtual environment.\n%s", e.proc.stdout)
-            except FileNotFoundError:
-                logger.error("Python version '%s' not found.", py)
             else:
                 if skip_deps:
                     logger.info("Skipping global deps install.")
diff --git a/riotfile.py b/riotfile.py
index af332c9..237d620 100644
--- a/riotfile.py
+++ b/riotfile.py
@@ -1,12 +1,12 @@
 from riot import latest, Venv
 
 venv = Venv(
-    pys=3,
+    pys=["3"],
     venvs=[
         Venv(
             name="test",
             command="pytest {cmdargs}",
-            pys=[3.6, 3.7, 3.8, 3.9],
+            pys=["3.6", "3.7", "3.8", "3.9"],
             pkgs={
                 "pytest": latest,
                 "pytest-cov": latest,
@@ -15,7 +15,7 @@
         ),
         Venv(
             pkgs={
-                "black": "==20.8b1",
+                "black": ["==20.8b1"],
             },
             venvs=[
                 Venv(
diff --git a/setup.py b/setup.py
index 3ffce94..2ed4e99 100644
--- a/setup.py
+++ b/setup.py
@@ -28,6 +28,7 @@
     install_requires=[
         "dataclasses; python_version<'3.7'",
         "click>=7,<8",
+        "packaging",
         "virtualenv",
     ],
     setup_requires=["setuptools_scm"],
diff --git a/tests/data/simple_riotfile.py b/tests/data/simple_riotfile.py
index 3bf815f..c32b753 100644
--- a/tests/data/simple_riotfile.py
+++ b/tests/data/simple_riotfile.py
@@ -10,7 +10,7 @@
                     pkgs={
                         "pytest": ["==5.4.3", ""],
                     },
-                    pys=[3],
+                    pys=["3"],
                 ),
             ],
         ),
diff --git a/tests/test_cli.py b/tests/test_cli.py
index db00a91..b7820d5 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -10,7 +10,6 @@
 import pytest
 import riot.cli
 import riot.riot
-from riot.riot import Interpreter
 
 
 HERE = os.path.abspath(os.path.dirname(__file__))
@@ -136,39 +135,30 @@ def test_list_with_venv_pattern(cli: click.testing.CliRunner) -> None:
             ],
         )
         assert result.exit_code == 0
-        assert result.stdout == "test  Python Interpreter(_hint='3') 'pytest==5.4.3'\n"
+        assert result.stdout.startswith("test  ")
+        assert result.stdout.endswith("'pytest==5.4.3'\n")
 
 
 def test_list_with_python(cli: click.testing.CliRunner) -> None:
     """Running list with a python passes through the python."""
-    with mock.patch("riot.cli.Session.list_venvs") as list_venvs:
+    with mock.patch("riot.cli.Session.list_venvs"):
         with with_riotfile(cli, "empty_riotfile.py"):
-            result = cli.invoke(riot.cli.main, ["list", "--python", "3.6"])
+            result = cli.invoke(riot.cli.main, ["list", "--python", "3"])
             # Success, but no output because we don't have a matching pattern
             assert result.exit_code == 0
             assert result.stdout == ""
 
-            list_venvs.assert_called_once()
-            assert list_venvs.call_args.kwargs["pythons"] == (Interpreter("3.6"),)
-
     # multiple pythons
-    with mock.patch("riot.cli.Session.list_venvs") as list_venvs:
+    with mock.patch("riot.cli.Session.list_venvs"):
         with with_riotfile(cli, "empty_riotfile.py"):
             result = cli.invoke(
                 riot.cli.main,
-                ["list", "--python", "3.6", "-p", "3.8", "--python", "2.7"],
+                ["list", "-p", "3.8", "--python", "2.7"],
             )
             # Success, but no output because we don't have a matching pattern
             assert result.exit_code == 0
             assert result.stdout == ""
 
-            list_venvs.assert_called_once()
-            assert list_venvs.call_args.kwargs["pythons"] == (
-                Interpreter("3.6"),
-                Interpreter("3.8"),
-                Interpreter("2.7"),
-            )
-
 
 def test_run(cli: click.testing.CliRunner) -> None:
     """Running run with default options."""
@@ -272,8 +262,6 @@ def test_run_no_venv_pattern(cli: click.testing.CliRunner) -> None:
             ],
         )
         assert result.exit_code == 0
-        assert "✓ test:  pythonInterpreter(_hint='3') 'pytest==5.4.3'" in result.stdout
-        assert "✓ test:  pythonInterpreter(_hint='3') 'pytest'" in result.stdout
         assert "2 passed with 0 warnings, 0 failed" in result.stdout
 
 
diff --git a/tests/test_unit.py b/tests/test_unit.py
deleted file mode 100644
index 15985b4..0000000
--- a/tests/test_unit.py
+++ /dev/null
@@ -1,28 +0,0 @@
-import pytest
-from riot.riot import Interpreter
-
-
-@pytest.mark.parametrize(
-    "v1,v2,equal",
-    [
-        (3.6, 3.6, True),
-        (3.6, "3.6", True),
-        ("3.6", "3.6", True),
-        ("3.6", 3.6, True),
-        (3.6, 3.7, False),
-        (3.6, "3.7", False),
-        (3.7, 3.7, True),
-        (3, 3, True),
-        (3, "3", True),
-        ("3", 3, True),
-    ],
-)
-def test_interpreter(v1, v2, equal):
-    if equal:
-        assert Interpreter(v1) == Interpreter(v2)
-        assert hash(Interpreter(v1)) == hash(Interpreter(v2))
-        assert repr(Interpreter(v1)) == repr(Interpreter(v2))
-    else:
-        assert Interpreter(v1) != Interpreter(v2)
-        assert hash(Interpreter(v1)) != hash(Interpreter(v2))
-        assert repr(Interpreter(v1)) != repr(Interpreter(v2))

From 40a8de4d6ac30a2c31070e3aa545411d4e1b2874 Mon Sep 17 00:00:00 2001
From: Kyle Verhoog <kyle@verhoog.ca>
Date: Thu, 20 May 2021 21:49:23 -0400
Subject: [PATCH 2/8] Inline return

---
 riot/cli.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/riot/cli.py b/riot/cli.py
index dce3375..2c0b031 100644
--- a/riot/cli.py
+++ b/riot/cli.py
@@ -22,8 +22,7 @@ class InterpreterParamType(click.ParamType):
     name = "interpreter"
 
     def convert(self, value, param, ctx):
-        interp = Interpreter.resolve(value)
-        return interp
+        return Interpreter.resolve(value)
 
 
 PATTERN_ARG = click.argument("pattern", envvar="RIOT_PATTERN", default=r".*")

From 36e1d1933ebbefbe4d0102f90e1ff25629ffa941 Mon Sep 17 00:00:00 2001
From: Kyle Verhoog <kyle@verhoog.ca>
Date: Thu, 20 May 2021 21:55:03 -0400
Subject: [PATCH 3/8] Atomic directory read

---
 riot/riot.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/riot/riot.py b/riot/riot.py
index 58c9ac6..33c7ba5 100644
--- a/riot/riot.py
+++ b/riot/riot.py
@@ -70,6 +70,7 @@ def to_list(x: t.Union[_K, t.List[_K]]) -> t.List[_K]:
 class Interpreter:
     _path: str = dataclasses.field()
     _version: Version = dataclasses.field(init=False)
+    _version_expr = re.compile(f"python{VERSION_PATTERN}$", re.IGNORECASE | re.VERBOSE)
 
     def __post_init__(self):
         """Get the version of the interpreter."""
@@ -140,13 +141,13 @@ def find(cls) -> t.List["Interpreter"]:
         interp_paths: t.List[str] = []
         # packaging says that the regular expression needs to be compiled
         # with the following flags
-        expr = re.compile(f"python{VERSION_PATTERN}$", re.IGNORECASE | re.VERBOSE)
         for d in ex_dirs:
-            if not os.path.isdir(d):
+            try:
+                for f in os.listdir(d):
+                    if cls._version_expr.match(f):
+                        interp_paths.append(os.path.join(d, f))
+            except (FileNotFoundError, NotADirectoryError):
                 continue
-            for f in os.listdir(d):
-                if expr.match(f):
-                    interp_paths.append(os.path.join(d, f))
 
         interps: t.List[Interpreter] = []
         for i in interp_paths:

From 68ef1cd23e00257678ac60af88ffcee198ed045f Mon Sep 17 00:00:00 2001
From: Kyle Verhoog <kyle@verhoog.ca>
Date: Thu, 20 May 2021 22:02:11 -0400
Subject: [PATCH 4/8] Don't wrap invalid version errors on resolve

---
 riot/riot.py | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/riot/riot.py b/riot/riot.py
index 33c7ba5..7ddff24 100644
--- a/riot/riot.py
+++ b/riot/riot.py
@@ -188,12 +188,8 @@ def resolve(cls, s: str) -> "Interpreter":
         if ex:
             return cls(ex)
 
-        try:
-            version = Version(s)
-        except InvalidVersion as e:
-            raise FileNotFoundError("No interpreter found for %r" % s) from e
-        else:
-            return cls.find_match(version)
+        version = InterpreterVersion(s)
+        return cls.find_match(version)
 
     @property
     def version(self) -> Version:

From 63af679dfd691e6638e5af041c7f7c2f8af1a3b5 Mon Sep 17 00:00:00 2001
From: Kyle Verhoog <kyle@verhoog.ca>
Date: Thu, 20 May 2021 22:04:07 -0400
Subject: [PATCH 5/8] interpreter_version -> interpreter_hint

A bit more clear that the "version" is not necessarily a version.
---
 riot/riot.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/riot/riot.py b/riot/riot.py
index 7ddff24..020574f 100644
--- a/riot/riot.py
+++ b/riot/riot.py
@@ -258,7 +258,7 @@ class Venv:
         name (str): Name of the instance. Overrides parent value.
         command (str): Command to run in the virtual environment. Overrides parent value.
         pys  (List[str]): Python version(s) to use. Can be a file path to an interpreter, an executable name (locatable
-            on the PATH or a version number of an interpreter that can be found on the PATH. Overrides parent value.
+        on the PATH) or a version number of an interpreter that can be found on the PATH. Overrides parent value.
         pkgs (Dict[str, Union[str, List[str]]]): Packages and version(s) to install into the virtual env. Merges and overrides parent values.
         env  (Dict[str, Union[str, List[str]]]): Environment variables to define in the virtual env. Merges and overrides parent values.
         venvs (List[Venv]): List of Venvs that inherit the properties of this Venv (unless they are overridden).
@@ -333,7 +333,7 @@ def instances(
                         yield VenvInstance(
                             name=resolved.name,
                             command=resolved.command,
-                            interpreter_version=py,
+                            interpreter_hint=py,
                             interpreter=interpreter,
                             env=env,
                             pkgs=pkgs,
@@ -346,7 +346,7 @@ class VenvInstance:
     env: t.Tuple[t.Tuple[str, str]]
     name: t.Optional[str]
     pkgs: t.Tuple[t.Tuple[str, str]]
-    interpreter_version: str
+    interpreter_hint: str
     """A null interpreter means that it was not found."""
     interpreter: t.Optional[Interpreter]
 
@@ -354,7 +354,7 @@ def venv_path(self) -> str:
         """Return path to directory of the instance."""
         if not self.interpreter:
             raise FileNotFoundError(
-                "No interpreter found for %r" % self.interpreter_version
+                "No interpreter found for %r" % self.interpreter_hint
             )
         base_path = self.interpreter.venv_path()
         venv_postfix = "_".join([f"{n}{rmchars('<=>.,', v)}" for n, v in self.pkgs])

From 65a1b553ca54a060173bb0a149bdf6a1f7ab830a Mon Sep 17 00:00:00 2001
From: Kyle Verhoog <kyle@verhoog.ca>
Date: Fri, 21 May 2021 21:46:08 -0400
Subject: [PATCH 6/8] Refactor run with RunFailure

CmdFailure was overloaded to handle two different error cases which was
confusing.
---
 riot/riot.py | 97 ++++++++++++++++++++++++++++------------------------
 1 file changed, 53 insertions(+), 44 deletions(-)

diff --git a/riot/riot.py b/riot/riot.py
index 020574f..0cdfe50 100644
--- a/riot/riot.py
+++ b/riot/riot.py
@@ -364,8 +364,8 @@ def venv_path(self) -> str:
 @dataclasses.dataclass
 class VenvInstanceResult:
     instance: VenvInstance
-    venv_name: str
-    pkgstr: str
+    venv_name: t.Optional[str] = None
+    pkgstr: t.Optional[str] = None
     code: int = 1
     output: str = ""
 
@@ -378,6 +378,13 @@ def __init__(self, msg, completed_proc):
         super().__init__(self, msg)
 
 
+class RunFailure(Exception):
+    def __init__(self, msg):
+        self.msg = msg
+        self.code = 1
+        super().__init__(self, msg)
+
+
 @dataclasses.dataclass
 class Session:
     venv: Venv
@@ -454,50 +461,55 @@ def run(
         )
 
         for inst in self.venv.instances(pattern=pattern):
-            if not inst.interpreter:
-                logger.warning("Skipping %s due to missing interpreter", inst)
-                continue
-            if pythons and inst.interpreter not in pythons:
-                logger.debug("Skipping %s due to interpreter mismatch", inst)
-                continue
+            # Result which will be updated with the test outcome.
+            result = VenvInstanceResult(instance=inst)
 
             try:
-                base_venv_path: str = inst.interpreter.venv_path()
-            except FileNotFoundError:
-                if skip_missing:
-                    logger.warning("Skipping missing interpreter %s", inst.interpreter)
+                if inst.interpreter is None:
+                    if skip_missing:
+                        logger.warning(
+                            f"Skipping %s due to missing interpreter for '{inst.interpreter_hint}'",
+                            inst,
+                        )
+                        continue
+                    else:
+                        raise RunFailure(
+                            f"Missing interpreter for '{inst.interpreter_hint}'"
+                        )
+                if pythons and inst.interpreter not in pythons:
+                    logger.debug("Skipping %s due to interpreter mismatch", inst)
                     continue
-                else:
-                    raise
 
-            logger.info("Running with %s", inst.interpreter)
+                logger.info("Running with %s", inst.interpreter)
+                base_venv_path = inst.interpreter.venv_path()
 
-            # Resolve the packages required for this instance.
-            pkgs: t.Dict[str, str] = {
-                name: version for name, version in inst.pkgs if version is not None
-            }
+                # Resolve the packages required for this instance.
+                pkgs: t.Dict[str, str] = {
+                    name: version for name, version in inst.pkgs if version is not None
+                }
 
-            if pkgs:
-                venv_path = inst.venv_path()
-                pkg_str = " ".join(
-                    [f"'{get_pep_dep(lib, version)}'" for lib, version in pkgs.items()]
-                )
-            else:
-                venv_path = base_venv_path
-                pkg_str = ""
+                if pkgs:
+                    venv_path = inst.venv_path()
+                    pkg_str = " ".join(
+                        [
+                            f"'{get_pep_dep(lib, version)}'"
+                            for lib, version in pkgs.items()
+                        ]
+                    )
+                else:
+                    venv_path = base_venv_path
+                    pkg_str = ""
 
-            if not venv_pattern.search(venv_path):
-                logger.debug(
-                    "Skipping venv instance '%s' due to pattern mismatch", venv_path
-                )
-                continue
+                result.pkgstr = pkg_str
 
-            # Result which will be updated with the test outcome.
-            result = VenvInstanceResult(
-                instance=inst, venv_name=venv_path, pkgstr=pkg_str
-            )
+                if not venv_pattern.search(venv_path):
+                    logger.debug(
+                        "Skipping venv instance '%s' due to pattern mismatch", venv_path
+                    )
+                    continue
+
+                result.venv_name = venv_path
 
-            try:
                 if pkgs:
                     # Copy the base venv to use for this venv.
                     logger.info(
@@ -521,9 +533,8 @@ def run(
                             f"pip --disable-pip-version-check install {pkg_str}",
                         )
                     except CmdFailure as e:
-                        raise CmdFailure(
-                            f"Failed to install venv dependencies {pkg_str}\n{e.proc.stdout}",
-                            e.proc,
+                        raise RunFailure(
+                            f"Failed to install venv dependencies {pkg_str}\n{e.proc.stdout}"
                         )
 
                 # Generate the environment for the instance.
@@ -550,10 +561,8 @@ def run(
                     )
                     result.output = output.stdout
                 except CmdFailure as e:
-                    raise CmdFailure(
-                        f"Test failed with exit code {e.proc.returncode}", e.proc
-                    )
-            except CmdFailure as e:
+                    raise RunFailure(f"Test failed with exit code {e.proc.returncode}")
+            except RunFailure as e:
                 result.code = e.code
                 click.echo(click.style(e.msg, fg="red"))
                 if exit_first:

From 7f4eca35f2cec3394d1a8340873636c3b0b8b760 Mon Sep 17 00:00:00 2001
From: Kyle Verhoog <kyle@verhoog.ca>
Date: Fri, 21 May 2021 22:21:44 -0400
Subject: [PATCH 7/8] Fix list with missing interpreters

Print out the venv explicitly saying that the specified interpreter is
missing.
---
 riot/riot.py | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/riot/riot.py b/riot/riot.py
index 0cdfe50..e55afd5 100644
--- a/riot/riot.py
+++ b/riot/riot.py
@@ -611,11 +611,25 @@ def run(
         if any(True for r in results if r.code != 0):
             sys.exit(1)
 
-    def list_venvs(self, pattern, venv_pattern, pythons=None):
+    def list_venvs(
+        self,
+        pattern,
+        venv_pattern: t.Pattern[str],
+        pythons: t.Optional[t.Set[Interpreter]] = None,
+    ) -> None:
         for inst in self.venv.instances(pattern=pattern):
             if pythons and inst.interpreter not in pythons:
                 continue
-            if not inst.interpreter or not venv_pattern.search(inst.venv_path()):
+            if not inst.interpreter:
+                pkgs_str = " ".join(
+                    f"'{get_pep_dep(name, version)}'" for name, version in inst.pkgs
+                )
+                env_str = env_to_str(inst.env)
+                click.echo(
+                    f"{inst.name} {env_str} <Interpreter missing for '{inst.interpreter_hint}'> {pkgs_str}"
+                )
+                continue
+            if not venv_pattern.search(inst.venv_path()):
                 continue
 
             pkgs_str = " ".join(

From 67248665512370998e430305e1f6ecc817b833ff Mon Sep 17 00:00:00 2001
From: Kyle Verhoog <kyle@verhoog.ca>
Date: Fri, 21 May 2021 22:23:08 -0400
Subject: [PATCH 8/8] Fix generate venvs

---
 riot/riot.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/riot/riot.py b/riot/riot.py
index e55afd5..660da21 100644
--- a/riot/riot.py
+++ b/riot/riot.py
@@ -651,11 +651,12 @@ def generate_base_venvs(
 
         for inst in self.venv.instances(pattern=pattern):
             if inst.interpreter is None:
-                logger.warning(
-                    "Interpreter for %r not found.", inst.interpreter_version
-                )
                 continue
-            if not pythons or inst.interpreter in pythons:
+            if (
+                not pythons
+                or inst.interpreter in pythons
+                and inst.interpreter in required_pys
+            ):
                 required_pys.append(inst.interpreter)
 
         logger.info(