Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install all Python packages via pip wheel (or setup.py bdist_wheel), store wheels in $SAGE_LOCAL/var/lib/sage/wheels #29500

Closed
mkoeppe opened this issue Apr 13, 2020 · 45 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 13, 2020

Our current version of pip already tries to install packages via bdist_wheel.

In this ticket, we break the build process into these steps by modifying sdh_pip_install:

  1. Build the wheel explicitly using pip wheel --no-binary :all: --no-build-isolation -w "$SAGE_SPKG_WHEELS" (or setup.py bdist_wheel), which stores the wheel in SAGE_SPKG_WHEELS=$SAGE_LOCAL/var/lib/sage/wheels.

  2. Then install the wheel.

The wheels in $SAGE_SPKG_WHEELS persist after the completed build. We manage them using the DESTDIR staging mechanism -- there will be exactly 1 wheel for each installed package (and removing a package removes the wheel). Example (after rebuilding a few packages on this branch):

$ ls -l local/var/lib/sage/wheels/
...
-rw-r--r--  1 mkoeppe  staff    545293 Sep  8 12:44 Pillow-7.2.0-cp37-cp37m-macosx_10_9_x86_64.whl
-rw-r--r--  1 mkoeppe  staff   4573510 Sep  8 12:06 numpy-1.19.1-cp37-cp37m-macosx_10_9_x86_64.whl
-rw-r--r--  1 mkoeppe  staff      2237 Sep  8 12:44 sage_conf-9.2b12-py3-none-any.whl
-rw-r--r--  1 mkoeppe  staff  17981103 Sep  8 12:59 scipy-1.5.2-cp37-cp37m-macosx_10_9_x86_64.whl

Users can create virtual environments and install the wheels built by the Sage distribution into them, using standard tools such as pip install --find-links.

In this ticket, we keep using the DESTDIR staging mechanism also for the installation of the package from the wheel.


References:

CC: @slel @tobiasdiez @embray @jhpalmieri @tscrim

Component: build

Author: Matthias Koeppe

Branch: f2e7075

Reviewer: Tobias Diez, John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/29500

@mkoeppe mkoeppe added this to the sage-9.2 milestone Apr 13, 2020
@slel
Copy link
Member

slel commented May 9, 2020

comment:1

Would that be gambit, numpy and pillow?

And is this how to find them?

$ git grep "setup.py" build/pkgs

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Aug 13, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Install all Python packages using pip Install all Python packages via pip wheel Aug 27, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 30, 2020

comment:5

... Also consider pipenv + pipfile as suggested in #30317 ...

@embray
Copy link
Contributor

embray commented Sep 1, 2020

comment:7

I would prefer not to have two separate package installation and management systems. If the only motivation of this is the race condition, like I said this can be solved with an additional file lock to prevent multiple Python packages from being installed simultaneously.

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Install all Python packages via pip wheel Install all Python packages via pip wheel, create PEP 503 simple repository for wheels Sep 6, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 6, 2020

Dependencies: #30024

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 8, 2020

comment:15

Narrowed the scope of the ticket.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 8, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 8, 2020

New commits:

8aa6fd9build/bin/sage-dist-helpers (sdh_pip_install): Build a wheel, store it

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 8, 2020

Commit: 8aa6fd9

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Install all Python packages via pip wheel, create PEP 503 simple repository for wheels Install all Python packages via pip wheel, store wheels in $SAGE_LOCAL/var/lib/sage/wheels Sep 8, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 8, 2020

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 8, 2020

Changed dependencies from #30024 to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 9, 2020

Changed commit from ca58693 to 5a747c4

@jhpalmieri
Copy link
Member

comment:28
  • Why sdh_store_and_pip_install_wheel requires . as only argument? Why not just call it with no arguments?

  • This function should be documented in developer/packaging.rst.

  • Does sage-pip-uninstall leave the old wheels lying around?

  • Typo in sage-pip-uninstall (copied from sage-pip-install): "this command hould use"

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 17, 2020

comment:29

Replying to @jhpalmieri:

  • Why sdh_store_and_pip_install_wheel requires . as only argument? Why not just call it with no arguments?

I did this to emphasize the analogy to sage-pip-install.

  • This function should be documented in developer/packaging.rst.

Good idea, will do.

  • Does sage-pip-uninstall leave the old wheels lying around?

sage-pip-uninstall is not really used for regular uninstalls. Rather, it "force-uninstalls" previously installed versions - in case the site-packages directory was somehow messed up. (This code was previously part of sage-pip-install.)

The wheels are installed with the staging mechanism (DESTDIR), which records the installed files. In this way, the -clean targets etc. automatically install the wheel.

  • Typo in sage-pip-uninstall (copied from sage-pip-install): "this command hould use"

Thanks

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 17, 2020

comment:30

Replying to @mkoeppe:

The wheels are installed with the staging mechanism (DESTDIR), which records the installed files. In this way, the -clean targets etc. automatically install the wheel.

To clarify, both the wheels and the installation made from the wheels uses the staging mechanism.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 17, 2020

Changed commit from 5a747c4 to 4135e8b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 17, 2020

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

9ee2110build/bin/sage-dist-helpers: Also use $sudo for storing the wheel file
d7aac84src/doc/en/developer/packaging.rst: Update sdh_... documentation
9b7c7a0build/bin/sage-pip-{install,uninstall}: Fix typo in comment
4135e8bbuild/bin/sage-pip-install: Remove an outdated comment

@jhpalmieri
Copy link
Member

Changed reviewer from Tobias Diez, ... to Tobias Diez, John Palmieri

@jhpalmieri
Copy link
Member

comment:32

Seems okay to me, and it built and passed tests before, so I think it's okay. Of course, I can no longer build Sage anymore, so perhaps take this review with a grain of salt.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 18, 2020

comment:33

Thanks!

@vbraun
Copy link
Member

vbraun commented Sep 21, 2020

comment:34

Merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2020

Changed commit from 4135e8b to f2e7075

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 21, 2020

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

f2e7075Merge tag '9.2.beta13' into t/29500/install_all_python_packages_via_pip_wheel__create_pep_503_simple_repository_for_wheels

@vbraun
Copy link
Member

vbraun commented Sep 27, 2020

@vbraun vbraun closed this as completed in 82a9e2d Sep 27, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.2 Sep 27, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 15, 2020

comment:39

Follow-up: #30771

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 15, 2020

Changed commit from f2e7075 to none

@embray
Copy link
Contributor

embray commented Oct 22, 2020

comment:40

I'm not really sure what the advantage is of this complication. For the follow-up do you propose eliminating the installation manifests installed to /var/lib/sage/installed/? I would very much prefer to not do that. I see no reason for Python packages to be arbitrarily treated differently in this regard. But I'm not sure if that is really the plan.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 22, 2020

comment:41

Replying to @embray:

I'm not really sure what the advantage is of this complication.

https://wiki.sagemath.org/ReleaseTours/sage-9.2#Reusable_wheels_for_the_Python_packages_built_by_the_Sage_distribution explains an immediate use case.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 22, 2020

comment:42

Replying to @embray:

For the follow-up do you propose eliminating the installation manifests installed to /var/lib/sage/installed/?

I don't have a ticket for this at the moment. But I think it's unavoidable to make some changes. Already now these installation manifests (which duplicate the .egg-info record in site-packages) can fall out of sync with the actual installation when the user uses sage -pip install, which could lead to upgrades of some python package dependencies. It's best to use standard Python tools to manage Python packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants