From ff04d1bfc186a0563728707a3dc08ebdfc1a6f59 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Fri, 8 Apr 2022 00:58:20 +0200
Subject: Use the non-deprecated `MetaPathFinder` interface (#29745)

* Extract the MetaPathFinder and Loaders for packages in their own classes

https://peps.python.org/pep-0451/

Currently, RepoPath and Repo implement the (deprecated) interface of
MetaPathFinder (find_module) and of Loader (load_module). This commit
extracts both of them and places the code in their own classes.

The MetaPathFinder interface is updated to contain both the deprecated
"find_module" (for Python 2.7 support) and the recommended "find_spec".
Update of the Loader interface is deferred at a subsequent commit.

* Move the lines to be prepended inside "RepoLoader"

Also adjust the naming of a few variables too

* Remove spack.util.imp, since code is only used in spack.repo

* Remove support from loading Python modules Python > 3 but < 3.5

* Remove `Repo._create_namespace`

This function was interacting badly with the MetaPathFinder
and causing issues with "normal" imports. Removing the
function allows to do things like:
```python
import spack.pkg.builtin.mpich
cls = spack.pkg.builtin.mpich.Mpich
```

* Remove code needed to trigger the Singleton evaluation

The finder is coded in a way to trigger the Singleton,
so we don't need external code now that we register it
at module level into `sys.meta_path`.

* Add unit tests
---
 lib/spack/docs/conf.py                         |   1 +
 lib/spack/llnl/util/lang.py                    |   5 -
 lib/spack/spack/main.py                        |   3 +-
 lib/spack/spack/package.py                     |   6 +-
 lib/spack/spack/repo.py                        | 498 +++++++++++++------------
 lib/spack/spack/test/repo.py                   |  23 ++
 lib/spack/spack/util/imp/__init__.py           |  22 --
 lib/spack/spack/util/imp/imp_importer.py       |  67 ----
 lib/spack/spack/util/imp/importlib_importer.py |  48 ---
 9 files changed, 294 insertions(+), 379 deletions(-)
 delete mode 100644 lib/spack/spack/util/imp/__init__.py
 delete mode 100644 lib/spack/spack/util/imp/imp_importer.py
 delete mode 100644 lib/spack/spack/util/imp/importlib_importer.py

(limited to 'lib')

diff --git a/lib/spack/docs/conf.py b/lib/spack/docs/conf.py
index 5beb0980ba..5455aa0f28 100644
--- a/lib/spack/docs/conf.py
+++ b/lib/spack/docs/conf.py
@@ -180,6 +180,7 @@ nitpick_ignore = [
     ('py:class', '_frozen_importlib_external.SourceFileLoader'),
     # Spack classes that are private and we don't want to expose
     ('py:class', 'spack.provider_index._IndexBase'),
+    ('py:class', 'spack.repo._PrependFileLoader'),
 ]
 
 # The reST default role (used for this markup: `text`) to use for all documents.
diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py
index fb8b01aa84..3644ec11a7 100644
--- a/lib/spack/llnl/util/lang.py
+++ b/lib/spack/llnl/util/lang.py
@@ -889,11 +889,6 @@ def load_module_from_file(module_name, module_path):
             except KeyError:
                 pass
             raise
-    elif sys.version_info[0] == 3 and sys.version_info[1] < 5:
-        import importlib.machinery
-        loader = importlib.machinery.SourceFileLoader(  # novm
-            module_name, module_path)
-        module = loader.load_module()
     elif sys.version_info[0] == 2:
         import imp
         module = imp.load_source(module_name, module_path)
diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py
index 6e72b6a557..5f13f0736f 100644
--- a/lib/spack/spack/main.py
+++ b/lib/spack/spack/main.py
@@ -512,8 +512,7 @@ def setup_main_options(args):
         spack.config.set('config:locks', args.locks, scope='command_line')
 
     if args.mock:
-        rp = spack.repo.RepoPath(spack.paths.mock_packages_path)
-        spack.repo.set_path(rp)
+        spack.repo.path = spack.repo.RepoPath(spack.paths.mock_packages_path)
 
     # If the user asked for it, don't check ssl certs.
     if args.insecure:
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 96ad5cab8c..9bd7669de9 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -399,11 +399,7 @@ class PackageMeta(
     @property
     def namespace(self):
         """Spack namespace for the package, which identifies its repo."""
-        namespace, dot, module = self.__module__.rpartition('.')
-        prefix = '%s.' % spack.repo.repo_namespace
-        if namespace.startswith(prefix):
-            namespace = namespace[len(prefix):]
-        return namespace
+        return spack.repo.namespace_from_fullname(self.__module__)
 
     @property
     def fullname(self):
diff --git a/lib/spack/spack/repo.py b/lib/spack/spack/repo.py
index 6577310048..0a18079574 100644
--- a/lib/spack/spack/repo.py
+++ b/lib/spack/spack/repo.py
@@ -16,6 +16,7 @@ import re
 import shutil
 import stat
 import sys
+import tempfile
 import traceback
 import types
 from typing import Dict  # novm
@@ -36,19 +37,269 @@ import spack.patch
 import spack.provider_index
 import spack.spec
 import spack.tag
-import spack.util.imp as simp
 import spack.util.naming as nm
 import spack.util.path
 from spack.util.executable import which
 
-#: Super-namespace for all packages.
-#: Package modules are imported as spack.pkg.<namespace>.<pkg-name>.
-repo_namespace = 'spack.pkg'
+#: Package modules are imported as spack.pkg.<repo-namespace>.<pkg-name>
+ROOT_PYTHON_NAMESPACE = 'spack.pkg'
 
 
-def get_full_namespace(namespace):
-    """Returns the full namespace of a repository, given its relative one."""
-    return '{0}.{1}'.format(repo_namespace, namespace)
+def python_package_for_repo(namespace):
+    """Returns the full namespace of a repository, given its relative one
+
+    For instance:
+
+        python_package_for_repo('builtin') == 'spack.pkg.builtin'
+
+    Args:
+        namespace (str): repo namespace
+    """
+    return '{0}.{1}'.format(ROOT_PYTHON_NAMESPACE, namespace)
+
+
+def namespace_from_fullname(fullname):
+    """Return the repository namespace only for the full module name.
+
+    For instance:
+
+        namespace_from_fullname('spack.pkg.builtin.hdf5') == 'builtin'
+
+    Args:
+        fullname (str): full name for the Python module
+    """
+    namespace, dot, module = fullname.rpartition('.')
+    prefix_and_dot = '{0}.'.format(ROOT_PYTHON_NAMESPACE)
+    if namespace.startswith(prefix_and_dot):
+        namespace = namespace[len(prefix_and_dot):]
+    return namespace
+
+
+# The code below is needed to have a uniform Loader interface that could cover both
+# Python 2.7 and Python 3.X when we load Spack packages as Python modules, e.g. when
+# we do "import spack.pkg.builtin.mpich" in package recipes.
+if sys.version_info[0] == 2:
+    import imp
+
+    @contextlib.contextmanager
+    def import_lock():
+        try:
+            imp.acquire_lock()
+            yield
+        finally:
+            imp.release_lock()
+
+    def load_source(fullname, path, prepend=None):
+        """Import a Python module from source.
+
+        Load the source file and add it to ``sys.modules``.
+
+        Args:
+            fullname (str): full name of the module to be loaded
+            path (str): path to the file that should be loaded
+            prepend (str or None): some optional code to prepend to the
+                loaded module; e.g., can be used to inject import statements
+
+        Returns:
+            the loaded module
+        """
+        with import_lock():
+            with prepend_open(path, text=prepend) as f:
+                return imp.load_source(fullname, path, f)
+
+    @contextlib.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.
+
+        We have to write and read a tempfile for the ``imp``-based importer,
+        as the ``file`` argument to ``imp.load_source()`` requires a
+        low-level file handle.
+
+        See the ``importlib``-based importer for a faster way to do this in
+        later versions of python.
+        """
+        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
+
+    class _PrependFileLoader(object):
+        def __init__(self, fullname, path, prepend=None):
+            # Done to have a compatible interface with Python 3
+            #
+            # All the object attributes used in this method must be defined
+            # by a derived class
+            pass
+
+        def package_module(self):
+            try:
+                module = load_source(
+                    self.fullname, self.package_py, prepend=self._package_prepend
+                )
+            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
+                # user the correct package.py where the syntax error is located
+                msg = 'invalid syntax in {0:}, line {1:}'
+                raise SyntaxError(msg.format(self.package_py, e.lineno))
+
+            module.__package__ = self.repo.full_namespace
+            module.__loader__ = self
+            return module
+
+        def load_module(self, fullname):
+            # Compatibility method to support Python 2.7
+            if fullname in sys.modules:
+                return sys.modules[fullname]
+
+            namespace, dot, module_name = fullname.rpartition('.')
+
+            try:
+                module = self.package_module()
+            except Exception as e:
+                raise ImportError(str(e))
+
+            module.__loader__ = self
+            sys.modules[fullname] = module
+            if namespace != fullname:
+                parent = sys.modules[namespace]
+                if not hasattr(parent, module_name):
+                    setattr(parent, module_name, module)
+
+            return module
+
+else:
+    import importlib.machinery  # novm
+
+    class _PrependFileLoader(importlib.machinery.SourceFileLoader):  # novm
+        def __init__(self, fullname, path, prepend=None):
+            super(_PrependFileLoader, self).__init__(fullname, path)
+            self.prepend = prepend
+
+        def path_stats(self, path):
+            stats = super(_PrependFileLoader, self).path_stats(path)
+            if self.prepend:
+                stats["size"] += len(self.prepend) + 1
+            return stats
+
+        def get_data(self, path):
+            data = super(_PrependFileLoader, self).get_data(path)
+            if path != self.path or self.prepend is None:
+                return data
+            else:
+                return self.prepend.encode() + b"\n" + data
+
+
+class RepoLoader(_PrependFileLoader):
+    """Loads a Python module associated with a package in specific repository"""
+    #: 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.
+    _package_prepend = ('from __future__ import absolute_import;'
+                        'from spack.pkgkit import *')
+
+    def __init__(self, fullname, repo, package_name):
+        self.repo = repo
+        self.package_name = package_name
+        self.package_py = repo.filename_for_package_name(package_name)
+        self.fullname = fullname
+        super(RepoLoader, self).__init__(
+            self.fullname, self.package_py, prepend=self._package_prepend
+        )
+
+
+class SpackNamespaceLoader(object):
+    def create_module(self, spec):
+        return SpackNamespace(spec.name)
+
+    def exec_module(self, module):
+        module.__loader__ = self
+
+    def load_module(self, fullname):
+        # Compatibility method to support Python 2.7
+        if fullname in sys.modules:
+            return sys.modules[fullname]
+        module = SpackNamespace(fullname)
+        self.exec_module(module)
+
+        namespace, dot, module_name = fullname.rpartition('.')
+        sys.modules[fullname] = module
+        if namespace != fullname:
+            parent = sys.modules[namespace]
+            if not hasattr(parent, module_name):
+                setattr(parent, module_name, module)
+
+        return module
+
+
+class ReposFinder(object):
+    """MetaPathFinder class that loads a Python module corresponding to a Spack package
+
+    Return a loader based on the inspection of the current global repository list.
+    """
+    def find_spec(self, fullname, python_path, target=None):
+        # This function is Python 3 only and will not be called by Python 2.7
+        import importlib.util
+
+        # "target" is not None only when calling importlib.reload()
+        if target is not None:
+            raise RuntimeError('cannot reload module "{0}"'.format(fullname))
+
+        # Preferred API from https://peps.python.org/pep-0451/
+        if not fullname.startswith(ROOT_PYTHON_NAMESPACE):
+            return None
+
+        loader = self.compute_loader(fullname)
+        if loader is None:
+            return None
+        return importlib.util.spec_from_loader(fullname, loader)  # novm
+
+    def compute_loader(self, fullname):
+        # namespaces are added to repo, and package modules are leaves.
+        namespace, dot, module_name = fullname.rpartition('.')
+
+        # If it's a module in some repo, or if it is the repo's
+        # namespace, let the repo handle it.
+        for repo in path.repos:
+            # We are using the namespace of the repo and the repo contains the package
+            if namespace == repo.full_namespace:
+                # With 2 nested conditionals we can call "repo.real_name" only once
+                package_name = repo.real_name(module_name)
+                if package_name:
+                    return RepoLoader(fullname, repo, package_name)
+
+            # We are importing a full namespace like 'spack.pkg.builtin'
+            if fullname == repo.full_namespace:
+                return SpackNamespaceLoader()
+
+        # No repo provides the namespace, but it is a valid prefix of
+        # something in the RepoPath.
+        if path.by_namespace.is_prefix(fullname):
+            return SpackNamespaceLoader()
+
+        return None
+
+    def find_module(self, fullname, python_path=None):
+        # Compatibility method to support Python 2.7
+        if not fullname.startswith(ROOT_PYTHON_NAMESPACE):
+            return None
+        return self.compute_loader(fullname)
 
 
 #
@@ -62,22 +313,6 @@ package_file_name  = 'package.py'  # Filename for packages in a repository.
 #: 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 __future__ import absolute_import; from spack.pkgkit import *'
-
 
 def packages_path():
     """Get the test repo if it is active, otherwise the builtin repo."""
@@ -596,7 +831,7 @@ class RepoPath(object):
                 If default is provided, return it when the namespace
                 isn't found.  If not, raise an UnknownNamespaceError.
         """
-        full_namespace = get_full_namespace(namespace)
+        full_namespace = python_package_for_repo(namespace)
         if full_namespace not in self.by_namespace:
             if default == NOT_PROVIDED:
                 raise UnknownNamespaceError(namespace)
@@ -674,48 +909,6 @@ class RepoPath(object):
     def extensions_for(self, extendee_spec):
         return [p for p in self.all_packages() if p.extends(extendee_spec)]
 
-    def find_module(self, fullname, path=None):
-        """Implements precedence for overlaid namespaces.
-
-        Loop checks each namespace in self.repos for packages, and
-        also handles loading empty containing namespaces.
-
-        """
-        # namespaces are added to repo, and package modules are leaves.
-        namespace, dot, module_name = fullname.rpartition('.')
-
-        # If it's a module in some repo, or if it is the repo's
-        # namespace, let the repo handle it.
-        for repo in self.repos:
-            if namespace == repo.full_namespace:
-                if repo.real_name(module_name):
-                    return repo
-            elif fullname == repo.full_namespace:
-                return repo
-
-        # No repo provides the namespace, but it is a valid prefix of
-        # something in the RepoPath.
-        if self.by_namespace.is_prefix(fullname):
-            return self
-
-        return None
-
-    def load_module(self, fullname):
-        """Handles loading container namespaces when necessary.
-
-        See ``Repo`` for how actual package modules are loaded.
-        """
-        if fullname in sys.modules:
-            return sys.modules[fullname]
-
-        if not self.by_namespace.is_prefix(fullname):
-            raise ImportError("No such Spack repo: %s" % fullname)
-
-        module = SpackNamespace(fullname)
-        module.__loader__ = self
-        sys.modules[fullname] = module
-        return module
-
     def last_mtime(self):
         """Time a package file in this repo was last updated."""
         return max(repo.last_mtime() for repo in self.repos)
@@ -735,7 +928,7 @@ class RepoPath(object):
         # If the spec already has a namespace, then return the
         # corresponding repo if we know about it.
         if namespace:
-            fullspace = get_full_namespace(namespace)
+            fullspace = python_package_for_repo(namespace)
             if fullspace not in self.by_namespace:
                 raise UnknownNamespaceError(namespace)
             return self.by_namespace[fullspace]
@@ -849,7 +1042,7 @@ class Repo(object):
               "Namespaces must be valid python identifiers separated by '.'")
 
         # Set up 'full_namespace' to include the super-namespace
-        self.full_namespace = get_full_namespace(self.namespace)
+        self.full_namespace = python_package_for_repo(self.namespace)
 
         # Keep name components around for checking prefixes.
         self._names = self.full_namespace.split('.')
@@ -865,40 +1058,6 @@ class Repo(object):
         # Indexes for this repository, computed lazily
         self._repo_index = None
 
-        # make sure the namespace for packages in this repo exists.
-        self._create_namespace()
-
-    def _create_namespace(self):
-        """Create this repo's namespace module and insert it into sys.modules.
-
-        Ensures that modules loaded via the repo have a home, and that
-        we don't get runtime warnings from Python's module system.
-
-        """
-        parent = None
-        for i in range(1, len(self._names) + 1):
-            ns = '.'.join(self._names[:i])
-
-            if ns not in sys.modules:
-                module = SpackNamespace(ns)
-                module.__loader__ = self
-                sys.modules[ns] = module
-
-                # Ensure the namespace is an atrribute of its parent,
-                # if it has not been set by something else already.
-                #
-                # This ensures that we can do things like:
-                #    import spack.pkg.builtin.mpich as mpich
-                if parent:
-                    modname = self._names[i - 1]
-                    setattr(parent, modname, module)
-            else:
-                # no need to set up a module
-                module = sys.modules[ns]
-
-            # but keep track of the parent in this loop
-            parent = module
-
     def real_name(self, import_name):
         """Allow users to import Spack packages using Python identifiers.
 
@@ -929,52 +1088,6 @@ class Repo(object):
         parts = fullname.split('.')
         return self._names[:len(parts)] == parts
 
-    def find_module(self, fullname, path=None):
-        """Python find_module import hook.
-
-        Returns this Repo if it can load the module; None if not.
-        """
-        if self.is_prefix(fullname):
-            return self
-
-        namespace, dot, module_name = fullname.rpartition('.')
-        if namespace == self.full_namespace:
-            if self.real_name(module_name):
-                return self
-
-        return None
-
-    def load_module(self, fullname):
-        """Python importer load hook.
-
-        Tries to load the module; raises an ImportError if it can't.
-        """
-        if fullname in sys.modules:
-            return sys.modules[fullname]
-
-        namespace, dot, module_name = fullname.rpartition('.')
-
-        if self.is_prefix(fullname):
-            module = SpackNamespace(fullname)
-
-        elif namespace == self.full_namespace:
-            real_name = self.real_name(module_name)
-            if not real_name:
-                raise ImportError("No module %s in %s" % (module_name, self))
-            module = self._get_pkg_module(real_name)
-
-        else:
-            raise ImportError("No module %s in %s" % (fullname, self))
-
-        module.__loader__ = self
-        sys.modules[fullname] = module
-        if namespace != fullname:
-            parent = sys.modules[namespace]
-            if not hasattr(parent, module_name):
-                setattr(parent, module_name, module)
-
-        return module
-
     def _read_config(self):
         """Check for a YAML config file in this db's root directory."""
         try:
@@ -1164,46 +1277,6 @@ class Repo(object):
         """True if the package with this name is virtual, False otherwise."""
         return pkg_name in self.provider_index
 
-    def _get_pkg_module(self, pkg_name):
-        """Create a module for a particular package.
-
-        This caches the module within this Repo *instance*.  It does
-        *not* add it to ``sys.modules``.  So, you can construct
-        multiple Repos for testing and ensure that the module will be
-        loaded once per repo.
-
-        """
-        if pkg_name not in self._modules:
-            file_path = self.filename_for_package_name(pkg_name)
-
-            if not os.path.exists(file_path):
-                raise UnknownPackageError(pkg_name, self)
-
-            if not os.path.isfile(file_path):
-                tty.die("Something's wrong. '%s' is not a file!" % file_path)
-
-            if not os.access(file_path, os.R_OK):
-                tty.die("Cannot read '%s'!" % file_path)
-
-            # e.g., spack.pkg.builtin.mpich
-            fullname = "%s.%s" % (self.full_namespace, pkg_name)
-
-            try:
-                module = simp.load_source(fullname, file_path,
-                                          prepend=_package_prepend)
-            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
-                # user the correct package.py where the syntax error is located
-                raise SyntaxError('invalid syntax in {0:}, line {1:}'
-                                  .format(file_path, e.lineno))
-
-            module.__package__ = self.full_namespace
-            module.__loader__ = self
-            self._modules[pkg_name] = module
-
-        return self._modules[pkg_name]
-
     def get_pkg_class(self, pkg_name):
         """Get the class for the package out of its module.
 
@@ -1308,25 +1381,20 @@ def create_or_construct(path, namespace=None):
 
 
 def _path(repo_dirs=None):
-    """Get the singleton RepoPath instance for Spack.
-
-    Create a RepoPath, add it to sys.meta_path, and return it.
-
-    TODO: consider not making this a singleton.
-    """
+    """Get the singleton RepoPath instance for Spack."""
     repo_dirs = repo_dirs or spack.config.get('repos')
     if not repo_dirs:
         raise NoRepoConfiguredError(
             "Spack configuration contains no package repositories.")
-
-    path = RepoPath(*repo_dirs)
-    sys.meta_path.append(path)
-    return path
+    return RepoPath(*repo_dirs)
 
 
 #: Singleton repo path instance
 path = llnl.util.lang.Singleton(_path)
 
+# Add the finder to sys.meta_path
+sys.meta_path.append(ReposFinder())
+
 
 def get(spec):
     """Convenience wrapper around ``spack.repo.get()``."""
@@ -1338,22 +1406,6 @@ def all_package_names(include_virtuals=False):
     return path.all_package_names(include_virtuals)
 
 
-def set_path(repo):
-    """Set the path singleton to a specific value.
-
-    Overwrite ``path`` and register it as an importer in
-    ``sys.meta_path`` if it is a ``Repo`` or ``RepoPath``.
-    """
-    global path
-    path = repo
-
-    # make the new repo_path an importer if needed
-    append = isinstance(repo, (Repo, RepoPath))
-    if append:
-        sys.meta_path.append(repo)
-    return append
-
-
 @contextlib.contextmanager
 def additional_repository(repository):
     """Adds temporarily a repository to the default one.
@@ -1378,24 +1430,10 @@ def use_repositories(*paths_and_repos):
         Corresponding RepoPath object
     """
     global path
-
-    remove_from_meta = None
-
-    # Construct a temporary RepoPath object from
-    temporary_repositories = RepoPath(*paths_and_repos)
-
-    # Swap the current repository out
-    saved = path
-
+    path, saved = RepoPath(*paths_and_repos), path
     try:
-        remove_from_meta = set_path(temporary_repositories)
-
-        yield temporary_repositories
-
+        yield path
     finally:
-        # Restore _path and sys.meta_path
-        if remove_from_meta:
-            sys.meta_path.remove(temporary_repositories)
         path = saved
 
 
diff --git a/lib/spack/spack/test/repo.py b/lib/spack/spack/test/repo.py
index ac3138325e..e43c572065 100644
--- a/lib/spack/spack/test/repo.py
+++ b/lib/spack/spack/test/repo.py
@@ -7,6 +7,7 @@ import os
 
 import pytest
 
+import spack.package
 import spack.paths
 import spack.repo
 
@@ -98,3 +99,25 @@ def test_use_repositories_doesnt_change_class():
     with spack.repo.use_repositories(*current_paths):
         zlib_cls_inner = spack.repo.path.get_pkg_class('zlib')
     assert id(zlib_cls_inner) == id(zlib_cls_outer)
+
+
+def test_import_repo_prefixes_as_python_modules(mock_packages):
+    import spack.pkg.builtin.mock
+    assert isinstance(spack.pkg, spack.repo.SpackNamespace)
+    assert isinstance(spack.pkg.builtin, spack.repo.SpackNamespace)
+    assert isinstance(spack.pkg.builtin.mock, spack.repo.SpackNamespace)
+
+
+def test_absolute_import_spack_packages_as_python_modules(mock_packages):
+    import spack.pkg.builtin.mock.mpileaks
+    assert hasattr(spack.pkg.builtin.mock, 'mpileaks')
+    assert hasattr(spack.pkg.builtin.mock.mpileaks, 'Mpileaks')
+    assert isinstance(spack.pkg.builtin.mock.mpileaks.Mpileaks,
+                      spack.package.PackageMeta)
+    assert issubclass(spack.pkg.builtin.mock.mpileaks.Mpileaks, spack.package.Package)
+
+
+def test_relative_import_spack_packages_as_python_modules(mock_packages):
+    from spack.pkg.builtin.mock.mpileaks import Mpileaks
+    assert isinstance(Mpileaks, spack.package.PackageMeta)
+    assert issubclass(Mpileaks, spack.package.Package)
diff --git a/lib/spack/spack/util/imp/__init__.py b/lib/spack/spack/util/imp/__init__.py
deleted file mode 100644
index 4e1ff48a72..0000000000
--- a/lib/spack/spack/util/imp/__init__.py
+++ /dev/null
@@ -1,22 +0,0 @@
-# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other
-# Spack Project Developers. See the top-level COPYRIGHT file for details.
-#
-# SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-"""Consolidated module for all imports done by Spack.
-
-Many parts of Spack have to import Python code. This utility package
-wraps Spack's interface with Python's import system.
-
-We do this because Python's import system is confusing and changes from
-Python version to Python version, and we should be able to adapt our
-approach to the underlying implementation.
-
-Currently, this uses ``importlib.machinery`` where available and ``imp``
-when ``importlib`` is not completely usable.
-"""
-
-try:
-    from .importlib_importer import load_source  # noqa
-except ImportError:
-    from .imp_importer import load_source        # noqa
diff --git a/lib/spack/spack/util/imp/imp_importer.py b/lib/spack/spack/util/imp/imp_importer.py
deleted file mode 100644
index 9a09e95836..0000000000
--- a/lib/spack/spack/util/imp/imp_importer.py
+++ /dev/null
@@ -1,67 +0,0 @@
-# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other
-# Spack Project Developers. See the top-level COPYRIGHT file for details.
-#
-# SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-"""Implementation of Spack imports that uses imp underneath.
-
-``imp`` is deprecated in newer versions of Python, but is the only option
-in Python 2.6.
-"""
-import imp
-import tempfile
-from contextlib import contextmanager
-
-
-@contextmanager
-def import_lock():
-    imp.acquire_lock()
-    yield
-    imp.release_lock()
-
-
-def load_source(full_name, path, prepend=None):
-    """Import a Python module from source.
-
-    Load the source file and add it to ``sys.modules``.
-
-    Args:
-        full_name (str): full name of the module to be loaded
-        path (str): path to the file that should be loaded
-        prepend (str or None): some optional code to prepend to the
-            loaded module; e.g., can be used to inject import statements
-
-    Returns:
-        the loaded module
-    """
-    with import_lock():
-        if prepend is None:
-            return imp.load_source(full_name, path)
-        else:
-            with prepend_open(path, text=prepend) as f:
-                return imp.load_source(full_name, path, f)
-
-
-@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.
-
-    We have to write and read a tempfile for the ``imp``-based importer,
-    as the ``file`` argument to ``imp.load_source()`` requires a
-    low-level file handle.
-
-    See the ``importlib``-based importer for a faster way to do this in
-    later versions of python.
-    """
-    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
diff --git a/lib/spack/spack/util/imp/importlib_importer.py b/lib/spack/spack/util/imp/importlib_importer.py
deleted file mode 100644
index 8b2b55b718..0000000000
--- a/lib/spack/spack/util/imp/importlib_importer.py
+++ /dev/null
@@ -1,48 +0,0 @@
-# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other
-# Spack Project Developers. See the top-level COPYRIGHT file for details.
-#
-# SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-"""Implementation of Spack imports that uses importlib underneath.
-
-``importlib`` is only fully implemented in Python 3.
-"""
-from importlib.machinery import SourceFileLoader  # novm
-
-
-class PrependFileLoader(SourceFileLoader):
-    def __init__(self, full_name, path, prepend=None):
-        super(PrependFileLoader, self).__init__(full_name, path)
-        self.prepend = prepend
-
-    def path_stats(self, path):
-        stats = super(PrependFileLoader, self).path_stats(path)
-        if self.prepend:
-            stats["size"] += len(self.prepend) + 1
-        return stats
-
-    def get_data(self, path):
-        data = super(PrependFileLoader, self).get_data(path)
-        if path != self.path or self.prepend is None:
-            return data
-        else:
-            return self.prepend.encode() + b"\n" + data
-
-
-def load_source(full_name, path, prepend=None):
-    """Import a Python module from source.
-
-    Load the source file and add it to ``sys.modules``.
-
-    Args:
-        full_name (str): full name of the module to be loaded
-        path (str): path to the file that should be loaded
-        prepend (str or None): some optional code to prepend to the
-            loaded module; e.g., can be used to inject import statements
-
-    Returns:
-        the loaded module
-    """
-    # use our custom loader
-    loader = PrependFileLoader(full_name, path, prepend)
-    return loader.load_module()
-- 
cgit v1.2.3-60-g2f50