"spack uninstall": don't modify env#33711
Conversation
spackbot-app
bot
added
commands
core
…ut not uninstalled
…ned off everywhere, so instead I'm explaining why something looks weird
| " they are also used by another environment: {speclist}".format( | ||
| speclist=", ".join(x.name for x in remove_only) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
This is intended to resolve #26766 (comment) - I assume we don't want to record this as an error, since we want to support it.
All other comments from #26766 (review) are addressed without any caveats.
| find = SpackCommand("find") | ||
|
|
||
| @pytest.fixture | ||
| def environment_setup(self, mutable_mock_env_path, config, mock_packages, mutable_database): |
There was a problem hiding this comment.
These should use the install_mockery fixture, then you won't need to specify fake for all the installs
There was a problem hiding this comment.
Your suggestion is necessary as otherwise I will modify the real spack store, but --fake is still needed since the packages I'm using aren't legitimately install-able
scheibelp
changed the title
| with pytest.raises( | ||
| spack.error.SpackError, | ||
| match=r".*Will not uninstall.*There are still dependents.*use `spack env remove`", | ||
| ): |
There was a problem hiding this comment.
@becker33 this is my current attempt at following #33711 (comment) , and something I'm doing is incorrect and I also am having trouble following the logic to see how this output would make it into the exception: I assumed it wouldn't based on reading main.py:SpackCommand.__call__, but as noted in this comment, there are tests which seem to be able to examine output.
I'll note one difference between this and test/cmd/install.py:test_install_invalid_spec is that the error is actually raised as an exception (via parse.py) which triggers:
except BaseException as e:
tty.debug(e)
self.error = e
if fail_on_error:
self._log_command_output(out)
raise
So that's my theory of why tests which check for failures in commands typically run with fail_on_error=False (I originally copied this strategy from looking elsewhere, which is admittedly naive but on further inspection it would seem necessary pending a refactor of SpackCommand).
tgamblin
changed the title
"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.
"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.
3 participants
Supersedes #26766
This is a redo of #26766. I found it problematic to merge (even after applying
spack style --fixto the affected files, the merge listed a bunch of conflicts that seemed more effort than they were worth to resolve compared to "manual" merge).All the desired behaviors from #26766 (comment) apply here. I need to do a more comprehensive examination of the underlying and related logic to know if more-extensive changes are required, but FWIW porting the statements over was straightforward.