From 096bd69a949225590feeaeaf16545d673c4cee6d Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Thu, 25 Jun 2020 12:34:09 -0500 Subject: prevent multiple version sigils in the same spec (#17246) * prevent multiple version sigils in the same spec * fix packages with malformed versions --- lib/spack/spack/spec.py | 35 ++++++++++++++++------ lib/spack/spack/test/spec_syntax.py | 15 ++++++++++ lib/spack/spack/version.py | 3 ++ .../builtin/packages/netlib-scalapack/package.py | 6 ++-- .../repos/builtin/packages/protobuf/package.py | 4 +-- .../repos/builtin/packages/py-oauthlib/package.py | 2 +- var/spack/repos/builtin/packages/xeus/package.py | 2 +- 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') -- cgit v1.2.3-60-g2f50