Fix a few circular deps (#46373)

lib/spack/spack/__init__.py

@@ -3,6 +3,13 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import os
+import re
+from typing import Optional
+
+import spack.paths
+import spack.util.git
+
 #: PEP440 canonical <major>.<minor>.<micro>.<devN> string
 __version__ = "0.23.0.dev0"
 spack_version = __version__
@@ -19,4 +26,47 @@ def __try_int(v):
 spack_version_info = tuple([__try_int(v) for v in __version__.split(".")])
 
 
-__all__ = ["spack_version_info", "spack_version"]
+def get_spack_commit() -> Optional[str]:
+    """Get the Spack git commit sha.
+
+    Returns:
+        (str or None) the commit sha if available, otherwise None
+    """
+    git_path = os.path.join(spack.paths.prefix, ".git")
+    if not os.path.exists(git_path):
+        return None
+
+    git = spack.util.git.git()
+    if not git:
+        return None
+
+    rev = git(
+        "-C",
+        spack.paths.prefix,
+        "rev-parse",
+        "HEAD",
+        output=str,
+        error=os.devnull,
+        fail_on_error=False,
+    )
+    if git.returncode != 0:
+        return None
+
+    match = re.match(r"[a-f\d]{7,}$", rev)
+    return match.group(0) if match else None
+
+
+def get_version() -> str:
+    """Get a descriptive version of this instance of Spack.
+
+    Outputs '<PEP440 version> (<git commit sha>)'.
+
+    The commit sha is only added when available.
+    """
+    commit = get_spack_commit()
+    if commit:
+        return f"{spack_version} ({commit})"
+    return spack_version
+
+
+__all__ = ["spack_version_info", "spack_version", "get_version", "get_spack_commit"]

lib/spack/spack/binary_distribution.py

@@ -44,6 +44,7 @@
 import spack.oci.image
 import spack.oci.oci
 import spack.oci.opener
+import spack.paths
 import spack.platforms
 import spack.relocate as relocate
 import spack.repo
@@ -1447,7 +1448,9 @@ def _oci_push_pkg_blob(
     filename = os.path.join(tmpdir, f"{spec.dag_hash()}.tar.gz")
 
     # Create an oci.image.layer aka tarball of the package
-    compressed_tarfile_checksum, tarfile_checksum = spack.oci.oci.create_tarball(spec, filename)
+    compressed_tarfile_checksum, tarfile_checksum = _do_create_tarball(
+        filename, spec.prefix, get_buildinfo_dict(spec)
+    )
 
     blob = spack.oci.oci.Blob(
         Digest.from_sha256(compressed_tarfile_checksum),

lib/spack/spack/caches.py

@@ -13,7 +13,6 @@
 import spack.config
 import spack.error
 import spack.fetch_strategy
-import spack.mirror
 import spack.paths
 import spack.util.file_cache
 import spack.util.path

lib/spack/spack/ci.py

@@ -1219,8 +1219,8 @@ def main_script_replacements(cmd):
         # Capture the version of Spack used to generate the pipeline, that can be
         # passed to `git checkout` for version consistency. If we aren't in a Git
         # repository, presume we are a Spack release and use the Git tag instead.
-        spack_version = spack.main.get_version()
-        version_to_clone = spack.main.get_spack_commit() or f"v{spack.spack_version}"
+        spack_version = spack.get_version()
+        version_to_clone = spack.get_spack_commit() or f"v{spack.spack_version}"
 
         output_object["variables"] = {
             "SPACK_ARTIFACTS_ROOT": rel_artifacts_root,

lib/spack/spack/cmd/debug.py

@@ -13,11 +13,11 @@
 import llnl.util.tty as tty
 from llnl.util.filesystem import working_dir
 
+import spack
 import spack.config
 import spack.paths
 import spack.platforms
 import spack.util.git
-from spack.main import get_version
 from spack.util.executable import which
 
 description = "debugging commands for troubleshooting Spack"
@@ -89,7 +89,7 @@ def report(args):
     host_os = host_platform.operating_system("frontend")
     host_target = host_platform.target("frontend")
     architecture = spack.spec.ArchSpec((str(host_platform), str(host_os), str(host_target)))
-    print("* **Spack:**", get_version())
+    print("* **Spack:**", spack.get_version())
     print("* **Python:**", platform.python_version())
     print("* **Platform:**", architecture)
 

lib/spack/spack/container/writers.py

@@ -6,6 +6,7 @@
 convenience functions.
 """
 import copy
+import shlex
 from collections import namedtuple
 from typing import Optional
 
@@ -15,7 +16,7 @@
 import spack.tengine as tengine
 import spack.util.spack_yaml as syaml
 
-from ..images import (
+from .images import (
     bootstrap_template_for,
     build_info,
     checkout_command,
@@ -308,7 +309,54 @@ def __call__(self):
         return t.render(**self.to_dict())
 
 
-# Import after function definition all the modules in this package,
-# so that registration of writers will happen automatically
-from . import docker  # noqa: F401 E402
-from . import singularity  # noqa: F401 E402
+@writer("docker")
+class DockerContext(PathContext):
+    """Context used to instantiate a Dockerfile"""
+
+    #: Name of the template used for Dockerfiles
+    template_name = "container/Dockerfile"
+
+    @tengine.context_property
+    def manifest(self):
+        manifest_str = super().manifest
+        # Docker doesn't support HEREDOC, so we need to resort to
+        # a horrible echo trick to have the manifest in the Dockerfile
+        echoed_lines = []
+        for idx, line in enumerate(manifest_str.split("\n")):
+            quoted_line = shlex.quote(line)
+            if idx == 0:
+                echoed_lines.append("&&  (echo " + quoted_line + " \\")
+                continue
+            echoed_lines.append("&&   echo " + quoted_line + " \\")
+
+        echoed_lines[-1] = echoed_lines[-1].replace(" \\", ")")
+
+        return "\n".join(echoed_lines)
+
+
+@writer("singularity")
+class SingularityContext(PathContext):
+    """Context used to instantiate a Singularity definition file"""
+
+    #: Name of the template used for Singularity definition files
+    template_name = "container/singularity.def"
+
+    @property
+    def singularity_config(self):
+        return self.container_config.get("singularity", {})
+
+    @tengine.context_property
+    def runscript(self):
+        return self.singularity_config.get("runscript", "")
+
+    @tengine.context_property
+    def startscript(self):
+        return self.singularity_config.get("startscript", "")
+
+    @tengine.context_property
+    def test(self):
+        return self.singularity_config.get("test", "")
+
+    @tengine.context_property
+    def help(self):
+        return self.singularity_config.get("help", "")

lib/spack/spack/container/writers/docker.py

@@ -1,34 +0,0 @@
-# Copyright 2013-2024 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)
-import shlex
-
-import spack.tengine as tengine
-
-from . import PathContext, writer
-
-
-@writer("docker")
-class DockerContext(PathContext):
-    """Context used to instantiate a Dockerfile"""
-
-    #: Name of the template used for Dockerfiles
-    template_name = "container/Dockerfile"
-
-    @tengine.context_property
-    def manifest(self):
-        manifest_str = super().manifest
-        # Docker doesn't support HEREDOC, so we need to resort to
-        # a horrible echo trick to have the manifest in the Dockerfile
-        echoed_lines = []
-        for idx, line in enumerate(manifest_str.split("\n")):
-            quoted_line = shlex.quote(line)
-            if idx == 0:
-                echoed_lines.append("&&  (echo " + quoted_line + " \\")
-                continue
-            echoed_lines.append("&&   echo " + quoted_line + " \\")
-
-        echoed_lines[-1] = echoed_lines[-1].replace(" \\", ")")
-
-        return "\n".join(echoed_lines)

lib/spack/spack/container/writers/singularity.py

@@ -1,35 +0,0 @@
-# Copyright 2013-2024 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)
-import spack.tengine as tengine
-
-from . import PathContext, writer
-
-
-@writer("singularity")
-class SingularityContext(PathContext):
-    """Context used to instantiate a Singularity definition file"""
-
-    #: Name of the template used for Singularity definition files
-    template_name = "container/singularity.def"
-
-    @property
-    def singularity_config(self):
-        return self.container_config.get("singularity", {})
-
-    @tengine.context_property
-    def runscript(self):
-        return self.singularity_config.get("runscript", "")
-
-    @tengine.context_property
-    def startscript(self):
-        return self.singularity_config.get("startscript", "")
-
-    @tengine.context_property
-    def test(self):
-        return self.singularity_config.get("test", "")
-
-    @tengine.context_property
-    def help(self):
-        return self.singularity_config.get("help", "")

lib/spack/spack/environment/environment.py

@@ -24,25 +24,20 @@
 from llnl.util.link_tree import ConflictingSpecsError
 from llnl.util.symlink import readlink, symlink
 
+import spack
 import spack.caches
-import spack.cmd
 import spack.compilers
 import spack.concretize
 import spack.config
 import spack.deptypes as dt
 import spack.error
-import spack.fetch_strategy
 import spack.filesystem_view as fsv
 import spack.hash_types as ht
-import spack.hooks
-import spack.main
 import spack.paths
 import spack.repo
 import spack.schema.env
 import spack.spec
-import spack.stage
 import spack.store
-import spack.subprocess_context
 import spack.user_environment as uenv
 import spack.util.cpus
 import spack.util.environment
@@ -53,7 +48,6 @@
 import spack.util.spack_json as sjson
 import spack.util.spack_yaml as syaml
 import spack.util.url
-import spack.version
 from spack import traverse
 from spack.installer import PackageInstaller
 from spack.schema.env import TOP_LEVEL_KEY
@@ -2179,7 +2173,7 @@ def _to_lockfile_dict(self):
         root_specs = self._concrete_roots_dict()
 
         spack_dict = {"version": spack.spack_version}
-        spack_commit = spack.main.get_spack_commit()
+        spack_commit = spack.get_spack_commit()
         if spack_commit:
             spack_dict["type"] = "git"
             spack_dict["commit"] = spack_commit

lib/spack/spack/error.py

@@ -132,3 +132,15 @@ def __init__(self, provided, required, constraint_type):
 
 class FetchError(SpackError):
     """Superclass for fetch-related errors."""
+
+
+class NoSuchPatchError(SpackError):
+    """Raised when a patch file doesn't exist."""
+
+
+class PatchDirectiveError(SpackError):
+    """Raised when the wrong arguments are suppled to the patch directive."""
+
+
+class PatchLookupError(NoSuchPatchError):
+    """Raised when a patch file cannot be located from sha256."""

lib/spack/spack/main.py

@@ -32,6 +32,7 @@
 import llnl.util.tty.color as color
 from llnl.util.tty.log import log_output
 
+import spack
 import spack.cmd
 import spack.config
 import spack.environment as ev
@@ -44,8 +45,6 @@
 import spack.store
 import spack.util.debug
 import spack.util.environment
-import spack.util.git
-import spack.util.path
 from spack.error import SpackError
 
 #: names of profile statistics
@@ -122,51 +121,6 @@ def add_all_commands(parser):
         parser.add_command(cmd)
 
 
-def get_spack_commit():
-    """Get the Spack git commit sha.
-
-    Returns:
-        (str or None) the commit sha if available, otherwise None
-    """
-    git_path = os.path.join(spack.paths.prefix, ".git")
-    if not os.path.exists(git_path):
-        return None
-
-    git = spack.util.git.git()
-    if not git:
-        return None
-
-    rev = git(
-        "-C",
-        spack.paths.prefix,
-        "rev-parse",
-        "HEAD",
-        output=str,
-        error=os.devnull,
-        fail_on_error=False,
-    )
-    if git.returncode != 0:
-        return None
-
-    match = re.match(r"[a-f\d]{7,}$", rev)
-    return match.group(0) if match else None
-
-
-def get_version():
-    """Get a descriptive version of this instance of Spack.
-
-    Outputs '<PEP440 version> (<git commit sha>)'.
-
-    The commit sha is only added when available.
-    """
-    version = spack.spack_version
-    commit = get_spack_commit()
-    if commit:
-        version += " ({0})".format(commit)
-
-    return version
-
-
 def index_commands():
     """create an index of commands by section for this help level"""
     index = {}
@@ -954,7 +908,7 @@ def _main(argv=None):
 
     # version is special as it does not require a command or loading and additional infrastructure
     if args.version:
-        print(get_version())
+        print(spack.get_version())
         return 0
 
     # ------------------------------------------------------------------------

lib/spack/spack/oci/oci.py

@@ -15,7 +15,6 @@
 
 import llnl.util.tty as tty
 
-import spack.binary_distribution
 import spack.config
 import spack.error
 import spack.fetch_strategy
@@ -37,11 +36,6 @@ class Blob(NamedTuple):
     size: int
 
 
-def create_tarball(spec: spack.spec.Spec, tarfile_path):
-    buildinfo = spack.binary_distribution.get_buildinfo_dict(spec)
-    return spack.binary_distribution._do_create_tarball(tarfile_path, spec.prefix, buildinfo)
-
-
 def with_query_param(url: str, param: str, value: str) -> str:
     """Add a query parameter to a URL
 

lib/spack/spack/package_base.py

@@ -65,6 +65,7 @@
     install_test_root,
 )
 from spack.installer import InstallError, PackageInstaller
+from spack.solver.version_order import concretization_version_order
 from spack.stage import DevelopStage, ResourceStage, Stage, StageComposite, compute_stage_name
 from spack.util.executable import ProcessError, which
 from spack.util.package_hash import package_hash
@@ -116,7 +117,6 @@ def preferred_version(pkg: "PackageBase"):
     Arguments:
         pkg: The package whose versions are to be assessed.
     """
-    from spack.solver.asp import concretization_version_order
 
     version, _ = max(pkg.versions.items(), key=concretization_version_order)
     return version

lib/spack/spack/patch.py

@@ -113,7 +113,7 @@ def apply(self, stage: "spack.stage.Stage") -> None:
             stage: stage where source code lives
         """
         if not self.path or not os.path.isfile(self.path):
-            raise NoSuchPatchError(f"No such patch: {self.path}")
+            raise spack.error.NoSuchPatchError(f"No such patch: {self.path}")
 
         apply_patch(stage, self.path, self.level, self.working_dir, self.reverse)
 
@@ -275,14 +275,14 @@ def __init__(
         self.ordering_key = ordering_key
 
         if allowed_archive(self.url) and not archive_sha256:
-            raise PatchDirectiveError(
+            raise spack.error.PatchDirectiveError(
                 "Compressed patches require 'archive_sha256' "
                 "and patch 'sha256' attributes: %s" % self.url
             )
         self.archive_sha256 = archive_sha256
 
         if not sha256:
-            raise PatchDirectiveError("URL patches require a sha256 checksum")
+            raise spack.error.PatchDirectiveError("URL patches require a sha256 checksum")
         self.sha256 = sha256
 
     def apply(self, stage: "spack.stage.Stage") -> None:
@@ -480,7 +480,7 @@ def patch_for_package(self, sha256: str, pkg: "spack.package_base.PackageBase")
         """
         sha_index = self.index.get(sha256)
         if not sha_index:
-            raise PatchLookupError(
+            raise spack.error.PatchLookupError(
                 f"Couldn't find patch for package {pkg.fullname} with sha256: {sha256}"
             )
 
@@ -490,7 +490,7 @@ def patch_for_package(self, sha256: str, pkg: "spack.package_base.PackageBase")
             if patch_dict:
                 break
         else:
-            raise PatchLookupError(
+            raise spack.error.PatchLookupError(
                 f"Couldn't find patch for package {pkg.fullname} with sha256: {sha256}"
             )
 
@@ -573,15 +573,3 @@ def _index_patches(
                         index[patch.sha256] = {dspec_cls.fullname: patch_dict}
 
         return index
-
-
-class NoSuchPatchError(spack.error.SpackError):
-    """Raised when a patch file doesn't exist."""
-
-
-class PatchLookupError(NoSuchPatchError):
-    """Raised when a patch file cannot be located from sha256."""
-
-
-class PatchDirectiveError(spack.error.SpackError):
-    """Raised when the wrong arguments are suppled to the patch directive."""

lib/spack/spack/platforms/__init__.py

@@ -51,7 +51,6 @@ def __call__(self):
 def use_platform(new_platform):
     global host
 
-    import spack.compilers
     import spack.config
 
     msg = '"{0}" must be an instance of Platform'
@@ -61,16 +60,9 @@ def use_platform(new_platform):
 
     try:
         host = _PickleableCallable(new_platform)
-
-        # Clear configuration and compiler caches
         spack.config.CONFIG.clear_caches()
-        spack.compilers._cache_config_files = []
-
         yield new_platform
 
     finally:
         host = original_host_fn
-
-        # Clear configuration and compiler caches
         spack.config.CONFIG.clear_caches()
-        spack.compilers._cache_config_files = []

lib/spack/spack/reporters/cdash.py

@@ -19,9 +19,11 @@
 import llnl.util.tty as tty
 from llnl.util.filesystem import working_dir
 
+import spack
 import spack.build_environment
 import spack.fetch_strategy
 import spack.package_base
+import spack.paths
 import spack.platforms
 import spack.util.git
 from spack.error import SpackError
@@ -119,7 +121,7 @@ def __init__(self, configuration: CDashConfiguration):
         git = spack.util.git.git()
         with working_dir(spack.paths.spack_root):
             self.revision = git("rev-parse", "HEAD", output=str).strip()
-        self.generator = "spack-{0}".format(spack.main.get_version())
+        self.generator = "spack-{0}".format(spack.get_version())
         self.multiple_packages = False
 
     def report_build_name(self, pkg_name):

lib/spack/spack/solver/asp.py

@@ -64,6 +64,7 @@
     parse_term,
 )
 from .counter import FullDuplicatesCounter, MinimalDuplicatesCounter, NoDuplicatesCounter
+from .version_order import concretization_version_order
 
 GitOrStandardVersion = Union[spack.version.GitVersion, spack.version.StandardVersion]
 
@@ -579,20 +580,6 @@ def _is_checksummed_version(version_info: Tuple[GitOrStandardVersion, dict]):
     return _is_checksummed_git_version(version)
 
 
-def concretization_version_order(version_info: Tuple[GitOrStandardVersion, dict]):
-    """Version order key for concretization, where preferred > not preferred,
-    not deprecated > deprecated, finite > any infinite component; only if all are
-    the same, do we use default version ordering."""
-    version, info = version_info
-    return (
-        info.get("preferred", False),
-        not info.get("deprecated", False),
-        not version.isdevelop(),
-        not version.is_prerelease(),
-        version,
-    )
-
-
 def _spec_with_default_name(spec_str, name):
     """Return a spec with a default name if none is provided, used for requirement specs"""
     spec = spack.spec.Spec(spec_str)

lib/spack/spack/solver/version_order.py

@@ -0,0 +1,21 @@
+# Copyright 2013-2024 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)
+from typing import Tuple, Union
+
+from spack.version import GitVersion, StandardVersion
+
+
+def concretization_version_order(version_info: Tuple[Union[GitVersion, StandardVersion], dict]):
+    """Version order key for concretization, where preferred > not preferred,
+    not deprecated > deprecated, finite > any infinite component; only if all are
+    the same, do we use default version ordering."""
+    version, info = version_info
+    return (
+        info.get("preferred", False),
+        not info.get("deprecated", False),
+        not version.isdevelop(),
+        not version.is_prerelease(),
+        version,
+    )

lib/spack/spack/spec.py

@@ -68,6 +68,7 @@
 import llnl.util.tty as tty
 import llnl.util.tty.color as clr
 
+import spack
 import spack.compiler
 import spack.compilers
 import spack.config
@@ -75,7 +76,6 @@
 import spack.error
 import spack.hash_types as ht
 import spack.parser
-import spack.patch
 import spack.paths
 import spack.platforms
 import spack.provider_index
@@ -1307,7 +1307,7 @@ def copy(self, *args, **kwargs):
 
 
 def tree(
-    specs: List["spack.spec.Spec"],
+    specs: List["Spec"],
     *,
     color: Optional[bool] = None,
     depth: bool = False,
@@ -2032,7 +2032,7 @@ def _lookup_hash(self):
             raise InvalidHashError(self, self.abstract_hash)
 
         if len(matches) != 1:
-            raise spack.spec.AmbiguousHashError(
+            raise AmbiguousHashError(
                 f"Multiple packages specify hash beginning '{self.abstract_hash}'.", *matches
             )
 
@@ -3464,7 +3464,7 @@ def patches(self):
                     pkg_cls = spack.repo.PATH.get_pkg_class(self.name)
                     try:
                         patch = index.patch_for_package(sha256, pkg_cls)
-                    except spack.patch.PatchLookupError as e:
+                    except spack.error.PatchLookupError as e:
                         raise spack.error.SpecError(
                             f"{e}. This usually means the patch was modified or removed. "
                             "To fix this, either reconcretize or use the original package "
@@ -4535,7 +4535,7 @@ def merge_abstract_anonymous_specs(*abstract_specs: Spec):
     Args:
         *abstract_specs: abstract specs to be merged
     """
-    merged_spec = spack.spec.Spec()
+    merged_spec = Spec()
     for current_spec_constraint in abstract_specs:
         merged_spec.constrain(current_spec_constraint, deps=False)
 
@@ -4890,7 +4890,6 @@ def get_host_environment_metadata() -> Dict[str, str]:
     """Get the host environment, reduce to a subset that we can store in
     the install directory, and add the spack version.
     """
-    import spack.main
 
     environ = get_host_environment()
     return {
@@ -4898,7 +4897,7 @@ def get_host_environment_metadata() -> Dict[str, str]:
         "platform": environ["platform"],
         "host_target": environ["target"],
         "hostname": environ["hostname"],
-        "spack_version": spack.main.get_version(),
+        "spack_version": spack.get_version(),
         "kernel_version": platform.version(),
     }
 

lib/spack/spack/test/cmd/ci.py

@@ -305,8 +305,8 @@ def test_ci_generate_with_custom_settings(
     ci_generate_test, tmp_path, mock_binary_index, monkeypatch
 ):
     """Test use of user-provided scripts and attributes"""
-    monkeypatch.setattr(spack.main, "get_version", lambda: "0.15.3")
-    monkeypatch.setattr(spack.main, "get_spack_commit", lambda: "big ol commit sha")
+    monkeypatch.setattr(spack, "get_version", lambda: "0.15.3")
+    monkeypatch.setattr(spack, "get_spack_commit", lambda: "big ol commit sha")
     spack_yaml, outputfile, _ = ci_generate_test(
         f"""\
 spack:
@@ -1040,8 +1040,8 @@ def test_ci_generate_override_runner_attrs(
     inherit them from the top level, as well as when we override one or
     more at the runner level"""
     monkeypatch.setattr(spack, "spack_version", "0.20.0.test0")
-    monkeypatch.setattr(spack.main, "get_version", lambda: "0.20.0.test0 (blah)")
-    monkeypatch.setattr(spack.main, "get_spack_commit", lambda: git_version)
+    monkeypatch.setattr(spack, "get_version", lambda: "0.20.0.test0 (blah)")
+    monkeypatch.setattr(spack, "get_spack_commit", lambda: git_version)
     spack_yaml, outputfile, _ = ci_generate_test(
         f"""\
 spack:

lib/spack/spack/test/cmd/debug.py

@@ -9,9 +9,10 @@
 
 import pytest
 
+import spack
 import spack.config
 import spack.platforms
-from spack.main import SpackCommand, get_version
+from spack.main import SpackCommand
 from spack.util.executable import which
 
 debug = SpackCommand("debug")
@@ -55,6 +56,6 @@ def test_report():
     host_target = host_platform.target("frontend")
     architecture = spack.spec.ArchSpec((str(host_platform), str(host_os), str(host_target)))
 
-    assert get_version() in out
+    assert spack.get_version() in out
     assert platform.python_version() in out
     assert str(architecture) in out

lib/spack/spack/test/concretize.py

@@ -25,6 +25,7 @@
 import spack.platforms
 import spack.repo
 import spack.solver.asp
+import spack.solver.version_order
 import spack.store
 import spack.util.file_cache
 import spack.util.libc
@@ -2928,7 +2929,7 @@ def test_concretization_version_order():
     result = [
         v
         for v, _ in sorted(
-            versions, key=spack.solver.asp.concretization_version_order, reverse=True
+            versions, key=spack.solver.version_order.concretization_version_order, reverse=True
         )
     ]
     assert result == [

lib/spack/spack/test/main.py

@@ -8,10 +8,11 @@
 
 import llnl.util.filesystem as fs
 
+import spack
 import spack.paths
 import spack.util.executable as exe
 import spack.util.git
-from spack.main import get_version, main
+from spack.main import main
 
 pytestmark = pytest.mark.not_on_windows(
     "Test functionality supported but tests are failing on Win"
@@ -29,7 +30,7 @@ def test_version_git_nonsense_output(tmpdir, working_env, monkeypatch):
     fs.set_executable(git)
 
     monkeypatch.setattr(spack.util.git, "git", lambda: exe.which(git))
-    assert spack.spack_version == get_version()
+    assert spack.spack_version == spack.get_version()
 
 
 def test_version_git_fails(tmpdir, working_env, monkeypatch):
@@ -44,7 +45,7 @@ def test_version_git_fails(tmpdir, working_env, monkeypatch):
     fs.set_executable(git)
 
     monkeypatch.setattr(spack.util.git, "git", lambda: exe.which(git))
-    assert spack.spack_version == get_version()
+    assert spack.spack_version == spack.get_version()
 
 
 def test_git_sha_output(tmpdir, working_env, monkeypatch):
@@ -62,17 +63,17 @@ def test_git_sha_output(tmpdir, working_env, monkeypatch):
 
     monkeypatch.setattr(spack.util.git, "git", lambda: exe.which(git))
     expected = "{0} ({1})".format(spack.spack_version, sha)
-    assert expected == get_version()
+    assert expected == spack.get_version()
 
 
 def test_get_version_no_repo(tmpdir, monkeypatch):
     monkeypatch.setattr(spack.paths, "prefix", str(tmpdir))
-    assert spack.spack_version == get_version()
+    assert spack.spack_version == spack.get_version()
 
 
 def test_get_version_no_git(tmpdir, working_env, monkeypatch):
     monkeypatch.setattr(spack.util.git, "git", lambda: None)
-    assert spack.spack_version == get_version()
+    assert spack.spack_version == spack.get_version()
 
 
 def test_main_calls_get_version(tmpdir, capsys, working_env, monkeypatch):
@@ -96,4 +97,4 @@ def test_get_version_bad_git(tmpdir, working_env, monkeypatch):
     fs.set_executable(bad_git)
 
     monkeypatch.setattr(spack.util.git, "git", lambda: exe.which(bad_git))
-    assert spack.spack_version == get_version()
+    assert spack.spack_version == spack.get_version()

lib/spack/spack/test/patch.py

@@ -13,6 +13,7 @@
 
 from llnl.util.filesystem import mkdirp, touch, working_dir
 
+import spack.error
 import spack.patch
 import spack.paths
 import spack.repo
@@ -434,7 +435,7 @@ def test_patch_no_file():
 
     patch = spack.patch.Patch(fp, "nonexistent_file", 0, "")
     patch.path = "test"
-    with pytest.raises(spack.patch.NoSuchPatchError, match="No such patch:"):
+    with pytest.raises(spack.error.NoSuchPatchError, match="No such patch:"):
         patch.apply("")
 
 
@@ -444,10 +445,10 @@ def test_patch_no_sha256():
     fp = FakePackage("fake-package", "test", "fake-package")
     url = url_util.path_to_file_url("foo.tgz")
     match = "Compressed patches require 'archive_sha256' and patch 'sha256' attributes: file://"
-    with pytest.raises(spack.patch.PatchDirectiveError, match=match):
+    with pytest.raises(spack.error.PatchDirectiveError, match=match):
         spack.patch.UrlPatch(fp, url, sha256="", archive_sha256="")
     match = "URL patches require a sha256 checksum"
-    with pytest.raises(spack.patch.PatchDirectiveError, match=match):
+    with pytest.raises(spack.error.PatchDirectiveError, match=match):
         spack.patch.UrlPatch(fp, url, sha256="", archive_sha256="abc")
 
 

lib/spack/spack/util/environment.py

@@ -12,6 +12,7 @@
 import pickle
 import re
 import shlex
+import subprocess
 import sys
 from functools import wraps
 from typing import Any, Callable, Dict, List, MutableMapping, Optional, Tuple, Union
@@ -20,8 +21,6 @@
 from llnl.util import tty
 from llnl.util.lang import dedupe
 
-from .executable import Executable, which
-
 if sys.platform == "win32":
     SYSTEM_PATHS = [
         "C:\\",
@@ -1034,8 +1033,6 @@ def environment_after_sourcing_files(
         source_command = kwargs.get("source_command", "source")
     concatenate_on_success = kwargs.get("concatenate_on_success", "&&")
 
-    shell = Executable(shell_cmd)
-
     def _source_single_file(file_and_args, environment):
         shell_options_list = shell_options.split()
 
@@ -1043,26 +1040,21 @@ def _source_single_file(file_and_args, environment):
         source_file.extend(x for x in file_and_args)
         source_file = " ".join(source_file)
 
-        # If the environment contains 'python' use it, if not
-        # go with sys.executable. Below we just need a working
-        # Python interpreter, not necessarily sys.executable.
-        python_cmd = which("python3", "python", "python2")
-        python_cmd = python_cmd.path if python_cmd else sys.executable
-
         dump_cmd = "import os, json; print(json.dumps(dict(os.environ)))"
-        dump_environment_cmd = python_cmd + f' -E -c "{dump_cmd}"'
+        dump_environment_cmd = sys.executable + f' -E -c "{dump_cmd}"'
 
         # Try to source the file
         source_file_arguments = " ".join(
             [source_file, suppress_output, concatenate_on_success, dump_environment_cmd]
         )
-        output = shell(
-            *shell_options_list,
-            source_file_arguments,
-            output=str,
+
+        with subprocess.Popen(
+            [shell_cmd, *shell_options_list, source_file_arguments],
             env=environment,
-            ignore_quotes=True,
-        )
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+        ) as shell:
+            output, _ = shell.communicate()
 
         return json.loads(output)
 

lib/spack/spack/util/executable.py

@@ -12,6 +12,7 @@
 import llnl.util.tty as tty
 
 import spack.error
+from spack.util.environment import EnvironmentModifications
 
 __all__ = ["Executable", "which", "ProcessError"]
 
@@ -27,7 +28,6 @@ def __init__(self, name):
         self.exe = [file_path]
 
         self.default_env = {}
-        from spack.util.environment import EnvironmentModifications  # no cycle
 
         self.default_envmod = EnvironmentModifications()
         self.returncode = None