Skip to content

Quote path settings containing # and spaces #1698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog/1700.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow {/} to refer to os.sep. - by :user:`jayvdb`
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is #1701

Without this, the tests need to do hacks to convert / to \ on Windows. It gets messy because the bugs related to this are about real native paths, including constructed path based on preset paths.

1 change: 1 addition & 0 deletions docs/changelog/763.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Quote paths in settings containing `` `` or ``#``. - by :user:`jayvdb`
6 changes: 6 additions & 0 deletions docs/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,12 @@ Globally available substitutions
OS-specific path separator (``:`` os \*nix family, ``;`` on Windows). May be used in ``setenv``,
when target variable is path variable (e.g. PATH or PYTHONPATH).

``{/}``
OS-specific directory separator (``/`` os \*nix family, ``\\`` on Windows).
Useful for deriving filenames from preset paths, as arguments for commands
that requires ``\\`` on Windows. e.g. ``{distdir}{/}file.txt``.
It is not usually needed when using commands written in Python.

Substitutions for virtualenv-related sections
+++++++++++++++++++++++++++++++++++++++++++++

Expand Down
74 changes: 59 additions & 15 deletions src/tox/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -61,6 +62,7 @@
_ENVSTR_SPLIT_PATTERN = re.compile(r"((?:\{[^}]+\})+)|,")
_ENVSTR_EXPAND_PATTERN = re.compile(r"\{([^}]+)\}")
_WHITESPACE_PATTERN = re.compile(r"\s+")
_UNESCAPED_DOUBLEQUOTE = re.compile(r"((?<!\{1})'){2}")


def get_plugin_manager(plugins=()):
Expand Down Expand Up @@ -390,7 +392,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=True)
res = res.replace("\\{", "{").replace("\\}", "}")
self.resolved[name] = res
finally:
Expand Down Expand Up @@ -1591,7 +1593,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)
Expand Down Expand Up @@ -1680,7 +1682,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:
Expand All @@ -1698,10 +1708,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):
Expand All @@ -1715,9 +1725,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):
Expand All @@ -1736,14 +1746,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):
Expand All @@ -1770,6 +1783,7 @@ class Replacer:
def __init__(self, reader, crossonly=False):
self.reader = reader
self.crossonly = crossonly
self._path_quoted = False

def do_replace(self, value):
"""
Expand Down Expand Up @@ -1811,6 +1825,8 @@ def _replace_match(self, match):
"Malformed substitution; no substitution type provided",
)

if not sub_type and not g["default_value"] and sub_value == "/":
return os.sep
if sub_type == "env":
return self._replace_env(match)
if sub_type == "tty":
Expand Down Expand Up @@ -1853,6 +1869,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))
Expand All @@ -1864,6 +1881,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
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has partially been done.
I was thinking of switching quoting char to avoid clashes with '. The quoting could even be done with an invalid filename char (e.g. non-printables), as long as it makes it through the regex engine.

if "'" not in val and ("#" in val or " " in val):
val = "'{}'".format(val)
self._path_quoted = True
return str(val)


Expand Down Expand Up @@ -1895,7 +1918,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)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unquote_path=False is added liberally to aid reviewing, so all invocations of _replace(..) can be seen.
IMO they should removed before being merged.

commands.extend(cls.getargvlist(reader, replaced))
else:
commands.append(cls.processcommand(reader, current_command, replace))
Expand Down Expand Up @@ -1924,8 +1947,11 @@ def processcommand(cls, reader, command, replace=True):
continue

new_arg = ""
new_word = reader._replace(word)
new_word = reader._replace(new_word)
had_dual_quote = re.search(_UNESCAPED_DOUBLEQUOTE, word)
new_word = reader._replace(word, unquote_path=False)
new_word = reader._replace(new_word, unquote_path=False)
if not had_dual_quote:
new_word = re.sub(_UNESCAPED_DOUBLEQUOTE, "'", new_word)
new_word = new_word.replace("\\{", "{").replace("\\}", "}")
new_arg += new_word
newcommand += new_arg
Expand Down Expand Up @@ -1960,8 +1986,14 @@ def word_has_ended():
and ps.word
and ps.word[-1] not in string.whitespace
)
or (cur_char == "{" and ps.depth == 0 and not ps.word.endswith("\\"))
or (ps.depth == 0 and ps.word and ps.word[-1] == "}")
or (
cur_char == "{"
and ps.depth == 0
and not ps.word.endswith("\\")
and ps.word != "'"
)
or (ps.depth == 0 and ps.word and ps.word[-1] == "}" and peek() != "'")
or (ps.depth == 0 and ps.word and ps.word[-2:] == "}'")
or (cur_char not in string.whitespace and ps.word and ps.word.strip() == "")
)

Expand All @@ -1975,6 +2007,12 @@ def yield_if_word_ended():
if word_has_ended():
yield_this_word()

def peek():
try:
return self.command[_i + 1]
except IndexError:
return ""

def accumulate():
ps.word += cur_char

Expand All @@ -1984,7 +2022,7 @@ def push_substitution():
def pop_substitution():
ps.depth -= 1

for cur_char in self.command:
for _i, cur_char in enumerate(self.command):
if cur_char in string.whitespace:
if ps.depth == 0:
yield_if_word_ended()
Expand All @@ -1996,6 +2034,12 @@ def pop_substitution():
elif cur_char == "}":
accumulate()
pop_substitution()
elif cur_char == "'":
if ps.depth == 0 and ps.word[:2] == "'{" and ps.word[-1] == "}":
accumulate()
else:
yield_if_word_ended()
accumulate()
else:
yield_if_word_ended()
accumulate()
Expand Down
Loading