sage-spkg does not correctly overwrite existing directories

[jdemeyer]

This line in sage-spkg

$SAGE_SUDO mv "$PREFIX/$filename" "${SAGE_LOCAL%/}/$filename"

might move a file $filename (to be installed) into a directory $filename (if the latter was an existing directory).

This can occur in particular with symlinks to directories, since those can be interpreted both as file and as directory. This is partially responsible for #26152.

[embray]

Note: This is probably fixed by #26011

[embray]

if the latter was an existing directory

Or, it seems, a symlink pointing to a directory. That's the real problem.

[jdemeyer]

Replying to embray:

Or, it seems, a symlink pointing to a directory. That's the real problem.

Yes, I clarified that in an edit of the description that you reverted.

[embray]

It looks like we were just posting at the same time--sorry, I didn't notice the warning.

[dimpase]

Do I understand it right that the branch on #26011 fixes this?

[embray]

It would. The problem here was that I was manually calling mv on each file one-by-one, which in its own right is a bit slow when you have lots of files to move. But there are some unfortunate subtleties to mv, especially when symlinks are involved, that were not taken into account (see the problem Jeroen described in this ticket). #26011 would replace N mv calls with a single cp -r of the entire directory tree, which AFAICT handles cases like symlinks well too.

In #26011 Jeroen raised some valid concerns about that approach which I attempted to answer, but I admit I don't know what the best play is here (though most online resources I've scoured really do seem to suggest that using cp is the most robust approach in most cases, even if that means needless copying of data. But I'm definitely up for other approaches if they can be suggested...

[vbraun]

Duplicate?

[embray]

I wouldn't say it's a duplicate per se, but since #26011 has been accepted we should be able to call this fixed as well. I'm still not sure if Jeroen will be satisfied with the solution to this (I'm not entirely either) but if we revisit the issue we should just make sure this case is preserved s well.

[jdemeyer]

It's not really "fixed" by #26011 in the sense that the condition now becomes a hard error (after #26642), failing the installation. Ideally, it should overwrite any existing directories.

That being said, I think that you can only do so much with simple shell scripts. Anything more complicated should really be done in Python.

[embray]

Yes, ideally sage-spkg should still be rewritten mostly if not entirely in Python.