Skip to content

Spack can automatically remove unused specs#13534

Merged
tgamblin merged 15 commits intospack:developfrom
alalazo:features/remove_unused_packages
Jan 7, 2020
Merged

Spack can automatically remove unused specs#13534
tgamblin merged 15 commits intospack:developfrom
alalazo:features/remove_unused_packages

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Nov 1, 2019 •
edited
Loading

fixes #4382
fixes #8291

This PR adds a new command called spack autoremove spack gc that removes all the specs that were necessary to build other specs but are currently unused. This includes build dependencies and implicit dependency of specs that have been already uninstalled.

It provides a useful way to keep the environment clean after an upgrade or to minimize the size of installations within e.g. container images.

The name autoremove has been selected to be consistent with what yum and apt use for the same functionality.

@alalazo alalazo added feature A feature is missing in Spack specs user-experience labels Nov 1, 2019
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Nov 1, 2019

Example of use:

$ spack find
==> 36 installed packages
-- linux-ubuntu18.04-broadwell / [email protected] ----------------------
[email protected]    [email protected]      [email protected]          [email protected]  [email protected]              [email protected]           [email protected]
[email protected]  [email protected]       [email protected]        [email protected]    [email protected]             [email protected]             [email protected]
[email protected]    [email protected]     [email protected]             [email protected]    [email protected]  [email protected]           [email protected]
[email protected]      [email protected]  [email protected]        [email protected]        [email protected]          [email protected]
[email protected]    [email protected]       [email protected]  [email protected]        [email protected]          [email protected]

-- linux-ubuntu18.04-x86_64 / [email protected] -------------------------
[email protected]  [email protected]  [email protected]

$ spack autoremove
==> The following packages will be uninstalled:

    -- linux-ubuntu18.04-broadwell / [email protected] ----------------------
    vn47edz [email protected]%gcc 
    ki6nfw5 [email protected]%gcc 
    mrptssh [email protected]%gcc 
    fklde6b [email protected]%gcc 
    ubl6bgk [email protected]%gcc 
    b6pswuo [email protected]%gcc  patches=3877ab548f88597ab2327a2230ee048d2d07ace1062efe81fc92e91b7f39cd00,fc9b61654a3ba1a8d6cd78ce087e7c96366c290bc8d2c299f09828d793b853c8 +sigsegv
    k3s2csy [email protected]%gcc +cpanm+shared+threads
    5vmfbrq [email protected]%gcc 
    urdw22a [email protected]%gcc 
==> Do you want to proceed? [y/N] y
==> Successfully uninstalled [email protected]%[email protected] arch=linux-ubuntu18.04-broadwell/mrptssh
==> Successfully uninstalled [email protected]%[email protected] arch=linux-ubuntu18.04-broadwell/ubl6bgk
==> Successfully uninstalled [email protected]%[email protected] arch=linux-ubuntu18.04-broadwell/5vmfbrq
==> Successfully uninstalled [email protected]%[email protected] arch=linux-ubuntu18.04-broadwell/vn47edz
==> Successfully uninstalled [email protected]%[email protected] arch=linux-ubuntu18.04-broadwell/ki6nfw5
==> Successfully uninstalled [email protected]%[email protected]+cpanm+shared+threads arch=linux-ubuntu18.04-broadwell/k3s2csy
==> Successfully uninstalled [email protected]%[email protected] patches=3877ab548f88597ab2327a2230ee048d2d07ace1062efe81fc92e91b7f39cd00,fc9b61654a3ba1a8d6cd78ce087e7c96366c290bc8d2c299f09828d793b853c8 +sigsegv arch=linux-ubuntu18.04-broadwell/b6pswuo
==> Successfully uninstalled [email protected]%[email protected] arch=linux-ubuntu18.04-broadwell/fklde6b
==> Successfully uninstalled [email protected]%[email protected] arch=linux-ubuntu18.04-broadwell/urdw22a

$ spack find
==> 27 installed packages
-- linux-ubuntu18.04-broadwell / [email protected] ----------------------
[email protected]  [email protected]       [email protected]    [email protected]        [email protected]  [email protected]             [email protected]          [email protected]
[email protected]    [email protected]  [email protected]  [email protected]  [email protected]      [email protected]  [email protected]           [email protected]
[email protected]     [email protected]       [email protected]       [email protected]      [email protected]     [email protected]          [email protected]  [email protected]

-- linux-ubuntu18.04-x86_64 / [email protected] -------------------------
[email protected]  [email protected]  [email protected]

alalazo
alalazo commented Nov 1, 2019
View reviewed changes
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Nov 1, 2019

@nazavode

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Nov 1, 2019

I need to check this PR copes well with environments, as it might not be the case.

@adamjstewart
Copy link
Copy Markdown
Member

adamjstewart commented Nov 1, 2019

Some comments:

  1. I guess I don't see a reason to make a new command for this? spack uninstall --autoremove would suffice. Just trying to prevent a repeat of Too Many Commands, Poorly Organized and of Low Quality #2867, and we've added a lot of new commands lately.
  2. I assume this also removes unused test dependencies?
  3. Now that I'm using Document how to use Spack to replace Homebrew/Conda #13083, I would really love a way to uninstall all packages that aren't in use in my current/only environment. I assume this is different than your PR? Would it be possible to do this?
  4. Don't forget to update the bash completion script 😃

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Nov 1, 2019

I guess I don't see a reason to make a new command for this? spack uninstall --autoremove would suffice. Just trying to prevent a repeat of #2867, and we've added a lot of new commands lately.

I have two reasons, even though I have to say that I don't feel strongly about this:

  1. All the major package managers have a command for doing that, rather than an option
  2. Having a command makes the logic that handles conflicting options simpler

This PR started out like you suggest - see first commit. Factoring out a new command avoided me to check for error conditions among incompatible options of uninstall.

Even for the name of the command I am open to anything. I chose autoremove as it is what yum and apt are using and I thought it might have been familiar to people. @tgamblin was also suggesting spack gc for "garbage collection".

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Nov 1, 2019

I assume this also removes unused test dependencies?

It removes anything with reference count equal to 0 and that was not explicitly installed, and iterates to find the unused specs until nothing new is found:

spack/lib/spack/spack/store.py

Lines 129 to 150 in c296b13

# Get all the implicit specs available and start building the
# list of unused specs
implicit_specs = scratch_db.query_local(explicit=False)
unused = []
while True:
# Try to discover new unused specs
discovered = [s for s in implicit_specs
if scratch_db.get_record(s).ref_count == 0]
if not discovered:
break
unused.extend(discovered)
# Now uninstall the newly discovered specs from the scratch_db and
# iterate again
for s in discovered:
scratch_db.remove(s)
# Update the current list of implicit specs
implicit_specs = scratch_db.query_local(explicit=False)
return unused

If test dependencies are treated like build dependencies then I think the short answer is "yes".

@alalazo alalazo self-assigned this Nov 1, 2019
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Nov 2, 2019 •
edited
Loading

This is ready for a first review + test drive. The 80% coverage comes from the fact that there are functions requiring user interaction that are not covered - I'll try to check if there's any way to fake passing user input but I'm not so confident it exists. If anyone already knows it feel free to post 🙂

@alalazo alalazo force-pushed the features/remove_unused_packages branch from 292a87c to 9d41f83 Compare November 6, 2019 11:51
@alalazo alalazo force-pushed the features/remove_unused_packages branch from 9d41f83 to dd4c97d Compare December 10, 2019 14:54
@alalazo alalazo force-pushed the features/remove_unused_packages branch from dd4c97d to f0864b0 Compare December 19, 2019 09:01
@alalazo

This comment has been minimized.

Sign in to view
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Dec 19, 2019

@tgamblin I tried a few use cases moving in and out of environments. Preserving the root specs when doing garbage collection on the entire store seems highly unintuitive. The new behavior is currently to restrict garbage collection to the active environment, if any:

$ # Outside of an environment
$ spack gc
==> The following packages will be uninstalled:

    -- linux-ubuntu18.04-broadwell / [email protected] ----------------------
    vn47edz [email protected]    6m3f2qn [email protected]  ubl6bgk [email protected]  pksawhz [email protected]  urdw22a [email protected]
    ki6nfw5 [email protected]  fklde6b [email protected]      b6pswuo [email protected]      k3s2csy [email protected]     lp5ya3t [email protected]
    ylvgsov [email protected]     5omotir [email protected]  leuzbbh [email protected]    5vmfbrq [email protected]   5bmv4tg [email protected]

$ # Inside an environment with only cmake and still to be installed
$ spack find
==> In environment /home/culpo/tmp/spack-issues/13534
==> Root specs
cmake 

==> 0 installed packages

$ spack gc
==> Restricting the garbage collection to the "/home/culpo/tmp/spack-issues/13534" environment
==> The following packages will be uninstalled:

    -- linux-ubuntu18.04-broadwell / [email protected] ----------------------
    fklde6b [email protected]  k3s2csy [email protected]  5vmfbrq [email protected]  urdw22a [email protected]

==> Do you want to proceed? [y/N]

@alalazo alalazo mentioned this pull request Dec 27, 2019
Merged
@alalazo alalazo force-pushed the features/remove_unused_packages branch from 3f6d094 to a3724c4 Compare January 2, 2020 09:12
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Jan 2, 2020

@tgamblin Can we prioritize reviewing this so that I can update #14202 templates and static JSON data accordingly once the PR will be merged?

tgamblin
tgamblin requested changes Jan 6, 2020
View reviewed changes
alalazo added 2 commits January 7, 2020 11:04
@alalazo
Spack can uninstall unused specs
08a6429 
fixes spack#4382

Added an option to spack uninstall that removes all unused specs i.e.
build dependencies or transitive dependencies that are left
in the store after the specs that pulled them in have been removed.
@alalazo
Moved the functionality to its own command
77af505 
The command has been named 'spack autoremove' to follow the naming used
for the same functionality by other widely known package managers i.e.
yum and apt.
@alalazo alalazo force-pushed the features/remove_unused_packages branch from a3724c4 to 7aa4cd3 Compare January 7, 2020 10:15
alalazo added 3 commits January 7, 2020 13:36
@alalazo
Updated documentation according to review
f8e9f14 
Rephrased a couple of sentences, added references to
`spack find` and dependency types.
@alalazo
Updated function naming and docstrings
dd205c1
@alalazo
Simplified computation of unused specs
53d8b17 
Since the new approach uses private attributes of the DB
it has been coded as a method of that class rather than a
freestanding function.
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Jan 7, 2020

@tgamblin I think I addressed the comments in the review, let me know if there a more changes needed.

@tgamblin tgamblin merged commit 08d0267 into spack:develop Jan 7, 2020
@alalazo alalazo deleted the features/remove_unused_packages branch January 7, 2020 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgamblin tgamblin tgamblin approved these changes

@scheibelp scheibelp Awaiting requested review from scheibelp

@adamjstewart adamjstewart Awaiting requested review from adamjstewart

@becker33 becker33 Awaiting requested review from becker33

@tldahlgren tldahlgren Awaiting requested review from tldahlgren

Assignees

@alalazo alalazo

Labels

feature A feature is missing in Spack specs user-experience

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PR: Garbage Collection for Spack Environments uninstall a package with its dependencies if they are not used elsewhere

3 participants

@alalazo @adamjstewart @tgamblin