summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--lib/spack/spack/cmd/__init__.py84
-rw-r--r--lib/spack/spack/cmd/spec.py3
-rw-r--r--lib/spack/spack/test/cmd/common/arguments.py17
-rw-r--r--lib/spack/spack/test/cmd/spec.py54
-rw-r--r--lib/spack/spack/test/spec_semantics.py6
5 files changed, 145 insertions, 19 deletions
diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py
index ff6c3d7bf4..608db2776b 100644
--- a/lib/spack/spack/cmd/__init__.py
+++ b/lib/spack/spack/cmd/__init__.py
@@ -8,7 +8,10 @@ from __future__ import print_function
import argparse
import os
import re
+import shlex
import sys
+from textwrap import dedent
+from typing import List, Tuple
import ruamel.yaml as yaml
import six
@@ -147,6 +150,58 @@ def get_command(cmd_name):
return getattr(get_module(cmd_name), pname)
+class _UnquotedFlags(object):
+ """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').
+
+ 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()`.
+ """
+
+ flags_arg_pattern = re.compile(
+ r'^({0})=([^\'"].*)$'.format(
+ '|'.join(spack.spec.FlagMap.valid_compiler_flags()),
+ ))
+
+ def __init__(self, all_unquoted_flag_pairs):
+ # type: (List[Tuple[re.Match, str]]) -> None
+ self._flag_pairs = all_unquoted_flag_pairs
+
+ def __bool__(self):
+ # type: () -> bool
+ return bool(self._flag_pairs)
+
+ @classmethod
+ def extract(cls, sargs):
+ # type: (str) -> _UnquotedFlags
+ all_unquoted_flag_pairs = [] # type: List[Tuple[re.Match, 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):
+ # type: () -> 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):
"""Convenience function for parsing arguments from specs. Handles common
exceptions and dies if there are errors.
@@ -157,15 +212,28 @@ def parse_specs(args, **kwargs):
sargs = args
if not isinstance(args, six.string_types):
- sargs = ' '.join(spack.util.string.quote(args))
- specs = spack.spec.parse(sargs)
- for spec in specs:
- if concretize:
- spec.concretize(tests=tests) # implies normalize
- elif normalize:
- spec.normalize(tests=tests)
+ sargs = ' '.join(args)
+ unquoted_flags = _UnquotedFlags.extract(sargs)
- return specs
+ try:
+ specs = spack.spec.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
+ if unquoted_flags:
+ msg += '\n\n'
+ msg += unquoted_flags.report()
+
+ raise spack.error.SpackError(msg)
def matching_spec_from_env(spec):
diff --git a/lib/spack/spack/cmd/spec.py b/lib/spack/spack/cmd/spec.py
index 74a47d154d..1e5b213fe4 100644
--- a/lib/spack/spack/cmd/spec.py
+++ b/lib/spack/spack/cmd/spec.py
@@ -80,7 +80,8 @@ def spec(parser, args):
# Use command line specified specs, otherwise try to use environment specs.
if args.specs:
input_specs = spack.cmd.parse_specs(args.specs)
- specs = [(s, s.concretized()) for s in input_specs]
+ concretized_specs = spack.cmd.parse_specs(args.specs, concretize=True)
+ specs = list(zip(input_specs, concretized_specs))
else:
env = ev.active_environment()
if env:
diff --git a/lib/spack/spack/test/cmd/common/arguments.py b/lib/spack/spack/test/cmd/common/arguments.py
index 5f1299a94a..c527cc074a 100644
--- a/lib/spack/spack/test/cmd/common/arguments.py
+++ b/lib/spack/spack/test/cmd/common/arguments.py
@@ -45,21 +45,22 @@ def test_negative_integers_not_allowed_for_parallel_jobs(job_parser):
assert 'expected a positive integer' in str(exc_info.value)
-@pytest.mark.parametrize('specs,expected_variants,unexpected_variants', [
- (['coreutils', 'cflags=-O3 -g'], [], ['g']),
- (['coreutils', 'cflags=-O3', '-g'], ['g'], []),
+@pytest.mark.parametrize('specs,cflags,negated_variants', [
+ (['coreutils cflags="-O3 -g"'], ['-O3', '-g'], []),
+ (['coreutils', 'cflags=-O3 -g'], ['-O3'], ['g']),
+ (['coreutils', 'cflags=-O3', '-g'], ['-O3'], ['g']),
])
@pytest.mark.regression('12951')
-def test_parse_spec_flags_with_spaces(
- specs, expected_variants, unexpected_variants
-):
+def test_parse_spec_flags_with_spaces(specs, cflags, negated_variants):
spec_list = spack.cmd.parse_specs(specs)
assert len(spec_list) == 1
s = spec_list.pop()
- assert all(x not in s.variants for x in unexpected_variants)
- assert all(x in s.variants for x in expected_variants)
+ assert s.compiler_flags['cflags'] == cflags
+ assert list(s.variants.keys()) == negated_variants
+ for v in negated_variants:
+ assert '~{0}'.format(v) in s
@pytest.mark.usefixtures('config')
diff --git a/lib/spack/spack/test/cmd/spec.py b/lib/spack/spack/test/cmd/spec.py
index 42f4c7a59a..e406fabe26 100644
--- a/lib/spack/spack/test/cmd/spec.py
+++ b/lib/spack/spack/test/cmd/spec.py
@@ -4,10 +4,12 @@
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import re
+from textwrap import dedent
import pytest
import spack.environment as ev
+import spack.error
import spack.spec
import spack.store
from spack.main import SpackCommand, SpackCommandError
@@ -55,6 +57,54 @@ def test_spec_concretizer_args(mutable_config, mutable_database):
assert h in output
+def test_spec_parse_dependency_variant_value():
+ """Verify that we can provide multiple key=value variants to multiple separate
+ packages within a spec string."""
+ output = spec('multivalue-variant fee=barbaz ^ a foobar=baz')
+
+ assert 'fee=barbaz' in output
+ assert 'foobar=baz' in output
+
+
+def test_spec_parse_cflags_quoting():
+ """Verify that compiler flags can be provided to a spec from the command line."""
+ output = spec('--yaml', 'gcc cflags="-Os -pipe" cxxflags="-flto -Os"')
+ gh_flagged = spack.spec.Spec.from_yaml(output)
+
+ assert ['-Os', '-pipe'] == gh_flagged.compiler_flags['cflags']
+ 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.
+ assert str(cm.value) == dedent('''\
+ No installed spec matches the hash: 'usr'
+
+ 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"''')
+
+ # 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 [happened during concretization of gcc cflags="-Os" ~pipe]') # noqa: E501
+ assert cm.endswith('(1) cflags=-Os -pipe => cflags="-Os -pipe"')
+
+
def test_spec_yaml():
output = spec('--yaml', 'mpileaks')
@@ -125,14 +175,14 @@ def test_spec_returncode():
def test_spec_parse_error():
- with pytest.raises(spack.spec.SpecParseError) as e:
+ with pytest.raises(spack.error.SpackError) as e:
spec("1.15:")
# make sure the error is formatted properly
error_msg = """\
1.15:
^"""
- assert error_msg in e.value.long_message
+ assert error_msg in str(e.value)
def test_env_aware_spec(mutable_mock_env_path):
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index edc31e4f09..db1f68f396 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -15,6 +15,7 @@ from spack.spec import (
SpecFormatSigilError,
SpecFormatStringError,
UnconstrainableDependencySpecError,
+ UnsupportedCompilerError,
)
from spack.variant import (
InvalidVariantValueError,
@@ -1320,3 +1321,8 @@ def test_concretize_partial_old_dag_hash_spec(mock_packages, config):
# make sure package hash is NOT recomputed
assert not getattr(spec["dt-diamond-bottom"], '_package_hash', None)
+
+
+def test_unsupported_compiler():
+ with pytest.raises(UnsupportedCompilerError):
+ Spec('gcc%fake-compiler').validate_or_raise()