diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/spack/spack/parse.py | 6 | ||||
| -rw-r--r-- | lib/spack/spack/repo.py | 17 | ||||
| -rw-r--r-- | lib/spack/spack/spec.py | 97 | ||||
| -rw-r--r-- | lib/spack/spack/test/spec_syntax.py | 217 |
4 files changed, 318 insertions, 19 deletions
diff --git a/lib/spack/spack/parse.py b/lib/spack/spack/parse.py index 836b40c3a6..6bfdec7b43 100644 --- a/lib/spack/spack/parse.py +++ b/lib/spack/spack/parse.py @@ -12,7 +12,7 @@ from six import string_types import spack.error -class Token: +class Token(object): """Represents tokens; generated from input by lexer and fed to parse().""" def __init__(self, type, value='', start=0, end=0): @@ -25,7 +25,7 @@ class Token: return str(self) def __str__(self): - return "'%s'" % self.value + return "<%d: '%s'>" % (self.type, self.value) def is_a(self, type): return self.type == type @@ -128,7 +128,7 @@ class Parser(object): raise ParseError(message, self.text, self.token.start) def unexpected_token(self): - self.next_token_error("Unexpected token") + self.next_token_error("Unexpected token: '%s'" % self.next.value) def expect(self, id): """Like accept(), but fails if we don't like the next token.""" diff --git a/lib/spack/spack/repo.py b/lib/spack/spack/repo.py index d726fafacc..3ea196dd4d 100644 --- a/lib/spack/spack/repo.py +++ b/lib/spack/spack/repo.py @@ -564,7 +564,7 @@ class RepoPath(object): def providers_for(self, vpkg_spec): providers = self.provider_index.providers_for(vpkg_spec) if not providers: - raise UnknownPackageError(vpkg_spec.name) + raise UnknownPackageError(vpkg_spec.fullname) return providers @autospec @@ -967,7 +967,7 @@ class Repo(object): def providers_for(self, vpkg_spec): providers = self.provider_index.providers_for(vpkg_spec) if not providers: - raise UnknownPackageError(vpkg_spec.name) + raise UnknownPackageError(vpkg_spec.fullname) return providers @autospec @@ -1283,10 +1283,17 @@ class UnknownPackageError(UnknownEntityError): def __init__(self, name, repo=None): msg = None if repo: - msg = "Package %s not found in repository %s" % (name, repo) + msg = "Package '%s' not found in repository '%s'" % (name, repo) else: - msg = "Package %s not found." % name - super(UnknownPackageError, self).__init__(msg) + msg = "Package '%s' not found." % name + + # special handling for specs that may have been intended as filenames + # prompt the user to ask whether they intended to write './<name>' + long_msg = None + if name.endswith(".yaml"): + long_msg = "Did you mean to specify a filename with './%s'?" % name + + super(UnknownPackageError, self).__init__(msg, long_msg) self.name = name diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index c7926ad3a3..dca06101b8 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3804,10 +3804,11 @@ class LazySpecCache(collections.defaultdict): return value -# -# These are possible token types in the spec grammar. -# -HASH, DEP, AT, COLON, COMMA, ON, OFF, PCT, EQ, ID, VAL = range(11) +#: These are possible token types in the spec grammar. +HASH, DEP, AT, COLON, COMMA, ON, OFF, PCT, EQ, ID, VAL, FILE = range(12) + +#: Regex for fully qualified spec names. (e.g., builtin.hdf5) +spec_id_re = r'\w[\w.-]*' class SpecLexer(spack.parse.Lexer): @@ -3816,7 +3817,6 @@ class SpecLexer(spack.parse.Lexer): def __init__(self): super(SpecLexer, self).__init__([ - (r'/', lambda scanner, val: self.token(HASH, val)), (r'\^', lambda scanner, val: self.token(DEP, val)), (r'\@', lambda scanner, val: self.token(AT, val)), (r'\:', lambda scanner, val: self.token(COLON, val)), @@ -3826,9 +3826,18 @@ class SpecLexer(spack.parse.Lexer): (r'\~', lambda scanner, val: self.token(OFF, val)), (r'\%', lambda scanner, val: self.token(PCT, val)), (r'\=', lambda scanner, val: self.token(EQ, val)), - # This is more liberal than identifier_re (see above). - # Checked by check_identifier() for better error messages. - (r'\w[\w.-]*', lambda scanner, val: self.token(ID, val)), + + # Filenames match before identifiers, so no initial filename + # component is parsed as a spec (e.g., in subdir/spec.yaml) + (r'[/\w.-]+\.yaml[^\b]*', lambda scanner, v: self.token(FILE, v)), + + # Hash match after filename. No valid filename can be a hash + # (files end w/.yaml), but a hash can match a filename prefix. + (r'/', lambda scanner, val: self.token(HASH, val)), + + # Identifiers match after filenames and hashes. + (spec_id_re, lambda scanner, val: self.token(ID, val)), + (r'\s+', lambda scanner, val: None)], [EQ], [(r'[\S].*', lambda scanner, val: self.token(VAL, val)), @@ -3859,7 +3868,14 @@ class SpecParser(spack.parse.Parser): try: while self.next: - # TODO: clean this parsing up a bit + # Try a file first, but if it doesn't succeed, keep parsing + # as from_file may backtrack and try an id. + if self.accept(FILE): + spec = self.spec_from_file() + if spec: + specs.append(spec) + continue + if self.accept(ID): self.previous = self.token if self.accept(EQ): @@ -3899,12 +3915,18 @@ class SpecParser(spack.parse.Parser): self.push_tokens([self.token]) specs.append(self.spec(None)) else: - if self.accept(HASH): + dep = None + if self.accept(FILE): + # this may return None, in which case we backtrack + dep = self.spec_from_file() + + if not dep and self.accept(HASH): # We're finding a dependency by hash for an # anonymous spec dep = self.spec_by_hash() dep = dep.copy(deps=('link', 'run')) - else: + + if not dep: # We're adding a dependency to the last spec self.expect(ID) dep = self.spec(self.token.value) @@ -3944,6 +3966,51 @@ class SpecParser(spack.parse.Parser): s._set_architecture(platform=platform_default) return specs + def spec_from_file(self): + """Read a spec from a filename parsed on the input stream. + + There is some care taken here to ensure that filenames are a last + resort, and that any valid package name is parsed as a name + before we consider it as a file. Specs are used in lots of places; + we don't want the parser touching the filesystem unnecessarily. + + The parse logic is as follows: + + 1. We require that filenames end in .yaml, which means that no valid + filename can be interpreted as a hash (hashes can't have '.') + + 2. We avoid treating paths like /path/to/spec.yaml as hashes, or paths + like subdir/spec.yaml as ids by lexing filenames before hashes. + + 3. For spec names that match file and id regexes, like 'builtin.yaml', + we backtrack from spec_from_file() and treat them as spec names. + + """ + path = self.token.value + + # don't treat builtin.yaml, builtin.yaml-cpp, etc. as filenames + if re.match(spec_id_re + '$', path): + self.push_tokens([spack.parse.Token(ID, self.token.value)]) + return None + + # Special case where someone omits a space after a filename. Consider: + # + # libdwarf^/some/path/to/libelf.yamllibdwarf ^../../libelf.yaml + # + # The error is clearly an omitted space. To handle this, the FILE + # regex admits text *beyond* .yaml, and we raise a nice error for + # file names that don't end in .yaml. + if not path.endswith(".yaml"): + raise SpecFilenameError( + "Spec filename must end in .yaml: '{0}'".format(path)) + + # if we get here, we're *finally* interpreting path as a filename + if not os.path.exists(path): + raise NoSuchSpecFileError("No such spec file: '{0}'".format(path)) + + with open(path) as f: + return Spec.from_yaml(f) + def parse_compiler(self, text): self.setup(text) return self.compiler() @@ -4281,6 +4348,14 @@ class NoSuchHashError(SpecError): % hash) +class SpecFilenameError(SpecError): + """Raised when a spec file name is invalid.""" + + +class NoSuchSpecFileError(SpecFilenameError): + """Raised when a spec file doesn't exist.""" + + class RedundantSpecError(SpecError): def __init__(self, spec, addition): super(RedundantSpecError, self).__init__( diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 699ecc987c..238b21f535 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -3,9 +3,14 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import os import pytest import shlex +import llnl.util.filesystem as fs + +import spack.hash_types as ht +import spack.repo import spack.store import spack.spec as sp from spack.parse import Token @@ -14,6 +19,7 @@ from spack.spec import SpecParseError, RedundantSpecError from spack.spec import AmbiguousHashError, InvalidHashError, NoSuchHashError from spack.spec import DuplicateArchitectureError, DuplicateVariantError from spack.spec import DuplicateDependencyError, DuplicateCompilerSpecError +from spack.spec import SpecFilenameError, NoSuchSpecFileError # Sample output for a complex lexing. @@ -109,6 +115,7 @@ class TestSpecSyntax(object): def _check_raises(self, exc_type, items): for item in items: with pytest.raises(exc_type): + print("CHECKING: ", item, "=======================") Spec(item) # ======================================================================== @@ -433,6 +440,216 @@ class TestSpecSyntax(object): ] self._check_raises(DuplicateArchitectureError, duplicates) + @pytest.mark.usefixtures('config') + def test_parse_yaml_simple(self, mock_packages, tmpdir): + s = Spec('libdwarf') + s.concretize() + + specfile = tmpdir.join('libdwarf.yaml') + + with specfile.open('w') as f: + f.write(s.to_yaml(hash=ht.build_hash)) + + # Check an absolute path to spec.yaml by itself: + # "spack spec /path/to/libdwarf.yaml" + specs = sp.parse(specfile.strpath) + assert len(specs) == 1 + + # Check absolute path to spec.yaml mixed with a clispec, e.g.: + # "spack spec mvapich_foo /path/to/libdwarf.yaml" + specs = sp.parse('mvapich_foo {0}'.format(specfile.strpath)) + assert len(specs) == 2 + + @pytest.mark.usefixtures('config') + def test_parse_filename_missing_slash_as_spec(self, mock_packages, tmpdir): + """Ensure that libelf.yaml parses as a spec, NOT a file.""" + s = Spec('libelf') + s.concretize() + + specfile = tmpdir.join('libelf.yaml') + + # write the file to the current directory to make sure it exists, + # and that we still do not parse the spec as a file. + with specfile.open('w') as f: + f.write(s.to_yaml(hash=ht.build_hash)) + + # Check the spec `libelf.yaml` in the working directory, which + # should evaluate to a spec called `yaml` in the `libelf` + # namespace, NOT a spec for `libelf`. + with tmpdir.as_cwd(): + specs = sp.parse("libelf.yaml") + assert len(specs) == 1 + + spec = specs[0] + assert spec.name == "yaml" + assert spec.namespace == "libelf" + assert spec.fullname == "libelf.yaml" + + # check that if we concretize this spec, we get a good error + # message that mentions we might've meant a file. + with pytest.raises(spack.repo.UnknownPackageError) as exc_info: + spec.concretize() + assert exc_info.value.long_message + assert ("Did you mean to specify a filename with './libelf.yaml'?" + in exc_info.value.long_message) + + # make sure that only happens when the spec ends in yaml + with pytest.raises(spack.repo.UnknownPackageError) as exc_info: + Spec('builtin.mock.doesnotexist').concretize() + assert ( + not exc_info.value.long_message or ( + "Did you mean to specify a filename with" not in + exc_info.value.long_message + ) + ) + + @pytest.mark.usefixtures('config') + def test_parse_yaml_dependency(self, mock_packages, tmpdir): + s = Spec('libdwarf') + s.concretize() + + specfile = tmpdir.join('libelf.yaml') + + with specfile.open('w') as f: + f.write(s['libelf'].to_yaml(hash=ht.build_hash)) + + print("") + print("") + print("PARSING HERE") + + # Make sure we can use yaml path as dependency, e.g.: + # "spack spec libdwarf ^ /path/to/libelf.yaml" + specs = sp.parse('libdwarf ^ {0}'.format(specfile.strpath)) + assert len(specs) == 1 + + @pytest.mark.usefixtures('config') + def test_parse_yaml_relative_paths(self, mock_packages, tmpdir): + s = Spec('libdwarf') + s.concretize() + + specfile = tmpdir.join('libdwarf.yaml') + + with specfile.open('w') as f: + f.write(s.to_yaml(hash=ht.build_hash)) + + file_name = specfile.basename + parent_dir = os.path.basename(specfile.dirname) + + # Relative path to specfile + with fs.working_dir(specfile.dirname): + # Test for command like: "spack spec libelf.yaml" + # This should parse a single spec, but should not concretize. + # See test_parse_filename_missing_slash_as_spec() + specs = sp.parse('{0}'.format(file_name)) + assert len(specs) == 1 + + # Make sure this also works: "spack spec ./libelf.yaml" + specs = sp.parse('./{0}'.format(file_name)) + assert len(specs) == 1 + + # Should also be accepted: "spack spec ../<cur-dir>/libelf.yaml" + specs = sp.parse('../{0}/{1}'.format(parent_dir, file_name)) + assert len(specs) == 1 + + # Should also handle mixed clispecs and relative paths, e.g.: + # "spack spec mvapich_foo ../<cur-dir>/libelf.yaml" + specs = sp.parse('mvapich_foo ../{0}/{1}'.format( + parent_dir, file_name)) + assert len(specs) == 2 + + @pytest.mark.usefixtures('config') + def test_parse_yaml_relative_subdir_path(self, mock_packages, tmpdir): + s = Spec('libdwarf') + s.concretize() + + specfile = tmpdir.mkdir('subdir').join('libdwarf.yaml') + + with specfile.open('w') as f: + f.write(s.to_yaml(hash=ht.build_hash)) + + file_name = specfile.basename + + # Relative path to specfile + with tmpdir.as_cwd(): + assert os.path.exists('subdir/{0}'.format(file_name)) + + # Test for command like: "spack spec libelf.yaml" + specs = sp.parse('subdir/{0}'.format(file_name)) + assert len(specs) == 1 + + @pytest.mark.usefixtures('config') + def test_parse_yaml_dependency_relative_paths(self, mock_packages, tmpdir): + s = Spec('libdwarf') + s.concretize() + + specfile = tmpdir.join('libelf.yaml') + + with specfile.open('w') as f: + f.write(s['libelf'].to_yaml(hash=ht.build_hash)) + + file_name = specfile.basename + parent_dir = os.path.basename(specfile.dirname) + + # Relative path to specfile + with fs.working_dir(specfile.dirname): + # Test for command like: "spack spec libelf.yaml" + specs = sp.parse('libdwarf^{0}'.format(file_name)) + assert len(specs) == 1 + + # Make sure this also works: "spack spec ./libelf.yaml" + specs = sp.parse('libdwarf^./{0}'.format(file_name)) + assert len(specs) == 1 + + # Should also be accepted: "spack spec ../<cur-dir>/libelf.yaml" + specs = sp.parse('libdwarf^../{0}/{1}'.format( + parent_dir, file_name)) + assert len(specs) == 1 + + def test_parse_yaml_error_handling(self): + self._check_raises(NoSuchSpecFileError, [ + # Single spec that looks like a yaml path + '/bogus/path/libdwarf.yaml', + '../../libdwarf.yaml', + './libdwarf.yaml', + # Dependency spec that looks like a yaml path + 'libdwarf^/bogus/path/libelf.yaml', + 'libdwarf ^../../libelf.yaml', + 'libdwarf^ ./libelf.yaml', + # Multiple specs, one looks like a yaml path + 'mvapich_foo /bogus/path/libelf.yaml', + 'mvapich_foo ../../libelf.yaml', + 'mvapich_foo ./libelf.yaml', + ]) + + def test_nice_error_for_no_space_after_spec_filename(self): + """Ensure that omitted spaces don't give weird errors about hashes.""" + self._check_raises(SpecFilenameError, [ + '/bogus/path/libdwarf.yamlfoobar', + 'libdwarf^/bogus/path/libelf.yamlfoobar ^/path/to/bogus.yaml', + ]) + + @pytest.mark.usefixtures('config') + def test_yaml_spec_not_filename(self, mock_packages, tmpdir): + with pytest.raises(spack.repo.UnknownPackageError): + Spec('builtin.mock.yaml').concretize() + + with pytest.raises(spack.repo.UnknownPackageError): + Spec('builtin.mock.yamlfoobar').concretize() + + @pytest.mark.usefixtures('config') + def test_parse_yaml_variant_error(self, mock_packages, tmpdir): + s = Spec('a') + s.concretize() + + specfile = tmpdir.join('a.yaml') + + with specfile.open('w') as f: + f.write(s.to_yaml(hash=ht.build_hash)) + + with pytest.raises(RedundantSpecError): + # Trying to change a variant on a concrete spec is an error + sp.parse('{0} ~bvv'.format(specfile.strpath)) + # ======================================================================== # Lex checks # ======================================================================== |