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

@@ -111,28 +111,6 @@
 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
 

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 @@
 #: String containing Spack version joined with .'s
 spack_version = '.'.join(str(v) for v in spack_version_info)
 
-
+__all__ = ['spack_version_info', 'spack_version']
-#-----------------------------------------------------------------------------
-# 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']

lib/spack/spack/build_environment.py

@@ -65,6 +65,7 @@
 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):

lib/spack/spack/cmd/setup.py

@@ -33,6 +33,7 @@
 
 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))

lib/spack/spack/main.py

@@ -42,6 +42,8 @@
 
 import spack
 import spack.config
+import spack.cmd
+import spack.hooks
 import spack.paths
 import spack.repo
 import spack.util.debug

lib/spack/spack/package.py

@@ -58,6 +58,7 @@
 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.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 @@ def build_process():
             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.")

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

lib/spack/spack/repo.py

@@ -32,6 +32,7 @@
 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.naming import NamespaceTrie, valid_module_name
 from spack.util.naming import mod_to_class, possible_spack_module_names
 
-#
+#: Super-namespace for all packages.
-# Super-namespace for all packages.
+#: Package modules are imported as spack.pkg.<namespace>.<pkg-name>.
-# Package modules are imported as spack.pkg.<namespace>.<pkg-name>.
-#
 repo_namespace     = 'spack.pkg'
 
 #
@@ -73,9 +72,25 @@
 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 @@ def _get_pkg_module(self, pkg_name):
             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."""

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 @@ def __init__(self, spec):
     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."""