From 6b2c49648a7ad53022a0d83dcb043399d7041027 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Thu, 10 May 2018 10:01:55 -0700 Subject: init: move symbols in `spack` to `spack.pkgkit` - Spack packages were originally expected to call `from spack import *` themselves, but it has become difficult to manage imports in the Spack core. - the top-level namespace polluted by package symbols, and it's not possible to avoid circular dependencies and unnecessary module loads in the core, given all the stuff the packages need. - This makes the top-level `spack` package essentially empty, save for a version tuple and a version string, and `from spack import *` is now essentially a no-op. - The common routines and directives that packages need are now in `spack.pkgkit`, and the import system forces packages to automatically include this so that old packages that call `from spack import *` will continue to work without modification. - Since `from spack import *` is no longer required, we could consider removing ``from spack import *`` from packages in the future and shifting to ``from spack.pkgkit import *``, but we can wait a while to do this. --- lib/spack/docs/conf.py | 22 ----------- lib/spack/spack/__init__.py | 77 +----------------------------------- lib/spack/spack/build_environment.py | 5 ++- lib/spack/spack/cmd/setup.py | 3 +- lib/spack/spack/main.py | 2 + lib/spack/spack/package.py | 4 +- lib/spack/spack/pkgkit.py | 66 +++++++++++++++++++++++++++++++ lib/spack/spack/repo.py | 57 ++++++++++++++++++++++---- lib/spack/spack/util/package_hash.py | 23 +++++------ 9 files changed, 137 insertions(+), 122 deletions(-) create mode 100644 lib/spack/spack/pkgkit.py diff --git a/lib/spack/docs/conf.py b/lib/spack/docs/conf.py index 0b664eb5c0..f543346809 100644 --- a/lib/spack/docs/conf.py +++ b/lib/spack/docs/conf.py @@ -111,28 +111,6 @@ apidoc_args = [ sphinx_apidoc(apidoc_args + ['../spack']) sphinx_apidoc(apidoc_args + ['../llnl']) -# -# Exclude everything in spack.__all__ from indexing. All of these -# symbols are imported from elsewhere in spack; their inclusion in -# __all__ simply allows package authors to use `from spack import *`. -# Excluding them ensures they're only documented in their "real" module. -# -# This also avoids issues where some of these symbols shadow core spack -# modules. Sphinx will complain about duplicate docs when this happens. -# -import fileinput -handling_spack = False -for line in fileinput.input('spack.rst', inplace=1): - if handling_spack: - if not line.startswith(' :noindex:'): - print(' :noindex: %s' % ' '.join(spack.__all__)) - handling_spack = False - - if line.startswith('.. automodule::'): - handling_spack = (line == '.. automodule:: spack\n') - - sys.stdout.write(line) - # Enable todo items todo_include_todos = True diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py index 0b5b2717b5..515b36cf26 100644 --- a/lib/spack/spack/__init__.py +++ b/lib/spack/spack/__init__.py @@ -1,4 +1,3 @@ -# flake8: noqa ############################################################################## # Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. # Produced at the Lawrence Livermore National Laboratory. @@ -30,78 +29,4 @@ spack_version_info = (0, 11, 2) #: String containing Spack version joined with .'s spack_version = '.'.join(str(v) for v in spack_version_info) - -#----------------------------------------------------------------------------- -# When packages call 'from spack import *', we import a set of things that -# should be useful for builds. -#----------------------------------------------------------------------------- -__all__ = [] - -from spack.package import Package, run_before, run_after, on_package_attributes -from spack.build_systems.makefile import MakefilePackage -from spack.build_systems.aspell_dict import AspellDictPackage -from spack.build_systems.autotools import AutotoolsPackage -from spack.build_systems.cmake import CMakePackage -from spack.build_systems.cuda import CudaPackage -from spack.build_systems.qmake import QMakePackage -from spack.build_systems.scons import SConsPackage -from spack.build_systems.waf import WafPackage -from spack.build_systems.octave import OctavePackage -from spack.build_systems.python import PythonPackage -from spack.build_systems.r import RPackage -from spack.build_systems.perl import PerlPackage -from spack.build_systems.intel import IntelPackage - -__all__ += [ - 'run_before', - 'run_after', - 'on_package_attributes', - 'Package', - 'MakefilePackage', - 'AspellDictPackage', - 'AutotoolsPackage', - 'CMakePackage', - 'CudaPackage', - 'QMakePackage', - 'SConsPackage', - 'WafPackage', - 'OctavePackage', - 'PythonPackage', - 'RPackage', - 'PerlPackage', - 'IntelPackage', -] - -from spack.mixins import filter_compiler_wrappers -__all__ += ['filter_compiler_wrappers'] - -from spack.version import Version, ver -__all__ += ['Version', 'ver'] - -from spack.spec import Spec -__all__ += ['Spec'] - -from spack.dependency import all_deptypes -__all__ += ['all_deptypes'] - -from spack.multimethod import when -__all__ += ['when'] - -import llnl.util.filesystem -from llnl.util.filesystem import * -__all__ += llnl.util.filesystem.__all__ - -import spack.directives -from spack.directives import * -__all__ += spack.directives.__all__ - -import spack.util.executable -from spack.util.executable import * -__all__ += spack.util.executable.__all__ - -from spack.package import \ - install_dependency_symlinks, flatten_dependencies, \ - DependencyConflictError, InstallError, ExternalPackageError -__all__ += [ - 'install_dependency_symlinks', 'flatten_dependencies', - 'DependencyConflictError', 'InstallError', 'ExternalPackageError'] +__all__ = ['spack_version_info', 'spack_version'] diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index cdf3e86577..5a98006a49 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -65,6 +65,7 @@ import llnl.util.tty as tty from llnl.util.tty.color import colorize from llnl.util.filesystem import mkdirp, install, install_tree +import spack.build_systems.cmake import spack.config import spack.main import spack.paths @@ -374,7 +375,7 @@ def set_module_variables_for_package(pkg, module): m.ctest = Executable('ctest') # Standard CMake arguments - m.std_cmake_args = spack.CMakePackage._std_args(pkg) + m.std_cmake_args = spack.build_systems.cmake.CMakePackage._std_args(pkg) # Put spack compiler paths in module scope. link_dir = spack.paths.build_env_path @@ -540,7 +541,7 @@ def get_std_cmake_args(pkg): Returns: list of str: arguments for cmake """ - return spack.CMakePackage._std_args(pkg) + return spack.build_systems.cmake.CMakePackage._std_args(pkg) def parent_class_modules(cls): diff --git a/lib/spack/spack/cmd/setup.py b/lib/spack/spack/cmd/setup.py index ad603704a3..dcace5a668 100644 --- a/lib/spack/spack/cmd/setup.py +++ b/lib/spack/spack/cmd/setup.py @@ -33,6 +33,7 @@ from llnl.util.filesystem import set_executable import spack.repo import spack.store +import spack.build_systems.cmake import spack.cmd import spack.cmd.install as install import spack.cmd.common.arguments as arguments @@ -147,7 +148,7 @@ def setup(self, args): spec.concretize() package = spack.repo.get(spec) - if not isinstance(package, spack.CMakePackage): + if not isinstance(package, spack.build_systems.cmake.CMakePackage): tty.die( 'Support for {0} derived packages not yet implemented'.format( package.build_system_class)) diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py index 7af096c821..61e992bed4 100644 --- a/lib/spack/spack/main.py +++ b/lib/spack/spack/main.py @@ -42,6 +42,8 @@ from llnl.util.tty.log import log_output import spack import spack.config +import spack.cmd +import spack.hooks import spack.paths import spack.repo import spack.util.debug diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 3fdaf56b8d..c7a246f274 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -58,6 +58,7 @@ import spack.paths import spack.store import spack.compilers import spack.directives +import spack.directory_layout import spack.error import spack.fetch_strategy as fs import spack.hooks @@ -75,7 +76,6 @@ from llnl.util.lang import memoized from llnl.util.link_tree import LinkTree from llnl.util.tty.log import log_output from llnl.util.tty.color import colorize -from spack import directory_layout from spack.util.executable import which from spack.stage import Stage, ResourceStage, StageComposite from spack.util.environment import dump_environment @@ -1581,7 +1581,7 @@ class PackageBase(with_metaclass(PackageMeta, object)): spack.store.db.add( self.spec, spack.store.layout, explicit=explicit ) - except directory_layout.InstallDirectoryAlreadyExistsError: + except spack.directory_layout.InstallDirectoryAlreadyExistsError: # Abort install if install directory exists. # But do NOT remove it (you'd be overwriting someone else's stuff) tty.warn("Keeping existing install prefix in place.") diff --git a/lib/spack/spack/pkgkit.py b/lib/spack/spack/pkgkit.py new file mode 100644 index 0000000000..563fe0f64a --- /dev/null +++ b/lib/spack/spack/pkgkit.py @@ -0,0 +1,66 @@ +# flake8: noqa: F401 +############################################################################## +# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/spack/spack +# Please also see the NOTICE and LICENSE files for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +"""pkgkit is a set of useful build tools and directives for packages. + +Everything in this module is automatically imported into Spack package files. +""" +import llnl.util.filesystem +from llnl.util.filesystem import * + +from spack.package import Package, run_before, run_after, on_package_attributes +from spack.build_systems.makefile import MakefilePackage +from spack.build_systems.aspell_dict import AspellDictPackage +from spack.build_systems.autotools import AutotoolsPackage +from spack.build_systems.cmake import CMakePackage +from spack.build_systems.cuda import CudaPackage +from spack.build_systems.qmake import QMakePackage +from spack.build_systems.scons import SConsPackage +from spack.build_systems.waf import WafPackage +from spack.build_systems.octave import OctavePackage +from spack.build_systems.python import PythonPackage +from spack.build_systems.r import RPackage +from spack.build_systems.perl import PerlPackage +from spack.build_systems.intel import IntelPackage + +from spack.mixins import filter_compiler_wrappers + +from spack.version import Version, ver + +from spack.spec import Spec + +from spack.dependency import all_deptypes + +from spack.multimethod import when + +import spack.directives +from spack.directives import * + +import spack.util.executable +from spack.util.executable import * + +from spack.package import \ + install_dependency_symlinks, flatten_dependencies, \ + DependencyConflictError, InstallError, ExternalPackageError diff --git a/lib/spack/spack/repo.py b/lib/spack/spack/repo.py index d0cf1b0ce1..d71ab2452e 100644 --- a/lib/spack/spack/repo.py +++ b/lib/spack/spack/repo.py @@ -32,6 +32,7 @@ import inspect import imp import re import traceback +import tempfile import json from contextlib import contextmanager from six import string_types @@ -59,10 +60,8 @@ from spack.util.path import canonicalize_path from spack.util.naming import NamespaceTrie, valid_module_name from spack.util.naming import mod_to_class, possible_spack_module_names -# -# Super-namespace for all packages. -# Package modules are imported as spack.pkg... -# +#: Super-namespace for all packages. +#: Package modules are imported as spack.pkg... repo_namespace = 'spack.pkg' # @@ -73,9 +72,25 @@ repo_index_name = 'index.yaml' # Top-level filename for repository index. packages_dir_name = 'packages' # Top-level repo directory containing pkgs. package_file_name = 'package.py' # Filename for packages in a repository. -# Guaranteed unused default value for some functions. +#: Guaranteed unused default value for some functions. NOT_PROVIDED = object() +#: Code in ``_package_prepend`` is prepended to imported packages. +#: +#: Spack packages were originally expected to call `from spack import *` +#: themselves, but it became difficult to manage and imports in the Spack +#: core the top-level namespace polluted by package symbols this way. To +#: solve this, the top-level ``spack`` package contains very few symbols +#: of its own, and importing ``*`` is essentially a no-op. The common +#: routines and directives that packages need are now in ``spack.pkgkit``, +#: and the import system forces packages to automatically include +#: this. This way, old packages that call ``from spack import *`` will +#: continue to work without modification, but it's no longer required. +#: +#: TODO: At some point in the future, consider removing ``from spack import *`` +#: TODO: from packages and shifting to from ``spack.pkgkit import *`` +_package_prepend = 'from spack.pkgkit import *' + def _autospec(function): """Decorator that automatically converts the argument of a single-arg @@ -118,8 +133,7 @@ class FastPackageChecker(Mapping): _paths_cache = {} def __init__(self, packages_path): - - #: The path of the repository managed by this instance + # The path of the repository managed by this instance self.packages_path = packages_path # If the cache we need is not there yet, then build it appropriately @@ -976,7 +990,9 @@ class Repo(object): fullname = "%s.%s" % (self.full_namespace, pkg_name) try: - module = imp.load_source(fullname, file_path) + with import_lock(): + with prepend_open(file_path, text=_package_prepend) as f: + module = imp.load_source(fullname, file_path, f) except SyntaxError as e: # SyntaxError strips the path from the filename so we need to # manually construct the error message in order to give the @@ -1126,6 +1142,31 @@ def set_path(repo): return append +@contextmanager +def import_lock(): + imp.acquire_lock() + yield + imp.release_lock() + + +@contextmanager +def prepend_open(f, *args, **kwargs): + """Open a file for reading, but prepend with some text prepended + + Arguments are same as for ``open()``, with one keyword argument, + ``text``, specifying the text to prepend. + """ + text = kwargs.get('text', None) + + with open(f, *args) as f: + with tempfile.NamedTemporaryFile(mode='w+') as tf: + if text: + tf.write(text + '\n') + tf.write(f.read()) + tf.seek(0) + yield tf.file + + @contextmanager def swap(repo_path): """Temporarily use another RepoPath.""" diff --git a/lib/spack/spack/util/package_hash.py b/lib/spack/spack/util/package_hash.py index 0acb211312..21514745e3 100644 --- a/lib/spack/spack/util/package_hash.py +++ b/lib/spack/spack/util/package_hash.py @@ -22,15 +22,16 @@ # along with this program; if not, write to the Free Software Foundation, # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA ############################################################################## -import spack.repo -from spack import directives -from spack.error import SpackError -from spack.spec import Spec -from spack.util.naming import mod_to_class - import ast import hashlib +import spack.repo +import spack.package +import spack.directives +import spack.error +import spack.spec +import spack.util.naming + class RemoveDocstrings(ast.NodeTransformer): """Transformer that removes docstrings from a Python AST.""" @@ -61,15 +62,15 @@ class RemoveDirectives(ast.NodeTransformer): def is_directive(self, node): return (isinstance(node, ast.Expr) and node.value and isinstance(node.value, ast.Call) and - node.value.func.id in directives.__all__) + node.value.func.id in spack.directives.__all__) def is_spack_attr(self, node): return (isinstance(node, ast.Assign) and node.targets and isinstance(node.targets[0], ast.Name) and - node.targets[0].id in spack.Package.metadata_attrs) + node.targets[0].id in spack.package.Package.metadata_attrs) def visit_ClassDef(self, node): - if node.name == mod_to_class(self.spec.name): + if node.name == spack.util.naming.mod_to_class(self.spec.name): node.body = [ c for c in node.body if (not self.is_directive(c) and not self.is_spack_attr(c))] @@ -129,7 +130,7 @@ def package_hash(spec, content=None): def package_ast(spec): - spec = Spec(spec) + spec = spack.spec.Spec(spec) filename = spack.repo.path.filename_for_package_name(spec.name) with open(filename) as f: @@ -147,5 +148,5 @@ def package_ast(spec): return root -class PackageHashError(SpackError): +class PackageHashError(spack.error.SpackError): """Raised for all errors encountered during package hashing.""" -- cgit v1.2.3-70-g09d2