Opened 20 months ago
Closed 19 months ago
#30687 closed defect (fixed)
"make python3-clean" should always remove local/bin/python3*
Reported by: | dimpase | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | sage-9.2 |
Component: | build | Keywords: | |
Cc: | mkoeppe, mjo, jhpalmieri | Merged in: | |
Authors: | Matthias Koeppe | Reviewers: | John Palmieri |
Report Upstream: | N/A | Work issues: | |
Branch: | ad7d2c9 (Commits, GitHub, GitLab) | Commit: | ad7d2c929bf0a7d083a0734ad66df4b2751bda63 |
Dependencies: | Stopgaps: |
Description (last modified by )
make python3-clean
does not remove $SAGE_LOCAL/bin/python3*
- certainly not if an external python3
is used.
We do the uninstallation in an spkg-legacy-uninstall
script.
(We also repair the spkg-legacy-uninstall
mechanism - it was broken in #29096.)
Critical for Sage 9.2 because it will help avoid puzzling incremental build behavior, which could include corruption of the installed system python if that is writable for the user.
Change History (34)
comment:1 Changed 20 months ago by
comment:2 Changed 20 months ago by
- Priority changed from major to critical
comment:3 Changed 20 months ago by
- Branch set to u/mkoeppe/_make_python3_clean__should_always_remove_local_lib_python3_
comment:4 Changed 20 months ago by
- Commit set to 0157c38bcd365310e822cfb63557d8e88bb04507
comment:5 Changed 20 months ago by
comment:6 Changed 20 months ago by
"Of course" the spkg-legacy-uninstall
mechanism is broken... It is one of the WRAPPED_SCRIPTS
of build/bin/sage-spkg
, but sage_bootstrap.uninstall.legacy_uninstall
tries to execute spkg-legacy-uninstall
directly out of SAGE_ROOT/build
with bash...
comment:7 Changed 20 months ago by
- Commit changed from 0157c38bcd365310e822cfb63557d8e88bb04507 to ad7d2c929bf0a7d083a0734ad66df4b2751bda63
Branch pushed to git repo; I updated commit sha1. New commits:
e55c0c1 | build/pkgs/*/spkg-legacy-uninstall: Make executable, add #!, remove .in extension
|
0a2f15e | build/bin/sage-spkg: Do not wrap the spkg-legacy-uninstall script
|
ad7d2c9 | build/sage_bootstrap/uninstall.py (legacy_uninstall): Execute spkg-legacy-uninstall script directly, do not go through bash
|
comment:8 Changed 20 months ago by
- Description modified (diff)
comment:9 Changed 20 months ago by
- Status changed from new to needs_review
comment:10 Changed 20 months ago by
- Cc jhpalmieri added
comment:11 follow-up: ↓ 12 Changed 20 months ago by
So the problem with #29096 is that the uninstall script wasn't being run by sage-spkg
, so the conversion from SCRIPT.in
to SCRIPT
didn't do anything useful?
I also don't understand how the use of the spkg-legacy-uninstall
script compares to its documentation. R and gap, for example, are modern packages. Why do they need legacy uninstall scripts?
comment:12 in reply to: ↑ 11 Changed 20 months ago by
Replying to jhpalmieri:
So the problem with #29096 is that the uninstall script wasn't being run by
sage-spkg
, so the conversion fromSCRIPT.in
toSCRIPT
didn't do anything useful?
Yes, prior to #29096 there was an inconsistency - the file build/pkgs/SPKG/spkg-legacy-uninstall
was transformed to the wrapped script (in the temporary build location) but that was never used. Instead, sage-spkg-uninstall
was directly invoked spkg-legacy-uninstall
.
I unfortunately overlooked this in #29096 and so the renaming broke spkg-legacy-uninstall
for all packages - this script was no longer getting invoked for any package.
I also don't understand how the use of the
spkg-legacy-uninstall
script compares to its usage. R and gap, for example, are modern packages. Why do they need legacy uninstall scripts?
I think it is for cleaning out very old installs in long-lived installation trees. Given that #29096 has not led to obvious error reports, it does not seem to be an important function.
comment:13 Changed 20 months ago by
Okay, good. The ticket description says that we should remove $SAGE_LOCAL/lib/python3*
, but the script removes $SAGE_LOCAL/bin/python3*
. Which is correct?
comment:14 Changed 20 months ago by
- Description modified (diff)
- Summary changed from "make python3-clean" should always remove local/lib/python3* to "make python3-clean" should always remove local/bin/python3*
Not sure if the original report really meant "lib", and I don't think there is any problem with it. The branch, in any case, takes care of removing the problematic symlinks in "bin". I have update ticket summary and description.
comment:15 follow-up: ↓ 17 Changed 20 months ago by
The original report did mean lib
rather than bin
. I guess both should be taken care of.
comment:16 follow-ups: ↓ 18 ↓ 19 Changed 20 months ago by
If I've built Sage with a system Python 3, I'm not sure what make python3-clean
should do. Should it really remove all Python site libraries? If it does this, it has to somehow let the build system know to rebuild all of those packages.
comment:17 in reply to: ↑ 15 Changed 20 months ago by
Replying to dimpase:
The original report did mean
lib
rather thanbin
. I guess both should be taken care of.
If you install python3
from spkg, then make clean
already works because it goes through DESTDIR staging.
If you use system python3 (venv), then nothing is installed in /usr/lib/python3.8
... except for stuff installed into site-packages
, which have their own uninstallation procedures.
comment:18 in reply to: ↑ 16 Changed 20 months ago by
Replying to jhpalmieri:
If I've built Sage with a system Python 3, I'm not sure what
make python3-clean
should do.
It should be a no-op because python3
is not installed.
comment:19 in reply to: ↑ 16 Changed 20 months ago by
Replying to jhpalmieri:
Should it really remove all Python site libraries?
No, the files that are installed there belong to the individual spkgs.
comment:20 Changed 20 months ago by
Right, I understand that, so Dima, what did you have in mind?
comment:21 follow-ups: ↓ 22 ↓ 23 Changed 20 months ago by
while these files are installed by individual packages, they "belong" to python.
Also there should be no difference in what happens to the $SAGE_LOCAL/lib/python3* whether python3 is installed by Sage or not. Isn't it true that cleaning a Sage-installed Python leads to removal of $SAGE_LOCAL/lib/python3* ?
comment:22 in reply to: ↑ 21 Changed 20 months ago by
Replying to dimpase:
Isn't it true that cleaning a Sage-installed Python leads to removal of $SAGE_LOCAL/lib/python3* ?
No. The files that belong to the package, recorded in SAGE_LOCAL/var/lib/sage/installed
are removed. It does not remove the installed site-packages.
When you reinstall the python3 spkg, then dependencies cause all python packages to be rebuilt.
comment:23 in reply to: ↑ 21 Changed 20 months ago by
Replying to dimpase:
while these files are installed by individual packages, they "belong" to python.
I think we need to start from the beginning here. What failure do you observe, with or without the branch on this ticket.
comment:24 Changed 20 months ago by
weird things happen if you switch from one external python3 to another.
comment:25 Changed 20 months ago by
I think the branch of this ticket addresses this by making make python3-clean
remove the interpreter symlinks and the pyvenv.cfg
. Please try
comment:26 Changed 20 months ago by
Let's please get this ticket in.
comment:27 Changed 20 months ago by
I don't have any objections to it. Dima?
comment:28 follow-up: ↓ 29 Changed 20 months ago by
A nitpick, but I don't like mass renaming of spkg-legacy-uninstall.in involving adding boilerplate into each of them. Couldn't build/sage_bootstrap/uninstall.py, the only place it is called from, just prepend the #!
line?
comment:29 in reply to: ↑ 28 Changed 20 months ago by
Replying to dimpase:
A nitpick, but I don't like mass renaming of spkg-legacy-uninstall.in involving adding boilerplate into each of them. Couldn't build/sage_bootstrap/uninstall.py, the only place it is called from, just prepend the
#!
line?
This is literally just undoing a change from #29096 which completely broke the use of the spkg-legacy-uninstall
scripts. Otherwise I would agree with you.
comment:30 Changed 20 months ago by
- Reviewers set to John Palmieri
- Status changed from needs_review to positive_review
comment:31 Changed 20 months ago by
Thanks!
comment:32 Changed 20 months ago by
- Description modified (diff)
comment:33 Changed 20 months ago by
- Description modified (diff)
- Priority changed from critical to blocker
comment:34 Changed 19 months ago by
- Branch changed from u/mkoeppe/_make_python3_clean__should_always_remove_local_lib_python3_ to ad7d2c929bf0a7d083a0734ad66df4b2751bda63
- Resolution set to fixed
- Status changed from positive_review to closed
Not really
make python3-clean
, more likemake python3_venv-clean
; see #29708 ("Fix switching between--with-system-python3
and--without-system-python3
"). I had hoped to fix it via #29386, but this won't be ready for Sage 9.2.We can try a hotfix for this issue by adding
build/pkgs/python3/legacy-uninstall