merge default

4 files changed, 288 insertions, 147 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 37c99ca..1358ca6 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,12 @@
 2.3.0 (unreleased)
 -----
 
+- 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. Thanks Nelfin for some tests and initial
+  work on a PR.
+
 - allow "#" in commands.  This is slightly incompatible with commands
   sections that used a comment after a "\" line continuation.
   Thanks David Stanek for the PR.
diff --git a/tests/test_config.py b/tests/test_config.py
index a9a97b8..f727a1f 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -283,18 +283,9 @@ class TestIniParserAgainstCommandsKey:
            commands =
              ls {env:TEST}
         """)
-        reader = SectionReader("testenv:py27", config._cfg)
-        x = reader.getargvlist("commands")
-        assert x == [
-            "ls testvalue".split()
-        ]
-        assert x != [
-            "ls {env:TEST}".split()
-        ]
-        y = reader.getargvlist("setenv")
-        assert y == [
-            "TEST=testvalue".split()
-        ]
+        envconfig = config.envconfigs["py27"]
+        assert envconfig.commands == [["ls", "testvalue"]]
+        assert envconfig.setenv["TEST"] == "testvalue"
 
 
 class TestIniParser:
@@ -653,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.
@@ -744,46 +735,6 @@ class TestConfigTestEnv:
         if bp == "jython":
             assert envconfig.envpython == envconfig.envbindir.join(bp)
 
-    def test_setenv_overrides(self, tmpdir, newconfig):
-        config = newconfig("""
-            [testenv]
-            setenv =
-                PYTHONPATH = something
-                ANOTHER_VAL=else
-        """)
-        assert len(config.envconfigs) == 1
-        envconfig = config.envconfigs['python']
-        assert 'PYTHONPATH' in envconfig.setenv
-        assert 'ANOTHER_VAL' in envconfig.setenv
-        assert envconfig.setenv['PYTHONPATH'] == 'something'
-        assert envconfig.setenv['ANOTHER_VAL'] == 'else'
-
-    def test_setenv_with_envdir_and_basepython(self, tmpdir, newconfig):
-        config = newconfig("""
-            [testenv]
-            setenv =
-                VAL = {envdir}
-            basepython = {env:VAL}
-        """)
-        assert len(config.envconfigs) == 1
-        envconfig = config.envconfigs['python']
-        assert 'VAL' in envconfig.setenv
-        assert envconfig.setenv['VAL'] == envconfig.envdir
-        assert envconfig.basepython == envconfig.envdir
-
-    def test_setenv_ordering_1(self, tmpdir, newconfig):
-        config = newconfig("""
-            [testenv]
-            setenv=
-                VAL={envdir}
-            commands=echo {env:VAL}
-        """)
-        assert len(config.envconfigs) == 1
-        envconfig = config.envconfigs['python']
-        assert 'VAL' in envconfig.setenv
-        assert envconfig.setenv['VAL'] == envconfig.envdir
-        assert str(envconfig.envdir) in envconfig.commands[0]
-
     @pytest.mark.parametrize("plat", ["win32", "linux2"])
     def test_passenv_as_multiline_list(self, tmpdir, newconfig, monkeypatch, plat):
         monkeypatch.setattr(sys, "platform", plat)
@@ -1525,7 +1476,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)
@@ -1574,7 +1525,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 = """
@@ -1618,6 +1569,125 @@ class TestHashseedOption:
         self._check_hashseed(envconfigs["hash2"], '123456789')
 
 
+class TestSetenv:
+    def test_getdict_lazy(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")
+        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")
+        config = newconfig("""
+            [testenv:env1]
+            setenv =
+                X = {env:X}
+        """)
+        assert config.envconfigs["env1"].setenv["X"] == "1"
+
+    def test_setenv_default_os_environ(self, tmpdir, newconfig, monkeypatch):
+        monkeypatch.delenv("X", raising=False)
+        config = newconfig("""
+            [testenv:env1]
+            setenv =
+                X = {env:X:2}
+        """)
+        assert config.envconfigs["env1"].setenv["X"] == "2"
+
+    def test_setenv_uses_other_setenv(self, tmpdir, newconfig):
+        config = newconfig("""
+            [testenv:env1]
+            setenv =
+                Y = 5
+                X = {env:Y}
+        """)
+        assert config.envconfigs["env1"].setenv["X"] == "5"
+
+    def test_setenv_recursive_direct(self, tmpdir, newconfig):
+        config = newconfig("""
+            [testenv:env1]
+            setenv =
+                X = {env:X:3}
+        """)
+        assert config.envconfigs["env1"].setenv["X"] == "3"
+
+    def test_setenv_overrides(self, tmpdir, newconfig):
+        config = newconfig("""
+            [testenv]
+            setenv =
+                PYTHONPATH = something
+                ANOTHER_VAL=else
+        """)
+        assert len(config.envconfigs) == 1
+        envconfig = config.envconfigs['python']
+        assert 'PYTHONPATH' in envconfig.setenv
+        assert 'ANOTHER_VAL' in envconfig.setenv
+        assert envconfig.setenv['PYTHONPATH'] == 'something'
+        assert envconfig.setenv['ANOTHER_VAL'] == 'else'
+
+    def test_setenv_with_envdir_and_basepython(self, tmpdir, newconfig):
+        config = newconfig("""
+            [testenv]
+            setenv =
+                VAL = {envdir}
+            basepython = {env:VAL}
+        """)
+        assert len(config.envconfigs) == 1
+        envconfig = config.envconfigs['python']
+        assert 'VAL' in envconfig.setenv
+        assert envconfig.setenv['VAL'] == envconfig.envdir
+        assert envconfig.basepython == envconfig.envdir
+
+    def test_setenv_ordering_1(self, tmpdir, newconfig):
+        config = newconfig("""
+            [testenv]
+            setenv=
+                VAL={envdir}
+            commands=echo {env:VAL}
+        """)
+        assert len(config.envconfigs) == 1
+        envconfig = config.envconfigs['python']
+        assert 'VAL' in envconfig.setenv
+        assert envconfig.setenv['VAL'] == envconfig.envdir
+        assert str(envconfig.envdir) in envconfig.commands[0]
+
+    @pytest.mark.xfail(reason="we don't implement cross-section substitution for setenv")
+    def test_setenv_cross_section_subst(self, monkeypatch, newconfig):
+        """test that we can do cross-section substitution with setenv"""
+        monkeypatch.delenv('TEST', raising=False)
+        config = newconfig("""
+            [section]
+            x =
+              NOT_TEST={env:TEST:defaultvalue}
+
+            [testenv]
+            setenv = {[section]x}
+        """)
+        envconfig = config.envconfigs["python"]
+        assert envconfig.setenv["NOT_TEST"] == "defaultvalue"
+
+
 class TestIndexServer:
     def test_indexserver(self, tmpdir, newconfig):
         config = newconfig("""
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 2caeb1a..d34a597 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(name)
+                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)
@@ -323,11 +366,22 @@ def tox_addoption(parser):
     parser.add_argument("args", nargs="*",
                         help="additional arguments available to command positional substitution")
 
-    # add various core venv interpreter attributes
     parser.add_testenv_attribute(
         name="envdir", type="path", default="{toxworkdir}/{envname}",
         help="venv directory")
 
+    # add various core venv interpreter attributes
+    def setenv(testenv_config, value):
+        setenv = value
+        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_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:
@@ -385,17 +439,6 @@ def tox_addoption(parser):
         name="recreate", type="bool", default=False, postprocess=recreate,
         help="always recreate this test environment.")
 
-    def setenv(testenv_config, value):
-        setenv = value
-        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", postprocess=setenv,
-        help="list of X=Y lines with environment variable settings")
-
     def passenv(testenv_config, value):
         # Flatten the list to deal with space-separated values.
         value = list(
@@ -515,8 +558,7 @@ 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"
                 and "jython" not in self.basepython
@@ -526,7 +568,15 @@ class TestenvConfig:
             return self.envdir.join("bin")
 
     @property
+    def envbindir(self):
+        return self.get_envbindir()
+
+    @property
     def envpython(self):
+        """ path to python executable. """
+        return self.get_envpython()
+
+    def get_envpython(self):
         """ path to python/jython executable. """
         if "jython" in str(self.basepython):
             name = "jython"
@@ -534,8 +584,7 @@ class TestenvConfig:
             name = "python"
         return self.envbindir.join(name)
 
-    # no @property to avoid early calling (see callable(subst[key]) checks)
-    def envsitepackagesdir(self):
+    def get_envsitepackagesdir(self):
         """ return sitepackagesdir of the virtualenv environment.
         (only available during execution, not parsing)
         """
@@ -696,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", "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":
@@ -716,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):
@@ -799,16 +848,6 @@ class IndexServerConfig:
 is_section_substitution = re.compile("{\[[^{}\s]+\]\S+?}").match
 
 
-RE_ITEM_REF = re.compile(
-    r'''
-    (?<!\\)[{]
-    (?:(?P<sub_type>[^[:{}]+):)?    # optional sub_type for special rules
-    (?P<substitution_value>[^{}]*)  # substitution key
-    [}]
-    ''',
-    re.VERBOSE)
-
-
 class SectionReader:
     def __init__(self, section_name, cfgparser, fallbacksections=None, factors=()):
         self.section_name = section_name
@@ -817,6 +856,12 @@ class SectionReader:
         self.factors = factors
         self._subs = {}
         self._subststack = []
+        self._setenv = None
+
+    def get_environ_value(self, 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)
@@ -836,17 +881,26 @@ class SectionReader:
         return [x.strip() for x in s.split(sep) if x.strip()]
 
     def getdict(self, name, default=None, sep="\n"):
-        s = self.getstring(name, None)
-        if s is None:
+        value = self.getstring(name, None)
+        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:
             return default or {}
 
-        value = {}
-        for line in s.split(sep):
+        d = {}
+        for line in value.split(sep):
             if line.strip():
                 name, rest = line.split('=', 1)
-                value[name.strip()] = rest.strip()
+                d[name.strip()] = rest.strip()
 
-        return value
+        return d
 
     def getbool(self, name, default=None):
         s = self.getstring(name, default)
@@ -888,11 +942,7 @@ class SectionReader:
             x = self._apply_factors(x)
 
         if replace and x and hasattr(x, 'replace'):
-            self._subststack.append((self.section_name, name))
-            try:
-                x = self._replace(x)
-            finally:
-                assert self._subststack.pop() == (self.section_name, name)
+            x = self._replace(x, name=name)
         # print "getstring", self.section_name, name, "returned", repr(x)
         return x
 
@@ -909,8 +959,58 @@ class SectionReader:
         lines = s.strip().splitlines()
         return '\n'.join(filter(None, map(factor_line, lines)))
 
+    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).do_replace(value)
+        finally:
+            assert self._subststack.pop() == (section_name, name)
+
+
+class Replacer:
+    RE_ITEM_REF = re.compile(
+        r'''
+        (?<!\\)[{]
+        (?:(?P<sub_type>[^[:{}]+):)?    # optional sub_type for special rules
+        (?P<substitution_value>[^{}]*)  # substitution key
+        [}]
+        ''',
+        re.VERBOSE)
+
+    def __init__(self, reader):
+        self.reader = reader
+
+    def do_replace(self, x):
+        return self.RE_ITEM_REF.sub(self._replace_match, x)
+
+    def _replace_match(self, match):
+        g = match.groupdict()
+
+        # special case: opts and packages. Leave {opts} and
+        # {packages} intact, they are replaced manually in
+        # _venv.VirtualEnv.run_install_command.
+        sub_value = g['substitution_value']
+        if sub_value in ('opts', 'packages'):
+            return '{%s}' % sub_value
+
+        try:
+            sub_type = g['sub_type']
+        except KeyError:
+            raise tox.exception.ConfigError(
+                "Malformed substitution; no substitution type provided")
+
+        if sub_type == "env":
+            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):
-        env_list = self.getdict('setenv')
         match_value = match.group('substitution_value')
         if not match_value:
             raise tox.exception.ConfigError(
@@ -924,75 +1024,40 @@ class SectionReader:
         else:
             envkey = match_value
 
-        if envkey not in os.environ and default is None:
-            if envkey not in env_list and default is None:
+        envvalue = self.reader.get_environ_value(envkey)
+        if envvalue is None:
+            if default is None:
                 raise tox.exception.ConfigError(
-                    "substitution env:%r: unknown environment variable %r" %
+                    "substitution env:%r: unknown environment variable %r "
+                    " or recursive definition." %
                     (envkey, envkey))
-        if envkey in os.environ:
-            return os.environ.get(envkey, default)
-        else:
-            return env_list.get(envkey, default)
+            return default
+        return envvalue
 
     def _substitute_from_other_section(self, key):
         if key.startswith("[") and "]" in key:
             i = key.find("]")
             section, item = key[1:i], key[i + 1:]
-            if section in self._cfg and item in self._cfg[section]:
-                if (section, item) in self._subststack:
+            cfg = self.reader._cfg
+            if section in cfg and item in cfg[section]:
+                if (section, item) in self.reader._subststack:
                     raise ValueError('%s already in %s' % (
-                        (section, item), self._subststack))
-                x = str(self._cfg[section][item])
-                self._subststack.append((section, item))
-                try:
-                    return self._replace(x)
-                finally:
-                    self._subststack.pop()
+                        (section, item), self.reader._subststack))
+                x = str(cfg[section][item])
+                return self.reader._replace(x, name=item, section_name=section)
 
         raise tox.exception.ConfigError(
             "substitution key %r not found" % key)
 
     def _replace_substitution(self, match):
         sub_key = match.group('substitution_value')
-        val = self._subs.get(sub_key, None)
+        val = self.reader._subs.get(sub_key, None)
         if val is None:
             val = self._substitute_from_other_section(sub_key)
         if py.builtin.callable(val):
             val = val()
         return str(val)
 
-    def _replace_match(self, match):
-        g = match.groupdict()
-
-        # special case: opts and packages. Leave {opts} and
-        # {packages} intact, they are replaced manually in
-        # _venv.VirtualEnv.run_install_command.
-        sub_value = g['substitution_value']
-        if sub_value in ('opts', 'packages'):
-            return '{%s}' % sub_value
-
-        handlers = {
-            'env': self._replace_env,
-            None: self._replace_substitution,
-        }
-        try:
-            sub_type = g['sub_type']
-        except KeyError:
-            raise tox.exception.ConfigError(
-                "Malformed substitution; no substitution type provided")
-
-        try:
-            handler = handlers[sub_type]
-        except KeyError:
-            raise tox.exception.ConfigError("No support for the %s substitution type" % sub_type)
-
-        return handler(match)
-
-    def _replace(self, x):
-        if '{' in x:
-            return RE_ITEM_REF.sub(self._replace_match, x)
-        return x
-
 
 class _ArgvlistReader:
     @classmethod