Revert "package import: remove magic import line (#39183)" (#39380)

This reverts commit 8e7c53a8ba.

lib/spack/spack/build_environment.py

@@ -1265,8 +1265,12 @@ def make_stack(tb, stack=None):
     # Point out the location in the install method where we failed.
     filename = inspect.getfile(frame.f_code)
     lineno = frame.f_lineno
+    if os.path.basename(filename) == "package.py":
+        # subtract 1 because we inject a magic import at the top of package files.
+        # TODO: get rid of the magic import.
+        lineno -= 1
 
-    lines = [f"{filename}:{lineno:d}, in {frame.f_code.co_name}:"]
+    lines = ["{0}:{1:d}, in {2}:".format(filename, lineno, frame.f_code.co_name)]
 
     # Build a message showing context in the install method.
     sourcelines, start = inspect.getsourcelines(frame)

lib/spack/spack/repo.py

@@ -78,6 +78,43 @@ def namespace_from_fullname(fullname):
     return namespace
 
 
+class _PrependFileLoader(importlib.machinery.SourceFileLoader):
+    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 are expected to call `from spack.package import *`
+    #: themselves, but we are allowing a deprecation period before breaking
+    #: external repos that don't do this yet.
+    _package_prepend = "from spack.package 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().__init__(self.fullname, self.package_py, prepend=self._package_prepend)
+
+
 class SpackNamespaceLoader:
     def create_module(self, spec):
         return SpackNamespace(spec.name)
@@ -118,9 +155,7 @@ def compute_loader(self, fullname):
                 # With 2 nested conditionals we can call "repo.real_name" only once
                 package_name = repo.real_name(module_name)
                 if package_name:
-                    return importlib.machinery.SourceFileLoader(
+                    return RepoLoader(fullname, repo, package_name)
-                        fullname, repo.filename_for_package_name(package_name)
-                    )
 
             # We are importing a full namespace like 'spack.pkg.builtin'
             if fullname == repo.full_namespace:
@@ -1201,11 +1236,6 @@ def get_pkg_class(self, pkg_name):
             raise UnknownPackageError(fullname)
         except Exception as e:
             msg = f"cannot load package '{pkg_name}' from the '{self.namespace}' repository: {e}"
-            if isinstance(e, NameError):
-                msg += (
-                    ". This usually means `from spack.package import *` "
-                    "is missing at the top of the package.py file."
-                )
             raise RepoError(msg) from e
 
         cls = getattr(module, class_name)

lib/spack/spack/test/concretize.py

@@ -145,8 +145,6 @@ def repo_with_changing_recipe(tmpdir_factory, mutable_mock_repo):
 
     packages_dir = repo_dir.ensure("packages", dir=True)
     root_pkg_str = """
-from spack.package import *
-
 class Root(Package):
     homepage = "http://www.example.com"
     url      = "http://www.example.com/root-1.0.tar.gz"
@@ -159,8 +157,6 @@ class Root(Package):
     packages_dir.join("root", "package.py").write(root_pkg_str, ensure=True)
 
     changing_template = """
-from spack.package import *
-
 class Changing(Package):
     homepage = "http://www.example.com"
     url      = "http://www.example.com/changing-1.0.tar.gz"

lib/spack/spack/test/concretize_requirements.py

@@ -29,8 +29,6 @@ def update_packages_config(conf_str):
 _pkgx = (
     "x",
     """\
-from spack.package import *
-
 class X(Package):
     version("1.1")
     version("1.0")
@@ -47,8 +45,6 @@ class X(Package):
 _pkgy = (
     "y",
     """\
-from spack.package import *
-
 class Y(Package):
     version("2.5")
     version("2.4")
@@ -63,8 +59,6 @@ class Y(Package):
 _pkgv = (
     "v",
     """\
-from spack.package import *
-
 class V(Package):
     version("2.1")
     version("2.0")
@@ -75,8 +69,6 @@ class V(Package):
 _pkgt = (
     "t",
     """\
-from spack.package import *
-
 class T(Package):
     version('2.1')
     version('2.0')
@@ -89,8 +81,6 @@ class T(Package):
 _pkgu = (
     "u",
     """\
-from spack.package import *
-
 class U(Package):
     version('1.1')
     version('1.0')

lib/spack/spack/test/packages.py

@@ -4,13 +4,11 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import os
-import re
 
 import pytest
 
 import spack.directives
 import spack.fetch_strategy
-import spack.package_base
 import spack.repo
 from spack.paths import mock_packages_path
 from spack.spec import Spec
@@ -320,24 +318,3 @@ def test_package_deprecated_version(mock_packages, mock_fetch, mock_stage):
 
     assert spack.package_base.deprecated_version(pkg_cls, "1.1.0")
     assert not spack.package_base.deprecated_version(pkg_cls, "1.0.0")
-
-
-def test_package_with_deprecated_magic_import_has_a_useful_error(tmpdir, mutable_config):
-    """Test that a package that's missing `from spack.package import *` gets a useful error,
-    suggesting that it be added."""
-    tmpdir.join("repo.yaml").write("repo:\n  namespace: old_package")
-    tmpdir.join("packages", "old-package").ensure(dir=True).join("package.py").write(
-        """\
-class OldPackage(Package):
-    version('1.0', '0123456789abcdef0123456789abcdef')
-"""
-    )
-    with spack.repo.use_repositories(str(tmpdir)) as repo:
-        with pytest.raises(
-            spack.repo.RepoError,
-            match=re.escape(
-                "This usually means `from spack.package import *` "
-                "is missing at the top of the package.py file."
-            ),
-        ):
-            repo.get_pkg_class("old-package")

share/spack/templates/mock-repository/package.pyt

@@ -1,5 +1,3 @@
-from spack.package import *
-
 class {{ cls_name }}(Package):
     homepage = "http://www.example.com"
     url = "http://www.example.com/root-1.0.tar.gz"