Skip to content

Commit abb1a51

Browse files
author
Release Manager
committed
gh-36743: Normal Python packages: postpone wheel installation to the post-install phase <!-- ^^^^^ 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 --> Previously, on `sdh_pip_install`, the wheel file is staged in DESTDIR, but the wheel is installed immediately. Now we store a new script `spkg-pipinst`, which is run after unloading DESTDIR (and before any `spkg-postinst` script), which does the actual installation of the wheel. - This resolves #30956 (fixing the wheel URLs shown in `sage -pip freeze`) -- except when `SAGE_SUDO` is set. Apart from this and some changes to the messages displayed during package installation, this should make no difference for any of our packages. Just so that it is tested for at least one package in CI, we include a small package update. Together with - #36738 and - #36740, this is preparation for requiring only the build dependencies ("build- system requires") while building a wheel for the package, and to require the runtime dependencies ("install-requires") only later, when the wheel is to be installed. <!-- Why is this change required? What problem does it solve? --> <!-- 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. - [x] 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: #36743 Reported by: Matthias Köppe Reviewer(s): John H. Palmieri
2 parents 46b03d5 + f981b15 commit abb1a51

File tree

5 files changed

+62
-23
lines changed

5 files changed

+62
-23
lines changed

build/bin/sage-dist-helpers

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -355,38 +355,58 @@ sdh_store_and_pip_install_wheel() {
355355
shift
356356
done
357357
sdh_store_wheel "$@"
358-
if [ -n "$SAGE_SUDO" ]; then
359-
# Trac #29585: Do the SAGE_DESTDIR staging of the wheel installation
360-
# ONLY if SAGE_SUDO is set (in that case, we still do the staging so
361-
# that we do not invoke pip as root).
358+
359+
wheel_basename="${wheel##*/}"
360+
distname="${wheel_basename%%-*}"
361+
362+
if [ -d "$SAGE_BUILD_DIR/$PKG_NAME" ]; then
363+
# Normal package install through sage-spkg;
364+
# scripts live in the package's build directory
365+
# until copied to the final destination by sage-spkg.
366+
script_dir="$SAGE_BUILD_DIR/$PKG_NAME"
367+
else
368+
script_dir="$SAGE_SPKG_SCRIPTS/$PKG_BASE"
369+
fi
370+
371+
if [ -n "$SAGE_DESTDIR" -a -z "$SAGE_SUDO" ]; then
372+
# We stage the wheel file and do the actual installation in post.
373+
echo "sdh_actually_pip_install_wheel $distname $pip_options -r \"\$SAGE_SPKG_SCRIPTS/\$PKG_BASE/spkg-requirements.txt\"" >> "$script_dir"/spkg-pipinst
374+
else
362375
if [ -n "$SAGE_DESTDIR" ]; then
376+
# Trac #29585: Do the SAGE_DESTDIR staging of the wheel installation
377+
# ONLY if SAGE_SUDO is set (in that case, we still do the staging so
378+
# that we do not invoke pip as root).
363379
# --no-warn-script-location: Suppress a warning caused by --root
364380
local sudo=""
365381
local root="--root=$SAGE_DESTDIR --no-warn-script-location"
366-
else
367-
# Trac #32361: Of course, this can only be done for normal packages,
368-
# whose installation goes through sage-spkg.
369-
# For script packages, we do have to invoke pip as root.
382+
elif [ -n "$SAGE_SUDO" ]; then
383+
# Trac #32361: For script packages, we do have to invoke pip as root.
370384
local sudo="$SAGE_SUDO"
371385
local root=""
386+
else
387+
#
388+
local sudo=""
389+
local root=""
372390
fi
373-
else
374-
local sudo=""
375-
local root=""
391+
sdh_actually_pip_install_wheel $distname $root $pip_options "$wheel"
376392
fi
393+
echo "sdh_pip_uninstall -r \"\$SAGE_SPKG_SCRIPTS/\$PKG_BASE/spkg-requirements.txt\"" >> "$script_dir"/spkg-piprm
394+
}
395+
396+
sdh_actually_pip_install_wheel() {
397+
distname=$1
398+
shift
377399
# Trac #32659: pip no longer reinstalls local wheels if the version is the same.
378400
# Because neither (1) applying patches nor (2) local changes (in the case
379401
# of sage-conf, sage-setup, etc.) bump the version number, we need to
380402
# override this behavior. The pip install option --force-reinstall does too
381403
# much -- it also reinstalls all dependencies, which we do not want.
382-
wheel_basename="${wheel##*/}"
383-
distname="${wheel_basename%%-*}"
384404
$sudo sage-pip-uninstall "$distname"
385405
if [ $? -ne 0 ]; then
386406
echo "(ignoring error)" >&2
387407
fi
388-
$sudo sage-pip-install $root $pip_options "$wheel" || \
389-
sdh_die "Error installing ${wheel##*/}"
408+
$sudo sage-pip-install "$@" || \
409+
sdh_die "Error installing $distname"
390410
}
391411

392412
sdh_pip_uninstall() {

build/bin/sage-spkg

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -583,9 +583,13 @@ __EOF__
583583

584584
# Prepare script for uninstallation of packages that use sdh_pip_install
585585
# or sdh_store_and_pip_install_wheel.
586-
echo 'sdh_pip_uninstall -r $SAGE_SPKG_SCRIPTS/$PKG_BASE/spkg-requirements.txt' > spkg-piprm.in
586+
touch spkg-piprm.in
587587

588-
for script in $WRAPPED_SCRIPTS; do
588+
# Prepare script for deferred installation of packages that use sdh_pip_install
589+
# or sdh_store_and_pip_install_wheel.
590+
touch spkg-pipinst.in
591+
592+
for script in $WRAPPED_SCRIPTS pipinst; do # pipinst can be added to WRAPPED_SCRIPTS later
589593
# 'Installed' scripts are not run immediately out of the package build
590594
# directory, and may be run later, so set their root directory to
591595
# $SAGE_ROOT
@@ -730,10 +734,11 @@ INSTALLED_SCRIPTS_DEST="$SAGE_SPKG_SCRIPTS/$PKG_BASE"
730734

731735
if [ ! -f $INSTALLED_SCRIPTS_DEST/spkg-requirements.txt ]; then
732736
# No packages to uninstall with pip, so remove the prepared uninstall script
733-
rm -f spkg-piprm spkg-piprm.in
737+
# and the prepared deferred installation script
738+
rm -f spkg-piprm spkg-piprm.in spkg-pipinst spkg-pipinst.in
734739
fi
735740

736-
for script in $INSTALLED_SCRIPTS; do
741+
for script in $INSTALLED_SCRIPTS pipinst; do # pipinst can be added to WRAPPED_SCRIPTS later
737742
script="spkg-$script"
738743

739744
if [ -f "$script" ]; then
@@ -754,6 +759,15 @@ done
754759

755760
post_install() { #################################################
756761
# Run the post-install script, if any
762+
# But first complete the delayed installation of wheels.
763+
if [ -f spkg-pipinst ]; then
764+
echo "Running pip-install script for $PKG_NAME."
765+
$SAGE_SUDO ./spkg-pipinst
766+
if [ $? -ne 0 ]; then
767+
error_msg "Error running the pipinst script for $PKG_NAME."
768+
exit 1
769+
fi
770+
fi
757771
if [ -f spkg-postinst ]; then
758772
echo "Running post-install script for $PKG_NAME."
759773
time $SAGE_SUDO ./spkg-postinst

build/pkgs/pip/spkg-pipinst.in

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
cd src
2+
3+
# pip can install its own wheel! But first we need to ensure that the pip
4+
# source directory in on the PYTHONPATH
5+
export PYTHONPATH="$(pwd)/src"

build/pkgs/texttable/checksums.ini

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
tarball=texttable-VERSION.tar.gz
2-
sha1=25e1b92e02c8e919dc0da053efbe8c4874418a8d
3-
md5=83eb15fb541dd857ff051a8d0c979b9c
4-
cksum=3183998721
2+
sha1=0fa175fa6e0fefea31434746641bedc8cbb60248
3+
md5=e5d380c04fab132ccf0bbfd4f761bd51
4+
cksum=274394355
55
upstream_url=https://pypi.io/packages/source/t/texttable/texttable-VERSION.tar.gz
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.6.7
1+
1.7.0

0 commit comments

Comments
 (0)