From bf610a379f1b613d75cf2c9d7e7575bf59f57059 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sun, 24 Sep 2017 12:00:35 -0700 Subject: Clean up exceptions and function names in directives. - Functions returned by directives were all called `_execute`, which made reading stack traces hard because you couldn't tell what directive a frame came from. - renamed them all to `_execute_` - Exceptions in directives were only really used in one or two places -- get rid of the boilerplate init functions and let the callsite specify the message. --- lib/spack/spack/directives.py | 64 ++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 41 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index db07d2b4c1..57063c7f63 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -133,8 +133,8 @@ class DirectiveMetaMixin(type): for d in DirectiveMetaMixin._directive_names: setattr(cls, d, {}) # Lazy execution of directives - for command in cls._directives_to_be_executed: - command(cls) + for directive in cls._directives_to_be_executed: + directive(cls) super(DirectiveMetaMixin, cls).__init__(name, bases, attr_dict) @@ -218,7 +218,7 @@ def version(ver, checksum=None, **kwargs): dictionary. Package must turn it into a valid fetch strategy later. """ - def _execute(pkg): + def _execute_version(pkg): # TODO: checksum vs md5 distinction is confusing -- fix this. # special case checksum for backward compatibility if checksum: @@ -226,7 +226,7 @@ def version(ver, checksum=None, **kwargs): # Store kwargs for the package to later with a fetch_strategy. pkg.versions[Version(ver)] = kwargs - return _execute + return _execute_version def _depends_on(pkg, spec, when=None, type=default_deptype): @@ -240,7 +240,8 @@ def _depends_on(pkg, spec, when=None, type=default_deptype): dep_spec = Spec(spec) if pkg.name == dep_spec.name: - raise CircularReferenceError('depends_on', pkg.name) + raise CircularReferenceError( + "Package '%s' cannot depend on itself." % pkg.name) type = canonical_deptype(type) conditions = pkg.dependencies.setdefault(dep_spec.name, {}) @@ -272,7 +273,7 @@ def conflicts(conflict_spec, when=None, msg=None): when (Spec): optional constraint that triggers the conflict msg (str): optional user defined message """ - def _execute(pkg): + def _execute_conflicts(pkg): # If when is not specified the conflict always holds condition = pkg.name if when is None else when when_spec = parse_anonymous_spec(condition, pkg.name) @@ -280,7 +281,7 @@ def conflicts(conflict_spec, when=None, msg=None): # Save in a list the conflicts and the associated custom messages when_spec_list = pkg.conflicts.setdefault(conflict_spec, []) when_spec_list.append((when_spec, msg)) - return _execute + return _execute_conflicts @directive(('dependencies')) @@ -289,9 +290,9 @@ def depends_on(spec, when=None, type=default_deptype): This directive is to be used inside a Package definition to declare that the package requires other packages to be built first. @see The section "Dependency specs" in the Spack Packaging Guide.""" - def _execute(pkg): + def _execute_depends_on(pkg): _depends_on(pkg, spec, when=when, type=type) - return _execute + return _execute_depends_on @directive(('extendees', 'dependencies')) @@ -309,7 +310,7 @@ def extends(spec, **kwargs): mechanism. """ - def _execute(pkg): + def _execute_extends(pkg): # if pkg.extendees: # directive = 'extends' # msg = 'Packages can extend at most one other package.' @@ -318,7 +319,7 @@ def extends(spec, **kwargs): when = kwargs.pop('when', pkg.name) _depends_on(pkg, spec, when=when) pkg.extendees[spec] = (Spec(spec), kwargs) - return _execute + return _execute_extends @directive('provided') @@ -327,18 +328,20 @@ def provides(*specs, **kwargs): 'mpi', other packages can declare that they depend on "mpi", and spack can use the providing package to satisfy the dependency. """ - def _execute(pkg): + def _execute_provides(pkg): spec_string = kwargs.get('when', pkg.name) provider_spec = parse_anonymous_spec(spec_string, pkg.name) for string in specs: for provided_spec in spack.spec.parse(string): if pkg.name == provided_spec.name: - raise CircularReferenceError('depends_on', pkg.name) + raise CircularReferenceError( + "Package '%s' cannot provide itself.") + if provided_spec not in pkg.provided: pkg.provided[provided_spec] = set() pkg.provided[provided_spec].add(provider_spec) - return _execute + return _execute_provides @directive('patches') @@ -359,7 +362,7 @@ def patch(url_or_filename, level=1, when=None, **kwargs): if it comes from a url) """ - def _execute(pkg): + def _execute_patch(pkg): constraint = pkg.name if when is None else when when_spec = parse_anonymous_spec(constraint, pkg.name) @@ -368,7 +371,7 @@ def patch(url_or_filename, level=1, when=None, **kwargs): cur_patches = pkg.patches.setdefault(when_spec, []) cur_patches.append(Patch.create(pkg, url_or_filename, level, **kwargs)) - return _execute + return _execute_patch @directive('variants') @@ -414,7 +417,7 @@ def variant( default = default description = str(description).strip() - def _execute(pkg): + def _execute_variant(pkg): if not re.match(spack.spec.identifier_re, name): directive = 'variant' msg = "Invalid variant name in {0}: '{1}'" @@ -423,7 +426,7 @@ def variant( pkg.variants[name] = Variant( name, default, description, values, multi, validator ) - return _execute + return _execute_variant @directive('resources') @@ -443,7 +446,7 @@ def resource(**kwargs): * 'placement' : (optional) gives the possibility to fine tune how the resource is moved into the main package stage area. """ - def _execute(pkg): + def _execute_resource(pkg): when = kwargs.get('when', pkg.name) destination = kwargs.get('destination', "") placement = kwargs.get('placement', None) @@ -472,33 +475,12 @@ def resource(**kwargs): name = kwargs.get('name') fetcher = from_kwargs(**kwargs) resources.append(Resource(name, fetcher, destination, placement)) - return _execute + return _execute_resource class DirectiveError(spack.error.SpackError): """This is raised when something is wrong with a package directive.""" - def __init__(self, directive, message): - super(DirectiveError, self).__init__(message) - self.directive = directive - class CircularReferenceError(DirectiveError): """This is raised when something depends on itself.""" - - def __init__(self, directive, package): - super(CircularReferenceError, self).__init__( - directive, - "Package '%s' cannot pass itself to %s" % (package, directive)) - self.package = package - - -class UnknownDependencyTypeError(DirectiveError): - """This is raised when a dependency is of an unknown type.""" - - def __init__(self, directive, package, deptype): - super(UnknownDependencyTypeError, self).__init__( - directive, - "Package '%s' cannot depend on a package via %s." - % (package, deptype)) - self.package = package -- cgit v1.2.3-60-g2f50