From 554ae9b3552a40ed253250bdebf548e4d8b01976 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Tue, 15 Apr 2014 08:34:04 -0700 Subject: bugfix for SPACK-20: add dependency bin directories to PATH - Consolidated build environment stuff from package.py into build_environment.py - package.py calls build_environment.py functions just before calling install(), in do_install() - Organization is better b/c SPACK_xxxx environment variables are now in build_environment, the only place they're used. Were previously cluttering globals.py. --- lib/spack/env/cc | 24 +++--- lib/spack/spack/build_environment.py | 160 +++++++++++++++++++++++++++++++++++ lib/spack/spack/globals.py | 9 +- lib/spack/spack/package.py | 114 ++----------------------- 4 files changed, 181 insertions(+), 126 deletions(-) create mode 100644 lib/spack/spack/build_environment.py diff --git a/lib/spack/env/cc b/lib/spack/env/cc index 0711c873e3..e5dbf21beb 100755 --- a/lib/spack/env/cc +++ b/lib/spack/env/cc @@ -62,15 +62,17 @@ options, other_args = parser.parse_known_args() rpaths, other_args = parse_rpaths(other_args) # Add dependencies' include and lib paths to our compiler flags. -def append_if_dir(path_list, *dirs): - full_path = os.path.join(*dirs) - if os.path.isdir(full_path): - path_list.append(full_path) +def add_if_dir(path_list, directory, index=None): + if os.path.isdir(directory): + if index is None: + path_list.append(directory) + else: + path_list.insert(index, directory) for dep_dir in spack_deps: - append_if_dir(options.include_path, dep_dir, "include") - append_if_dir(options.lib_path, dep_dir, "lib") - append_if_dir(options.lib_path, dep_dir, "lib64") + add_if_dir(options.include_path, os.path.join(dep_dir, "include")) + add_if_dir(options.lib_path, os.path.join(dep_dir, "lib")) + add_if_dir(options.lib_path, os.path.join(dep_dir, "lib64")) # Add our modified arguments to it. arguments = ['-I%s' % path for path in options.include_path] @@ -95,11 +97,9 @@ for var in ["LD_LIBRARY_PATH", "LD_RUN_PATH", "DYLD_LIBRARY_PATH"]: os.environ.pop(var) # Ensure that the delegated command doesn't just call this script again. -clean_path = get_path("PATH") -for item in ['.'] + spack_env_path: - if item in clean_path: - clean_path.remove(item) -os.environ["PATH"] = ":".join(clean_path) +remove_paths = ['.'] + spack_env_path +path = [p for p in get_path("PATH") if p not in remove_paths] +os.environ["PATH"] = ":".join(path) full_command = [command] + arguments diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py new file mode 100644 index 0000000000..d6becb77db --- /dev/null +++ b/lib/spack/spack/build_environment.py @@ -0,0 +1,160 @@ +""" +This module contains all routines related to setting up the package +build environment. All of this is set up by package.py just before +install() is called. + +There are two parts to the bulid environment: + +1. Python build environment (i.e. install() method) + + This is how things are set up when install() is called. Spack + takes advantage of each package being in its own module by adding a + bunch of command-like functions (like configure(), make(), etc.) in + the package's module scope. Ths allows package writers to call + them all directly in Package.install() without writing 'self.' + everywhere. No, this isn't Pythonic. Yes, it makes the code more + readable and more like the shell script from whcih someone is + likely porting. + +2. Build execution environment + + This is the set of environment variables, like PATH, CC, CXX, + etc. that control the build. There are also a number of + environment variables used to pass information (like RPATHs and + other information about dependencies) to Spack's compiler wrappers. + All of these env vars are also set up here. + +Skimming this module is a nice way to get acquainted with the types of +calls you can make from within the install() function. +""" +import os +import shutil +import multiprocessing +import platform +from llnl.util.filesystem import * + +import spack +from spack.util.executable import Executable, which +from spack.util.environment import * + +# +# This can be set by the user to globally disable parallel builds. +# +SPACK_NO_PARALLEL_MAKE = 'SPACK_NO_PARALLEL_MAKE' + +# +# These environment variables are set by +# set_build_environment_variables and used to pass parameters to +# Spack's compiler wrappers. +# +SPACK_LIB = 'SPACK_LIB' +SPACK_ENV_PATH = 'SPACK_ENV_PATH' +SPACK_DEPENDENCIES = 'SPACK_DEPENDENCIES' +SPACK_PREFIX = 'SPACK_PREFIX' +SPACK_BUILD_ROOT = 'SPACK_BUILD_ROOT' + + +class MakeExecutable(Executable): + """Special callable executable object for make so the user can + specify parallel or not on a per-invocation basis. Using + 'parallel' as a kwarg will override whatever the package's + global setting is, so you can either default to true or false + and override particular calls. + + Note that if the SPACK_NO_PARALLEL_MAKE env var is set it overrides + everything. + """ + def __init__(self, name, parallel): + super(MakeExecutable, self).__init__(name) + self.parallel = parallel + + def __call__(self, *args, **kwargs): + parallel = kwargs.get('parallel', self.parallel) + disable_parallel = env_flag(SPACK_NO_PARALLEL_MAKE) + + if parallel and not disable_parallel: + jobs = "-j%d" % multiprocessing.cpu_count() + args = (jobs,) + args + + super(MakeExecutable, self).__call__(*args, **kwargs) + + +def set_build_environment_variables(pkg): + """This ensures a clean install environment when we build packages. + """ + # This tells the compiler script where to find the Spack installation. + os.environ[SPACK_LIB] = spack.lib_path + + # Add spack build environment path with compiler wrappers first in + # the path. We handle case sensitivity conflicts like "CC" and + # "cc" by putting one in the /case-insensitive + # directory. Add that to the path too. + env_paths = [spack.build_env_path, + join_path(spack.build_env_path, 'case-insensitive')] + path_put_first("PATH", env_paths) + path_set(SPACK_ENV_PATH, env_paths) + + # Prefixes of all of the package's dependencies go in + # SPACK_DEPENDENCIES + dep_prefixes = [d.package.prefix for d in pkg.spec.dependencies.values()] + path_set(SPACK_DEPENDENCIES, dep_prefixes) + + # Install prefix + os.environ[SPACK_PREFIX] = pkg.prefix + + # Build root for logging. + os.environ[SPACK_BUILD_ROOT] = pkg.stage.expanded_archive_path + + # Remove these vars from the environment during build becaus they + # can affect how some packages find libraries. We want to make + # sure that builds never pull in unintended external dependencies. + pop_keys(os.environ, "LD_LIBRARY_PATH", "LD_RUN_PATH", "DYLD_LIBRARY_PATH") + + # Add bin directories from dependencies to the PATH for the build. + bin_dirs = ['%s/bin' % prefix for prefix in dep_prefixes] + path_put_first('PATH', [bin for bin in bin_dirs if os.path.isdir(bin)]) + + +def set_module_variables_for_package(pkg): + """Populate the module scope of install() with some useful functions. + This makes things easier for package writers. + """ + m = pkg.module + + m.make = MakeExecutable('make', pkg.parallel) + m.gmake = MakeExecutable('gmake', pkg.parallel) + + # number of jobs spack prefers to build with. + m.make_jobs = multiprocessing.cpu_count() + + # Find the configure script in the archive path + # Don't use which for this; we want to find it in the current dir. + m.configure = Executable('./configure') + + # TODO: shouldn't really use "which" here. Consider adding notion + # TODO: of build dependencies, as opposed to link dependencies. + # TODO: Currently, everything is a link dependency, but tools like + # TODO: this shouldn't be. + m.cmake = which("cmake") + + # standard CMake arguments + m.std_cmake_args = ['-DCMAKE_INSTALL_PREFIX=%s' % pkg.prefix, + '-DCMAKE_BUILD_TYPE=None'] + if platform.mac_ver()[0]: + m.std_cmake_args.append('-DCMAKE_FIND_FRAMEWORK=LAST') + + # Emulate some shell commands for convenience + m.cd = os.chdir + m.mkdir = os.mkdir + m.makedirs = os.makedirs + m.remove = os.remove + m.removedirs = os.removedirs + + m.mkdirp = mkdirp + m.install = install + m.rmtree = shutil.rmtree + m.move = shutil.move + + # Useful directories within the prefix are encapsulated in + # a Prefix object. + m.prefix = pkg.prefix diff --git a/lib/spack/spack/globals.py b/lib/spack/spack/globals.py index 4ff50a3e7e..096c1334f5 100644 --- a/lib/spack/spack/globals.py +++ b/lib/spack/spack/globals.py @@ -40,7 +40,7 @@ spack_file = join_path(prefix, "bin", "spack") # spack directory hierarchy lib_path = join_path(prefix, "lib", "spack") -env_path = join_path(lib_path, "env") +build_env_path = join_path(lib_path, "env") module_path = join_path(lib_path, "spack") compilers_path = join_path(module_path, "compilers") test_path = join_path(module_path, "test") @@ -130,10 +130,3 @@ sys_type = None # mirrors = [] -# Important environment variables -SPACK_NO_PARALLEL_MAKE = 'SPACK_NO_PARALLEL_MAKE' -SPACK_LIB = 'SPACK_LIB' -SPACK_ENV_PATH = 'SPACK_ENV_PATH' -SPACK_DEPENDENCIES = 'SPACK_DEPENDENCIES' -SPACK_PREFIX = 'SPACK_PREFIX' -SPACK_BUILD_ROOT = 'SPACK_BUILD_ROOT' diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 27b7b8aef5..b8f66bbb76 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -38,25 +38,22 @@ import os import re import subprocess import platform as py_platform -import shutil import multiprocessing from urlparse import urlparse import llnl.util.tty as tty -from llnl.util.tty.color import cwrite from llnl.util.filesystem import * from llnl.util.lang import * import spack import spack.spec import spack.error +import spack.build_environment as build_env import spack.url as url import spack.util.crypto as crypto from spack.version import * from spack.stage import Stage from spack.util.web import get_pages -from spack.util.environment import * -from spack.util.executable import Executable, which from spack.util.compression import allowed_archive """Allowed URL schemes for spack packages.""" @@ -413,46 +410,6 @@ class Package(object): return self._stage - def add_commands_to_module(self): - """Populate the module scope of install() with some useful functions. - This makes things easier for package writers. - """ - m = self.module - - m.make = MakeExecutable('make', self.parallel) - m.gmake = MakeExecutable('gmake', self.parallel) - - # number of jobs spack prefers to build with. - m.make_jobs = multiprocessing.cpu_count() - - # Find the configure script in the archive path - # Don't use which for this; we want to find it in the current dir. - m.configure = Executable('./configure') - m.cmake = which("cmake") - - # standard CMake arguments - m.std_cmake_args = ['-DCMAKE_INSTALL_PREFIX=%s' % self.prefix, - '-DCMAKE_BUILD_TYPE=None'] - if py_platform.mac_ver()[0]: - m.std_cmake_args.append('-DCMAKE_FIND_FRAMEWORK=LAST') - - # Emulate some shell commands for convenience - m.cd = os.chdir - m.mkdir = os.mkdir - m.makedirs = os.makedirs - m.remove = os.remove - m.removedirs = os.removedirs - - m.mkdirp = mkdirp - m.install = install - m.rmtree = shutil.rmtree - m.move = shutil.move - - # Useful directories within the prefix are encapsulated in - # a Prefix object. - m.prefix = self.prefix - - def preorder_traversal(self, visited=None, **kwargs): """This does a preorder traversal of the package's dependence DAG.""" virtual = kwargs.get("virtual", False) @@ -682,19 +639,16 @@ class Package(object): self.do_install_dependencies() self.do_patch() - self.setup_install_environment() - - # Add convenience commands to the package's module scope to - # make building easier. - self.add_commands_to_module() - - tty.msg("Building %s." % self.name) # create the install directory (allow the layout to handle this in # case it needs to add extra files) spack.install_layout.make_path_for_spec(self.spec) + tty.msg("Building %s." % self.name) try: + build_env.set_build_environment_variables(self) + build_env.set_module_variables_for_package(self) + self.install(self.spec, self.prefix) if not os.path.isdir(self.prefix): tty.die("Install failed for %s. No install dir created." % self.name) @@ -713,36 +667,6 @@ class Package(object): self.stage.destroy() - def setup_install_environment(self): - """This ensures a clean install environment when we build packages.""" - pop_keys(os.environ, "LD_LIBRARY_PATH", "LD_RUN_PATH", "DYLD_LIBRARY_PATH") - - # Add spack environment at front of path and pass the - # lib location along so the compiler script can find spack - os.environ[spack.SPACK_LIB] = spack.lib_path - - # Fix for case-insensitive file systems. Conflicting links are - # in directories called "case*" within the env directory. - env_paths = [spack.env_path] - for file in os.listdir(spack.env_path): - path = join_path(spack.env_path, file) - if file.startswith("case") and os.path.isdir(path): - env_paths.append(path) - path_put_first("PATH", env_paths) - path_set(spack.SPACK_ENV_PATH, env_paths) - - # Pass along prefixes of dependencies here - path_set( - spack.SPACK_DEPENDENCIES, - [dep.package.prefix for dep in self.spec.dependencies.values()]) - - # Install location - os.environ[spack.SPACK_PREFIX] = self.prefix - - # Build root for logging. - os.environ[spack.SPACK_BUILD_ROOT] = self.stage.expanded_archive_path - - def do_install_dependencies(self): # Pass along paths of dependencies here for dep in self.spec.dependencies.values(): @@ -786,7 +710,8 @@ class Package(object): def clean(self): """By default just runs make clean. Override if this isn't good.""" try: - make = MakeExecutable('make', self.parallel) + # TODO: should we really call make clean, ro just blow away the directory? + make = build_env.MakeExecutable('make', self.parallel) make('clean') tty.msg("Successfully cleaned %s" % self.name) except subprocess.CalledProcessError, e: @@ -871,30 +796,6 @@ def find_versions_of_archive(archive_url, **kwargs): return versions -class MakeExecutable(Executable): - """Special Executable for make so the user can specify parallel or - not on a per-invocation basis. Using 'parallel' as a kwarg will - override whatever the package's global setting is, so you can - either default to true or false and override particular calls. - - Note that if the SPACK_NO_PARALLEL_MAKE env var is set it overrides - everything. - """ - def __init__(self, name, parallel): - super(MakeExecutable, self).__init__(name) - self.parallel = parallel - - def __call__(self, *args, **kwargs): - parallel = kwargs.get('parallel', self.parallel) - disable_parallel = env_flag(spack.SPACK_NO_PARALLEL_MAKE) - - if parallel and not disable_parallel: - jobs = "-j%d" % multiprocessing.cpu_count() - args = (jobs,) + args - - super(MakeExecutable, self).__call__(*args, **kwargs) - - def validate_package_url(url_string): """Determine whether spack can handle a particular URL or not.""" url = urlparse(url_string) @@ -911,6 +812,7 @@ def print_pkg(message): if mac_ver and Version(mac_ver) >= Version('10.7'): print u"\U0001F4E6" + tty.indent, else: + from llnl.util.tty.color import cwrite cwrite('@*g{[+]} ') print message -- cgit v1.2.3-60-g2f50