Skip to content

spack uninstall can uninstall specs with multiple roots#11977

Merged
scheibelp merged 4 commits intospack:developfrom
alalazo:fixes/uninstall_broken
Jul 15, 2019
Merged

spack uninstall can uninstall specs with multiple roots#11977
scheibelp merged 4 commits intospack:developfrom
alalazo:fixes/uninstall_broken

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Jul 10, 2019

fixes #3690
fixes #5637

Uninstalling dependents of a spec was relying on a traversal of the parents done by inspecting spec._dependents. This is in turn a DependencyMap that maps a package name to a single DependencySpec object (an edge in the DAG) and cannot thus model the case where a spec has multiple configurations of the same parent package installed.

This commit works around this issue by constructing the list of specs to be uninstalled in an alternative way, and adds tests to verify the behavior. The core issue with DependencyMap is not resolved here.

@alalazo alalazo added bug Something isn't working specs pkg-database labels Jul 10, 2019
@alalazo alalazo requested review from adamjstewart and tgamblin July 10, 2019 15:21
Copy link
Copy Markdown
Member

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR works like a charm! I successfully uninstalled all 28 Python installations that Spack decided I needed with a single call to uninstall. I noticed an unrelated uninstall bug that I'll report in a separate issue.

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Jul 10, 2019

Trying to uninstall on a docker image:

$ spack install hpx build_type=Release cxxstd=14 instrumentation=valgrind ^[email protected] [email protected]

without c6f0f20:

$ time spack uninstall -aRy
...

real	0m15.976s
user	0m14.538s
sys	0m1.370s

with c6f0f20:

$ time spack uninstall -aRy
...

real	0m9.252s
user	0m7.334s
sys	0m1.514s

For smaller tests involving only one or two simple specs, the gain seems to be only around 10%.

@adamjstewart
Copy link
Copy Markdown
Member

I did notice things were rather slow when trying to uninstall all hundreds of packages in my installation. Hopefully that will speed things up.

@alalazo alalazo requested a review from healther July 11, 2019 07:21
Copy link
Copy Markdown
Member

@scheibelp scheibelp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a couple requests.

alalazo added 4 commits July 13, 2019 12:59
fixes spack#3690

Uninstalling dependents of a spec was relying on a traversal of the
parents done by inspecting spec._dependents. This is in turn a
DependencyMap that maps a package name to a single DependencySpec object
(an edge in the DAG) and cannot thus model the case where a spec has
multiple configurations of the same parent package installed.

This commit works around this issue by constructing the list of specs to
be uninstalled in an alternative way, and adds tests to verify the
behavior. The core issue with DependencyMap is not resolved here.
This approach seems to uninstall specs considerably faster, in
particular for large number of specs.
@alalazo alalazo force-pushed the fixes/uninstall_broken branch from 53fb82f to b0a4dc4 Compare July 13, 2019 11:01
@scheibelp scheibelp merged commit 5acbe44 into spack:develop Jul 15, 2019
@scheibelp
Copy link
Copy Markdown
Member

Thanks!

@alalazo alalazo deleted the fixes/uninstall_broken branch July 15, 2019 17:35
dev-zero pushed a commit to dev-zero/spack that referenced this pull request Aug 13, 2019
Fixes spack#3690
Fixes spack#5637

Uninstalling dependents of a spec was relying on a traversal of the
parents done by inspecting spec._dependents. This is in turn a
DependencyMap that maps a package name to a single DependencySpec object
(an edge in the DAG) and cannot thus model the case where a spec has
multiple configurations of the same parent package installed (for
example if different versions of the same Python library depend on
the same Python installation).

This commit works around this issue by constructing the list of specs to
be uninstalled in an alternative way, and adds tests to verify the
behavior. The core issue with DependencyMap is not resolved here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working specs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spack uninstall --dependents fails when multiple dependents exist spack uninstall --dependents broken

3 participants