prevent multiple version sigils in the same spec (#17246)

* prevent multiple version sigils in the same spec

* fix packages with malformed versions

7 files changed, 51 insertions, 16 deletions

diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index b23142e101..175d160855 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -511,8 +511,17 @@ class CompilerSpec(object):
             raise TypeError(
                 "__init__ takes 1 or 2 arguments. (%d given)" % nargs)
 
-    def _add_version(self, version):
-        self.versions.add(version)
+    def _add_versions(self, version_list):
+        # If it already has a non-trivial version list, this is an error
+        if self.versions and self.versions != vn.VersionList(':'):
+            # Note: This may be impossible to reach by the current parser
+            # Keeping it in case the implementation changes.
+            raise MultipleVersionError(
+                'A spec cannot contain multiple version signifiers.'
+                ' Use a version list instead.')
+        self.versions = vn.VersionList()
+        for version in version_list:
+            self.versions.add(version)
 
     def _autospec(self, compiler_spec_like):
         if isinstance(compiler_spec_like, CompilerSpec):
@@ -1050,9 +1059,16 @@ class Spec(object):
     #
     # Private routines here are called by the parser when building a spec.
     #
-    def _add_version(self, version):
+    def _add_versions(self, version_list):
         """Called by the parser to add an allowable version."""
-        self.versions.add(version)
+        # If it already has a non-trivial version list, this is an error
+        if self.versions and self.versions != vn.VersionList(':'):
+            raise MultipleVersionError(
+                'A spec cannot contain multiple version signifiers.'
+                ' Use a version list instead.')
+        self.versions = vn.VersionList()
+        for version in version_list:
+            self.versions.add(version)
 
     def _add_flag(self, name, value):
         """Called by the parser to add a known flag.
@@ -4157,9 +4173,7 @@ class SpecParser(spack.parse.Parser):
         while self.next:
             if self.accept(AT):
                 vlist = self.version_list()
-                spec.versions = vn.VersionList()
-                for version in vlist:
-                    spec._add_version(version)
+                spec._add_versions(vlist)
 
             elif self.accept(ON):
                 name = self.variant()
@@ -4251,8 +4265,7 @@ class SpecParser(spack.parse.Parser):
         compiler.versions = vn.VersionList()
         if self.accept(AT):
             vlist = self.version_list()
-            for version in vlist:
-                compiler._add_version(version)
+            compiler._add_versions(vlist)
         else:
             compiler.versions = vn.VersionList(':')
         return compiler
@@ -4325,6 +4338,10 @@ class DuplicateDependencyError(spack.error.SpecError):
     """Raised when the same dependency occurs in a spec twice."""
 
 
+class MultipleVersionError(spack.error.SpecError):
+    """Raised when version constraints occur in a spec twice."""
+
+
 class DuplicateCompilerSpecError(spack.error.SpecError):
     """Raised when the same compiler occurs in a spec twice."""
 
diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py
index c9a2100b09..52a842912e 100644
--- a/lib/spack/spack/test/spec_syntax.py
+++ b/lib/spack/spack/test/spec_syntax.py
@@ -20,6 +20,7 @@ from spack.spec import AmbiguousHashError, InvalidHashError, NoSuchHashError
 from spack.spec import DuplicateArchitectureError
 from spack.spec import DuplicateDependencyError, DuplicateCompilerSpecError
 from spack.spec import SpecFilenameError, NoSuchSpecFileError
+from spack.spec import MultipleVersionError
 from spack.variant import DuplicateVariantError
 
 
@@ -149,6 +150,9 @@ class TestSpecSyntax(object):
         self.check_parse("openmpi ^hwloc ^libunwind",
                          "openmpi^hwloc^libunwind")
 
+    def test_version_after_compiler(self):
+        self.check_parse('foo@2.0%bar@1.0', 'foo %bar@1.0 @2.0')
+
     def test_dependencies_with_versions(self):
         self.check_parse("openmpi ^hwloc@1.2e6")
         self.check_parse("openmpi ^hwloc@1.2e6:")
@@ -432,6 +436,17 @@ class TestSpecSyntax(object):
         ]
         self._check_raises(DuplicateVariantError, duplicates)
 
+    def test_multiple_versions(self):
+        multiples = [
+            'x@1.2@2.3',
+            'x@1.2:2.3@1.4',
+            'x@1.2@2.3:2.4',
+            'x@1.2@2.3,2.4',
+            'x@1.2 +foo~bar @2.3',
+            'x@1.2%y@1.2@2.3:2.4',
+        ]
+        self._check_raises(MultipleVersionError, multiples)
+
     def test_duplicate_dependency(self):
         self._check_raises(DuplicateDependencyError, ["x ^y ^y"])
 
diff --git a/lib/spack/spack/version.py b/lib/spack/spack/version.py
index bb7a79772e..0c7399c3fb 100644
--- a/lib/spack/spack/version.py
+++ b/lib/spack/spack/version.py
@@ -782,6 +782,9 @@ class VersionList(object):
     def __len__(self):
         return len(self.versions)
 
+    def __bool__(self):
+        return bool(self.versions)
+
     @coerced
     def __eq__(self, other):
         return other is not None and self.versions == other.versions
diff --git a/var/spack/repos/builtin/packages/netlib-scalapack/package.py b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
index 75993c4701..26fe53190a 100644
--- a/var/spack/repos/builtin/packages/netlib-scalapack/package.py
+++ b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
@@ -41,11 +41,11 @@ class NetlibScalapack(CMakePackage):
     depends_on('cmake', when='@2.0.0:', type='build')
 
     # See: https://github.com/Reference-ScaLAPACK/scalapack/issues/9
-    patch("cmake_fortran_mangle.patch", when='@2.0.2:@2.0.99')
+    patch("cmake_fortran_mangle.patch", when='@2.0.2:2.0.99')
     # See: https://github.com/Reference-ScaLAPACK/scalapack/pull/10
-    patch("mpi2-compatibility.patch", when='@2.0.2:@2.0.99')
+    patch("mpi2-compatibility.patch", when='@2.0.2:2.0.99')
     # See: https://github.com/Reference-ScaLAPACK/scalapack/pull/16
-    patch("int_overflow.patch", when='@2.0.0:@2.1.0')
+    patch("int_overflow.patch", when='@2.0.0:2.1.0')
 
     @property
     def libs(self):
diff --git a/var/spack/repos/builtin/packages/protobuf/package.py b/var/spack/repos/builtin/packages/protobuf/package.py
index 82d2d05d75..1cbd94b314 100644
--- a/var/spack/repos/builtin/packages/protobuf/package.py
+++ b/var/spack/repos/builtin/packages/protobuf/package.py
@@ -56,10 +56,10 @@ class Protobuf(Package):
     # first fixed in 3.4.0: https://github.com/google/protobuf/pull/3406
     patch('pkgconfig.patch', when='@3.0.2:3.3.2')
 
-    patch('intel-v1.patch', when='@3.2:@3.6 %intel')
+    patch('intel-v1.patch', when='@3.2:3.6 %intel')
 
     # See https://github.com/protocolbuffers/protobuf/pull/7197
-    patch('intel-v2.patch', when='@3.7:@3.11.4 %intel')
+    patch('intel-v2.patch', when='@3.7:3.11.4 %intel')
 
     patch('protoc2.5.0_aarch64.patch', sha256='7b44fcdb794f421174d619f83584e00a36012a16da09079e2fad9c12f7337451', when='@2.5.0 target=aarch64:')
 
diff --git a/var/spack/repos/builtin/packages/py-oauthlib/package.py b/var/spack/repos/builtin/packages/py-oauthlib/package.py
index 163db071ad..93fbb9aacc 100644
--- a/var/spack/repos/builtin/packages/py-oauthlib/package.py
+++ b/var/spack/repos/builtin/packages/py-oauthlib/package.py
@@ -32,6 +32,6 @@ class PyOauthlib(PythonPackage):
     depends_on('py-pytest-cov@2.6:', type='test')
 
     depends_on('py-nose', type='test', when='@2.0.2')
-    depends_on('py-unittest2', type='test', when='^python@2 @2.0.2')
+    depends_on('py-unittest2', type='test', when='^python@2.0.2')
 
     depends_on('python@2.7:2.8,3.4:', type=('build', 'run'))
diff --git a/var/spack/repos/builtin/packages/xeus/package.py b/var/spack/repos/builtin/packages/xeus/package.py
index 629bbb76a8..756a130aa4 100644
--- a/var/spack/repos/builtin/packages/xeus/package.py
+++ b/var/spack/repos/builtin/packages/xeus/package.py
@@ -27,7 +27,7 @@ class Xeus(CMakePackage):
     depends_on('cppzmq@4.3.0:')
     depends_on('cryptopp@7.0.0:')
     depends_on('xtl@0.4.0:')
-    depends_on('nlohmann-json@3.2.0', when='@develop@0.15.0:')
+    depends_on('nlohmann-json@3.2.0', when='@develop,0.15.0:')
     depends_on('nlohmann-json@3.1.1', when='@0.14.1')
     depends_on('libuuid')