Fix legacy uninstall

[jdemeyer]

It turns out that the legacy uninstallers spkg-legacy-uninstall are never actually run due to an always-false executable-bit check.

Also, it happens regularly that the stamp file $SAGE_LOCAL/var/lib/sage/installed/$PKG-$VERSION is inconsistent with what is actually installed. This can happen for example when package installation failed or was interrupted. This can lead to problems like #26996. As a solution, we suggest to always run the legacy uninstaller as fallback if there is no stamp file. This is more in line with how installation worked before the introduction of the new-style stamp files.

[jdemeyer]

New commits:

​0cf4613

Fix legacy uninstall

[git]

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

​bcef97e

rm -rf never fails; can't use sdh_die here anyway

[embray]

This is what I found as well. I was going to post a fix to this last night but didn't have time.

[embray]

I'm not sure why you changed this message:

     if keep_files:
-        print("Removing stamp file '{0}' but keeping package files".format(
-            stamp_file))
+        print("Removing stamp file but keeping package files")
         remove_stamp_files(stamp_files)
         return

Isn't it a little bit more helpful to actually specify what file the message is referring to?

[embray]

Other than that, this is just a slightly different formatted version of what I did, so you can set positive review after addressing my one question.

[jdemeyer]

Replying to embray:

Isn't it a little bit more helpful to actually specify what file the message is referring to?

Mainly because it's not guaranteed that there is exactly one stamp file.

Also, why should the filename of the stamp file be printed in this code path but not in other code paths? If you really want to see a message for removing stamp files, it would better be in the remove_stamp_files() function.

[embray]

Also, it happens regularly that the stamp file $SAGE_LOCAL/var/lib/sage/installed/$PKG-$VERSION is inconsistent with what is actually installed. This can happen for example when package installation failed or was interrupted.

I do consider this a separate issue. I would like to eventually remove the spkg-legacy-uninstall files entirely. Most of them are not well maintained and their use at all is inconsistent (this is true before the idea of moving that functionality into a separate file).

In other words, even spkg-legacy-uninstall scripts are not a reliable way to deal with partial/failed installations. I would rather fix this more directly. For example, I think the stamp file could be written before copying files into $SAGE_LOCAL. That way, even if the "installation" (which at this point is just copying files) fails, there is at least still a manifest of what files were expected to be in place, so a partial installation can be rolled back.

There should also be better error/interrupt handling in the sage-spkg script so that "partial installs" can be immediately rolled back when possible.

The downside to this approach is if an install is interrupted before it's complete, you still have a valid stamp file suggesting the installation was completed. To address this, maybe it should be written first to a different location (after all, in this case it is serving merely as a file manifest) and then moved into its final location after the successful installation.

[embray]

Replying to jdemeyer:

Replying to embray:

Isn't it a little bit more helpful to actually specify what file the message is referring to?

Mainly because it's not guaranteed that there is exactly one stamp file.

That is rarely the case though. And in any case the message still says "Removing stamp file", singular.

Also, why should the filename of the stamp file be printed in this code path but not in other code paths? If you really want to see a message for removing stamp files, it would better be in the remove_stamp_files() function.

That's fine; I don't care where it goes.

[jdemeyer]

Replying to embray:

For example, I think the stamp file could be written before copying files into $SAGE_LOCAL. That way, even if the "installation" (which at this point is just copying files) fails, there is at least still a manifest of what files were expected to be in place, so a partial installation can be rolled back.

I think it's too hard to ensure that the stamp file is consistent with the actual installed files. I've had similar issues with pip too where pip would think that a package is not installed but still some files belonging to that package are installed.

So rather than improving the stamp files, I would rather improve installation in the case that the stamp files are missing or wrong.

[embray]

Under normal circumstances it's actually exactly consistent, except in the cases of files that are created post-installation which are handled differently. So I think you'd have to be more specific what you think the problem is.

I'm also not talking about "improving stamp files", but rather strategies for managing broken installations. The "stamp file" here is really serving two purposes (where previously it served only one):

• It is an indication that a package installed successfully.
• It is a manifest of (expected) installed files.

Installation, when done correctly, is just copying a tree of files from one place to another. In order to roll back a partial/unsuccessful installation you want to have that manifest somewhere, but not in the actual "stamp file" as its existence indicates a successful install. So what I'm suggesting is instead writing it to something like ".<pkg-name>-<version>.tmp" prior to installation, and then following a successful installing moving it to "<pkg-name>-<version>".

The uninstall program would also be able to look for these ".<pkg-name>-<version>.tmp" files in order to clean up broken installations.

[git]

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

​3c97621

Always print message when removing stamp file

[embray]

Looks good to me. I'm sorry I didn't say anything sooner, though I will say your fix is a little cleaner than mine--you restructured things a bit such that there was less duplication than what I had.