From 285c5444ab0e1522b8e04b2052bfe5649f6838a3 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sat, 17 May 2014 13:01:00 -0700 Subject: Add CompilerSpec class and loading capability. - spack.spec.Compiler is now spack.spec.CompilerSpec - Can load a spack.compilers.* module for a particular spec - e.g. load Gcc module for gcc@4.7 spec. --- lib/spack/docs/packaging_guide.rst | 4 +-- lib/spack/spack/build_environment.py | 6 ++++ lib/spack/spack/compilers/__init__.py | 59 +++++++++++++++++++++++++++------ lib/spack/spack/concretize.py | 9 ++++++ lib/spack/spack/package.py | 1 + lib/spack/spack/packages.py | 51 ++--------------------------- lib/spack/spack/spec.py | 51 ++++++++++++++++++----------- lib/spack/spack/test/concretize.py | 4 +-- lib/spack/spack/test/packages.py | 11 ++++--- lib/spack/spack/test/spec_syntax.py | 12 +++---- lib/spack/spack/util/naming.py | 61 +++++++++++++++++++++++++++++++++++ 11 files changed, 177 insertions(+), 92 deletions(-) create mode 100644 lib/spack/spack/util/naming.py (limited to 'lib') diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index 4e076aa991..da944b33c8 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -107,7 +107,7 @@ Package class names The **class name** (``Libelf`` in our example) is formed by converting words separated by `-` or ``_`` in the file name to camel case. If the name starts with a number, we prefix the class name with -``Num_``. Here are some examples: +``_``. Here are some examples: ================= ================= Module Name Class Name @@ -115,7 +115,7 @@ the name starts with a number, we prefix the class name with ``foo_bar`` ``FooBar`` ``docbook-xml`` ``DocbookXml`` ``FooBar`` ``Foobar`` - ``3proxy`` ``Num_3proxy`` + ``3proxy`` ``_3proxy`` ================= ================= The class name is needed by Spack to properly import a package, but diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index d6becb77db..8d3d0909db 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -34,6 +34,7 @@ import platform from llnl.util.filesystem import * import spack +from spack.compilers import compiler_for_spec from spack.util.executable import Executable, which from spack.util.environment import * @@ -79,6 +80,11 @@ class MakeExecutable(Executable): super(MakeExecutable, self).__call__(*args, **kwargs) +def set_compiler_environment_variables(pkg): + assert(pkg.spec.concrete) + compiler = compiler_for_spec(pkg.spec.compiler) + + def set_build_environment_variables(pkg): """This ensures a clean install environment when we build packages. """ diff --git a/lib/spack/spack/compilers/__init__.py b/lib/spack/spack/compilers/__init__.py index 2b97cd1036..095e26edb5 100644 --- a/lib/spack/spack/compilers/__init__.py +++ b/lib/spack/spack/compilers/__init__.py @@ -25,11 +25,21 @@ # # This needs to be expanded for full compiler support. # +import imp + from llnl.util.lang import memoized, list_modules +from llnl.util.filesystem import join_path import spack +import spack.error import spack.spec +from spack.compiler import Compiler from spack.util.executable import which +from spack.util.naming import mod_to_class + +_imported_compilers_module = 'spack.compiler.versions' +_imported_versions_module = 'spack.compilers' + @memoized def supported_compilers(): @@ -41,29 +51,58 @@ def supported_compilers(): return sorted(c for c in list_modules(spack.compilers_path)) +def supported(compiler_spec): + """Test if a particular compiler is supported.""" + if not isinstance(compiler_spec, spack.spec.CompilerSpec): + compiler_spec = spack.spec.CompilerSpec(compiler_spec) + return compiler_spec.name in supported_compilers() + + def available_compilers(): """Return a list of specs for all the compiler versions currently available to build with. These are instances of - spack.spec.Compiler. + CompilerSpec. """ - return [spack.spec.Compiler(c) + return [spack.spec.CompilerSpec(c) for c in list_modules(spack.compiler_version_path)] -def supported(compiler_spec): - """Test if a particular compiler is supported.""" - if not isinstance(compiler_spec, spack.spec.Compiler): - compiler_spec = spack.spec.Compiler(compiler_spec) - return compiler_spec.name in supported_compilers() +def compiler_for_spec(compiler_spec): + """This gets an instance of an actual spack.compiler.Compiler object + from a compiler spec. The spec needs to be concrete for this to + work; it will raise an error if passed an abstract compiler. + """ + matches = [c for c in available_compilers() if c.satisfies(compiler_spec)] + + # TODO: do something when there are zero matches. + assert(len(matches) >= 1) + + compiler = matches[0] + file_path = join_path(spack.compiler_version_path, "%s.py" % compiler) + + mod = imp.load_source(_imported_versions_module, file_path) + compiler_class = class_for_compiler_name(compiler.name) + + return compiler_class(mod.cc, mod.cxx, mod.f77, mod.f90) + + +def class_for_compiler_name(compiler_name): + assert(supported(compiler_name)) + + file_path = join_path(spack.compilers_path, compiler_name + ".py") + compiler_mod = imp.load_source(_imported_compilers_module, file_path) + return getattr(compiler_mod, mod_to_class(compiler_name)) @memoized def default_compiler(): - """Get the spec for the default compiler supported by Spack. + """Get the spec for the default compiler on the system. Currently just returns the system's default gcc. - TODO: provide a better way to specify/find this on startup. + TODO: provide a more sensible default. e.g. on Intel systems + we probably want icc. On Mac OS, clang. Probably need + to inspect the system and figure this out. """ gcc = which('gcc', required=True) version = gcc('-dumpversion', return_output=True) - return spack.spec.Compiler('gcc', version) + return spack.spec.CompilerSpec('gcc', version) diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 8679d3e13e..28efcaea64 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -99,6 +99,15 @@ class DefaultConcretizer(object): this one has a strict compiler requirement. Otherwise, try to build with the compiler that will be used by libraries that link to this one, to maximize compatibility. + + TODO: In many cases we probably want to look for installed + versions of each package and use *that* version if we + can link to it. The policy implemented here will + tend to rebuild a lot of stuff becasue it will prefer + a compiler in the spec to any compiler already- + installed things were built with. There is likely + some better policy that finds some middle ground + between these two extremes. """ if spec.compiler and spec.compiler.concrete: return diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 59d7ce7907..b40448df37 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -657,6 +657,7 @@ class Package(object): spack.install_layout.make_path_for_spec(self.spec) # Set up process's build environment before running install. + build_env.set_compiler_environment_variables(self) build_env.set_build_environment_variables(self) build_env.set_module_variables_for_package(self) diff --git a/lib/spack/spack/packages.py b/lib/spack/spack/packages.py index 08ded5cdbb..36f3d4286a 100644 --- a/lib/spack/spack/packages.py +++ b/lib/spack/spack/packages.py @@ -22,10 +22,8 @@ # along with this program; if not, write to the Free Software Foundation, # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA ############################################################################## -import re import os import sys -import string import inspect import glob import imp @@ -38,6 +36,7 @@ import spack import spack.error import spack.spec from spack.virtual import ProviderIndex +from spack.util.naming import mod_to_class, validate_module_name # Name of module under which packages are imported _imported_packages_module = 'spack.packages' @@ -45,42 +44,6 @@ _imported_packages_module = 'spack.packages' # Name of the package file inside a package directory _package_file_name = 'package.py' -# Valid package names can contain '-' but can't start with it. -valid_package_re = r'^\w[\w-]*$' - -# Don't allow consecutive [_-] in package names -invalid_package_re = r'[_-][_-]+' - - -def valid_package_name(pkg_name): - """Return whether the pkg_name is valid for use in Spack.""" - return (re.match(valid_package_re, pkg_name) and - not re.search(invalid_package_re, pkg_name)) - - -def validate_package_name(pkg_name): - """Raise an exception if pkg_name is not valid.""" - if not valid_package_name(pkg_name): - raise InvalidPackageNameError(pkg_name) - - -def class_name_for_package_name(pkg_name): - """Get a name for the class the package file should contain. Note that - conflicts don't matter because the classes are in different modules. - """ - validate_package_name(pkg_name) - - class_name = pkg_name.replace('_', '-') - class_name = string.capwords(class_name, '-') - class_name = class_name.replace('-', '') - - # If a class starts with a number, prefix it with Number_ to make it a valid - # Python class name. - if re.match(r'^[0-9]', class_name): - class_name = "Num_%s" % class_name - - return class_name - def _autospec(function): """Decorator that automatically converts the argument of a single-arg @@ -143,7 +106,7 @@ class PackageDB(object): package doesn't exist yet, so callers will need to ensure the package exists before importing. """ - validate_package_name(pkg_name) + validate_module_name(pkg_name) pkg_dir = self.dirname_for_package_name(pkg_name) return join_path(pkg_dir, _package_file_name) @@ -200,7 +163,7 @@ class PackageDB(object): else: raise UnknownPackageError(pkg_name) - class_name = class_name_for_package_name(pkg_name) + class_name = mod_to_class(pkg_name) try: module_name = _imported_packages_module + '.' + pkg_name module = imp.load_source(module_name, file_path) @@ -259,14 +222,6 @@ class PackageDB(object): out.write('}\n') -class InvalidPackageNameError(spack.error.SpackError): - """Raised when we encounter a bad package name.""" - def __init__(self, name): - super(InvalidPackageNameError, self).__init__( - "Invalid package name: " + name) - self.name = name - - class UnknownPackageError(spack.error.SpackError): """Raised when we encounter a package spack doesn't have.""" def __init__(self, name): diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index d0b08cca00..f0244695bc 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -102,8 +102,7 @@ from llnl.util.tty.color import * import spack import spack.parse import spack.error -import spack.compilers -import spack.compilers.gcc +from spack.compilers import supported as supported_compiler from spack.version import * from spack.util.string import * @@ -169,17 +168,29 @@ def colorize_spec(spec): @key_ordering -class Compiler(object): - """The Compiler field represents the compiler or range of compiler - versions that a package should be built with. Compilers have a +class CompilerSpec(object): + """The CompilerSpec field represents the compiler or range of compiler + versions that a package should be built with. CompilerSpecs have a name and a version list. """ def __init__(self, *args): nargs = len(args) if nargs == 1: - # If there is one argument, it's a spec to parse - c = SpecParser().parse_compiler(args[0]) - self.name = c.name - self.versions = c.versions + arg = args[0] + # If there is one argument, it's either another CompilerSpec + # to copy or a string to parse + if isinstance(arg, basestring): + c = SpecParser().parse_compiler(arg) + self.name = c.name + self.versions = c.versions + + elif isinstance(arg, CompilerSpec): + self.name = arg.name + self.versions = arg.versions.copy() + + else: + raise TypeError( + "Can only build CompilerSpec from string or CompilerSpec." + + " Found %s" % type(arg)) elif nargs == 2: name, version = args @@ -197,12 +208,14 @@ class Compiler(object): def _autospec(self, compiler_spec_like): - if not isinstance(compiler_spec_like, Compiler): - return Compiler(compiler_spec_like) - return compiler_spec_like + if isinstance(compiler_spec_like, CompilerSpec): + return compiler_spec_like + return CompilerSpec(compiler_spec_like) def satisfies(self, other): + # TODO: This should not just look for overlapping versions. + # TODO: e.g., 4.7.3 should satisfy a requirement for 4.7. other = self._autospec(other) return (self.name == other.name and self.versions.overlaps(other.versions)) @@ -218,7 +231,7 @@ class Compiler(object): @property def concrete(self): - """A Compiler spec is concrete if its versions are concrete.""" + """A CompilerSpec is concrete if its versions are concrete.""" return self.versions.concrete @@ -230,7 +243,7 @@ class Compiler(object): def copy(self): - clone = Compiler.__new__(Compiler) + clone = CompilerSpec.__new__(CompilerSpec) clone.name = self.name clone.versions = self.versions.copy() return clone @@ -353,7 +366,7 @@ class Spec(object): def _set_compiler(self, compiler): """Called by the parser to set the compiler.""" - if self.compiler: raise DuplicateCompilerError( + if self.compiler: raise DuplicateCompilerSpecError( "Spec for '%s' cannot have two compilers." % self.name) self.compiler = compiler @@ -808,7 +821,7 @@ class Spec(object): # validate compiler in addition to the package name. if spec.compiler: - if not spack.compilers.supported(spec.compiler): + if not supported_compiler(spec.compiler): raise UnsupportedCompilerError(spec.compiler.name) @@ -1320,7 +1333,7 @@ class SpecParser(spack.parse.Parser): self.expect(ID) self.check_identifier() - compiler = Compiler.__new__(Compiler) + compiler = CompilerSpec.__new__(CompilerSpec) compiler.name = self.token.value compiler.versions = VersionList() if self.accept(AT): @@ -1402,10 +1415,10 @@ class DuplicateVariantError(SpecError): super(DuplicateVariantError, self).__init__(message) -class DuplicateCompilerError(SpecError): +class DuplicateCompilerSpecError(SpecError): """Raised when the same compiler occurs in a spec twice.""" def __init__(self, message): - super(DuplicateCompilerError, self).__init__(message) + super(DuplicateCompilerSpecError, self).__init__(message) class UnsupportedCompilerError(SpecError): diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 2a989f6766..6ad2ef29d8 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -25,7 +25,7 @@ import unittest import spack -from spack.spec import Spec, Compiler +from spack.spec import Spec, CompilerSpec from spack.test.mock_packages_test import * class ConcretizeTest(MockPackagesTest): @@ -169,7 +169,7 @@ class ConcretizeTest(MockPackagesTest): spec = Spec('mpileaks') spec.normalize() - spec['dyninst'].compiler = Compiler('clang') + spec['dyninst'].compiler = CompilerSpec('clang') spec.concretize() # TODO: not exactly the syntax I would like. diff --git a/lib/spack/spack/test/packages.py b/lib/spack/spack/test/packages.py index 1b2e0ab07a..a8183cf6a6 100644 --- a/lib/spack/spack/test/packages.py +++ b/lib/spack/spack/test/packages.py @@ -28,6 +28,7 @@ from llnl.util.filesystem import join_path import spack import spack.packages as packages +from spack.util.naming import mod_to_class from spack.test.mock_packages_test import * @@ -58,8 +59,8 @@ class PackagesTest(MockPackagesTest): def test_package_class_names(self): - self.assertEqual('Mpich', packages.class_name_for_package_name('mpich')) - self.assertEqual('PmgrCollective', packages.class_name_for_package_name('pmgr_collective')) - self.assertEqual('PmgrCollective', packages.class_name_for_package_name('pmgr-collective')) - self.assertEqual('Pmgrcollective', packages.class_name_for_package_name('PmgrCollective')) - self.assertEqual('Num_3db', packages.class_name_for_package_name('3db')) + self.assertEqual('Mpich', mod_to_class('mpich')) + self.assertEqual('PmgrCollective', mod_to_class('pmgr_collective')) + self.assertEqual('PmgrCollective', mod_to_class('pmgr-collective')) + self.assertEqual('Pmgrcollective', mod_to_class('PmgrCollective')) + self.assertEqual('_3db', mod_to_class('3db')) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 20468be1b1..404f38906e 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -133,12 +133,12 @@ class SpecSyntaxTest(unittest.TestCase): self.assertRaises(DuplicateDependencyError, self.check_parse, "x ^y ^y") def test_duplicate_compiler(self): - self.assertRaises(DuplicateCompilerError, self.check_parse, "x%intel%intel") - self.assertRaises(DuplicateCompilerError, self.check_parse, "x%intel%gcc") - self.assertRaises(DuplicateCompilerError, self.check_parse, "x%gcc%intel") - self.assertRaises(DuplicateCompilerError, self.check_parse, "x ^y%intel%intel") - self.assertRaises(DuplicateCompilerError, self.check_parse, "x ^y%intel%gcc") - self.assertRaises(DuplicateCompilerError, self.check_parse, "x ^y%gcc%intel") + self.assertRaises(DuplicateCompilerSpecError, self.check_parse, "x%intel%intel") + self.assertRaises(DuplicateCompilerSpecError, self.check_parse, "x%intel%gcc") + self.assertRaises(DuplicateCompilerSpecError, self.check_parse, "x%gcc%intel") + self.assertRaises(DuplicateCompilerSpecError, self.check_parse, "x ^y%intel%intel") + self.assertRaises(DuplicateCompilerSpecError, self.check_parse, "x ^y%intel%gcc") + self.assertRaises(DuplicateCompilerSpecError, self.check_parse, "x ^y%gcc%intel") # ================================================================================ diff --git a/lib/spack/spack/util/naming.py b/lib/spack/spack/util/naming.py new file mode 100644 index 0000000000..782afbd4bb --- /dev/null +++ b/lib/spack/spack/util/naming.py @@ -0,0 +1,61 @@ +# Need this because of spack.util.string +from __future__ import absolute_import +import string +import re + +import spack + +# Valid module names can contain '-' but can't start with it. +_valid_module_re = r'^\w[\w-]*$' + + +def mod_to_class(mod_name): + """Convert a name from module style to class name style. Spack mostly + follows `PEP-8 `_: + + * Module and package names use lowercase_with_underscores. + * Class names use the CapWords convention. + + Regular source code follows these convetions. Spack is a bit + more liberal with its Package names nad Compiler names: + + * They can contain '-' as well as '_', but cannot start with '-'. + * They can start with numbers, e.g. "3proxy". + + This function converts from the module convention to the class + convention by removing _ and - and converting surrounding + lowercase text to CapWords. If mod_name starts with a number, + the class name returned will be prepended with '_' to make a + valid Python identifier. + """ + validate_module_name(mod_name) + + class_name = re.sub(r'[-_]+', '-', mod_name) + class_name = string.capwords(class_name, '-') + class_name = class_name.replace('-', '') + + # If a class starts with a number, prefix it with Number_ to make it a valid + # Python class name. + if re.match(r'^[0-9]', class_name): + class_name = "_%s" % class_name + + return class_name + + +def valid_module_name(mod_name): + """Return whether the mod_name is valid for use in Spack.""" + return bool(re.match(_valid_module_re, mod_name)) + + +def validate_module_name(mod_name): + """Raise an exception if mod_name is not valid.""" + if not valid_module_name(mod_name): + raise InvalidModuleNameError(mod_name) + + +class InvalidModuleNameError(spack.error.SpackError): + """Raised when we encounter a bad module name.""" + def __init__(self, name): + super(InvalidModuleNameError, self).__init__( + "Invalid module name: " + name) + self.name = name -- cgit v1.2.3-60-g2f50