Quote setting paths containing pound and space

Author: jayvdb

docs/changelog/763.feature.rst

@@ -0,0 +1 @@
+Quote paths in settings containing ` ` or `#`. - by :user:`jayvdb`

src/tox/config/__init__.py

@@ -20,6 +20,7 @@
 import toml
 from packaging import requirements
 from packaging.utils import canonicalize_name
+from py._path.common import PathBase
 
 import tox
 from tox.constants import INFO
@@ -390,7 +391,7 @@ def get(self, name, default=None):
                 return os.environ.get(name, default)
             self._lookupstack.append(name)
             try:
-                res = self.reader._replace(val)
+                res = self.reader._replace(val, unquote_path=False)
                 res = res.replace("\\{", "{").replace("\\}", "}")
                 self.resolved[name] = res
             finally:
@@ -1591,7 +1592,7 @@ def addsubstitutions(self, _posargs=None, **kw):
             self.posargs = _posargs
 
     def getpath(self, name, defaultpath, replace=True):
-        path = self.getstring(name, defaultpath, replace=replace)
+        path = self.getstring(name, defaultpath, replace=replace, unquote_path=True)
         if path is not None:
             toxinidir = self._subs["toxinidir"]
             return toxinidir.join(path, abs=True)
@@ -1680,7 +1681,15 @@ def getargv_install_command(self, name, default="", replace=True):
 
         return _ArgvlistReader.getargvlist(self, s, replace=replace)[0]
 
-    def getstring(self, name, default=None, replace=True, crossonly=False, no_fallback=False):
+    def getstring(
+        self,
+        name,
+        default=None,
+        replace=True,
+        crossonly=False,
+        no_fallback=False,
+        unquote_path=False,
+    ):
         x = None
         sections = [self.section_name] + ([] if no_fallback else self.fallbacksections)
         for s in sections:
@@ -1698,10 +1707,10 @@ def getstring(self, name, default=None, replace=True, crossonly=False, no_fallba
             # process. Once they are unwrapped, we call apply factors
             # again for those new dependencies.
             x = self._apply_factors(x)
-            x = self._replace_if_needed(x, name, replace, crossonly)
+            x = self._replace_if_needed(x, name, replace, crossonly, unquote_path=unquote_path)
             x = self._apply_factors(x)
 
-        x = self._replace_if_needed(x, name, replace, crossonly)
+        x = self._replace_if_needed(x, name, replace, crossonly, unquote_path=unquote_path)
         return x
 
     def getposargs(self, default=None):
@@ -1715,9 +1724,9 @@ def getposargs(self, default=None):
         else:
             return default or ""
 
-    def _replace_if_needed(self, x, name, replace, crossonly):
+    def _replace_if_needed(self, x, name, replace, crossonly, unquote_path=False):
         if replace and x and hasattr(x, "replace"):
-            x = self._replace(x, name=name, crossonly=crossonly)
+            x = self._replace(x, name=name, crossonly=crossonly, unquote_path=unquote_path)
         return x
 
     def _apply_factors(self, s):
@@ -1736,14 +1745,17 @@ def factor_line(line):
         lines = s.strip().splitlines()
         return "\n".join(filter(None, map(factor_line, lines)))
 
-    def _replace(self, value, name=None, section_name=None, crossonly=False):
+    def _replace(self, value, name=None, section_name=None, crossonly=False, unquote_path=False):
         if "{" not in value:
             return value
 
         section_name = section_name if section_name else self.section_name
         self._subststack.append((section_name, name))
         try:
-            replaced = Replacer(self, crossonly=crossonly).do_replace(value)
+            replacer = Replacer(self, crossonly=crossonly)
+            replaced = replacer.do_replace(value)
+            if unquote_path and replacer._path_quoted:
+                replaced = replaced.replace("'", "")
             assert self._subststack.pop() == (section_name, name)
         except tox.exception.MissingSubstitution:
             if not section_name.startswith(testenvprefix):
@@ -1770,6 +1782,7 @@ class Replacer:
     def __init__(self, reader, crossonly=False):
         self.reader = reader
         self.crossonly = crossonly
+        self._path_quoted = False
 
     def do_replace(self, value):
         """
@@ -1853,6 +1866,7 @@ def _substitute_from_other_section(self, key):
                     name=item,
                     section_name=section,
                     crossonly=self.crossonly,
+                    unquote_path=False,
                 )
 
         raise tox.exception.ConfigError("substitution key {!r} not found".format(key))
@@ -1864,6 +1878,12 @@ def _replace_substitution(self, match):
             val = self._substitute_from_other_section(sub_key)
         if callable(val):
             val = val()
+        if isinstance(val, PathBase):
+            val = str(val)
+            # XXX handle ' and " in paths
+            if "'" not in val and ("#" in val or " " in val):
+                val = "'{}'".format(val)
+                self._path_quoted = True
         return str(val)
 
 
@@ -1895,7 +1915,7 @@ def getargvlist(cls, reader, value, replace=True):
             current_command += line
 
             if is_section_substitution(current_command):
-                replaced = reader._replace(current_command, crossonly=True)
+                replaced = reader._replace(current_command, crossonly=True, unquote_path=False)
                 commands.extend(cls.getargvlist(reader, replaced))
             else:
                 commands.append(cls.processcommand(reader, current_command, replace))
@@ -1924,8 +1944,8 @@ def processcommand(cls, reader, command, replace=True):
                     continue
 
                 new_arg = ""
-                new_word = reader._replace(word)
-                new_word = reader._replace(new_word)
+                new_word = reader._replace(word, unquote_path=False)
+                new_word = reader._replace(new_word, unquote_path=False)
                 new_word = new_word.replace("\\{", "{").replace("\\}", "}")
                 new_arg += new_word
                 newcommand += new_arg

tests/unit/config/test_config.py

@@ -91,6 +91,28 @@ def test_envdir_set_manually_with_substitutions(self, newconfig):
         envconfig = config.envconfigs["dev"]
         assert envconfig.envdir == config.toxworkdir.join("foobar")
 
+    def test_envdir_set_manually_with_pound(self, newconfig):
+        config = newconfig(
+            [],
+            """
+            [testenv:dev]
+            envdir = {toxworkdir}/foo#bar
+        """,
+        )
+        envconfig = config.envconfigs["dev"]
+        assert envconfig.envdir == config.toxworkdir.join("foo#bar")
+
+    def test_envdir_set_manually_with_whitespace(self, newconfig):
+        config = newconfig(
+            [],
+            """
+            [testenv:dev]
+            envdir = {toxworkdir}/foo bar
+        """,
+        )
+        envconfig = config.envconfigs["dev"]
+        assert envconfig.envdir == config.toxworkdir.join("foo bar")
+
     def test_force_dep_version(self, initproj):
         """
         Make sure we can override dependencies configured in tox.ini when using the command line
@@ -496,6 +518,56 @@ def test_regression_issue595(self, newconfig):
         assert config.envconfigs["bar"].setenv["VAR"] == "x"
         assert "VAR" not in config.envconfigs["baz"].setenv
 
+    def test_command_substitution_pound(self, tmpdir, newconfig):
+        """Ensure pound in path is kept in commands."""
+        config = newconfig(
+            """
+            [tox]
+            toxworkdir = {toxinidir}/.tox#dir
+
+            [testenv:py27]
+            commands = {envpython} {toxworkdir}
+        """,
+        )
+
+        assert config.toxworkdir.realpath() == tmpdir.join(".tox#dir").realpath()
+
+        envconfig = config.envconfigs["py27"]
+
+        assert envconfig.envbindir.realpath() in [
+            tmpdir.join(".tox#dir/py27/bin").realpath(),
+            tmpdir.join(".tox#dir/py27/Scripts").realpath(),
+        ]
+
+        assert envconfig.commands == [
+            [str(envconfig.envbindir.join("python")), str(config.toxworkdir.realpath())]
+        ]
+
+    def test_command_substitution_whitespace(self, tmpdir, newconfig):
+        """Ensure spaces in path is kept in commands."""
+        config = newconfig(
+            """
+            [tox]
+            toxworkdir = {toxinidir}/.tox dir
+
+            [testenv:py27]
+            commands = {envpython} {toxworkdir}
+        """,
+        )
+
+        assert config.toxworkdir.realpath() == tmpdir.join(".tox dir").realpath()
+
+        envconfig = config.envconfigs["py27"]
+
+        assert envconfig.envbindir.realpath() in [
+            tmpdir.join(".tox dir/py27/bin").realpath(),
+            tmpdir.join(".tox dir/py27/Scripts").realpath(),
+        ]
+
+        assert envconfig.commands == [
+            [str(envconfig.envbindir.join("python")), str(config.toxworkdir.realpath())]
+        ]
+
 
 class TestIniParser:
     def test_getstring_single(self, newconfig):
@@ -1084,6 +1156,46 @@ def test_envbindir(self, newconfig):
         envconfig = config.envconfigs["python"]
         assert envconfig.envpython == envconfig.envbindir.join("python")
 
+    def test_envbindir_with_pound(self, newconfig):
+        config = newconfig(
+            """
+            [tox]
+            toxworkdir = {toxinidir}/.tox#dir
+            [testenv]
+            basepython=python
+        """,
+        )
+        assert len(config.envconfigs) == 1
+        envconfig = config.envconfigs["python"]
+
+        assert "/.tox#dir" in str(envconfig.envbindir)
+        assert "/.tox#dir" in str(envconfig.envpython)
+
+        assert "'" not in str(envconfig.envbindir)
+        assert "'" not in str(envconfig.envpython)
+
+        assert envconfig.envpython == envconfig.envbindir.join("python")
+
+    def test_envbindir_with_whitespace(self, newconfig):
+        config = newconfig(
+            """
+            [tox]
+            toxworkdir = {toxinidir}/.tox dir
+            [testenv]
+            basepython=python
+        """,
+        )
+        assert len(config.envconfigs) == 1
+        envconfig = config.envconfigs["python"]
+
+        assert "/.tox dir" in str(envconfig.envbindir)
+        assert "/.tox dir" in str(envconfig.envpython)
+
+        assert "'" not in str(envconfig.envbindir)
+        assert "'" not in str(envconfig.envpython)
+
+        assert envconfig.envpython == envconfig.envbindir.join("python")
+
     @pytest.mark.parametrize("bp", ["jython", "pypy", "pypy3"])
     def test_envbindir_jython(self, newconfig, bp):
         config = newconfig(
@@ -1511,7 +1623,7 @@ def test_rewrite_posargs(self, tmpdir, newconfig):
         argv = conf.commands
         assert argv[0] == ["cmd1", "hello"]
 
-    def test_rewrite_simple_posargs(self, tmpdir, newconfig):
+    def test_rewrite_posargs_simple(self, tmpdir, newconfig):
         inisource = """
             [testenv:py27]
             args_are_paths = True
@@ -1531,6 +1643,46 @@ def test_rewrite_simple_posargs(self, tmpdir, newconfig):
         argv = conf.commands
         assert argv[0] == ["cmd1", "hello"]
 
+    def test_rewrite_posargs_pound(self, tmpdir, newconfig):
+        inisource = """
+            [testenv:py27]
+            args_are_paths = True
+            changedir = test#dir
+            commands = cmd1 {posargs:hello}
+        """
+        conf = newconfig([], inisource).envconfigs["py27"]
+        argv = conf.commands
+        assert argv[0] == ["cmd1", "hello"]
+
+        conf = newconfig(["test#dir/hello"], inisource).envconfigs["py27"]
+        argv = conf.commands
+        assert argv[0] == ["cmd1", "test#dir/hello"]
+
+        tmpdir.ensure("test#dir", "hello")
+        conf = newconfig(["test#dir/hello"], inisource).envconfigs["py27"]
+        argv = conf.commands
+        assert argv[0] == ["cmd1", "hello"]
+
+    def test_rewrite_posargs_whitespace(self, tmpdir, newconfig):
+        inisource = """
+            [testenv:py27]
+            args_are_paths = True
+            changedir = test dir
+            commands = cmd1 {posargs:hello}
+        """
+        conf = newconfig([], inisource).envconfigs["py27"]
+        argv = conf.commands
+        assert argv[0] == ["cmd1", "hello"]
+
+        conf = newconfig(["test dir/hello"], inisource).envconfigs["py27"]
+        argv = conf.commands
+        assert argv[0] == ["cmd1", "test dir/hello"]
+
+        tmpdir.ensure("test dir", "hello")
+        conf = newconfig(["test dir/hello"], inisource).envconfigs["py27"]
+        argv = conf.commands
+        assert argv[0] == ["cmd1", "hello"]
+
     @pytest.mark.parametrize(
         "envlist, deps",
         [