Skip to content

Commit

Permalink
gh-36737: Fix make SPKG-uninstall for Python packages after #36452
Browse files Browse the repository at this point in the history
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes #1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
#36452 broke `make SPKG-uninstall` for Python packages.

Reproducer:
```
$ make zipp-uninstall
...
if [ -d '/Users/mkoeppe/s/sage/sage-rebasing/worktree-
clean/local/var/lib/sage/venv-python3.10' ]; then sage-spkg-uninstall
zipp '/Users/mkoeppe/s/sage/sage-rebasing/worktree-
clean/local/var/lib/sage/venv-python3.10'; fi
Uninstalling existing 'zipp'
Running pip-uninstall script for 'zipp'
ERROR: Could not open requirements file: [Errno 2] No such file or
directory: '/Users/mkoeppe/s/sage/sage-rebasing/worktree-
clean/local/var/lib/sage/venv-python3.10/var/lib/sage/scripts/zipp/spkg-
requirements.txt'
Warning: pip exited with status 0
Removing stamp file '/Users/mkoeppe/s/sage/sage-rebasing/worktree-
clean/local/var/lib/sage/venv-
python3.10/var/lib/sage/installed/zipp-3.11.0'
$ ./sage -pip list | grep zipp
zipp                           3.11.0
```



<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes #12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- #12345: short description why this is a dependency
- #34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #36737
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
  • Loading branch information
Release Manager committed Nov 27, 2023
2 parents 202a245 + c5b6f9d commit 2994fa6
Showing 1 changed file with 11 additions and 9 deletions.
20 changes: 11 additions & 9 deletions build/sage_bootstrap/uninstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,17 @@ def modern_uninstall(spkg_name, sage_local, files, verbose=False):
else:
sys.exit(1)

# Run the package's piprm script, if it exists.
# Since #36452 the spkg-requirements.txt file appears in the installation
# manifest, so this step has to happen before removing the files.
try:
run_spkg_script(spkg_name, spkg_scripts, 'piprm',
'pip-uninstall')
except Exception:
print("Warning: Error running the pip-uninstall script for "
"'{0}'; uninstallation may have left behind some files".format(
spkg_name), file=sys.stderr)

def rmdir(dirname):
if pth.isdir(dirname):
if not os.listdir(dirname):
Expand Down Expand Up @@ -200,15 +211,6 @@ def rmdir(dirname):
# Remove file's directory if it is now empty
rmdir(dirname)

# Run the package's piprm script, if it exists.
try:
run_spkg_script(spkg_name, spkg_scripts, 'piprm',
'pip-uninstall')
except Exception:
print("Warning: Error running the pip-uninstall script for "
"'{0}'; uninstallation may have left behind some files".format(
spkg_name), file=sys.stderr)

# Run the package's postrm script, if it exists.
# If an error occurs here print a warning, but complete the
# uninstallation; otherwise we leave the package in a broken
Expand Down

0 comments on commit 2994fa6

Please sign in to comment.