-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Spec interface for build information (like lib names, includes, etc.) #1821
Description
Per discussion in #1682, reposting this for discussion.
One thing I thought about when looking at this PR: I don't really like it that some dependents still search for their dependencies libraries, though I realize that it can be necessary. Basically there are two guiding principles here:
- The dependencies know themselves best, and if they can expose their build information consistently, they should do that and let dependents access it.
- Dependents should prefer to ask dependencies for build information (lib locations, etc.) and only search if necessary. Searching complicates build logic and Spack should already know where things are because it already installed the dependencies.
(2) breaks down for external packages, which Spack didn't install and which may even be laid out differently than Spack would lay them out (e.g. a install of boost might have al the weird lib suffixes that boost supports, and I think Spack wouldn't currently know what to do with that). Even better is that find_libraries is hidden behind a property interface so the client code doesn't know how the library names are being assigned.
This PR follows both of these principles for LAPACK and BLAS. Using properties on specs for things like blas_libs and lapack_libs is great. It doesn't follow these for fftw, arpack, superlu, etc. See the install method for armadillo, which does its own find routines.
I think this PR makes it clearer to me what a good convention for passing args among packages in a build should look like. You want to ask the package for a property, and the package needs to know a) the property name and b) the vdep it's being asked as. For non-virtual packages, (b) doesn't matter and is always just the plain package name, but when you're asked to provide the libs/headers for a particular interface, you need to know the interface name.
I guess the question is whether this should be a convention or a more formal interface. The convention currently looks like:
spec['blas'].blas_libs
spec['lapack'].lapack_libsSlightly annoying things about it:
- You have to write the interface twice even though you know what you asked for.
- The interface name is written once as a string and once as a property name prefix.
So, think we could have a convention like this:
def provider_property(function):
"""Function object for grabbing build properties from dependency Packages."""
def __init__(self, name):
self.name = name
def __call__(self, provider):
"""Get provider-specific property if available, or default to generic property."""
specific = provider.replace('-', '_') + '_' + self.name
dep = spec[provider]
if hasattr(dep, specific):
return getattr(dep, specific)
else:
return getattr(dep, self.name) # default to generic attribute
class Spec(object):
...
libs = provider_property('libs')
includes = provider_property('includes')Now the interface looks like this:
spec.libs('blas')
spec.libs('lapack')And you've got a convention for properties to add to packages (libs, <provider>_libs, etc.). Package could provide a default implementation:
class Package(object):
...
def libs(self):
# requiring installation allows us to be more generic b/c we can search,
# but it does restrict when you can call the method.
assert(self.installed)
return find_libraries(... something that returns all the libs in `lib` or `lib64`...)And packages that need to can add things like blas_libs, lapack_libs, superlu_dist_libs, etc. as they do in this PR. Packages like arpack and superlu can do nothing b/c we find their libs automatically and they're available through, e.g. spec.libs('arpack').
I guess the question is whether the interface above is so much better than the simple convention we've got that it's worthwhile. The current convention is not so bad. I think it boils down which is simpler to document and teach to package authors.
The other question is whether this convention is worthwhile for non-virtuals. Sometimes the client needs to be specific about which lib names it needs, in which case find_libraries seems like the right thing to do.
What do you all think?