Add buildcache creation to distribution cmd (#637)

Author: jnhealy2

manager/manager_cmds/distribution.py

@@ -6,11 +6,12 @@
 import spack.config
 import spack.environment
 import spack.extensions
+import spack.llnl.util.filesystem as fs
 import spack.llnl.util.tty as tty
+import spack.spec
 import spack.util.path
 import spack.util.spack_yaml
 from spack.cmd.mirror import create_mirror_for_all_specs, filter_externals
-from spack.llnl.util.filesystem import working_dir
 from spack.main import SpackCommand
 from spack.paths import spack_root
 
@@ -49,6 +50,23 @@ def add_command(parser, command_dict):
             "into the root of the packaged distribution"
         ),
     )
+    group = subparser.add_mutually_exclusive_group()
+    group.add_argument(
+        "--source-only",
+        action="store_true",
+        help=(
+            "Only create a source mirror for packages in the environment. "
+            "Default is to create a source and binary mirror"
+        ),
+    )
+    group.add_argument(
+        "--binary-only",
+        action="store_true",
+        help=(
+            "Only create a binary mirror for packages in the environment. "
+            "Default is to create a source and binary mirror"
+        ),
+    )
     command_dict["distribution"] = distribution
 
 
@@ -135,7 +153,8 @@ def __init__(self, env, root, includes=None, excludes=None, extra_data=None):
         self.path = root
         self.package_repos = os.path.join(self.path, "spack_repo")
         self.extensions = os.path.join(self.path, "extensions")
-        self.mirror = os.path.join(self.path, "mirror")
+        self.source_mirror = os.path.join(self.path, "source-mirror")
+        self.binary_mirror = os.path.join(self.path, "binary-mirror")
         self.bootstrap_mirror = os.path.join(self.path, "bootstrap-mirror")
         self.spack_dir = os.path.join(self.path, "spack")
 
@@ -283,28 +302,46 @@ def configure_source_mirror(self):
         # However, this causes issues for packages that are not downloadable,
         # so we do a first-shot mirror creation with the original environment active.
         with self.environment_to_package:
-            tty.msg(f"Creating mirror at {self.mirror}....")
+            tty.msg(f"Creating source mirror at {self.source_mirror}....")
             create_mirror_for_all_specs(
                 mirror_specs=filter_externals(self.environment_to_package.all_specs()),
-                path=self.mirror,
+                path=self.source_mirror,
                 skip_unstable_versions=False,
             )
 
         with self.env:
-            tty.msg(f"Updating mirror at {self.mirror}....")
+            tty.msg(f"Updating mirror at {self.source_mirror}....")
             create_mirror_for_all_specs(
                 mirror_specs=filter_externals(self.env.all_specs()),
-                path=self.mirror,
+                path=self.source_mirror,
                 skip_unstable_versions=False,
             )
             mirror_path = os.path.join(
-                os.path.relpath(self.path, self.env.path), os.path.basename(self.mirror)
+                os.path.relpath(self.path, self.env.path), os.path.basename(self.source_mirror)
             )
 
             mirrorer = SpackCommand("mirror")
             tty.msg(f"Adding mirror to env: {self.env.name}....")
             with self.env.write_transaction():
-                mirrorer("add", "internal", mirror_path)
+                mirrorer("add", "internal-source", mirror_path)
+
+    def configure_binary_mirror(self):
+        cacher = SpackCommand("buildcache")
+        with self.environment_to_package:
+            tty.msg(f"Creating binary mirror at {self.binary_mirror}....")
+            cacher("push", "--unsigned", self.binary_mirror)
+            cacher("keys", "--install", "--trust")
+
+        mirrorer = SpackCommand("mirror")
+        mirror_path = os.path.join(
+            os.path.relpath(self.path, self.env.path), os.path.basename(self.binary_mirror)
+        )
+        mirror_name = "internal-binary"
+
+        with self.env:
+            tty.msg(f"Adding mirror to env: {self.env.name}....")
+            with self.env.write_transaction():
+                mirrorer("add", "--type", "binary", "--unsigned", mirror_name, mirror_path)
 
     def configure_bootstrap_mirror(self):
         tty.msg(f"Creating bootstrap mirror at {self.bootstrap_mirror}....")
@@ -317,7 +354,7 @@ def configure_bootstrap_mirror(self):
             os.path.relpath(self.bootstrap_mirror, self.env.path), "metadata", "binaries"
         )
         with self.env:
-            with working_dir(self.env.path):
+            with fs.working_dir(self.env.path):
                 with self.env.write_transaction():
                     strapper(
                         "add",
@@ -355,15 +392,40 @@ def _write(self, data):
             spack.util.spack_yaml.dump(data, outf, default_flow_style=False)
 
 
+def is_installed(spec):
+    status = True
+    for dependency in spec.dependencies(deptype=("link", "run")):
+        status = is_installed(dependency)
+    bad_statuses = [spack.spec.InstallStatus.absent, spack.spec.InstallStatus.missing]
+    return status and spec.install_status() not in bad_statuses
+
+
+def correct_mirror_args(env, args):
+    specs_to_check = list(env.concretized_specs())
+    install_status = [len(specs_to_check)] + [is_installed(x) for _, x in specs_to_check]
+    has_installed_specs = all(install_status)
+    if not args.source_only and not has_installed_specs:
+        tty.warn("Environment contains uninstalled specs, defaulting to source-only package")
+        if args.binary_only:
+            tty.die(
+                "Binary distribution requested, but the environment does not "
+                "include the necessary installed binary packages"
+            )
+        args.source_only = True
+
+
 def distribution(parser, args):
     env = spack.cmd.require_active_env(cmd_name="manager distribution")
+    correct_mirror_args(env, args)
+
     packager = DistributionPackager(
         env,
         args.distro_dir,
         includes=args.include,
         excludes=args.exclude,
         extra_data=args.extra_data,
     )
+
     with packager:
         packager.configure_specs()
         packager.configure_includes()
@@ -372,7 +434,10 @@ def distribution(parser, args):
         packager.configure_package_settings(filter_externals=args.filter_externals)
         packager.filter_excludes()
         packager.concretize()
-        packager.configure_source_mirror()
+        if not args.binary_only:
+            packager.configure_source_mirror()
+        if not args.source_only:
+            packager.configure_binary_mirror()
         packager.configure_bootstrap_mirror()
         packager.bundle_spack()
         packager.bundle_extra_data()

tests/test_distribution.py

@@ -9,10 +9,15 @@
 import os
 from argparse import ArgumentParser
 
+import manager.manager_cmds.distribution as distribution
+import pytest
+
 import spack
 import spack.cmd.bootstrap as test_bootstrap_parse
+import spack.cmd.buildcache as test_buildcache_parse
+import spack.cmd.mirror as test_mirror_parse
 import spack.environment
-import spack.extensions.manager.manager_cmds.distribution as distribution
+import spack.spec
 import spack.util.spack_yaml
 
 
@@ -202,6 +207,152 @@ def test_get_valid_env_scopes(tmpdir):
     assert len(scope_names) == 2
 
 
+class MockArgs:
+    def __init__(self, source=False, binary=False):
+        self.source_only = source
+        self.binary_only = binary
+
+
+class MockSpec:
+    def __init__(self, name, status, dependencies=None):
+        self.name = name
+        self.status = status
+        self._dependencies = dependencies or []
+
+    def install_status(self):
+        return self.status
+
+    def dependencies(self, *args, **kwargs):
+        return self._dependencies
+
+
+class MockEnv:
+    def __init__(self, specs):
+        self.specs = [(x, f"{x}.concrete") for x in specs]
+
+    def concretized_specs(self):
+        return self.specs
+
+
+def test_is_installed_true():
+    """
+    This test verifies that `is_installed` returns False if a package reports its
+    status as something other than missing or absent.
+    """
+    spec = MockSpec("hdf5", "fake_valid_status")
+    result = distribution.is_installed(spec)
+    assert result
+
+
+def test_is_installed_missing():
+    """
+    This test verifies that `is_installed` returns False if a package reports
+    its status as missing.
+    """
+    spec = MockSpec("hdf5", spack.spec.InstallStatus.missing)
+    result = distribution.is_installed(spec)
+    assert not result
+
+
+def test_is_installed_absent():
+    """
+    This test verifies that `is_installed` returns False if a package reports
+    its status as absent.
+    """
+    spec = MockSpec("hdf5", spack.spec.InstallStatus.absent)
+    result = distribution.is_installed(spec)
+    assert not result
+
+
+def test_is_installed_missing_dependency():
+    """
+    This test verifies that `is_installed` returns False if a package is installed but its
+    depenency is missing.
+    """
+    deps = [
+        MockSpec("a", spack.spec.InstallStatus.installed),
+        MockSpec("b", spack.spec.InstallStatus.missing),
+    ]
+    spec = MockSpec("hdf5", spack.spec.InstallStatus.installed, dependencies=deps)
+    result = distribution.is_installed(spec)
+    assert not result
+
+
+def test_correct_mirror_args_does_not_modify_args_when_source_only(monkeypatch):
+    """
+    This test verifies that `correct_mirror_args` does nothing if source_only was requested.
+    """
+    args = MockArgs(source=True)
+    env = MockEnv(["hdf5"])
+
+    assert args.source_only
+    assert not args.binary_only
+    monkeypatch.setattr(distribution, "is_installed", lambda x: False)
+    distribution.correct_mirror_args(env, args)
+    assert args.source_only
+    assert not args.binary_only
+
+
+def test_correct_mirror_args_sets_source_only_if_no_concretized_specs_in_env():
+    """
+    This test verifies that `correct_mirror_args` reverts to source_only if binaries aren't in env.
+    """
+    args = MockArgs()
+    env = MockEnv([])
+    assert not args.source_only
+    assert not args.binary_only
+    distribution.correct_mirror_args(env, args)
+    assert args.source_only
+    assert not args.binary_only
+
+
+def test_correct_mirror_args_sets_source_only_if_install_not_verified(monkeypatch):
+    """
+    This test verifies that `correct_mirror_args` reverts to source_only if binaries aren't
+    verified as correct.
+    """
+    args = MockArgs()
+    env = MockEnv(["hdf5.error"])
+
+    assert not args.source_only
+    assert not args.binary_only
+    monkeypatch.setattr(distribution, "is_installed", lambda x: False)
+    distribution.correct_mirror_args(env, args)
+    assert args.source_only
+    assert not args.binary_only
+
+
+def test_correct_mirror_args_does_no_modification_if_install_verified(monkeypatch):
+    """
+    This test verifies that `correct_mirror_args` does nothing if binaries exist.
+    """
+    args = MockArgs()
+    env = MockEnv(["hdf5.valid"])
+
+    assert not args.source_only
+    assert not args.binary_only
+    monkeypatch.setattr(distribution, "is_installed", lambda x: True)
+    distribution.correct_mirror_args(env, args)
+    assert not args.source_only
+    assert not args.binary_only
+
+
+def test_correct_mirror_args_does_errors_if_binary_only_but_no_binaries_exist(monkeypatch):
+    """
+    This test verifies that `correct_mirror_args` errors out if binary_only
+    is True and source_only gets defaulted to True.
+    """
+    args = MockArgs(binary=True)
+    env = MockEnv(["hdf5.error"])
+
+    assert not args.source_only
+    assert args.binary_only
+
+    monkeypatch.setattr(distribution, "is_installed", lambda x: False)
+    with pytest.raises(SystemExit):
+        distribution.correct_mirror_args(env, args)
+
+
 def test_DistributionPackager_init_distro_dir(tmpdir):
     """
     This test verifies that `init_distro_dir` correctly creates a directory
@@ -578,7 +729,7 @@ def mirror_for_specs(*args, **kwargs):
     pkgr.configure_source_mirror()
 
     content = get_manifest(pkgr.env)
-    expected = {"internal": "../mirror"}
+    expected = {"internal-source": "../source-mirror"}
     assert "mirrors" in content["spack"]
     assert content["spack"]["mirrors"] == expected
 
@@ -631,7 +782,7 @@ def mirror_for_specs(*args, **kwargs):
 
     content = get_manifest(pkgr.env)
     assert "mirrors" in content["spack"]
-    assert content["spack"]["mirrors"] == {"internal": "../mirror"}
+    assert content["spack"]["mirrors"] == {"internal-source": "../source-mirror"}
 
 
 def test_DistributionPackager_configure_bootstrap_mirror(tmpdir, monkeypatch):
@@ -670,3 +821,44 @@ def __call__(self, *args, **kwargs):
     with pkgr.env:
         for call in MockCommand.call_args:
             parser.parse_args(call)
+
+
+def test_DistributionPackager_configure_binary_mirror(tmpdir, monkeypatch):
+    """
+    This test verifies that `configure_binary_mirror` does not construct a call to
+    `spack buildcache` or `spack mirror` that violates its API.
+    """
+    manifest = os.path.join(tmpdir.strpath, "base-env", "spack.yaml")
+    create_spack_manifest(manifest)
+    env = spack.environment.Environment(os.path.dirname(manifest))
+    root = os.path.join(tmpdir.strpath, "root")
+    pkgr = distribution.DistributionPackager(env, root)
+
+    class MockCommand:
+        args = []
+        kwargs = {}
+        call_args = []
+
+        def __init__(self, *args, **kwargs):
+            self.args += list(args)
+            self.kwargs.update(kwargs)
+
+        def __call__(self, *args, **kwargs):
+            self.call_args.append(list(args))
+            self.kwargs.update(kwargs)
+
+    monkeypatch.setattr(distribution, "SpackCommand", MockCommand)
+    pkgr.configure_binary_mirror()
+
+    assert MockCommand.args == ["buildcache", "mirror"]
+    assert MockCommand.kwargs == {}
+    assert len(MockCommand.call_args) == 3
+
+    buildcache_parser = ArgumentParser()
+    test_buildcache_parse.setup_parser(buildcache_parser)
+    mirror_parser = ArgumentParser()
+    test_mirror_parse.setup_parser(mirror_parser)
+    with pkgr.env:
+        buildcache_parser.parse_args(MockCommand.call_args[0])
+        buildcache_parser.parse_args(MockCommand.call_args[1])
+        mirror_parser.parse_args(MockCommand.call_args[2])