make sagemath_categories-check, make pypi-wheels-check
#36452
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rd wheel file name in venv/var/lib/sage/scripts/PKG_BASE/spkg-requirements.txt
…re running spkg-install
…script spkg-postinstcheck if it exists; new target SPKG-check
mkoeppe
force-pushed
the
wheel_requirements
branch
from
3614d14
to
07fbc3c
Compare
mkoeppe
force-pushed
the
wheel_requirements
branch
from
07fbc3c
to
af6d0ad
Compare
mkoeppe
changed the title
make sagemath_categories-checkmake sagemath_categories-check, make pypi-wheels-check
kwankyu
reviewed
Oct 12, 2023
|
|
||
| - name: Test changed files (sage -t --new) | ||
| run: | | ||
| # We run tests with "sage -t --new"; this only tests the uncommitted changes. |
There was a problem hiding this comment.
if: always() && steps.worktree.outcome == 'success'?
There was a problem hiding this comment.
thanks, i've made a similar change in 8224cb0
|
Thus previously worked, but will not work after this PR. Instead we will use Am I right? If so, then do we need to update some relevant part of the developer guide? |
No, it still works the same way. The makefile invokes the |
Closed
5 tasks
kwankyu
reviewed
Oct 13, 2023
| fi | ||
| chmod +x spkg-install | ||
| fi | ||
|
|
There was a problem hiding this comment.
Is this never used, and so removed?
There was a problem hiding this comment.
Yes, that's right. It was a leftover from the abandoned support for "old-style packages"
|
OK. It looks good as far as I can tell. It works well as described. |
kwankyu
approved these changes
Oct 13, 2023
|
Thank you! |
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
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 pull request
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
5 tasks
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Nov 27, 2023
…agemath#36452 <!-- ^^^^^ 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 --> <!-- Why is this change required? What problem does it solve? --> sagemath#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 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#36737 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Nov 30, 2023
…agemath#36452 <!-- ^^^^^ 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 --> <!-- Why is this change required? What problem does it solve? --> sagemath#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 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#36737 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We add
maketargets for testing wheel-building script packages that have already been installed. For example:The target
make pypi-wheels-checkcalls 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 #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.
Previously, this information was only known during the execution of the
spkg-installscript.In a follow-up, we can address #30956 by delaying the wheel installation to spkg-postinst.
📝 Checklist
⌛ Dependencies