diff --git a/lib/spack/docs/developer_guide.rst b/lib/spack/docs/developer_guide.rst index eca5accb15c7cb230b1fadd97a6e5cbdec67ac7b..47b98a211db06e024d9a145f7b4188b11ea1b6b8 100644 --- a/lib/spack/docs/developer_guide.rst +++ b/lib/spack/docs/developer_guide.rst @@ -3,6 +3,8 @@ Developer Guide ===================== +This guide is intended for people who want to work on Spack's inner +workings. Right now it's pretty sparse. Spec objects ------------------------- diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index 55f7391cd9817c74f7334d6b08ff999fc4e728c9..75e738efcda4614d4ba6db636a3398aa37485cfe 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -1060,17 +1060,17 @@ for example: # the default, called when no @when specs match pass - @when('mpi@3:') + @when('^mpi@3:') def setup_mpi(self): # this will be called when mpi is version 3 or higher pass - @when('mpi@2:') + @when('^mpi@2:') def setup_mpi(self): # this will be called when mpi is version 2 or higher pass - @when('mpi@1:') + @when('^mpi@1:') def setup_mpi(self): # this will be called when mpi is version 1 or higher pass diff --git a/lib/spack/spack/multimethod.py b/lib/spack/spack/multimethod.py index f4de6445daad3d032857815a628530edb2124953..46ddb90d329adaf7a0012d93dacf3c55e4544a94 100644 --- a/lib/spack/spack/multimethod.py +++ b/lib/spack/spack/multimethod.py @@ -52,20 +52,20 @@ class SpecMultiMethod(object): registers method versions with them. To register a method, you can do something like this: - mf = SpecMultiMethod() - mf.register("^chaos_5_x86_64_ib", some_method) + mm = SpecMultiMethod() + mm.register("^chaos_5_x86_64_ib", some_method) The object registered needs to be a Spec or some string that will parse to be a valid spec. - When the pmf is actually called, it selects a version of the + When the mm is actually called, it selects a version of the method to call based on the sys_type of the object it is called on. See the docs for decorators below for more details. """ def __init__(self, default=None): - self.method_map = {} + self.method_list = [] self.default = default if default: functools.update_wrapper(self, default) @@ -73,7 +73,7 @@ def __init__(self, default=None): def register(self, spec, method): """Register a version of a method for a particular sys_type.""" - self.method_map[spec] = method + self.method_list.append((spec, method)) if not hasattr(self, '__name__'): functools.update_wrapper(self, method) @@ -87,33 +87,25 @@ def __get__(self, obj, objtype): def __call__(self, package_self, *args, **kwargs): - """Try to find a method that matches package_self.sys_type. - If none is found, call the default method that this was - initialized with. If there is no default, raise an error. + """Find the first method with a spec that matches the + package's spec. If none is found, call the default + or if there is none, then raise a NoSuchMethodError. """ - spec = package_self.spec - matching_specs = [s for s in self.method_map if s.satisfies(spec)] - num_matches = len(matching_specs) - if num_matches == 0: - if self.default is None: - raise NoSuchMethodError(type(package_self), self.__name__, - spec, self.method_map.keys()) - else: - method = self.default - - elif num_matches == 1: - method = self.method_map[matching_specs[0]] + for spec, method in self.method_list: + if spec.satisfies(package_self.spec): + return method(package_self, *args, **kwargs) + if self.default: + return self.default(package_self, *args, **kwargs) else: - raise AmbiguousMethodError(type(package_self), self.__name__, - spec, matching_specs) - - return method(package_self, *args, **kwargs) + raise NoSuchMethodError( + type(package_self), self.__name__, spec, + [m[0] for m in self.method_list]) def __str__(self): return "SpecMultiMethod {\n\tdefault: %s,\n\tspecs: %s\n}" % ( - self.default, self.method_map) + self.default, self.method_list) class when(object): @@ -207,12 +199,3 @@ def __init__(self, cls, method_name, spec, possible_specs): "Package %s does not support %s called with %s. Options are: %s" % (cls.__name__, method_name, spec, ", ".join(str(s) for s in possible_specs))) - - -class AmbiguousMethodError(spack.error.SpackError): - """Raised when we can't find a version of a multi-method.""" - def __init__(self, cls, method_name, spec, matching_specs): - super(AmbiguousMethodError, self).__init__( - "Package %s has multiple versions of %s that match %s: %s" - % (cls.__name__, method_name, spec, - ",".join(str(s) for s in matching_specs))) diff --git a/lib/spack/spack/test/mock_packages/multimethod.py b/lib/spack/spack/test/mock_packages/multimethod.py index e5c2f5243e6a698ae81ca34857d7ca3852331968..6b6aca976732ae8e8f1d8d1852b807029b495667 100644 --- a/lib/spack/spack/test/mock_packages/multimethod.py +++ b/lib/spack/spack/test/mock_packages/multimethod.py @@ -27,16 +27,35 @@ def no_version_2(self): # - # These functions overlap too much, so there is ambiguity + # These functions overlap, so there is ambiguity, but we'll take + # the first one. # @when('@:4') def version_overlap(self): - pass + return 1 @when('@2:') def version_overlap(self): - pass + return 2 + + + # + # More complicated case with cascading versions. + # + def mpi_version(self): + return 0 + @when('^mpi@3:') + def mpi_version(self): + return 3 + + @when('^mpi@2:') + def mpi_version(self): + return 2 + + @when('^mpi@1:') + def mpi_version(self): + return 1 # diff --git a/lib/spack/spack/test/multimethod.py b/lib/spack/spack/test/multimethod.py index 969c987da854b4c97bf1ae5aff62295f8a6a8f37..3a164a3d20660fd03e7e8f34bc919dcd71a37d5d 100644 --- a/lib/spack/spack/test/multimethod.py +++ b/lib/spack/spack/test/multimethod.py @@ -30,8 +30,30 @@ def test_one_version_match(self): def test_version_overlap(self): - pkg = packages.get('multimethod@3.0') - self.assertRaises(AmbiguousMethodError, pkg.version_overlap) + pkg = packages.get('multimethod@2.0') + self.assertEqual(pkg.version_overlap(), 1) + + pkg = packages.get('multimethod@5.0') + self.assertEqual(pkg.version_overlap(), 2) + + + def test_mpi_version(self): + pkg = packages.get('multimethod^mpich@3.0.4') + self.assertEqual(pkg.mpi_version(), 3) + + pkg = packages.get('multimethod^mpich2@1.2') + self.assertEqual(pkg.mpi_version(), 2) + + pkg = packages.get('multimethod^mpich@1.0') + self.assertEqual(pkg.mpi_version(), 1) + + + def test_undefined_mpi_version(self): + # This currently fails because provides() doesn't do + # the right thing undefined version ranges. + # TODO: fix this. + pkg = packages.get('multimethod^mpich@0.4') + self.assertEqual(pkg.mpi_version(), 0) def test_default_works(self): @@ -69,11 +91,10 @@ def test_dependency_match(self): pkg = packages.get('multimethod^mpich') self.assertEqual(pkg.different_by_dep(), 'mpich') - - def test_ambiguous_dep(self): - """If we try to switch on some entirely different dep, it's ambiguous""" + # If we try to switch on some entirely different dep, it's ambiguous, + # but should take the first option pkg = packages.get('multimethod^foobar') - self.assertRaises(AmbiguousMethodError, pkg.different_by_dep) + self.assertEqual(pkg.different_by_dep(), 'mpich') def test_virtual_dep_match(self):