summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/cmd/__init__.py102
-rw-r--r--lib/spack/spack/parser.py77
-rw-r--r--lib/spack/spack/spec.py30
-rw-r--r--lib/spack/spack/test/cmd/common/arguments.py4
-rw-r--r--lib/spack/spack/test/cmd/spec.py36
-rw-r--r--lib/spack/spack/test/spec_syntax.py115
-rw-r--r--lib/spack/spack/variant.py22
7 files changed, 198 insertions, 188 deletions
diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py
index c26ab181c1..cb4083ed8f 100644
--- a/lib/spack/spack/cmd/__init__.py
+++ b/lib/spack/spack/cmd/__init__.py
@@ -6,10 +6,8 @@
import argparse
import os
import re
-import shlex
import sys
-from textwrap import dedent
-from typing import List, Match, Tuple
+from typing import List, Union
import llnl.string
import llnl.util.tty as tty
@@ -147,89 +145,37 @@ def get_command(cmd_name):
return getattr(get_module(cmd_name), pname)
-class _UnquotedFlags:
- """Use a heuristic in `.extract()` to detect whether the user is trying to set
- multiple flags like the docker ENV attribute allows (e.g. 'cflags=-Os -pipe').
+def quote_kvp(string: str) -> str:
+ """For strings like ``name=value`` or ``name==value``, quote and escape the value if needed.
- If the heuristic finds a match (which can be checked with `__bool__()`), a warning
- message explaining how to quote multiple flags correctly can be generated with
- `.report()`.
+ This is a compromise to respect quoting of key-value pairs on the CLI. The shell
+ strips quotes from quoted arguments, so we cannot know *exactly* how CLI arguments
+ were quoted. To compensate, we re-add quotes around anything staritng with ``name=``
+ or ``name==``, and we assume the rest of the argument is the value. This covers the
+ common cases of passign flags, e.g., ``cflags="-O2 -g"`` on the command line.
"""
+ match = re.match(spack.parser.SPLIT_KVP, string)
+ if not match:
+ return string
- flags_arg_pattern = re.compile(
- r'^({0})=([^\'"].*)$'.format("|".join(spack.spec.FlagMap.valid_compiler_flags()))
- )
+ key, delim, value = match.groups()
+ return f"{key}{delim}{spack.parser.quote_if_needed(value)}"
- def __init__(self, all_unquoted_flag_pairs: List[Tuple[Match[str], str]]):
- self._flag_pairs = all_unquoted_flag_pairs
-
- def __bool__(self) -> bool:
- return bool(self._flag_pairs)
-
- @classmethod
- def extract(cls, sargs: str) -> "_UnquotedFlags":
- all_unquoted_flag_pairs: List[Tuple[Match[str], str]] = []
- prev_flags_arg = None
- for arg in shlex.split(sargs):
- if prev_flags_arg is not None:
- all_unquoted_flag_pairs.append((prev_flags_arg, arg))
- prev_flags_arg = cls.flags_arg_pattern.match(arg)
- return cls(all_unquoted_flag_pairs)
-
- def report(self) -> str:
- single_errors = [
- "({0}) {1} {2} => {3}".format(
- i + 1,
- match.group(0),
- next_arg,
- '{0}="{1} {2}"'.format(match.group(1), match.group(2), next_arg),
- )
- for i, (match, next_arg) in enumerate(self._flag_pairs)
- ]
- return dedent(
- """\
- Some compiler or linker flags were provided without quoting their arguments,
- which now causes spack to try to parse the *next* argument as a spec component
- such as a variant instead of an additional compiler or linker flag. If the
- intent was to set multiple flags, try quoting them together as described below.
-
- Possible flag quotation errors (with the correctly-quoted version after the =>):
- {0}"""
- ).format("\n".join(single_errors))
-
-
-def parse_specs(args, **kwargs):
+
+def parse_specs(
+ args: Union[str, List[str]], concretize: bool = False, tests: bool = False
+) -> List[spack.spec.Spec]:
"""Convenience function for parsing arguments from specs. Handles common
exceptions and dies if there are errors.
"""
- concretize = kwargs.get("concretize", False)
- normalize = kwargs.get("normalize", False)
- tests = kwargs.get("tests", False)
-
- sargs = args
- if not isinstance(args, str):
- sargs = " ".join(args)
- unquoted_flags = _UnquotedFlags.extract(sargs)
+ args = [args] if isinstance(args, str) else args
+ arg_string = " ".join([quote_kvp(arg) for arg in args])
- try:
- specs = spack.parser.parse(sargs)
- for spec in specs:
- if concretize:
- spec.concretize(tests=tests) # implies normalize
- elif normalize:
- spec.normalize(tests=tests)
- return specs
-
- except spack.error.SpecError as e:
- msg = e.message
- if e.long_message:
- msg += e.long_message
- # Unquoted flags will be read as a variant or hash
- if unquoted_flags and ("variant" in msg or "hash" in msg):
- msg += "\n\n"
- msg += unquoted_flags.report()
-
- raise spack.error.SpackError(msg) from e
+ specs = spack.parser.parse(arg_string)
+ for spec in specs:
+ if concretize:
+ spec.concretize(tests=tests)
+ return specs
def matching_spec_from_env(spec):
diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py
index b0a8d8ad83..63c139b9f0 100644
--- a/lib/spack/spack/parser.py
+++ b/lib/spack/spack/parser.py
@@ -58,6 +58,7 @@ specs to avoid ambiguity. Both are provided because ~ can cause shell
expansion when it is the first character in an id typed on the command line.
"""
import enum
+import json
import pathlib
import re
import sys
@@ -95,13 +96,55 @@ if not IS_WINDOWS:
else:
FILENAME = WINDOWS_FILENAME
+#: These are legal values that *can* be parsed bare, without quotes on the command line.
VALUE = r"(?:[a-zA-Z_0-9\-+\*.,:=\~\/\\]+)"
-QUOTED_VALUE = r"[\"']+(?:[a-zA-Z_0-9\-+\*.,:=\~\/\\\s]+)[\"']+"
+
+#: Variant/flag values that match this can be left unquoted in Spack output
+NO_QUOTES_NEEDED = r"^[a-zA-Z0-9,/_.-]+$"
+
+#: Quoted values can be *anything* in between quotes, including escaped quotes.
+QUOTED_VALUE = r"(?:'(?:[^']|(?<=\\)')*'|\"(?:[^\"]|(?<=\\)\")*\")"
VERSION = r"=?(?:[a-zA-Z0-9_][a-zA-Z_0-9\-\.]*\b)"
VERSION_RANGE = rf"(?:(?:{VERSION})?:(?:{VERSION}(?!\s*=))?)"
VERSION_LIST = rf"(?:{VERSION_RANGE}|{VERSION})(?:\s*,\s*(?:{VERSION_RANGE}|{VERSION}))*"
+#: Regex with groups to use for splitting (optionally propagated) key-value pairs
+SPLIT_KVP = rf"^({NAME})(==?)(.*)$"
+
+#: Regex to strip quotes. Group 2 will be the unquoted string.
+STRIP_QUOTES = r"^(['\"])(.*)\1$"
+
+
+def strip_quotes_and_unescape(string: str) -> str:
+ """Remove surrounding single or double quotes from string, if present."""
+ match = re.match(STRIP_QUOTES, string)
+ if not match:
+ return string
+
+ # replace any escaped quotes with bare quotes
+ quote, result = match.groups()
+ return result.replace(rf"\{quote}", quote)
+
+
+def quote_if_needed(value: str) -> str:
+ """Add quotes around the value if it requires quotes.
+
+ This will add quotes around the value unless it matches ``NO_QUOTES_NEEDED``.
+
+ This adds:
+ * single quotes by default
+ * double quotes around any value that contains single quotes
+
+ If double quotes are used, we json-escpae the string. That is, we escape ``\\``,
+ ``"``, and control codes.
+
+ """
+ if re.match(spack.parser.NO_QUOTES_NEEDED, value):
+ return value
+
+ return json.dumps(value) if "'" in value else f"'{value}'"
+
class TokenBase(enum.Enum):
"""Base class for an enum type with a regex value"""
@@ -138,8 +181,8 @@ class TokenType(TokenBase):
# Variants
PROPAGATED_BOOL_VARIANT = rf"(?:(?:\+\+|~~|--)\s*{NAME})"
BOOL_VARIANT = rf"(?:[~+-]\s*{NAME})"
- PROPAGATED_KEY_VALUE_PAIR = rf"(?:{NAME}\s*==\s*(?:{VALUE}|{QUOTED_VALUE}))"
- KEY_VALUE_PAIR = rf"(?:{NAME}\s*=\s*(?:{VALUE}|{QUOTED_VALUE}))"
+ PROPAGATED_KEY_VALUE_PAIR = rf"(?:{NAME}==(?:{VALUE}|{QUOTED_VALUE}))"
+ KEY_VALUE_PAIR = rf"(?:{NAME}=(?:{VALUE}|{QUOTED_VALUE}))"
# Compilers
COMPILER_AND_VERSION = rf"(?:%\s*(?:{NAME})(?:[\s]*)@\s*(?:{VERSION_LIST}))"
COMPILER = rf"(?:%\s*(?:{NAME}))"
@@ -351,12 +394,14 @@ class SpecNodeParser:
# accept another package name afterwards in a node
if self.ctx.accept(TokenType.UNQUALIFIED_PACKAGE_NAME):
initial_spec.name = self.ctx.current_token.value
+
elif self.ctx.accept(TokenType.FULLY_QUALIFIED_PACKAGE_NAME):
parts = self.ctx.current_token.value.split(".")
name = parts[-1]
namespace = ".".join(parts[:-1])
initial_spec.name = name
initial_spec.namespace = namespace
+
elif self.ctx.accept(TokenType.FILENAME):
return FileParser(self.ctx).parse(initial_spec)
@@ -370,6 +415,7 @@ class SpecNodeParser:
compiler_name = self.ctx.current_token.value[1:]
initial_spec.compiler = spack.spec.CompilerSpec(compiler_name.strip(), ":")
self.has_compiler = True
+
elif self.ctx.accept(TokenType.COMPILER_AND_VERSION):
if self.has_compiler:
raise spack.spec.DuplicateCompilerSpecError(
@@ -381,6 +427,7 @@ class SpecNodeParser:
compiler_name.strip(), compiler_version
)
self.has_compiler = True
+
elif (
self.ctx.accept(TokenType.VERSION_HASH_PAIR)
or self.ctx.accept(TokenType.GIT_VERSION)
@@ -395,31 +442,39 @@ class SpecNodeParser:
)
initial_spec.attach_git_version_lookup()
self.has_version = True
+
elif self.ctx.accept(TokenType.BOOL_VARIANT):
variant_value = self.ctx.current_token.value[0] == "+"
initial_spec._add_flag(
self.ctx.current_token.value[1:].strip(), variant_value, propagate=False
)
+
elif self.ctx.accept(TokenType.PROPAGATED_BOOL_VARIANT):
variant_value = self.ctx.current_token.value[0:2] == "++"
initial_spec._add_flag(
self.ctx.current_token.value[2:].strip(), variant_value, propagate=True
)
+
elif self.ctx.accept(TokenType.KEY_VALUE_PAIR):
- name, value = self.ctx.current_token.value.split("=", maxsplit=1)
- name = name.strip("'\" ")
- value = value.strip("'\" ")
- initial_spec._add_flag(name, value, propagate=False)
+ match = re.match(SPLIT_KVP, self.ctx.current_token.value)
+ assert match, "SPLIT_KVP and KEY_VALUE_PAIR do not agree."
+
+ name, delim, value = match.groups()
+ initial_spec._add_flag(name, strip_quotes_and_unescape(value), propagate=False)
+
elif self.ctx.accept(TokenType.PROPAGATED_KEY_VALUE_PAIR):
- name, value = self.ctx.current_token.value.split("==", maxsplit=1)
- name = name.strip("'\" ")
- value = value.strip("'\" ")
- initial_spec._add_flag(name, value, propagate=True)
+ match = re.match(SPLIT_KVP, self.ctx.current_token.value)
+ assert match, "SPLIT_KVP and PROPAGATED_KEY_VALUE_PAIR do not agree."
+
+ name, delim, value = match.groups()
+ initial_spec._add_flag(name, strip_quotes_and_unescape(value), propagate=True)
+
elif self.ctx.expect(TokenType.DAG_HASH):
if initial_spec.abstract_hash:
break
self.ctx.accept(TokenType.DAG_HASH)
initial_spec.abstract_hash = self.ctx.current_token.value[1:]
+
else:
break
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 88184de30f..600ce988d7 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -910,19 +910,23 @@ class FlagMap(lang.HashableMap):
yield flags
def __str__(self):
- sorted_keys = [k for k in sorted(self.keys()) if self[k] != []]
- cond_symbol = " " if len(sorted_keys) > 0 else ""
- return (
- cond_symbol
- + " ".join(
- key
- + ('=="' if True in [f.propagate for f in self[key]] else '="')
- + " ".join(self[key])
- + '"'
- for key in sorted_keys
- )
- + cond_symbol
- )
+ sorted_items = sorted((k, v) for k, v in self.items() if v)
+
+ result = ""
+ for flag_type, flags in sorted_items:
+ normal = [f for f in flags if not f.propagate]
+ if normal:
+ result += f" {flag_type}={spack.parser.quote_if_needed(' '.join(normal))}"
+
+ propagated = [f for f in flags if f.propagate]
+ if propagated:
+ result += f" {flag_type}=={spack.parser.quote_if_needed(' '.join(propagated))}"
+
+ # TODO: somehow add this space only if something follows in Spec.format()
+ if sorted_items:
+ result += " "
+
+ return result
def _sort_by_dep_types(dspec: DependencySpec):
diff --git a/lib/spack/spack/test/cmd/common/arguments.py b/lib/spack/spack/test/cmd/common/arguments.py
index e889fe55d8..dcfe86aa51 100644
--- a/lib/spack/spack/test/cmd/common/arguments.py
+++ b/lib/spack/spack/test/cmd/common/arguments.py
@@ -50,8 +50,8 @@ def test_negative_integers_not_allowed_for_parallel_jobs(job_parser):
[
(['coreutils cflags="-O3 -g"'], ["-O3", "-g"], [False, False], []),
(['coreutils cflags=="-O3 -g"'], ["-O3", "-g"], [True, True], []),
- (["coreutils", "cflags=-O3 -g"], ["-O3"], [False], ["g"]),
- (["coreutils", "cflags==-O3 -g"], ["-O3"], [True], ["g"]),
+ (["coreutils", "cflags=-O3 -g"], ["-O3", "-g"], [False, False], []),
+ (["coreutils", "cflags==-O3 -g"], ["-O3", "-g"], [True, True], []),
(["coreutils", "cflags=-O3", "-g"], ["-O3"], [False], ["g"]),
],
)
diff --git a/lib/spack/spack/test/cmd/spec.py b/lib/spack/spack/test/cmd/spec.py
index 763d83bf0a..39fff0bbd4 100644
--- a/lib/spack/spack/test/cmd/spec.py
+++ b/lib/spack/spack/test/cmd/spec.py
@@ -4,7 +4,6 @@
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import re
-from textwrap import dedent
import pytest
@@ -74,41 +73,6 @@ def test_spec_parse_cflags_quoting():
assert ["-flto", "-Os"] == gh_flagged.compiler_flags["cxxflags"]
-def test_spec_parse_unquoted_flags_report():
- """Verify that a useful error message is produced if unquoted compiler flags are
- provided."""
- # This should fail during parsing, since /usr/include is interpreted as a spec hash.
- with pytest.raises(spack.error.SpackError) as cm:
- # We don't try to figure out how many following args were intended to be part of
- # cflags, we just explain how to fix it for the immediate next arg.
- spec("gcc cflags=-Os -pipe -other-arg-that-gets-ignored cflags=-I /usr/include")
- # Verify that the generated error message is nicely formatted.
-
- expected_message = dedent(
- '''\
- Some compiler or linker flags were provided without quoting their arguments,
- which now causes spack to try to parse the *next* argument as a spec component
- such as a variant instead of an additional compiler or linker flag. If the
- intent was to set multiple flags, try quoting them together as described below.
-
- Possible flag quotation errors (with the correctly-quoted version after the =>):
- (1) cflags=-Os -pipe => cflags="-Os -pipe"
- (2) cflags=-I /usr/include => cflags="-I /usr/include"'''
- )
-
- assert expected_message in str(cm.value)
-
- # Verify that the same unquoted cflags report is generated in the error message even
- # if it fails during concretization, not just during parsing.
- with pytest.raises(spack.error.SpackError) as cm:
- spec("gcc cflags=-Os -pipe")
- cm = str(cm.value)
- assert cm.startswith(
- 'trying to set variant "pipe" in package "gcc", but the package has no such variant'
- )
- assert cm.endswith('(1) cflags=-Os -pipe => cflags="-Os -pipe"')
-
-
def test_spec_yaml():
output = spec("--yaml", "mpileaks")
diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py
index 3cbb59e69f..e2282df745 100644
--- a/lib/spack/spack/test/spec_syntax.py
+++ b/lib/spack/spack/test/spec_syntax.py
@@ -2,6 +2,7 @@
# Spack Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import inspect
import itertools
import os
import re
@@ -9,6 +10,7 @@ import sys
import pytest
+import spack.cmd
import spack.platforms.test
import spack.spec
import spack.variant
@@ -203,7 +205,8 @@ def specfile_for(default_mock_concretization):
"mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1~qt_4 debug=2 ^stackwalker@8.1_1e",
),
(
- "mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1 cppflags=-O3 +debug~qt_4 ^stackwalker@8.1_1e", # noqa: E501
+ "mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1 cppflags=-O3 +debug~qt_4 "
+ "^stackwalker@8.1_1e",
[
Token(TokenType.UNQUALIFIED_PACKAGE_NAME, value="mvapich_foo"),
Token(TokenType.DEPENDENCY, value="^"),
@@ -217,7 +220,8 @@ def specfile_for(default_mock_concretization):
Token(TokenType.UNQUALIFIED_PACKAGE_NAME, value="stackwalker"),
Token(TokenType.VERSION, value="@8.1_1e"),
],
- 'mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1 cppflags="-O3" +debug~qt_4 ^stackwalker@8.1_1e', # noqa: E501
+ "mvapich_foo ^_openmpi@1.2:1.4,1.6%intel@12.1 cppflags=-O3 +debug~qt_4 "
+ "^stackwalker@8.1_1e",
),
# Specs containing YAML or JSON in the package name
(
@@ -424,7 +428,7 @@ def specfile_for(default_mock_concretization):
compiler_with_version_range("%gcc@10.1.0,12.2.1:"),
compiler_with_version_range("%gcc@:8.4.3,10.2.1:12.1.0"),
# Special key value arguments
- ("dev_path=*", [Token(TokenType.KEY_VALUE_PAIR, value="dev_path=*")], "dev_path=*"),
+ ("dev_path=*", [Token(TokenType.KEY_VALUE_PAIR, value="dev_path=*")], "dev_path='*'"),
(
"dev_path=none",
[Token(TokenType.KEY_VALUE_PAIR, value="dev_path=none")],
@@ -444,33 +448,28 @@ def specfile_for(default_mock_concretization):
(
"cflags=a=b=c",
[Token(TokenType.KEY_VALUE_PAIR, value="cflags=a=b=c")],
- 'cflags="a=b=c"',
+ "cflags='a=b=c'",
),
(
"cflags=a=b=c",
[Token(TokenType.KEY_VALUE_PAIR, value="cflags=a=b=c")],
- 'cflags="a=b=c"',
+ "cflags='a=b=c'",
),
(
"cflags=a=b=c+~",
[Token(TokenType.KEY_VALUE_PAIR, value="cflags=a=b=c+~")],
- 'cflags="a=b=c+~"',
+ "cflags='a=b=c+~'",
),
(
"cflags=-Wl,a,b,c",
[Token(TokenType.KEY_VALUE_PAIR, value="cflags=-Wl,a,b,c")],
- 'cflags="-Wl,a,b,c"',
+ "cflags=-Wl,a,b,c",
),
# Multi quoted
(
- "cflags=''-Wl,a,b,c''",
- [Token(TokenType.KEY_VALUE_PAIR, value="cflags=''-Wl,a,b,c''")],
- 'cflags="-Wl,a,b,c"',
- ),
- (
'cflags=="-O3 -g"',
[Token(TokenType.PROPAGATED_KEY_VALUE_PAIR, value='cflags=="-O3 -g"')],
- 'cflags=="-O3 -g"',
+ "cflags=='-O3 -g'",
),
# Whitespace is allowed in version lists
("@1.2:1.4 , 1.6 ", [Token(TokenType.VERSION, value="@1.2:1.4 , 1.6")], "@1.2:1.4,1.6"),
@@ -485,22 +484,6 @@ def specfile_for(default_mock_concretization):
"a@1:",
),
(
- "@1.2: develop = foo",
- [
- Token(TokenType.VERSION, value="@1.2:"),
- Token(TokenType.KEY_VALUE_PAIR, value="develop = foo"),
- ],
- "@1.2: develop=foo",
- ),
- (
- "@1.2:develop = foo",
- [
- Token(TokenType.VERSION, value="@1.2:"),
- Token(TokenType.KEY_VALUE_PAIR, value="develop = foo"),
- ],
- "@1.2: develop=foo",
- ),
- (
"% intel @ 12.1:12.6 + debug",
[
Token(TokenType.COMPILER_AND_VERSION, value="% intel @ 12.1:12.6"),
@@ -587,8 +570,8 @@ def specfile_for(default_mock_concretization):
)
def test_parse_single_spec(spec_str, tokens, expected_roundtrip):
parser = SpecParser(spec_str)
- assert parser.tokens() == tokens
- assert str(parser.next_spec()) == expected_roundtrip
+ assert tokens == parser.tokens()
+ assert expected_roundtrip == str(parser.next_spec())
@pytest.mark.parametrize(
@@ -655,19 +638,79 @@ def test_parse_multiple_specs(text, tokens, expected_specs):
@pytest.mark.parametrize(
+ "args,expected",
+ [
+ # Test that CLI-quoted flags/variant values are preserved
+ (["zlib", "cflags=-O3 -g", "+bar", "baz"], "zlib cflags='-O3 -g' +bar baz"),
+ # Test that CLI-quoted propagated flags/variant values are preserved
+ (["zlib", "cflags==-O3 -g", "+bar", "baz"], "zlib cflags=='-O3 -g' +bar baz"),
+ # An entire string passed on the CLI with embedded quotes also works
+ (["zlib cflags='-O3 -g' +bar baz"], "zlib cflags='-O3 -g' +bar baz"),
+ # Entire string *without* quoted flags splits -O3/-g (-g interpreted as a variant)
+ (["zlib cflags=-O3 -g +bar baz"], "zlib cflags=-O3 +bar~g baz"),
+ # If the entirety of "-O3 -g +bar baz" is quoted on the CLI, it's all taken as flags
+ (["zlib", "cflags=-O3 -g +bar baz"], "zlib cflags='-O3 -g +bar baz'"),
+ # If the string doesn't start with key=, it needs internal quotes for flags
+ (["zlib", " cflags=-O3 -g +bar baz"], "zlib cflags=-O3 +bar~g baz"),
+ # Internal quotes for quoted CLI args are considered part of *one* arg
+ (["zlib", 'cflags="-O3 -g" +bar baz'], """zlib cflags='"-O3 -g" +bar baz'"""),
+ # Use double quotes if internal single quotes are present
+ (["zlib", "cflags='-O3 -g' +bar baz"], '''zlib cflags="'-O3 -g' +bar baz"'''),
+ # Use single quotes and escape single quotes with internal single and double quotes
+ (["zlib", "cflags='-O3 -g' \"+bar baz\""], 'zlib cflags="\'-O3 -g\' \\"+bar baz\\""'),
+ # Ensure that empty strings are handled correctly on CLI
+ (["zlib", "ldflags=", "+pic"], "zlib+pic"),
+ # These flags are assumed to be quoted by the shell, but the space doesn't matter because
+ # flags are space-separated.
+ (["zlib", "ldflags= +pic"], "zlib ldflags='+pic'"),
+ (["ldflags= +pic"], "ldflags='+pic'"),
+ # If the name is not a flag name, the space is preserved verbatim, because variant values
+ # are comma-separated.
+ (["zlib", "foo= +pic"], "zlib foo=' +pic'"),
+ (["foo= +pic"], "foo=' +pic'"),
+ # You can ensure no quotes are added parse_specs() by starting your string with space,
+ # but you still need to quote empty strings properly.
+ ([" ldflags= +pic"], SpecTokenizationError),
+ ([" ldflags=", "+pic"], SpecTokenizationError),
+ ([" ldflags='' +pic"], "+pic"),
+ ([" ldflags=''", "+pic"], "+pic"),
+ # Ensure that empty strings are handled properly in quoted strings
+ (["zlib ldflags='' +pic"], "zlib+pic"),
+ # Ensure that $ORIGIN is handled correctly
+ (["zlib", "ldflags=-Wl,-rpath=$ORIGIN/_libs"], "zlib ldflags='-Wl,-rpath=$ORIGIN/_libs'"),
+ # Ensure that passing escaped quotes on the CLI raises a tokenization error
+ (["zlib", '"-g', '-O2"'], SpecTokenizationError),
+ ],
+)
+def test_cli_spec_roundtrip(args, expected):
+ if inspect.isclass(expected) and issubclass(expected, BaseException):
+ with pytest.raises(expected):
+ spack.cmd.parse_specs(args)
+ return
+
+ specs = spack.cmd.parse_specs(args)
+ output_string = " ".join(str(spec) for spec in specs)
+ assert expected == output_string
+
+
+@pytest.mark.parametrize(
"text,expected_in_error",
[
- ("x@@1.2", "x@@1.2\n ^^^^^"),
- ("y ^x@@1.2", "y ^x@@1.2\n ^^^^^"),
- ("x@1.2::", "x@1.2::\n ^"),
- ("x::", "x::\n ^^"),
+ ("x@@1.2", r"x@@1.2\n ^"),
+ ("y ^x@@1.2", r"y ^x@@1.2\n ^"),
+ ("x@1.2::", r"x@1.2::\n ^"),
+ ("x::", r"x::\n ^^"),
+ ("cflags=''-Wl,a,b,c''", r"cflags=''-Wl,a,b,c''\n ^ ^ ^ ^^"),
+ ("@1.2: develop = foo", r"@1.2: develop = foo\n ^^"),
+ ("@1.2:develop = foo", r"@1.2:develop = foo\n ^^"),
],
)
def test_error_reporting(text, expected_in_error):
parser = SpecParser(text)
with pytest.raises(SpecTokenizationError) as exc:
parser.tokens()
- assert expected_in_error in str(exc), parser.tokens()
+
+ assert expected_in_error in str(exc), parser.tokens()
@pytest.mark.parametrize(
diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py
index 9bea903aac..f9f6f8b960 100644
--- a/lib/spack/spack/variant.py
+++ b/lib/spack/spack/variant.py
@@ -19,6 +19,7 @@ from llnl.string import comma_or
import spack.directives
import spack.error as error
+import spack.parser
special_variant_values = [None, "none", "*"]
@@ -399,13 +400,12 @@ class AbstractVariant:
return item in self._value
def __repr__(self):
- cls = type(self)
- return "{0.__name__}({1}, {2})".format(cls, repr(self.name), repr(self._original_value))
+ return f"{type(self).__name__}({repr(self.name)}, {repr(self._original_value)})"
def __str__(self):
- if self.propagate:
- return "{0}=={1}".format(self.name, ",".join(str(x) for x in self.value))
- return "{0}={1}".format(self.name, ",".join(str(x) for x in self.value))
+ delim = "==" if self.propagate else "="
+ values = spack.parser.quote_if_needed(",".join(str(v) for v in self.value))
+ return f"{self.name}{delim}{values}"
class MultiValuedVariant(AbstractVariant):
@@ -443,15 +443,14 @@ class MultiValuedVariant(AbstractVariant):
self._original_value = ",".join(self._value)
def __str__(self):
- # Special-case patches to not print the full 64 character hashes
+ # Special-case patches to not print the full 64 character sha256
if self.name == "patches":
values_str = ",".join(x[:7] for x in self.value)
else:
values_str = ",".join(str(x) for x in self.value)
- if self.propagate:
- return "{0}=={1}".format(self.name, values_str)
- return "{0}={1}".format(self.name, values_str)
+ delim = "==" if self.propagate else "="
+ return f"{self.name}{delim}{spack.parser.quote_if_needed(values_str)}"
class SingleValuedVariant(AbstractVariant):
@@ -467,9 +466,8 @@ class SingleValuedVariant(AbstractVariant):
self._value = str(self._value[0])
def __str__(self):
- if self.propagate:
- return "{0}=={1}".format(self.name, self.value)
- return "{0}={1}".format(self.name, self.value)
+ delim = "==" if self.propagate else "="
+ return f"{self.name}{delim}{spack.parser.quote_if_needed(self.value)}"
@implicit_variant_conversion
def satisfies(self, other):
generated by cgit v1.2.3-70-g09d2 (git 2.46.0) at 2024-11-28 17:15:06 +0000