Opened 3 years ago

Closed 11 months ago

#22510 closed enhancement (worksforme)

Enhanced package uninstallation for Sage spkgs

Reported by: embray Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords: uninstall
Cc: Merged in:
Authors: Erik Bray Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by embray)

Currently spkg uninstallation is handled in an ad-hoc manner. Many packages contain some rough uninstallation steps in their spkg-install scripts (to be run, for example, when upgrading/reinstalling a package). But these are often imprecise and not thorough (though maybe still worth keeping where they already exist, as I will explain below).

implementation

  • #22509 - most packages are now installed with staged installation (i.e. DESTDIR installation), which enables creating a manifest of all files installed by that package (minus files created by the package's spkg-postinst script). This is mostly working now for almost all packages (see #24024).
  • #25139 - add an spkg-uninstall script: this would read the file manifest for a package (if it exists) and remove those files from $SAGE_LOCAL--thus uninstalling the package thoroughly and precisely.
    • #25139 - support legacy uninstallers: many existing spkgs have steps in their spkg-install for "uninstalling" that package--mostly just removing a few key files associated with the package (executables and libraries), but was rarely complete. However, some of these legacy uninstall steps are still necessary for properly building the package if the package has not already been uninstalled. This is an issue for the new system insofar as we want upgrades from older versions of Sage to work, so the legacy uninstallers are still needed at least for one upgrade, if uninstalling a package that doesn't have a proper file manifest. Since these steps are no longer needed in the spkg-install (in fact ideally spkg-install should not modify $SAGE_LOCAL at all, we move those steps into a separate spkg-legacy-uninstall script which is run only when uninstalling a package that is missing its file manifest.
      • #25140 - once this infrastructure is in place there are several packages with 'legacy uninstallers' that would need to be updated.
    • #25139 - also add support for an spkg-postrm script that would be the complement to a spkg-postinst script, and may clean up files generated by a package that are not part of the package's file manifest

old design discussion

Here are a few proposals for improving package uninstallation (which is useful even for required packages, such as in the case of upgrading them):

  • For all installed packages, maintain lists of the files installed by those packages. For this, something like #22509 is a prerequisite. This list could even go into the $SAGE_LOCAL/var/lib/sage/installed/ files we already have. Uninstalling a package would consist primarily of removing all files in this list. It would also remove directories that are left empty after all files are removed.
  • Add support an optional spkg-uninstall script for any spkgs. This can be used to perform any additional uninstallation steps. For example, it is a place to remove files / data added specifically for Sage by spkg-install (e.g. symlinks) that are not installed by the package's make install. This would be run before the full uninstall described in the previous bullet point (in case any files from the package are necessary to determine additional uninstall steps). So this would be sort of like a pre-uninstall.
  • Add support for an optional spkg-legacy-uninstall script. This would contain whatever commands the spkg-install scripts currently perform for uninstall. This would only be run when the installed file list described in the first bullet point is unavailable (e.g. for backwards compatibility when upgrading to this new system for the first time). This feature could maybe be removed entirely later, but would be good to add at first and keep around for a while.

The new process for upgrading / reinstalling a package could be (especially if more progress is made toward supporting #21726 in all spkgs):

  1. Build
  2. Install to temporary directory (#22509)
  3. Upon successful completion of 1. and 2., uninstall old package
  4. Install new package by copying from temp dir into $SAGE_LOCAL
  5. Run post-install scripts, if any

This means a cleaner rollback from failed upgrades.

Change History (20)

comment:1 Changed 3 years ago by embray

  • Dependencies set to #22509

comment:2 Changed 3 years ago by embray

  • Authors set to Erik Bray
  • Branch set to u/embray/build/uninstall
  • Commit set to ae503162358d7251f8132b02a568a154dda882bf

Adding initial work on the sage-spkg-uninstall script.

I've already created spkg-legacy-uninstall scripts for most standard packages, but I'm still testing this mechanism. These scripts are just created by taking any uninstall steps found in the spkg-install and splitting it out to a separate script. Implementation of uninstall steps is of course very uneven across packages, and I'm not creating legacy-uninstall scripts for those packages that didn't already have uninstall steps--if it wasn't needed before it isn't needed now. Many of the existing uninstall steps probably aren't needed either, but are (they reference specific tickets). To be on the safe side I just took what already exists more or less at face value and didn't change any of the details.

I haven't implemented support for the separate spkg-uninstall script yet as it hasn't been needed. Though there are a handful of packages where it might be useful (for example maxima's spkg-install has a step that removes files from $DOT_SAGE).


Last 10 new commits:

3a32187Roll back accidental change from dc46ea2dfa97be79d7ef8ddbe9130d4c717acc3d that broke build of Python 2 on Cygwin.
8a621afUpdated the python2/3 pacakges to use staged install.
1391089Remove the '-v' from the 'cp' command.
e294123Add staged install support for atlas
00f6de8This file should be deleted from the temp install dir during staged install on OSX
8b06de1This is pointless now that the symlinks are being installed in SAGE_INST_TEMP, so of course they don't exist
e91fce4Initial version of the sage-spkg-uninstall script, handling both legacy uninstall, and uninstall using file records in the new stamp file format.
867f468Add a secret --debug flag to sage-spkg-uninstall, useful for debugging exceptions
a476ccdGraceful fallback if it appears the package is not installed
ae50316Use sage-spkg-uninstal in sage-spkg

comment:3 Changed 3 years ago by embray

There is a slight possibility for a race condition here. If two packages share a directory (other than some top level directory like lib that almost all packages use), but say something like share/pari, it's possible one package can be trying to install to that directory while the other is uninstalling from it and this could lead to errors.

I haven't seen it come up in practice yet and it's probably pretty rare, but I might consider a mutex around the uninstall/install sections...

comment:4 Changed 3 years ago by git

  • Commit changed from ae503162358d7251f8132b02a568a154dda882bf to 18530eddafc2e11f08ad01e3c6aa194edd88a5d8

Branch pushed to git repo; I updated commit sha1. New commits:

18530edAdds separate spkg-legacy-uninstall scripts for most standard packages that even had explicit uninstall steps in their spkg-installs.

comment:5 follow-up: Changed 3 years ago by embray

Out of curiosity I had a look to see if any packages are installing (and hence uninstalling) the same file, as I figured there might be some such cases and indeed there are. I don't have an exhaustive list, because I have not installed every optional and experimental package.

The duplicate files fall into a few different categories:

  1. Most are due to packages that conflict with/replace another package. Either because two packages provide the same functionality (e.g. openblas/atlas, mpir/gmp) or because one package is a stripped down version of another (e.g. boost_cropped/boost, pari_seadata_small/database_pari).

    I think it might be good if we had explicit conflicts/replaces markers for some packages. If a package conflicts with an installed package, it will not be installed unless forced. If a package replaces another package, the package it replaces is explicitly uninstalled first. In effect this already happens implicitly. If I install pari_seadata_small, and then install database_pari, the latter will override the former. But I think it would be better to do this explicitly.
  1. share/info/dir--this is a file shared by many packages. It is written/updated by the install-info program. Of course, when installing into a temp root, there will be no existing share/info/dir file, and install-info will create it. But then each package that installs it overrides each other.

    Debian handles this file by stripping it from the installation, then calling install-info from a post-install (and pre-uninstall) script. This is the sort of thing for which I anticipated possibly needing post-install/uninstall hooks. That said, this is the only example like this I've encountered so far in Sage-the-dist. So I might just make a special case for it for now, directly in sage-spkg, rather than placing the onus on each package that updates this file to do it correctly.
  1. __init__.py in Python namespace packages (e.g. backports_ssl_match_hostname/backports_shutil_get_terminal_size). If both are installed and one clobbers the other that's fine, but if one is then uninstalled it should not remove the shared __init__.py. I'll have to look at how Debian handles this. It might be handled contextually--these files usually contain some boilerplate like __path__ = extend_path(__path__, __name__). But the boilerplate is not always spelled exactly the same, so it might be tricky to check for. Maybe this just needs to be handled explicitly on a case-by-case basis.
  1. bin/2to3--just one special case of a script that is being installed by both Python 2 and Python 3. This should of course be fixed in those packages.
Last edited 3 years ago by embray (previous) (diff)

comment:6 in reply to: ↑ 5 Changed 3 years ago by embray

Replying to embray:

  1. share/info/dir--this is a file shared by many packages. It is written/updated by the install-info program. Of course, when installing into a temp root, there will be no existing share/info/dir file, and install-info will create it. But then each package that installs it overrides each other.

    Debian handles this file by stripping it from the installation, then calling install-info from a post-install (and pre-uninstall) script. This is the sort of thing for which I anticipated possibly needing post-install/uninstall hooks. That said, this is the only example like this I've encountered so far in Sage-the-dist. So I might just make a special case for it for now, directly in sage-spkg, rather than placing the onus on each package that updates this file to do it correctly.

In retrospect, I've come back around to thinking there should be a mechanism for post-install/pre-uninstall scripts. For common examples like share/info/dir there can be a boilerplate that's linked to or something.

Another example I found where this would be useful, which granted is an experimental package, while be compilerwrapper. This package conflicts with gcc unless the step of creating the wrapper scripts is done as a post-install step (and undone, if necessary, pre-uninstall).

comment:7 Changed 2 years ago by git

  • Commit changed from 18530eddafc2e11f08ad01e3c6aa194edd88a5d8 to 620287c2606f30fe3ea5623c87015dbc1b73817c

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

8439367A little better debug output here
0c4fcd6Added the ability for packages to have an spkg-postrm script that is called
0e73f89Change how pkgconf is installed to better handle uninstallation.
3f07318Handle errors when running the spkg-postrm script.
b79d581Restore IFS after this loop so that later loops aren't broken.
edada31Better handling of missing directories when uninstalling broken packages
b043628Add the ability for packages to have a pre-uninstall script too.
0b316b5It seems the best way to handle notebook extensions is to disable them in a prerm script.
e1a9716These packages should have their versions bumped now that they require installing pre/post-uninstall scripts.
620287cMove execution of spkg- scripts to a simple utility function, avoiding a bit of boilerplate

comment:8 Changed 2 years ago by embray

  • Description modified (diff)
  • Milestone changed from sage-wishlist to sage-8.1
  • Status changed from new to needs_review

Rebased on latest #22509; addressed issue of properly uninstalling the widgetsnbextension. It's working pretty well now, after multiple around of install/uninstall and upgrading from old builds from before these changes.

Of course #22509 should be reviewed and merged first as it lays most of the groundwork for this. If diff'd against #22509 the changes in this ticket don't look as big.

comment:9 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Needs to be rebased (but maybe not right now).

comment:10 Changed 2 years ago by embray

Yes, there's more work needed to be done in breaking up #22509 into smaller bites before this ticket can even work again (at least for all packages--it will probably work for most packages with #24106).

comment:11 Changed 2 years ago by embray

  • Milestone changed from sage-8.1 to sage-8.2

comment:12 Changed 21 months ago by embray

  • Dependencies #22509 deleted

I've had this on hold pending total completion of #22509 but it's still taking some time to get all the packages updated for DESTDIR support (see #24024). However, for many packages this is already working, and the general infrastructure is in place. So there's no reason not to move forward with the uninstallation work--the only problem is that packages that aren't installed with staged installation won't be properly uninstallable...yet.

Similarly, this work can be split up into a few smaller tickets, so I will move forward on that.

comment:13 Changed 21 months ago by embray

  • Branch u/embray/build/uninstall deleted
  • Commit 620287c2606f30fe3ea5623c87015dbc1b73817c deleted
  • Description modified (diff)

Some new explanation of the work planned for this ticket, so I can break it up into multiple smaller chunks.

comment:14 Changed 20 months ago by embray

  • Description modified (diff)
  • Keywords uninstall added

comment:15 Changed 20 months ago by embray

  • Description modified (diff)

comment:16 Changed 20 months ago by embray

  • Milestone changed from sage-8.2 to sage-8.3

comment:17 Changed 17 months ago by embray

  • Milestone changed from sage-8.3 to sage-8.4

I believe this issue can reasonably be addressed for Sage 8.4.

comment:18 Changed 14 months ago by embray

  • Milestone changed from sage-8.4 to sage-8.5

comment:19 Changed 12 months ago by embray

  • Milestone changed from sage-8.5 to sage-8.7

Retargeting some of my tickets (somewhat optimistically for now).

comment:20 Changed 11 months ago by embray

  • Milestone changed from sage-8.7 to sage-duplicate/invalid/wontfix
  • Resolution set to worksforme
  • Status changed from needs_work to closed

This is essentially done since #25139. There are still some packages that should have spkg-legacy-uninstall scripts, but that is tracked in #25140. Some packages still need to be ported to the new install system in order to be properly uninstallable, but that is being tracked in #24024.

Note: See TracTickets for help on using tickets.