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

sdh_store_and_pip_install_wheel: Install from the persistent wheel directory #30956

Closed
mkoeppe opened this issue Nov 24, 2020 · 4 comments · Fixed by #36743
Closed

sdh_store_and_pip_install_wheel: Install from the persistent wheel directory #30956

mkoeppe opened this issue Nov 24, 2020 · 4 comments · Fixed by #36743

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 24, 2020

Python packages no longer use SAGE_DESTDIR except for staging/removing the .whl file.

This last bit of staging leads to sage -pip freeze showing wheel URLs in temporary build locations:

$ ./sage -pip freeze
alabaster @ file:///Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/tmp/sage/build/alabaster-0.7.12/src
appdirs==1.4.4
appnope @ file:///Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/tmp/sage/build/appnope-0.1.0.p0/src
argon2-cffi==20.1.0
attrs @ file:///Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/tmp/sage/build/attrs-19.3.0/src
Babel @ file:///Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/tmp/sage/build/babel-2.6.0/src

(These locations are recorded in direct_url.json according to https://www.python.org/dev/peps/pep-0610/; see https://packaging.python.org/en/latest/specifications/direct-url/)

In this ticket, we remove the staging, writing the .whl file directly into SAGE_VENV/var/lib/sage/wheels. (Uninstallation of the .whl file will be done by adding to the spkg-piprm script.)

Thus, sage -pip freeze will show the actual file URLs of the stored wheels.

CC: @jhpalmieri @tobiasdiez

Component: build

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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Nov 24, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 24, 2020

comment:1

This will have to wait until we get rid of using DESTDIR for Python packages

@mkoeppe mkoeppe removed this from the sage-9.3 milestone Nov 24, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe added this to the sage-9.6 milestone Dec 28, 2021
@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Mar 5, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Jan 7, 2023
@mkoeppe mkoeppe removed this from the sage-10.0 milestone Mar 16, 2023
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 15, 2023
…-check`

    
<!-- ^^^^^
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 sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

We add `make` targets for testing wheel-building script packages that
have already been installed. For example:
```
$ make SAGE_WHEELS=yes sagemath_categories  # builds and stores (but
does not install) a wheel
$ make sagemath_categories-check            # tests the wheel using tox
```
The target `make pypi-wheels-check` calls all of them. We restructure
the Build & Test CI so that all tests are run after build has been
completed. Later, as a follow up of this and sagemath#36446, we can store the
result of Build as a container image and then parallelize the various
tests on top of it.

This is implemented on top of an improvement of how we record
information on our wheels.
```
$ cat venv/var/lib/sage/scripts/sagemath_categories/spkg-
requirements.txt
sagemath_categories @ file:///Users/mkoeppe/s/sage/sage-
rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.
11/var/lib/sage/wheels/sagemath_categories-10.2b6-cp311-cp311-
macosx_13_0_x86_64.whl
```
Previously, this information was only known during the execution of the
`spkg-install` script.

In a follow-up, we can address sagemath#30956 by delaying the wheel installation
to spkg-postinst.

<!-- 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 sagemath#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
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36452
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 17, 2023
…-check`

    
<!-- ^^^^^
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 sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

We add `make` targets for testing wheel-building script packages that
have already been installed. For example:
```
$ make SAGE_WHEELS=yes sagemath_categories  # builds and stores (but
does not install) a wheel
$ make sagemath_categories-check            # tests the wheel using tox
```
The target `make pypi-wheels-check` calls all of them. We restructure
the Build & Test CI so that all tests are run after build has been
completed. Later, as a follow up of this and sagemath#36446, we can store the
result of Build as a container image and then parallelize the various
tests on top of it.

This is implemented on top of an improvement of how we record
information on our wheels.
```
$ cat venv/var/lib/sage/scripts/sagemath_categories/spkg-
requirements.txt
sagemath_categories @ file:///Users/mkoeppe/s/sage/sage-
rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.
11/var/lib/sage/wheels/sagemath_categories-10.2b6-cp311-cp311-
macosx_13_0_x86_64.whl
```
Previously, this information was only known during the execution of the
`spkg-install` script.

In a follow-up, we can address sagemath#30956 by delaying the wheel installation
to spkg-postinst.

<!-- 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 sagemath#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
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36452
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 19, 2023
…-check`

    
<!-- ^^^^^
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 sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

We add `make` targets for testing wheel-building script packages that
have already been installed. For example:
```
$ make SAGE_WHEELS=yes sagemath_categories  # builds and stores (but
does not install) a wheel
$ make sagemath_categories-check            # tests the wheel using tox
```
The target `make pypi-wheels-check` calls all of them. We restructure
the Build & Test CI so that all tests are run after build has been
completed. Later, as a follow up of this and sagemath#36446, we can store the
result of Build as a container image and then parallelize the various
tests on top of it.

This is implemented on top of an improvement of how we record
information on our wheels.
```
$ cat venv/var/lib/sage/scripts/sagemath_categories/spkg-
requirements.txt
sagemath_categories @ file:///Users/mkoeppe/s/sage/sage-
rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.
11/var/lib/sage/wheels/sagemath_categories-10.2b6-cp311-cp311-
macosx_13_0_x86_64.whl
```
Previously, this information was only known during the execution of the
`spkg-install` script.

In a follow-up, we can address sagemath#30956 by delaying the wheel installation
to spkg-postinst.

<!-- 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 sagemath#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
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36452
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Dec 9, 2023
… 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 sagemath#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 sagemath#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
- sagemath#36738 and
- sagemath#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 sagemath#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
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36743
Reported by: Matthias Köppe
Reviewer(s): John H. Palmieri
@mkoeppe mkoeppe added this to the sage-10.3 milestone Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant