From 12ab882eba1631f85f4ebd2acfe1a9c41857ca79 Mon Sep 17 00:00:00 2001 From: becker33 Date: Wed, 24 May 2017 17:13:18 -0700 Subject: [PATCH] Fix issues parsing multiple anonymous specs (#4199) * fix parser * Removed xfails * cleaned up debug print statements * make use of these changes in gcc * Added comment explaining unreachable line, line left for added protection --- lib/spack/spack/spec.py | 31 +++++++++++++------ lib/spack/spack/test/spec_syntax.py | 11 ++----- .../repos/builtin/packages/gcc/package.py | 19 +++++------- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index ff213c59866..9e89feb1483 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -2958,16 +2958,23 @@ def do_parse(self): # We're parsing an anonymous spec beginning with a # key-value pair. if not specs: + self.push_tokens([self.previous, self.token]) + self.previous = None specs.append(self.spec(None)) - self.expect(VAL) - # Raise an error if the previous spec is already - # concrete (assigned by hash) - if specs[-1]._hash: - raise RedundantSpecError(specs[-1], - 'key-value pair') - specs[-1]._add_flag( - self.previous.value, self.token.value) - self.previous = None + else: + if specs[-1].concrete: + # Trying to add k-v pair to spec from hash + raise RedundantSpecError(specs[-1], + 'key-value pair') + # We should never end up here. + # This requires starting a new spec with ID, EQ + # After another spec that is not concrete + # If the previous spec is not concrete, this is + # handled in the spec parsing loop + # If it is concrete, see the if statement above + # If there is no previous spec, we don't land in + # this else case. + self.unexpected_token() else: # We're parsing a new spec by name self.previous = None @@ -3151,7 +3158,11 @@ def version(self): if self.accept(COLON): if self.accept(ID): - end = self.token.value + if self.next and self.next.type is EQ: + # This is a start: range followed by a key=value pair + self.push_tokens([self.token]) + else: + end = self.token.value elif start: # No colon, but there was a version. return Version(start) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 2ee9ef486ce..906aa77bb2e 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -140,10 +140,9 @@ def test_anonymous_specs(self): self.check_parse("arch=test-None-None", "platform=test") self.check_parse('@2.7:') - @pytest.mark.xfail() def test_anonymous_specs_with_multiple_parts(self): # Parse anonymous spec with multiple tokens - self.check_parse('languages=go @4.2:') + self.check_parse('@4.2: languages=go', 'languages=go @4.2:') self.check_parse('@4.2: languages=go') def test_simple_dependence(self): @@ -551,12 +550,8 @@ def test_kv_with_spaces(self): @pytest.mark.parametrize('spec,anon_spec,spec_name', [ ('openmpi languages=go', 'languages=go', 'openmpi'), ('openmpi @4.6:', '@4.6:', 'openmpi'), - pytest.mark.xfail( - ('openmpi languages=go @4.6:', 'languages=go @4.6:', 'openmpi') - ), - pytest.mark.xfail( - ('openmpi @4.6: languages=go', '@4.6: languages=go', 'openmpi') - ), + ('openmpi languages=go @4.6:', 'languages=go @4.6:', 'openmpi'), + ('openmpi @4.6: languages=go', '@4.6: languages=go', 'openmpi'), ]) def test_parse_anonymous_specs(spec, anon_spec, spec_name): diff --git a/var/spack/repos/builtin/packages/gcc/package.py b/var/spack/repos/builtin/packages/gcc/package.py index 1bdee43e831..3d42bb98b5b 100644 --- a/var/spack/repos/builtin/packages/gcc/package.py +++ b/var/spack/repos/builtin/packages/gcc/package.py @@ -97,17 +97,14 @@ class Gcc(AutotoolsPackage): # depends_on('guile@1.4.1:', type='test') # See https://golang.org/doc/install/gccgo#Releases - provides('golang', when='languages=go') - # 'when' does not currently support multiple parts of a spec. - # See https://github.com/LLNL/spack/pull/4151 - # provides('golang', when='languages=go @4.6:') - # provides('golang@:1', when='languages=go @4.7.1:') - # provides('golang@:1.1', when='languages=go @4.8:') - # provides('golang@:1.1.2', when='languages=go @4.8.2:') - # provides('golang@:1.2', when='languages=go @4.9:') - # provides('golang@:1.4', when='languages=go @5:') - # provides('golang@:1.6.1', when='languages=go @6:') - # provides('golang@:1.8', when='languages=go @7:') + provides('golang', when='languages=go @4.6:') + provides('golang@:1', when='languages=go @4.7.1:') + provides('golang@:1.1', when='languages=go @4.8:') + provides('golang@:1.1.2', when='languages=go @4.8.2:') + provides('golang@:1.2', when='languages=go @4.9:') + provides('golang@:1.4', when='languages=go @5:') + provides('golang@:1.6.1', when='languages=go @6:') + provides('golang@:1.8', when='languages=go @7:') # For a list of valid languages for a specific release, # run the following command in the GCC source directory: