Better handling of set_env (#1784)

Author: gaborbernat

docs/changelog.rst

@@ -5,6 +5,30 @@ Release History
 
 .. towncrier release notes start
 
+v4.0.0a2 (2021-01-09)
+---------------------
+
+Features - 4.0.0a2
+~~~~~~~~~~~~~~~~~~
+- Add option to disable colored output, and support ``NO_COLOR`` and ``FORCE_COLOR`` environment variables - by
+  :user:`gaborbernat`. (`#1630 <https://github.com/tox-dev/tox/issues/1630>`_)
+
+Bugfixes - 4.0.0a2
+~~~~~~~~~~~~~~~~~~
+- Fix coverage generation in CI - by :user:`gaborbernat`. (`#1551 <https://github.com/tox-dev/tox/issues/1551>`_)
+- Fix the CI failures:
+
+  - drop Python 3.5 support as it's not expected to get to a release before EOL,
+  - fix test using ``\n`` instead of ``os.linesep``,
+  - Windows Python 3.6 does not contain ``_overlapped.ReadFileInto``
+
+  - by :user:`gaborbernat`. (`#1556 <https://github.com/tox-dev/tox/issues/1556>`_)
+
+Improved Documentation - 4.0.0a2
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+- Add base documentation by merging virtualenv structure with tox 3 - by :user:`gaborbernat`. (`#1551 <https://github.com/tox-dev/tox/issues/1551>`_)
+
+
 v4.0.0a1
 --------
 * First version all is brand new.

docs/changelog/1551.bugfix.rst

@@ -0,0 +1 @@
+Entries in the ``set_env`` does not reference environments from ``set_env`` - by :user:`gaborbernat`.

docs/changelog/1551.doc.rst

@@ -0,0 +1 @@
+``env`` substitution does not uses values from ``set_env`` - by :user:`gaborbernat`.

docs/changelog/1556.bugfix.rst

@@ -0,0 +1 @@
+Raise exception when set env enters into a circular reference - by :user:`gaborbernat`.

docs/changelog/1630.feature.rst

@@ -0,0 +1,7 @@
+- Python version markers are stripped in package dependencies (after wrongfully being detected as an extra marker).
+- In packaging APIs do not set ``PYTHONPATH`` (to empty string) if ``backend-path`` is empty.
+- Fix commands parsing on Windows (do not auto-escape ``\`` - instead users should use the new ``{\}``, and on parsed
+  arguments strip both ``'`` and ``"`` quoted outcomes).
+- Allow windows paths in substitution set/default (the ``:`` character used to separate substitution arguments may
+  also be present in paths on Windows - do not support single capital letter values as substitution arguments) -
+  by :user:`gaborbernat`.

docs/changelog/1776.bugfix.rst

@@ -0,0 +1,4 @@
+- Raise exception when variable substitution enters into a circle.
+- Add ``{/}`` as substitution for os specific path separator.
+- Add ``{env_bin_dir}`` constant substitution.
+- Implement support for ``--discover`` flag - by :user:`gaborbernat`.

docs/changelog/1779.bugfix.rst

@@ -42,7 +42,7 @@ install_requires =
     packaging>=20.3
     pluggy>=0.13.1
     toml>=0.10
-    virtualenv>=20.0.20
+    virtualenv>=20.3
     importlib-metadata>=1.6.0;python_version<"3.8"
     typing-extensions>=3.7.4.2;python_version<"3.8"
 python_requires = >=3.6

docs/changelog/1779.feature.rst

@@ -51,7 +51,7 @@ def get(self, key: str, of_type: Type[Any]) -> Any:
                     result = None
                 else:
                     source = "file"
-                    value = self.ini.load(key, of_type=of_type, conf=None, env_name="tox")
+                    value = self.ini.load(key, of_type=of_type, conf=None, env_name="tox", chain=[key])
                     result = value, source
             except KeyError:  # just not found
                 result = None

docs/changelog/1784.bugfix.rst

@@ -1,6 +1,8 @@
 from abc import abstractmethod
 from argparse import ArgumentTypeError
-from typing import TYPE_CHECKING, Any, List, Mapping, Optional, Set, Type, TypeVar
+from concurrent.futures import Future
+from contextlib import contextmanager
+from typing import TYPE_CHECKING, Any, Generator, List, Mapping, Optional, Set, Type, TypeVar
 
 from tox.plugin.impl import impl
 
@@ -69,7 +71,9 @@ def found_keys(self) -> Set[str]:
     def __repr__(self) -> str:
         return f"{type(self).__name__}"
 
-    def load(self, key: str, of_type: Type[V], conf: Optional["Config"], env_name: Optional[str]) -> V:
+    def load(
+        self, key: str, of_type: Type[V], conf: Optional["Config"], env_name: Optional[str], chain: List[str]
+    ) -> V:
         """
         Load a value.
 
@@ -82,9 +86,25 @@ def load(self, key: str, of_type: Type[V], conf: Optional["Config"], env_name: O
         if key in self.overrides:
             return _STR_CONVERT.to(self.overrides[key].value, of_type)
         raw = self.load_raw(key, conf, env_name)
-        converted = self.to(raw, of_type)
+        future: "Future[V]" = Future()
+        with self.build(future, key, of_type, conf, env_name, raw, chain) as prepared:
+            converted = self.to(prepared, of_type)
+            future.set_result(converted)
         return converted
 
+    @contextmanager
+    def build(
+        self,
+        future: "Future[V]",
+        key: str,
+        of_type: Type[V],
+        conf: Optional["Config"],
+        env_name: Optional[str],
+        raw: T,
+        chain: List[str],
+    ) -> Generator[T, None, None]:
+        yield raw
+
 
 @impl
 def tox_add_option(parser: "ToxParser") -> None:

docs/changelog/1784.feature.rst

@@ -1,11 +1,15 @@
+import inspect
+from concurrent.futures import Future
 from configparser import ConfigParser, SectionProxy
-from typing import List, Optional, Set, TypeVar
+from contextlib import contextmanager
+from typing import Generator, List, Optional, Set, Type, TypeVar
 
 from tox.config.loader.api import Loader, Override
 from tox.config.loader.ini.factor import filter_for_env
 from tox.config.loader.ini.replace import replace
 from tox.config.loader.str_convert import StrConvert
 from tox.config.main import Config
+from tox.config.set_env import SetEnv
 from tox.report import HandledError
 
 V = TypeVar("V")
@@ -28,15 +32,42 @@ def load_raw(self, key: str, conf: Optional[Config], env_name: Optional[str]) ->
         value = self._section[key]
         collapsed_newlines = value.replace("\\\r\n", "").replace("\\\n", "")  # collapse explicit new-line escape
         if conf is None:  # conf is None when we're loading the global tox configuration file for the CLI
-            replaced = collapsed_newlines  # we don't support factor and replace functionality there
+            factor_filtered = collapsed_newlines  # we don't support factor and replace functionality there
         else:
-            factor_selected = filter_for_env(collapsed_newlines, env_name)  # select matching factors
-            try:
-                replaced = replace(factor_selected, conf, env_name, self)  # do replacements
-            except Exception as exception:
-                msg = f"replace failed in {'tox' if env_name is None else env_name}.{key} with {exception!r}"
-                raise HandledError(msg)
-        return replaced
+            factor_filtered = filter_for_env(collapsed_newlines, env_name)  # select matching factors
+        return factor_filtered
+
+    @contextmanager
+    def build(
+        self,
+        future: "Future[V]",
+        key: str,
+        of_type: Type[V],
+        conf: Optional["Config"],
+        env_name: Optional[str],
+        raw: str,
+        chain: List[str],
+    ) -> Generator[str, None, None]:
+        delay_replace = inspect.isclass(of_type) and issubclass(of_type, SetEnv)
+
+        def replacer(raw_: str, chain_: List[str]) -> str:
+            if conf is None:
+                replaced = raw_  # no replacement supported in the core section
+            else:
+                try:
+                    replaced = replace(conf, env_name, self, raw_, chain_)  # do replacements
+                except Exception as exception:
+                    msg = f"replace failed in {'tox' if env_name is None else env_name}.{key} with {exception!r}"
+                    raise HandledError(msg) from exception
+            return replaced
+
+        if not delay_replace:
+            raw = replacer(raw, chain)
+        yield raw
+        if delay_replace:
+            converted = future.result()
+            if hasattr(converted, "replacer"):  # pragma: no branch
+                converted.replacer = replacer  # type: ignore[attr-defined]
 
     def found_keys(self) -> Set[str]:
         return set(self._section.keys())

setup.cfg

@@ -15,12 +15,13 @@ def filter_for_env(value: str, name: Optional[str]) -> str:
     overall = []
     for factors, content in expand_factors(value):
         if factors is None:
-            overall.append(content)
+            if content:
+                overall.append(content)
         else:
             for group in factors:
                 for a_name, negate in group:
                     contains = a_name in current
-                    if contains == negate:
+                    if not ((contains is True and negate is False) or (contains is False and negate is True)):
                         break
                 else:
                     overall.append(content)

src/tox/config/cli/ini.py

@@ -9,6 +9,7 @@
 
 from tox.config.loader.stringify import stringify
 from tox.config.main import Config
+from tox.config.set_env import SetEnv
 from tox.config.sets import ConfigSet
 from tox.execute.request import shell_cmd
 
@@ -18,18 +19,19 @@
 CORE_PREFIX = "tox"
 BASE_TEST_ENV = "testenv"
 
-ARGS_GROUP = re.compile(r"(?<!\\):")
+# split alongside :, unless it's esscaped, or it's preceded by a single capital letter (Windows drive letter in paths)
+ARGS_GROUP = re.compile(r"(?<!\\\\|:[A-Z]):")
 
 
-def replace(value: str, conf: Config, name: Optional[str], loader: "IniLoader") -> str:
+def replace(conf: Config, name: Optional[str], loader: "IniLoader", value: str, chain: List[str]) -> str:
     # perform all non-escaped replaces
     start, end = 0, 0
     while True:
-        start, end, match = _find_replace_part(value, start, end)
+        start, end, match = find_replace_part(value, start, end)
         if not match:
             break
         to_replace = value[start + 1 : end]
-        replaced = _replace_match(conf, name, loader, to_replace)
+        replaced = _replace_match(conf, name, loader, to_replace, chain.copy())
         if replaced is None:
             # if we cannot replace, keep what was there, and continue looking for additional replaces following
             # note, here we cannot raise because the content may be a factorial expression, and in those case we don't
@@ -47,7 +49,7 @@ def replace(value: str, conf: Config, name: Optional[str], loader: "IniLoader")
     return value
 
 
-def _find_replace_part(value: str, start: int, end: int) -> Tuple[int, int, bool]:
+def find_replace_part(value: str, start: int, end: int) -> Tuple[int, int, bool]:
     match = False
     while end != -1:
         end = value.find("}", end)
@@ -68,16 +70,20 @@ def _find_replace_part(value: str, start: int, end: int) -> Tuple[int, int, bool
     return start, end, match
 
 
-def _replace_match(conf: Config, current_env: Optional[str], loader: "IniLoader", value: str) -> Optional[str]:
+def _replace_match(
+    conf: Config, current_env: Optional[str], loader: "IniLoader", value: str, chain: List[str]
+) -> Optional[str]:
     of_type, *args = ARGS_GROUP.split(value)
-    if of_type == "env":
-        replace_value: Optional[str] = replace_env(args)
+    if of_type == "/":
+        replace_value: Optional[str] = os.sep
+    elif of_type == "env":
+        replace_value = replace_env(conf, current_env, args, chain)
     elif of_type == "tty":
         replace_value = replace_tty(args)
     elif of_type == "posargs":
         replace_value = replace_pos_args(args, conf.pos_args)
     else:
-        replace_value = replace_reference(conf, current_env, loader, value)
+        replace_value = replace_reference(conf, current_env, loader, value, chain)
     return replace_value
 
 
@@ -96,6 +102,7 @@ def replace_reference(
     current_env: Optional[str],
     loader: "IniLoader",
     value: str,
+    chain: List[str],
 ) -> Optional[str]:
     # a return value of None indicates could not replace
     match = _REPLACE_REF.match(value)
@@ -112,7 +119,7 @@ def replace_reference(
                 try:
                     if isinstance(src, SectionProxy):
                         return src[key]
-                    value = src[key]
+                    value = src.load(key, chain)
                     as_str, _ = stringify(value)
                     return as_str
                 except KeyError as exc:  # if fails, keep trying maybe another source can satisfy
@@ -172,10 +179,24 @@ def replace_pos_args(args: List[str], pos_args: Optional[Sequence[str]]) -> str:
     return replace_value
 
 
-def replace_env(args: List[str]) -> str:
+def replace_env(conf: Config, env_name: Optional[str], args: List[str], chain: List[str]) -> str:
     key = args[0]
-    default = "" if len(args) == 1 else args[1]
-    return os.environ.get(key, default)
+    new_key = f"env:{key}"
+
+    if env_name is not None:  # on core no set env support # pragma: no branch
+        if new_key not in chain:  # check if set env
+            chain.append(new_key)
+            env_conf = conf.get_env(env_name)
+            set_env: SetEnv = env_conf["set_env"]
+            if key in set_env:
+                return set_env.load(key, chain)
+        elif chain[-1] != new_key:  # if there's a chain but only self-refers than use os.environ
+            raise ValueError(f"circular chain between set env {', '.join(i[4:] for i in chain[chain.index(new_key):])}")
+
+    if key in os.environ:
+        return os.environ[key]
+
+    return "" if len(args) == 1 else args[1]
 
 
 def replace_tty(args: List[str]) -> str:
@@ -190,4 +211,5 @@ def replace_tty(args: List[str]) -> str:
     "CORE_PREFIX",
     "BASE_TEST_ENV",
     "replace",
+    "find_replace_part",
 )

src/tox/config/loader/api.py

@@ -1,10 +1,9 @@
 """Convert string configuration values to tox python configuration objects."""
-import re
 import shlex
 import sys
 from itertools import chain
 from pathlib import Path
-from typing import Any, Iterator, Tuple, Type
+from typing import Any, Iterator, List, Tuple, Type
 
 from tox.config.loader.convert import Convert
 from tox.config.types import Command, EnvList
@@ -51,18 +50,21 @@ def to_dict(value: str, of_type: Tuple[Type[Any], Type[Any]]) -> Iterator[Tuple[
 
     @staticmethod
     def to_command(value: str) -> Command:
-        win = sys.platform == "win32"
-        splitter = shlex.shlex(value, posix=not win)
+        is_win = sys.platform == "win32"
+        splitter = shlex.shlex(value, posix=not is_win)
         splitter.whitespace_split = True
-        if win:  # pragma: win32 cover
-            args = []
+        if is_win:  # pragma: win32 cover
+            args: List[str] = []
             for arg in splitter:
-                if arg[0] == "'" and arg[-1] == "'":  # remove outer quote - the arg is passed as one, so no need for it
+                # on Windows quoted arguments will remain quoted, strip it
+                if (
+                    len(arg) > 1
+                    and (arg.startswith('"') and arg.endswith('"'))
+                    or (arg.startswith("'") and arg.endswith("'"))
+                ):
                     arg = arg[1:-1]
-                if "/" in arg:  # normalize posix paths to nt paths
-                    arg = "\\".join(re.split(pattern=r"[\\/]", string=arg))
                 args.append(arg)
-        else:  # pragma: win32 no cover
+        else:
             args = list(splitter)
         return Command(args)
 

src/tox/config/loader/ini/__init__.py

@@ -2,6 +2,7 @@
 from pathlib import Path
 from typing import Any, Mapping, Sequence, Set, Tuple
 
+from tox.config.set_env import SetEnv
 from tox.config.types import Command, EnvList
 
 
@@ -26,6 +27,8 @@ def stringify(value: Any) -> Tuple[str, bool]:
         return "\n".join(e for e in value.envs), True
     if isinstance(value, Command):
         return value.shell, True
+    if isinstance(value, SetEnv):
+        return stringify({k: value.load(k) for k in sorted(list(value))})
     return str(value), False