From 03f491116f09cda6c7e4aec06c70118ce8c26898 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Fri, 23 Oct 2020 19:04:06 +0700 Subject: [PATCH] Replacer: Allow LocalPath to pass through Paths were already stored inside SectionReader._subs as path objects, however they are turned into strings when they go through Replacer, and various transforms occur on these paths, for example breaking paths containing '#' and ' '. This change defaults to path objects being retained through the Replacer, and path segments being joined together using path object joining semantics. This mode can be disabled for settings of type `path` by wrapping the value in quotes, and disabled for other values by disabling new global setting literal_paths. Fixes https://github.com/tox-dev/tox/issues/763 and https://github.com/tox-dev/tox/issues/924 --- docs/changelog/1713.feature.rst | 1 + docs/config.rst | 13 ++ src/tox/config/__init__.py | 97 ++++++++++--- tests/unit/config/test_config.py | 238 ++++++++++++++++++++++++++++++- 4 files changed, 330 insertions(+), 19 deletions(-) create mode 100644 docs/changelog/1713.feature.rst diff --git a/docs/changelog/1713.feature.rst b/docs/changelog/1713.feature.rst new file mode 100644 index 000000000..1f843e915 --- /dev/null +++ b/docs/changelog/1713.feature.rst @@ -0,0 +1 @@ +Preserve paths through the substitution engine. Adds global setting ``literal_paths`` to disable new behaviour. - by :user:`jayvdb` diff --git a/docs/config.rst b/docs/config.rst index 9986fa039..1c87a2345 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -143,6 +143,19 @@ Global settings are defined under the ``tox`` section as: configure :conf:`basepython` in the global testenv without affecting environments that have implied base python versions. +.. conf:: literal_paths ^ true|false ^ true + + .. versionadded:: 3.21.0 + + tox defaults to interpretting values commencing with a path as a literal path, with + only segments inside ``{..}`` being substituted, without any need for quoting. + + Disabling this setting to use shell-like syntax for all values, except settings of + type ``path``. + + For settings of type ``path``, shell-like syntax can be activate by commencing the + value with a quotation mark. + .. conf:: isolated_build ^ true|false ^ false .. versionadded:: 3.3.0 diff --git a/src/tox/config/__init__.py b/src/tox/config/__init__.py index 26bd429e1..9617848d2 100644 --- a/src/tox/config/__init__.py +++ b/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 @@ -163,6 +164,8 @@ def postprocess(self, testenv_config, value): deps = [] config = testenv_config.config for depline in value: + if isinstance(depline, PathBase): + depline = str(depline) m = re.match(r":(\w+):\s*(\S+)", depline) if m: iname, name = m.groups() @@ -416,7 +419,7 @@ def export(self): # such as {} being escaped using \{\}, suitable for use with # os.environ . return { - name: Replacer._unescape(value) + name: str(value) if isinstance(value, PathBase) else Replacer._unescape(value) for name, value in self.items() if value is not self._DUMMY } @@ -1151,6 +1154,8 @@ def line_of_default_to_zero(section, name=None): hash_seed = config.option.hashseed config.hashseed = hash_seed + config.literal_paths = reader.getbool("literal_paths", True) + reader.addsubstitutions(toxinidir=config.toxinidir, homedir=config.homedir) if config.option.workdir is None: @@ -1364,7 +1369,13 @@ def _list_section_factors(self, section): def make_envconfig(self, name, section, subs, config, replace=True): factors = set(name.split("-")) - reader = SectionReader(section, self._cfg, fallbacksections=["testenv"], factors=factors) + reader = SectionReader( + section, + self._cfg, + fallbacksections=["testenv"], + factors=factors, + literal_paths=config.literal_paths, + ) tc = TestenvConfig(name, config, factors, reader) reader.addsubstitutions( envname=name, @@ -1578,6 +1589,7 @@ def __init__( factors=(), prefix=None, posargs="", + literal_paths=True, ): if prefix is None: self.section_name = section_name @@ -1590,6 +1602,7 @@ def __init__( self._subststack = [] self._setenv = None self.posargs = posargs + self.literal_paths = literal_paths def get_environ_value(self, name): if self._setenv is None: @@ -1602,7 +1615,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, is_path=True) if path is not None: toxinidir = self._subs["toxinidir"] return toxinidir.join(path, abs=True) @@ -1611,6 +1624,8 @@ def getlist(self, name, sep="\n"): s = self.getstring(name, None) if s is None: return [] + if isinstance(s, PathBase): + return [s] return [x.strip() for x in s.split(sep) if x.strip()] def getdict(self, name, default=None, sep="\n", replace=True): @@ -1698,7 +1713,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, + is_path=False, + ): x = None sections = [self.section_name] + ([] if no_fallback else self.fallbacksections) for s in sections: @@ -1716,10 +1739,16 @@ 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, is_path=is_path) + if isinstance(x, PathBase): + return x x = self._apply_factors(x) - x = self._replace_if_needed(x, name, replace, crossonly) + if isinstance(x, PathBase): + raise RuntimeError(name) + return x + + x = self._replace_if_needed(x, name, replace, crossonly, is_path=is_path) return x def getposargs(self, default=None): @@ -1733,9 +1762,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, is_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, is_path=is_path) return x def _apply_factors(self, s): @@ -1754,14 +1783,18 @@ 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, is_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) + replaced = Replacer(self, crossonly=crossonly).do_replace( + value, + is_path=is_path, + literal_paths=self.literal_paths, + ) assert self._subststack.pop() == (section_name, name) except tox.exception.MissingSubstitution: if not section_name.startswith(testenvprefix): @@ -1789,19 +1822,41 @@ def __init__(self, reader, crossonly=False): self.reader = reader self.crossonly = crossonly - def do_replace(self, value): + def do_replace(self, value, is_path=False, literal_paths=True): """ Recursively expand substitutions starting from the innermost expression """ - def substitute_once(x): - return self.RE_ITEM_REF.sub(self._replace_match, x) - - expanded = substitute_once(value) + def substitute_each(s): + parts = [] + pos = 0 + for match in self.RE_ITEM_REF.finditer(s): + start = match.start() + if start: + parts.append(s[pos:start]) + parts.append(self._replace_match(match)) + pos = match.end() + + tail = s[pos:] + if tail: + parts.append(tail) + + if not parts: + return "" + if (literal_paths or is_path) and isinstance(parts[0], PathBase): + if len(parts) == 1: + return parts[0] + return parts[0].join(*parts[1:]) + + return "".join(str(part) for part in parts) + + expanded = substitute_each(value) + if isinstance(expanded, PathBase): + return expanded while expanded != value: # substitution found value = expanded - expanded = substitute_once(value) + expanded = substitute_each(value) return expanded @@ -1888,6 +1943,8 @@ def _replace_substitution(self, match): val = self._substitute_from_other_section(sub_key) if callable(val): val = val() + if isinstance(val, PathBase): + return val return str(val) @@ -1949,8 +2006,12 @@ def processcommand(cls, reader, command, replace=True): new_arg = "" new_word = reader._replace(word) - new_word = reader._replace(new_word) - new_word = Replacer._unescape(new_word) + if not isinstance(new_word, PathBase): + new_word = reader._replace(new_word) + if not isinstance(new_word, PathBase): + new_word = Replacer._unescape(new_word) + if isinstance(new_word, PathBase): + new_word = str(new_word) new_arg += new_word newcommand += new_arg else: diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 806d6dce4..292ac6a2e 100644 --- a/tests/unit/config/test_config.py +++ b/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 @@ -587,6 +609,139 @@ 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}'{/}'foo#bar' + """, + ) + + 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[0] == [ + str(envconfig.envbindir.join("python")), + str(config.toxworkdir.join("foo#bar")), + ] + + 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}'{/}'foo bar' + """, + ) + + 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[0] == [ + str(envconfig.envbindir.join("python")), + str(config.toxworkdir.join("foo bar").realpath()), + ] + + def test_command_env_substitution_bracket(self, tmpdir, newconfig): + """Ensure bracket in path is kept in commands using setenv.""" + config = newconfig( + """ + [tox] + toxworkdir = {toxinidir}/.tox{dir + + [testenv:py27] + setenv = VAR = '{toxworkdir}'{/}'foo{bar' + commands = '{envpython}' '{env:VAR}' + """, + ) + + 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[0] == [ + str(envconfig.envbindir.join("python")), + str(config.toxworkdir.join("foo{bar").realpath()), + ] + + def test_command_env_substitution_pound(self, tmpdir, newconfig): + """Ensure pound in path is kept in commands using setenv.""" + config = newconfig( + """ + [tox] + toxworkdir = {toxinidir}/.tox#dir + + [testenv:py27] + setenv = VAR = {toxworkdir}{/}foo#bar + commands = '{envpython}' '{env:VAR}' + """, + ) + + 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[0] == [ + str(envconfig.envbindir.join("python")), + str(config.toxworkdir.join("foo#bar").realpath()), + ] + + def test_command_env_substitution_whitespace(self, tmpdir, newconfig): + """Ensure spaces in path is kept in commands using setenv.""" + config = newconfig( + """ + [tox] + toxworkdir = {toxinidir}/.tox dir + + [testenv:py27] + setenv = VAR = {toxworkdir}{/}foo bar + commands = '{envpython}' '{env:VAR}' + """, + ) + + 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[0] == [ + str(envconfig.envbindir.join("python")), + str(config.toxworkdir.join("foo bar").realpath()), + ] + class TestIniParser: def test_getstring_single(self, newconfig): @@ -1175,6 +1330,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( @@ -1602,7 +1797,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 @@ -1622,6 +1817,47 @@ 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"] + + if sys.platform != "win32": + 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", [