summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Scheibel <scheibel1@llnl.gov>2022-11-07 19:24:51 -0800
committerGitHub <noreply@github.com>2022-11-08 03:24:51 +0000
commit1a3415619ed8e250cf473efdfda1f0e2119d9c3d (patch)
tree0119a607598dcf2042c66abbb50ecc7c295fb2f5
parent84a3d32aa36f8af902ef67a58ce1944050bd7e1d (diff)
downloadspack-1a3415619ed8e250cf473efdfda1f0e2119d9c3d.tar.gz
spack-1a3415619ed8e250cf473efdfda1f0e2119d9c3d.tar.bz2
spack-1a3415619ed8e250cf473efdfda1f0e2119d9c3d.tar.xz
spack-1a3415619ed8e250cf473efdfda1f0e2119d9c3d.zip
"spack uninstall": don't modify env (#33711)
"spack install foo" no longer adds package "foo" to the environment (i.e. to the list of root specs) by default: you must specify "--add". Likewise "spack uninstall foo" no longer removes package "foo" from the environment: you must specify --remove. Generally this means that install/uninstall commands will no longer modify the users list of root specs (which many users found problematic: they had to deactivate an environment if they wanted to uninstall a spec without changing their spack.yaml description). In more detail: if you have environments e1 and e2, and specs [P, Q, R] such that P depends on R, Q depends on R, [P, R] are in e1, and [Q, R] are in e2: * `spack uninstall --dependents --remove r` in e1: removes R from e1 (but does not uninstall it) and uninstalls (and removes) P * `spack uninstall -f --dependents r` in e1: will uninstall P, Q, and R (i.e. e2 will have dependent specs uninstalled as a side effect) * `spack uninstall -f --dependents --remove r` in e1: this uninstalls P, Q, and R, and removes [P, R] from e1 * `spack uninstall -f --remove r` in e1: uninstalls R (so it is "missing" in both environments) and removes R from e1 (note that e1 would still install R as a dependency of P, but it would no longer be listed as a root spec) * `spack uninstall --dependents r` in e1: will fail because e2 needs R Individual unit tests were created for each of these scenarios.
-rw-r--r--lib/spack/spack/cmd/ci.py1
-rw-r--r--lib/spack/spack/cmd/install.py37
-rw-r--r--lib/spack/spack/cmd/uninstall.py182
-rw-r--r--lib/spack/spack/test/cmd/ci.py5
-rw-r--r--lib/spack/spack/test/cmd/env.py46
-rw-r--r--lib/spack/spack/test/cmd/find.py2
-rw-r--r--lib/spack/spack/test/cmd/install.py38
-rw-r--r--lib/spack/spack/test/cmd/uninstall.py187
-rw-r--r--lib/spack/spack/test/modules/lmod.py2
-rwxr-xr-xshare/spack/spack-completion.bash4
10 files changed, 395 insertions, 109 deletions
diff --git a/lib/spack/spack/cmd/ci.py b/lib/spack/spack/cmd/ci.py
index 11a47d5016..6f29da1f4d 100644
--- a/lib/spack/spack/cmd/ci.py
+++ b/lib/spack/spack/cmd/ci.py
@@ -531,7 +531,6 @@ def ci_rebuild(args):
slash_hash = "/{}".format(job_spec.dag_hash())
deps_install_args = install_args
root_install_args = install_args + [
- "--no-add",
"--keep-stage",
"--only=package",
"--use-buildcache=package:never,dependencies:only",
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index 640b48a6d0..3f9a948a23 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -193,14 +193,22 @@ which is useful for CI pipeline troubleshooting""",
default=False,
help="(with environment) only install already concretized specs",
)
- subparser.add_argument(
- "--no-add",
+
+ updateenv_group = subparser.add_mutually_exclusive_group()
+ updateenv_group.add_argument(
+ "--add",
action="store_true",
default=False,
- help="""(with environment) partially install an environment, limiting
-to concrete specs in the environment matching the arguments.
-Non-roots remain installed implicitly.""",
+ help="""(with environment) add spec to the environment as a root.""",
)
+ updateenv_group.add_argument(
+ "--no-add",
+ action="store_false",
+ dest="add",
+ help="""(with environment) do not add spec to the environment as a
+root (the default behavior).""",
+ )
+
subparser.add_argument(
"-f",
"--file",
@@ -289,11 +297,12 @@ def install_specs_inside_environment(specs, install_kwargs, cli_args):
# the matches. Getting to this point means there were either
# no matches or exactly one match.
- if not m_spec and cli_args.no_add:
+ if not m_spec and not cli_args.add:
msg = (
- "You asked to install {0} without adding it (--no-add), but no such spec "
- "exists in environment"
- ).format(abstract.name)
+ "Cannot install '{0}' because it is not in the current environment."
+ " You can add it to the environment with 'spack add {0}', or as part"
+ " of the install command with 'spack install --add {0}'"
+ ).format(str(abstract))
tty.die(msg)
if not m_spec:
@@ -303,14 +312,16 @@ def install_specs_inside_environment(specs, install_kwargs, cli_args):
tty.debug("exactly one match for {0} in env -> {1}".format(m_spec.name, m_spec.dag_hash()))
- if m_spec in env.roots() or cli_args.no_add:
- # either the single match is a root spec (and --no-add is
- # the default for roots) or --no-add was stated explicitly
+ if m_spec in env.roots() or not cli_args.add:
+ # either the single match is a root spec (in which case
+ # the spec is not added to the env again), or the user did
+ # not specify --add (in which case it is assumed we are
+ # installing already-concretized specs in the env)
tty.debug("just install {0}".format(m_spec.name))
specs_to_install.append(m_spec)
else:
# the single match is not a root (i.e. it's a dependency),
- # and --no-add was not specified, so we'll add it as a
+ # and --add was specified, so we'll add it as a
# root before installing
tty.debug("add {0} then install it".format(m_spec.name))
specs_to_add.append((abstract, concrete))
diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py
index af85b64188..8c112840fd 100644
--- a/lib/spack/spack/cmd/uninstall.py
+++ b/lib/spack/spack/cmd/uninstall.py
@@ -5,7 +5,6 @@
from __future__ import print_function
-import itertools
import sys
from llnl.util import tty
@@ -61,6 +60,13 @@ def setup_parser(subparser):
dest="force",
help="remove regardless of whether other packages or environments " "depend on this one",
)
+ subparser.add_argument(
+ "--remove",
+ action="store_true",
+ dest="remove",
+ help="if in an environment, then the spec should also be removed from "
+ "the environment description",
+ )
arguments.add_common_arguments(
subparser, ["recurse_dependents", "yes_to_all", "installed_specs"]
)
@@ -134,13 +140,21 @@ def installed_dependents(specs, env):
env (spack.environment.Environment or None): the active environment, or None
Returns:
- tuple: two mappings: one from specs to their dependent environments in the
- active environment (or global scope if there is no environment), and one from
- specs to their dependents in *inactive* environments (empty if there is no
- environment
+ tuple: two mappings: one from specs to their dependent installs in the
+ active environment, and one from specs to dependent installs outside of
+ the active environment.
+
+ Any of the input specs may appear in both mappings (if there are
+ dependents both inside and outside the current environment).
+
+ If a dependent spec is used both by the active environment and by
+ an inactive environment, it will only appear in the first mapping.
+
+ If there is not current active environment, the first mapping will be
+ empty.
"""
active_dpts = {}
- inactive_dpts = {}
+ outside_dpts = {}
env_hashes = set(env.all_hashes()) if env else set()
@@ -153,12 +167,12 @@ def installed_dependents(specs, env):
# dpts that are outside this environment
for dpt in installed:
if dpt not in specs:
- if not env or dpt.dag_hash() in env_hashes:
+ if dpt.dag_hash() in env_hashes:
active_dpts.setdefault(spec, set()).add(dpt)
else:
- inactive_dpts.setdefault(spec, set()).add(dpt)
+ outside_dpts.setdefault(spec, set()).add(dpt)
- return active_dpts, inactive_dpts
+ return active_dpts, outside_dpts
def dependent_environments(specs):
@@ -262,31 +276,65 @@ def do_uninstall(env, specs, force):
def get_uninstall_list(args, specs, env):
- # Gets the list of installed specs that match the ones give via cli
- # args.all takes care of the case where '-a' is given in the cli
- uninstall_list = find_matching_specs(env, specs, args.all, args.force, args.origin)
+ """Returns uninstall_list and remove_list: these may overlap (some things
+ may be both uninstalled and removed from the current environment).
+
+ It is assumed we are in an environment if --remove is specified (this
+ method raises an exception otherwise).
- # Takes care of '-R'
- active_dpts, inactive_dpts = installed_dependents(uninstall_list, env)
+ uninstall_list is topologically sorted: dependents come before
+ dependencies (so if a user uninstalls specs in the order provided,
+ the dependents will always be uninstalled first).
+ """
+ if args.remove and not env:
+ raise ValueError("Can only use --remove when in an environment")
- # if we are in the global scope, we complain if you try to remove a
- # spec that's in an environment. If we're in an environment, we'll
- # just *remove* it from the environment, so we ignore this
- # error when *in* an environment
- spec_envs = dependent_environments(uninstall_list)
- spec_envs = inactive_dependent_environments(spec_envs)
+ # Gets the list of installed specs that match the ones given via cli
+ # args.all takes care of the case where '-a' is given in the cli
+ base_uninstall_specs = set(find_matching_specs(env, specs, args.all, args.force))
+
+ active_dpts, outside_dpts = installed_dependents(base_uninstall_specs, env)
+ # It will be useful to track the unified set of specs with dependents, as
+ # well as to separately track specs in the current env with dependents
+ spec_to_dpts = {}
+ for spec, dpts in active_dpts.items():
+ spec_to_dpts[spec] = list(dpts)
+ for spec, dpts in outside_dpts.items():
+ if spec in spec_to_dpts:
+ spec_to_dpts[spec].extend(dpts)
+ else:
+ spec_to_dpts[spec] = list(dpts)
+
+ all_uninstall_specs = set(base_uninstall_specs)
+ if args.dependents:
+ for spec, lst in active_dpts.items():
+ all_uninstall_specs.update(lst)
+ for spec, lst in outside_dpts.items():
+ all_uninstall_specs.update(lst)
+
+ # For each spec that we intend to uninstall, this tracks the set of
+ # environments outside the current active environment which depend on the
+ # spec. There may be environments not managed directly with Spack: such
+ # environments would not be included here.
+ spec_to_other_envs = inactive_dependent_environments(
+ dependent_environments(all_uninstall_specs)
+ )
- # Process spec_dependents and update uninstall_list
has_error = not args.force and (
- (active_dpts and not args.dependents) # dependents in the current env
- or (not env and spec_envs) # there are environments that need specs
+ # There are dependents in the current env and we didn't ask to remove
+ # dependents
+ (spec_to_dpts and not args.dependents)
+ # An environment different than the current env (if any) depends on
+ # one or more of the specs to be uninstalled. There may also be
+ # packages in those envs which depend on the base set of packages
+ # to uninstall, but this covers that scenario.
+ or (not args.remove and spec_to_other_envs)
)
- # say why each problem spec is needed
if has_error:
- specs = set(active_dpts)
- if not env:
- specs.update(set(spec_envs)) # environments depend on this
+ # say why each problem spec is needed
+ specs = set(spec_to_dpts)
+ specs.update(set(spec_to_other_envs)) # environments depend on this
for i, spec in enumerate(sorted(specs)):
# space out blocks of reasons
@@ -296,65 +344,85 @@ def get_uninstall_list(args, specs, env):
spec_format = "{name}{@version}{%compiler}{/hash:7}"
tty.info("Will not uninstall %s" % spec.cformat(spec_format), format="*r")
- dependents = active_dpts.get(spec)
- if dependents:
+ dependents = spec_to_dpts.get(spec)
+ if dependents and not args.dependents:
print("The following packages depend on it:")
spack.cmd.display_specs(dependents, **display_args)
- if not env:
- envs = spec_envs.get(spec)
- if envs:
- print("It is used by the following environments:")
- colify([e.name for e in envs], indent=4)
+ envs = spec_to_other_envs.get(spec)
+ if envs:
+ if env:
+ env_context_qualifier = " other"
+ else:
+ env_context_qualifier = ""
+ print("It is used by the following{0} environments:".format(env_context_qualifier))
+ colify([e.name for e in envs], indent=4)
msgs = []
- if active_dpts:
+ if spec_to_dpts and not args.dependents:
msgs.append("use `spack uninstall --dependents` to remove dependents too")
- if spec_envs:
+ if spec_to_other_envs:
msgs.append("use `spack env remove` to remove from environments")
print()
tty.die("There are still dependents.", *msgs)
- elif args.dependents:
- for spec, lst in active_dpts.items():
- uninstall_list.extend(lst)
- uninstall_list = list(set(uninstall_list))
-
- # only force-remove (don't completely uninstall) specs that still
- # have external dependent envs or pkgs
- removes = set(inactive_dpts)
- if env:
- removes.update(spec_envs)
+ # If we are in an environment, this will track specs in this environment
+ # which should only be removed from the environment rather than uninstalled
+ remove_only = set()
+ if args.remove and not args.force:
+ remove_only.update(spec_to_other_envs)
+ if remove_only:
+ tty.info(
+ "The following specs will be removed but not uninstalled because"
+ " they are also used by another environment: {speclist}".format(
+ speclist=", ".join(x.name for x in remove_only)
+ )
+ )
- # remove anything in removes from the uninstall list
- uninstall_list = set(uninstall_list) - removes
+ # Compute the set of specs that should be removed from the current env.
+ # This may overlap (some specs may be uninstalled and also removed from
+ # the current environment).
+ if args.remove:
+ remove_specs = set(base_uninstall_specs)
+ if args.dependents:
+ # Any spec matched from the cli, or dependent of, should be removed
+ # from the environment
+ for spec, lst in active_dpts.items():
+ remove_specs.update(lst)
+ else:
+ remove_specs = set()
+
+ all_uninstall_specs -= remove_only
+ # Inefficient topological sort: uninstall dependents before dependencies
+ all_uninstall_specs = sorted(
+ all_uninstall_specs, key=lambda x: sum(1 for i in x.traverse()), reverse=True
+ )
- return uninstall_list, removes
+ return list(all_uninstall_specs), list(remove_specs)
def uninstall_specs(args, specs):
env = ev.active_environment()
uninstall_list, remove_list = get_uninstall_list(args, specs, env)
- anything_to_do = set(uninstall_list).union(set(remove_list))
- if not anything_to_do:
+ if not uninstall_list:
tty.warn("There are no package to uninstall.")
return
if not args.yes_to_all:
- confirm_removal(anything_to_do)
+ confirm_removal(uninstall_list)
+
+ # Uninstall everything on the list
+ do_uninstall(env, uninstall_list, args.force)
if env:
- # Remove all the specs that are supposed to be uninstalled or just
- # removed.
with env.write_transaction():
- for spec in itertools.chain(remove_list, uninstall_list):
+ for spec in remove_list:
_remove_from_env(spec, env)
env.write()
- # Uninstall everything on the list
- do_uninstall(env, uninstall_list, args.force)
+ env.regenerate_views()
def confirm_removal(specs):
diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py
index b2e84e0c02..73b474c7df 100644
--- a/lib/spack/spack/test/cmd/ci.py
+++ b/lib/spack/spack/test/cmd/ci.py
@@ -1011,7 +1011,6 @@ def test_ci_rebuild(
assert "--keep-stage" in install_parts
assert "--no-check-signature" not in install_parts
- assert "--no-add" in install_parts
assert "-f" in install_parts
flag_index = install_parts.index("-f")
assert "archive-files.json" in install_parts[flag_index + 1]
@@ -1261,7 +1260,7 @@ spack:
with open(json_path, "w") as ypfd:
ypfd.write(spec_json)
- install_cmd("--keep-stage", json_path)
+ install_cmd("--add", "--keep-stage", json_path)
# env, spec, json_path, mirror_url, build_id, sign_binaries
ci.push_mirror_contents(env, json_path, mirror_url, True)
@@ -1623,7 +1622,7 @@ spack:
with open(json_path, "w") as ypfd:
ypfd.write(spec_json)
- install_cmd("--keep-stage", "-f", json_path)
+ install_cmd("--add", "--keep-stage", "-f", json_path)
buildcache_cmd("create", "-u", "-a", "-f", "--mirror-url", mirror_url, "callpath")
ci_cmd("rebuild-index")
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 801ff04669..6782e65179 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -221,7 +221,7 @@ def test_env_install_single_spec(install_mockery, mock_fetch):
e = ev.read("test")
with e:
- install("cmake-client")
+ install("--add", "cmake-client")
e = ev.read("test")
assert e.user_specs[0].name == "cmake-client"
@@ -256,7 +256,7 @@ def test_env_modifications_error_on_activate(install_mockery, mock_fetch, monkey
e = ev.read("test")
with e:
- install("cmake-client")
+ install("--add", "cmake-client")
def setup_error(pkg, env):
raise RuntimeError("cmake-client had issues!")
@@ -277,7 +277,7 @@ def test_activate_adds_transitive_run_deps_to_path(install_mockery, mock_fetch,
e = ev.read("test")
with e:
- install("depends-on-run-env")
+ install("--add", "depends-on-run-env")
env_variables = {}
spack.environment.shell.activate(e).apply_modifications(env_variables)
@@ -290,7 +290,7 @@ def test_env_install_same_spec_twice(install_mockery, mock_fetch):
e = ev.read("test")
with e:
# The first installation outputs the package prefix, updates the view
- out = install("cmake-client")
+ out = install("--add", "cmake-client")
assert "Updating view at" in out
# The second installation reports all packages already installed
@@ -449,7 +449,7 @@ def test_env_status_broken_view(
):
env_dir = str(tmpdir)
with ev.Environment(env_dir):
- install("trivial-install-test-package")
+ install("--add", "trivial-install-test-package")
# switch to a new repo that doesn't include the installed package
# test that Spack detects the missing package and warns the user
@@ -468,7 +468,7 @@ def test_env_activate_broken_view(
mutable_mock_env_path, mock_archive, mock_fetch, mock_custom_repository, install_mockery
):
with ev.create("test"):
- install("trivial-install-test-package")
+ install("--add", "trivial-install-test-package")
# switch to a new repo that doesn't include the installed package
# test that Spack detects the missing package and fails gracefully
@@ -1057,7 +1057,9 @@ def test_roots_display_with_variants():
assert "boost +shared" in out
-def test_uninstall_removes_from_env(mock_stage, mock_fetch, install_mockery):
+def test_uninstall_keeps_in_env(mock_stage, mock_fetch, install_mockery):
+ # 'spack uninstall' without --remove should not change the environment
+ # spack.yaml file, just uninstall specs
env("create", "test")
with ev.read("test"):
add("mpileaks")
@@ -1065,13 +1067,33 @@ def test_uninstall_removes_from_env(mock_stage, mock_fetch, install_mockery):
install("--fake")
test = ev.read("test")
- assert any(s.name == "mpileaks" for s in test.specs_by_hash.values())
- assert any(s.name == "libelf" for s in test.specs_by_hash.values())
+ # Save this spec to check later if it is still in the env
+ (mpileaks_hash,) = list(x for x, y in test.specs_by_hash.items() if y.name == "mpileaks")
+ orig_user_specs = test.user_specs
+ orig_concretized_specs = test.concretized_order
with ev.read("test"):
uninstall("-ya")
test = ev.read("test")
+ assert test.concretized_order == orig_concretized_specs
+ assert test.user_specs.specs == orig_user_specs.specs
+ assert mpileaks_hash in test.specs_by_hash
+ assert not test.specs_by_hash[mpileaks_hash].package.installed
+
+
+def test_uninstall_removes_from_env(mock_stage, mock_fetch, install_mockery):
+ # 'spack uninstall --remove' should update the environment
+ env("create", "test")
+ with ev.read("test"):
+ add("mpileaks")
+ add("libelf")
+ install("--fake")
+
+ with ev.read("test"):
+ uninstall("-y", "-a", "--remove")
+
+ test = ev.read("test")
assert not test.specs_by_hash
assert not test.concretized_order
assert not test.user_specs
@@ -1256,7 +1278,7 @@ def test_env_updates_view_install_package(tmpdir, mock_stage, mock_fetch, instal
view_dir = tmpdir.join("view")
env("create", "--with-view=%s" % view_dir, "test")
with ev.read("test"):
- install("--fake", "mpileaks")
+ install("--fake", "--add", "mpileaks")
assert os.path.exists(str(view_dir.join(".spack/mpileaks")))
@@ -1276,7 +1298,7 @@ def test_env_updates_view_uninstall(tmpdir, mock_stage, mock_fetch, install_mock
view_dir = tmpdir.join("view")
env("create", "--with-view=%s" % view_dir, "test")
with ev.read("test"):
- install("--fake", "mpileaks")
+ install("--fake", "--add", "mpileaks")
check_mpileaks_and_deps_in_view(view_dir)
@@ -1325,7 +1347,7 @@ def test_env_updates_view_force_remove(tmpdir, mock_stage, mock_fetch, install_m
view_dir = tmpdir.join("view")
env("create", "--with-view=%s" % view_dir, "test")
with ev.read("test"):
- install("--fake", "mpileaks")
+ install("--add", "--fake", "mpileaks")
check_mpileaks_and_deps_in_view(view_dir)
diff --git a/lib/spack/spack/test/cmd/find.py b/lib/spack/spack/test/cmd/find.py
index be83541096..9f3f453bad 100644
--- a/lib/spack/spack/test/cmd/find.py
+++ b/lib/spack/spack/test/cmd/find.py
@@ -356,7 +356,7 @@ def test_find_prefix_in_env(
"""Test `find` formats requiring concrete specs work in environments."""
env("create", "test")
with ev.read("test"):
- install("mpileaks")
+ install("--add", "mpileaks")
find("-p")
find("-l")
find("-L")
diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py
index 843e539eea..67e0dd3f80 100644
--- a/lib/spack/spack/test/cmd/install.py
+++ b/lib/spack/spack/test/cmd/install.py
@@ -771,7 +771,7 @@ def test_install_only_dependencies_in_env(
dep = Spec("dependency-install").concretized()
root = Spec("dependent-install").concretized()
- install("-v", "--only", "dependencies", "dependent-install")
+ install("-v", "--only", "dependencies", "--add", "dependent-install")
assert os.path.exists(dep.prefix)
assert not os.path.exists(root.prefix)
@@ -800,7 +800,7 @@ def test_install_only_dependencies_of_all_in_env(
def test_install_no_add_in_env(tmpdir, mock_fetch, install_mockery, mutable_mock_env_path):
- # To test behavior of --no-add option, we create the following environment:
+ # To test behavior of --add option, we create the following environment:
#
# mpileaks
# ^callpath
@@ -849,18 +849,19 @@ def test_install_no_add_in_env(tmpdir, mock_fetch, install_mockery, mutable_mock
# Assert using --no-add with a spec not in the env fails
inst_out = install("--no-add", "boost", fail_on_error=False, output=str)
- assert "no such spec exists in environment" in inst_out
+ assert "You can add it to the environment with 'spack add " in inst_out
- # Ensure using --no-add with an ambiguous spec fails
+ # Without --add, ensure that install fails if the spec matches more
+ # than one root
with pytest.raises(ev.SpackEnvironmentError) as err:
- inst_out = install("--no-add", "a", output=str)
+ inst_out = install("a", output=str)
assert "a matches multiple specs in the env" in str(err)
- # With "--no-add", install an unambiguous dependency spec (that already
- # exists as a dep in the environment) using --no-add and make sure it
- # gets installed (w/ deps), but is not added to the environment.
- install("--no-add", "dyninst")
+ # Install an unambiguous dependency spec (that already exists as a dep
+ # in the environment) and make sure it gets installed (w/ deps),
+ # but is not added to the environment.
+ install("dyninst")
find_output = find("-l", output=str)
assert "dyninst" in find_output
@@ -872,31 +873,30 @@ def test_install_no_add_in_env(tmpdir, mock_fetch, install_mockery, mutable_mock
assert all([s in env_specs for s in post_install_specs])
# Make sure we can install a concrete dependency spec from a spec.json
- # file on disk, using the ``--no-add` option, and the spec is installed
- # but not added as a root
+ # file on disk, and the spec is installed but not added as a root
mpi_spec_json_path = tmpdir.join("{0}.json".format(mpi_spec.name))
with open(mpi_spec_json_path.strpath, "w") as fd:
fd.write(mpi_spec.to_json(hash=ht.dag_hash))
- install("--no-add", "-f", mpi_spec_json_path.strpath)
+ install("-f", mpi_spec_json_path.strpath)
assert mpi_spec not in e.roots()
find_output = find("-l", output=str)
assert mpi_spec.name in find_output
- # Without "--no-add", install an unambiguous depependency spec (that
- # already exists as a dep in the environment) without --no-add and make
- # sure it is added as a root of the environment as well as installed.
+ # Install an unambiguous depependency spec (that already exists as a
+ # dep in the environment) with --add and make sure it is added as a
+ # root of the environment as well as installed.
assert b_spec not in e.roots()
- install("b")
+ install("--add", "b")
assert b_spec in e.roots()
assert b_spec not in e.uninstalled_specs()
- # Without "--no-add", install a novel spec and make sure it is added
- # as a root and installed.
- install("bowtie")
+ # Install a novel spec with --add and make sure it is added as a root
+ # and installed.
+ install("--add", "bowtie")
assert any([s.name == "bowtie" for s in e.roots()])
assert not any([s.name == "bowtie" for s in e.uninstalled_specs()])
diff --git a/lib/spack/spack/test/cmd/uninstall.py b/lib/spack/spack/test/cmd/uninstall.py
index a47f6c60a5..ddbd54e252 100644
--- a/lib/spack/spack/test/cmd/uninstall.py
+++ b/lib/spack/spack/test/cmd/uninstall.py
@@ -3,10 +3,13 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import sys
+
import pytest
import llnl.util.tty as tty
+import spack.environment
import spack.store
from spack.main import SpackCommand, SpackCommandError
@@ -166,3 +169,187 @@ def test_in_memory_consistency_when_uninstalling(mutable_database, monkeypatch):
monkeypatch.setattr(tty, "warn", _warn)
# Now try to uninstall and check this doesn't trigger warnings
uninstall("-y", "-a")
+
+
+# Note: I want to use https://docs.pytest.org/en/7.1.x/how-to/skipping.html#skip-all-test-functions-of-a-class-or-module
+# the style formatter insists on separating these two lines.
+pytest.mark.skipif(sys.platform == "win32", reason="Envs unsupported on Windows")
+
+
+class TestUninstallFromEnv(object):
+ """Tests an installation with two environments e1 and e2, which each have
+ shared package installations:
+
+ e1 has dt-diamond-left -> dt-diamond-bottom
+
+ e2 has dt-diamond-right -> dt-diamond-bottom
+ """
+
+ env = SpackCommand("env")
+ add = SpackCommand("add")
+ concretize = SpackCommand("concretize")
+ find = SpackCommand("find")
+
+ @pytest.fixture
+ def environment_setup(
+ self, mutable_mock_env_path, config, mock_packages, mutable_database, install_mockery
+ ):
+ TestUninstallFromEnv.env("create", "e1")
+ e1 = spack.environment.read("e1")
+ with e1:
+ TestUninstallFromEnv.add("dt-diamond-left")
+ TestUninstallFromEnv.add("dt-diamond-bottom")
+ TestUninstallFromEnv.concretize()
+ install("--fake")
+
+ TestUninstallFromEnv.env("create", "e2")
+ e2 = spack.environment.read("e2")
+ with e2:
+ TestUninstallFromEnv.add("dt-diamond-right")
+ TestUninstallFromEnv.add("dt-diamond-bottom")
+ TestUninstallFromEnv.concretize()
+ install("--fake")
+
+ def test_basic_env_sanity(self, environment_setup):
+ for env_name in ["e1", "e2"]:
+ e = spack.environment.read(env_name)
+ with e:
+ for _, concretized_spec in e.concretized_specs():
+ assert concretized_spec.package.installed
+
+ def test_uninstall_force_dependency_shared_between_envs(self, environment_setup):
+ """If you "spack uninstall -f --dependents dt-diamond-bottom" from
+ e1, then all packages should be uninstalled (but not removed) from
+ both e1 and e2.
+ """
+ e1 = spack.environment.read("e1")
+ with e1:
+ uninstall("-f", "-y", "--dependents", "dt-diamond-bottom")
+
+ # The specs should still be in the environment, since
+ # --remove was not specified
+ assert set(root.name for (root, _) in e1.concretized_specs()) == set(
+ ["dt-diamond-left", "dt-diamond-bottom"]
+ )
+
+ for _, concretized_spec in e1.concretized_specs():
+ assert not concretized_spec.package.installed
+
+ # Everything in e2 depended on dt-diamond-bottom, so should also
+ # have been uninstalled. The roots should be unchanged though.
+ e2 = spack.environment.read("e2")
+ with e2:
+ assert set(root.name for (root, _) in e2.concretized_specs()) == set(
+ ["dt-diamond-right", "dt-diamond-bottom"]
+ )
+ for _, concretized_spec in e2.concretized_specs():
+ assert not concretized_spec.package.installed
+
+ def test_uninstall_remove_dependency_shared_between_envs(self, environment_setup):
+ """If you "spack uninstall --dependents --remove dt-diamond-bottom" from
+ e1, then all packages are removed from e1 (it is now empty);
+ dt-diamond-left is also uninstalled (since only e1 needs it) but
+ dt-diamond-bottom is not uninstalled (since e2 needs it).
+ """
+ e1 = spack.environment.read("e1")
+ with e1:
+ dtdiamondleft = next(
+ concrete
+ for (_, concrete) in e1.concretized_specs()
+ if concrete.name == "dt-diamond-left"
+ )
+ output = uninstall("-y", "--dependents", "--remove", "dt-diamond-bottom")
+ assert "The following specs will be removed but not uninstalled" in output
+ assert not list(e1.roots())
+ assert not dtdiamondleft.package.installed
+
+ # Since -f was not specified, all specs in e2 should still be installed
+ # (and e2 should be unchanged)
+ e2 = spack.environment.read("e2")
+ with e2:
+ assert set(root.name for (root, _) in e2.concretized_specs()) == set(
+ ["dt-diamond-right", "dt-diamond-bottom"]
+ )
+ for _, concretized_spec in e2.concretized_specs():
+ assert concretized_spec.package.installed
+
+ def test_uninstall_dependency_shared_between_envs_fail(self, environment_setup):
+ """If you "spack uninstall --dependents dt-diamond-bottom" from
+ e1 (without --remove or -f), then this should fail (this is needed by
+ e2).
+ """
+ e1 = spack.environment.read("e1")
+ with e1:
+ output = uninstall("-y", "--dependents", "dt-diamond-bottom", fail_on_error=False)
+ assert "There are still dependents." in output
+ assert "use `spack env remove`" in output
+
+ # The environment should be unchanged and nothing should have been
+ # uninstalled
+ assert set(root.name for (root, _) in e1.concretized_specs()) == set(
+ ["dt-diamond-left", "dt-diamond-bottom"]
+ )
+ for _, concretized_spec in e1.concretized_specs():
+ assert concretized_spec.package.installed
+
+ def test_uninstall_force_and_remove_dependency_shared_between_envs(self, environment_setup):
+ """If you "spack uninstall -f --dependents --remove dt-diamond-bottom" from
+ e1, then all packages should be uninstalled and removed from e1.
+ All packages will also be uninstalled from e2, but the roots will
+ remain unchanged.
+ """
+ e1 = spack.environment.read("e1")
+ with e1:
+ dtdiamondleft = next(
+ concrete
+ for (_, concrete) in e1.concretized_specs()
+ if concrete.name == "dt-diamond-left"
+ )
+ uninstall("-f", "-y", "--dependents", "--remove", "dt-diamond-bottom")
+ assert not list(e1.roots())
+ assert not dtdiamondleft.package.installed
+
+ e2 = spack.environment.read("e2")
+ with e2:
+ assert set(root.name for (root, _) in e2.concretized_specs()) == set(
+ ["dt-diamond-right", "dt-diamond-bottom"]
+ )
+ for _, concretized_spec in e2.concretized_specs():
+ assert not concretized_spec.package.installed
+
+ def test_uninstall_keep_dependents_dependency_shared_between_envs(self, environment_setup):
+ """If you "spack uninstall -f --remove dt-diamond-bottom" from
+ e1, then dt-diamond-bottom should be uninstalled, which leaves
+ "dangling" references in both environments, since
+ dt-diamond-left and dt-diamond-right both need it.
+ """
+ e1 = spack.environment.read("e1")
+ with e1:
+ dtdiamondleft = next(
+ concrete
+ for (_, concrete) in e1.concretized_specs()
+ if concrete.name == "dt-diamond-left"
+ )
+ uninstall("-f", "-y", "--remove", "dt-diamond-bottom")
+ # dt-diamond-bottom was removed from the list of roots (note that
+ # it would still be installed since dt-diamond-left depends on it)
+ assert set(x.name for x in e1.roots()) == set(["dt-diamond-left"])
+ assert dtdiamondleft.package.installed
+
+ e2 = spack.environment.read("e2")
+ with e2:
+ assert set(root.name for (root, _) in e2.concretized_specs()) == set(
+ ["dt-diamond-right", "dt-diamond-bottom"]
+ )
+ dtdiamondright = next(
+ concrete
+ for (_, concrete) in e2.concretized_specs()
+ if concrete.name == "dt-diamond-right"
+ )
+ assert dtdiamondright.package.installed
+ dtdiamondbottom = next(
+ concrete
+ for (_, concrete) in e2.concretized_specs()
+ if concrete.name == "dt-diamond-bottom"
+ )
+ assert not dtdiamondbottom.package.installed
diff --git a/lib/spack/spack/test/modules/lmod.py b/lib/spack/spack/test/modules/lmod.py
index d24ec8ca0d..5d0d04032e 100644
--- a/lib/spack/spack/test/modules/lmod.py
+++ b/lib/spack/spack/test/modules/lmod.py
@@ -298,7 +298,7 @@ class TestLmod(object):
):
with ev.Environment(str(tmpdir), with_view=True) as e:
module_configuration("with_view")
- install("cmake")
+ install("--add", "cmake")
spec = spack.spec.Spec("cmake").concretized()
diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash
index f0588e5368..2461a0b7c7 100755
--- a/share/spack/spack-completion.bash
+++ b/share/spack/spack-completion.bash
@@ -1203,7 +1203,7 @@ _spack_info() {
_spack_install() {
if $list_options
then
- SPACK_COMPREPLY="-h --help --only -u --until -j --jobs --overwrite --fail-fast --keep-prefix --keep-stage --dont-restage --use-cache --no-cache --cache-only --use-buildcache --include-build-deps --no-check-signature --show-log-on-error --source -n --no-checksum --deprecated -v --verbose --fake --only-concrete --no-add -f --file --clean --dirty --test --log-format --log-file --help-cdash --cdash-upload-url --cdash-build --cdash-site --cdash-track --cdash-buildstamp -y --yes-to-all -U --fresh --reuse"
+ SPACK_COMPREPLY="-h --help --only -u --until -j --jobs --overwrite --fail-fast --keep-prefix --keep-stage --dont-restage --use-cache --no-cache --cache-only --use-buildcache --include-build-deps --no-check-signature --show-log-on-error --source -n --no-checksum --deprecated -v --verbose --fake --only-concrete --add --no-add -f --file --clean --dirty --test --log-format --log-file --help-cdash --cdash-upload-url --cdash-build --cdash-site --cdash-track --cdash-buildstamp -y --yes-to-all -U --fresh --reuse"
else
_all_packages
fi
@@ -1824,7 +1824,7 @@ _spack_undevelop() {
_spack_uninstall() {
if $list_options
then
- SPACK_COMPREPLY="-h --help -f --force -R --dependents -y --yes-to-all -a --all --origin"
+ SPACK_COMPREPLY="-h --help -f --force --remove -R --dependents -y --yes-to-all -a --all --origin"
else
_installed_packages
fi