refactor setenv processing into its own class so that

we can cleanly implement lazyness and get rid of
all kinds of ordering problems.

4 files changed, 111 insertions, 60 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f3b30ec..1a395fc 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,7 +1,10 @@
 2.3.0 (unreleased)
 -----
 
-- fix issue285 (WIP) setenv processing with self-references
+- fix issue285 make setenv processing fully lazy to fix regressions
+  of tox-2.2.X and so that we can now have testenv attributes like 
+  "basepython" depend on environment variables that are set in 
+  a setenv section.
 
 - allow "#" in commands.  This is slightly incompatible with commands
   sections that used a comment after a "\" line continuation.
diff --git a/tests/test_config.py b/tests/test_config.py
index 462e33c..573fdfe 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -644,7 +644,7 @@ class TestConfigTestEnv:
         assert envconfig.usedevelop is False
         assert envconfig.ignore_errors is False
         assert envconfig.envlogdir == envconfig.envdir.join("log")
-        assert list(envconfig.setenv.keys()) == ['PYTHONHASHSEED']
+        assert list(envconfig.setenv.definitions.keys()) == ['PYTHONHASHSEED']
         hashseed = envconfig.setenv['PYTHONHASHSEED']
         assert isinstance(hashseed, str)
         # The following line checks that hashseed parses to an integer.
@@ -1516,7 +1516,7 @@ class TestHashseedOption:
         return envconfigs["python"]
 
     def _check_hashseed(self, envconfig, expected):
-        assert envconfig.setenv == {'PYTHONHASHSEED': expected}
+        assert envconfig.setenv['PYTHONHASHSEED'] == expected
 
     def _check_testenv(self, newconfig, expected, args=None, tox_ini=None):
         envconfig = self._get_envconfig(newconfig, args=args, tox_ini=tox_ini)
@@ -1565,7 +1565,7 @@ class TestHashseedOption:
     def test_noset(self, tmpdir, newconfig):
         args = ['--hashseed', 'noset']
         envconfig = self._get_envconfig(newconfig, args=args)
-        assert envconfig.setenv == {}
+        assert not envconfig.setenv.definitions
 
     def test_noset_with_setenv(self, tmpdir, newconfig):
         tox_ini = """
@@ -1610,18 +1610,32 @@ class TestHashseedOption:
 
 
 class TestSetenv:
-    def test_getdict_lazy(self, tmpdir, newconfig):
+    def test_getdict_lazy(self, tmpdir, newconfig, monkeypatch):
+        monkeypatch.setenv("X", "2")
         config = newconfig("""
             [testenv:X]
             key0 =
                 key1 = {env:X}
-                key2 = {env:X:1}
+                key2 = {env:Y:1}
         """)
         envconfig = config.envconfigs["X"]
-        val = envconfig._reader.getdict_lazy("key0")
-        assert val == {"key1": "{env:X}",
-                       "key2": "{env:X:1}"}
+        val = envconfig._reader.getdict_setenv("key0")
+        assert val["key1"] == "2"
+        assert val["key2"] == "1"
 
+    def test_getdict_lazy_update(self, tmpdir, newconfig, monkeypatch):
+        monkeypatch.setenv("X", "2")
+        config = newconfig("""
+            [testenv:X]
+            key0 =
+                key1 = {env:X}
+                key2 = {env:Y:1}
+        """)
+        envconfig = config.envconfigs["X"]
+        val = envconfig._reader.getdict_setenv("key0")
+        d = {}
+        d.update(val)
+        assert d == {"key1": "2", "key2": "1"}
 
     def test_setenv_uses_os_environ(self, tmpdir, newconfig, monkeypatch):
         monkeypatch.setenv("X", "1")
diff --git a/tox.ini b/tox.ini
index d78bcbd..e79b882 100644
--- a/tox.ini
+++ b/tox.ini
@@ -5,7 +5,7 @@ envlist=py27,py26,py34,py33,pypy,flakes,py26-bare
 commands=echo {posargs}
 
 [testenv]
-commands= py.test --timeout=180 {posargs}
+commands= py.test --timeout=180 {posargs:tests}
 
 deps=pytest>=2.3.5
     pytest-timeout
diff --git a/tox/config.py b/tox/config.py
index 480704b..4fa0a0a 100644
--- a/tox/config.py
+++ b/tox/config.py
@@ -26,6 +26,8 @@ for version in '26,27,32,33,34,35,36'.split(','):
 
 hookimpl = pluggy.HookimplMarker("tox")
 
+_dummy = object()
+
 
 def get_plugin_manager():
     # initialize plugin manager
@@ -253,6 +255,47 @@ class CountAction(argparse.Action):
             setattr(namespace, self.dest, 0)
 
 
+class SetenvDict:
+    def __init__(self, dict, reader):
+        self.reader = reader
+        self.definitions = dict
+        self.resolved = {}
+        self._lookupstack = []
+
+    def __contains__(self, name):
+        return name in self.definitions
+
+    def get(self, name, default=None):
+        try:
+            return self.resolved[name]
+        except KeyError:
+            try:
+                if name in self._lookupstack:
+                    raise KeyError("recursion")
+                val = self.definitions[name]
+            except KeyError:
+                return os.environ.get(name, default)
+            self._lookupstack.append(name)
+            try:
+                self.resolved[name] = res = self.reader._replace(val)
+            finally:
+                self._lookupstack.pop()
+            return res
+
+    def __getitem__(self, name):
+        x = self.get(name, _dummy)
+        if x is _dummy:
+            raise KeyError(name)
+        return x
+
+    def keys(self):
+        return self.definitions.keys()
+
+    def __setitem__(self, name, value):
+        self.definitions[name] = value
+        self.resolved[name] = value
+
+
 @hookimpl
 def tox_addoption(parser):
     # formatter_class=argparse.ArgumentDefaultsHelpFormatter)
@@ -330,32 +373,15 @@ def tox_addoption(parser):
     # add various core venv interpreter attributes
     def setenv(testenv_config, value):
         setenv = value
-        reader = testenv_config._reader
-
-        # we need to resolve environment variable substitution
-
-        replacing = []  # for detecting direct recursion
-        def setenv_reader(name):
-            if name in setenv and name not in replacing:
-                return setenv[name]
-            return os.environ.get(name)
-        reader.set_envreader(setenv_reader)
-
-        for name, value in setenv.items():
-            replacing.append(name)
-            setenv[name] = reader._replace(value)
-            replacing.pop()
-
         config = testenv_config.config
         if "PYTHONHASHSEED" not in setenv and config.hashseed is not None:
             setenv['PYTHONHASHSEED'] = config.hashseed
         return setenv
 
     parser.add_testenv_attribute(
-        name="setenv", type="dict_lazy", postprocess=setenv,
+        name="setenv", type="dict_setenv", postprocess=setenv,
         help="list of X=Y lines with environment variable settings")
 
-
     def basepython_default(testenv_config, value):
         if value is None:
             for f in testenv_config.factors:
@@ -532,21 +558,33 @@ class TestenvConfig:
         self.factors = factors
         self._reader = reader
 
-    @property
-    def envbindir(self):
+    def get_envbindir(self):
         """ path to directory where scripts/binaries reside. """
-        if sys.platform == "win32":
+        if (sys.platform == "win32"
+                and "jython" not in self.basepython
+                and "pypy" not in self.basepython):
             return self.envdir.join("Scripts")
         else:
             return self.envdir.join("bin")
 
     @property
+    def envbindir(self):
+        return self.get_envbindir()
+
+    @property
     def envpython(self):
         """ path to python executable. """
-        return self.envbindir.join(self.basepython)
+        return self.get_envpython()
 
-    # no @property to avoid early calling (see callable(subst[key]) checks)
-    def envsitepackagesdir(self):
+    def get_envpython(self):
+        """ path to python/jython executable. """
+        if "jython" in str(self.basepython):
+            name = "jython"
+        else:
+            name = "python"
+        return self.envbindir.join(name)
+
+    def get_envsitepackagesdir(self):
         """ return sitepackagesdir of the virtualenv environment.
         (only available during execution, not parsing)
         """
@@ -707,10 +745,13 @@ class parseini:
         vc = TestenvConfig(config=config, envname=name, factors=factors, reader=reader)
         reader.addsubstitutions(**subs)
         reader.addsubstitutions(envname=name)
+        reader.addsubstitutions(envbindir=vc.get_envbindir,
+                                envsitepackagesdir=vc.get_envsitepackagesdir,
+                                envpython=vc.get_envpython)
 
         for env_attr in config._testenv_attr:
             atype = env_attr.type
-            if atype in ("bool", "path", "string", "dict", "dict_lazy", "argv", "argvlist"):
+            if atype in ("bool", "path", "string", "dict", "dict_setenv", "argv", "argvlist"):
                 meth = getattr(reader, "get" + atype)
                 res = meth(env_attr.name, env_attr.default)
             elif atype == "space-separated-list":
@@ -727,9 +768,6 @@ class parseini:
             if atype == "path":
                 reader.addsubstitutions(**{env_attr.name: res})
 
-            if env_attr.name == "basepython":
-                reader.addsubstitutions(envbindir=vc.envbindir, envpython=vc.envpython,
-                                        envsitepackagesdir=vc.envsitepackagesdir)
         return vc
 
     def _getenvdata(self, reader):
@@ -818,13 +856,12 @@ class SectionReader:
         self.factors = factors
         self._subs = {}
         self._subststack = []
-        self._envreader = os.environ.get
-
-    def set_envreader(self, envreader):
-        self._envreader = envreader
+        self._setenv = None
 
     def get_environ_value(self, name):
-        return self._envreader(name)
+        if self._setenv is None:
+            return os.environ.get(name)
+        return self._setenv.get(name)
 
     def addsubstitutions(self, _posargs=None, **kw):
         self._subs.update(kw)
@@ -847,9 +884,11 @@ class SectionReader:
         value = self.getstring(name, None)
         return self._getdict(value, default=default, sep=sep)
 
-    def getdict_lazy(self, name, default=None, sep="\n"):
-        value = self.getstring(name, None, replace="noenv")
-        return self._getdict(value, default=default, sep=sep)
+    def getdict_setenv(self, name, default=None, sep="\n"):
+        value = self.getstring(name, None, replace=False)
+        definitions = self._getdict(value, default=default, sep=sep)
+        self._setenv = SetenvDict(definitions, reader=self)
+        return self._setenv
 
     def _getdict(self, value, default, sep):
         if value is None:
@@ -903,7 +942,7 @@ class SectionReader:
             x = self._apply_factors(x)
 
         if replace and x and hasattr(x, 'replace'):
-            x = self._replace(x, name=name, opt_replace_env=(replace!="noenv"))
+            x = self._replace(x, name=name)
         # print "getstring", self.section_name, name, "returned", repr(x)
         return x
 
@@ -920,14 +959,14 @@ class SectionReader:
         lines = s.strip().splitlines()
         return '\n'.join(filter(None, map(factor_line, lines)))
 
-    def _replace(self, value, name=None, section_name=None, opt_replace_env=True):
+    def _replace(self, value, name=None, section_name=None):
         if '{' not in value:
             return value
 
         section_name = section_name if section_name else self.section_name
         self._subststack.append((section_name, name))
         try:
-            return Replacer(self, opt_replace_env=opt_replace_env).do_replace(value)
+            return Replacer(self).do_replace(value)
         finally:
             assert self._subststack.pop() == (section_name, name)
 
@@ -942,10 +981,8 @@ class Replacer:
         ''',
         re.VERBOSE)
 
-
-    def __init__(self, reader, opt_replace_env):
+    def __init__(self, reader):
         self.reader = reader
-        self.opt_replace_env = opt_replace_env
 
     def do_replace(self, x):
         return self.RE_ITEM_REF.sub(self._replace_match, x)
@@ -967,11 +1004,10 @@ class Replacer:
                 "Malformed substitution; no substitution type provided")
 
         if sub_type == "env":
-            if self.opt_replace_env:
-                return self._replace_env(match)
-            return "{env:%s}" %(g["substitution_value"])
-        if sub_type != None:
-            raise tox.exception.ConfigError("No support for the %s substitution type" % sub_type)
+            return self._replace_env(match)
+        if sub_type is not None:
+            raise tox.exception.ConfigError(
+                "No support for the %s substitution type" % sub_type)
         return self._replace_substitution(match)
 
     def _replace_env(self, match):
@@ -1007,8 +1043,7 @@ class Replacer:
                     raise ValueError('%s already in %s' % (
                         (section, item), self.reader._subststack))
                 x = str(cfg[section][item])
-                return self.reader._replace(x, name=item, section_name=section,
-                                            opt_replace_env=self.opt_replace_env)
+                return self.reader._replace(x, name=item, section_name=section)
 
         raise tox.exception.ConfigError(
             "substitution key %r not found" % key)
@@ -1023,7 +1058,6 @@ class Replacer:
         return str(val)
 
 
-
 class _ArgvlistReader:
     @classmethod
     def getargvlist(cls, reader, value):