-
-
Notifications
You must be signed in to change notification settings - Fork 491
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
pkgs/sage-conf_pypi
: Repair after #36400, #36435
#36533
Conversation
Let's get this in please for 10.2 |
52f47e0
to
1f30d72
Compare
rebased away from #36523 |
I followed the instructions of https://github.com/sagemath/sage#alternative-installation-using-pypi. I got
|
With this PR, I got
|
Looks like your system Python is 3.12 already. That won't work because sagemath-environment (like all of our distributions) still declares "python-requires < 3.12". |
My system python is python 3.11, which is at /ust/local/bin/python I added this to tox.ini
and ran
|
About |
You can try running |
I got
|
Hmmm... this looks suspicious: |
But the real problem is that it's still using Python 3.12 |
1f30d72
to
19d1ea7
Compare
It tests that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works well!
Thanks very much! |
Marking it as a blocker so that it gets merged in the 10.2 release. |
This is idea of patching "on-the-fly" seems really broken. From https://github.com/sagemath/sage/actions/runs/6785240332/job/18443170385?pr=35148
There seems to a very easy solution to "CI is broken on releases" without having to automerge PRs: if @vbraun would make a PR when a new release is ready and then only cut the release if CI passes. What would be the downside of that? I mean, 10.2.rc0 was released three days ago, and we already have 11 patches applied? wtf? To make things more egregious, in a different run: https://github.com/sagemath/sage/actions/runs/6785240365/job/18443164416?pr=35148 has all 11 merges applied. |
Looks like a bug in the script that applies the patches using |
They are the blockers for the 10.2 release. As documented in https://doc.sagemath.org/html/en/developer/review.html#the-release-process |
Manual check out and merge has no problem. |
I think we need an independent solution. |
We cannot blame the idea itself from this instance of failure. Fortunately, there is no disaster from the failure. |
Then overloading the same label for "blockers for 10.2 release" and "hotfix patches" doesn't seem like a good idea (even if you disagree with my opinion that "hotfix patches" is a bad idea). |
Perhaps, but because of the peculiarities of our release process I think it's quite appropriate that people who work on PRs in the release candidate stage get their PRs tested with what will be merged in the release. |
I think I've just found & fixed what went wrong when applying this PR as a fix. |
Documentation preview for this PR (built with commit 11a8d35; changes) is ready! 🎉 |
This reverts commit 19d1ea7.
This is an argument for only applying positive-reviewed blockers as hotfixes. |
I give up. Because of the peculiarities of sagemath CI, I'll just keep ignoring it. |
We introduced hot-fixes mechanism to avoid false negatives in ci. There should be zero possibility that hot-fixes themselves cause false negatives. |
<!-- ^^^^^ 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 --> This is a new maintenance command for adding/updating `# sage_setup: distribution` directives at the top of source files. Based on a discussion in sagemath#35884 (comment) <!-- 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". --> - Part of: sagemath#29705 - Cherry-picked from: sagemath#35095 <!-- 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: ... --> - Depends on sagemath#36533 (CI fix) <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36135 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe
<!-- ^^^^^ 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 --> This is a new maintenance command for adding/updating `# sage_setup: distribution` directives at the top of source files. Based on a discussion in sagemath#35884 (comment) <!-- 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". --> - Part of: sagemath#29705 - Cherry-picked from: sagemath#35095 <!-- 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: ... --> - Depends on sagemath#36533 (CI fix) <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36135 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe
… `pyproject.toml` <!-- ^^^^^ 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? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> - In part split out from sagemath#36489 - Part of sagemath#33577 <!-- 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. - [ ] 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: ... --> - Depends on sagemath#36533 (which changes setup.cfg, merged here) - Depends on sagemath#36885 <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36561 Reported by: Matthias Köppe Reviewer(s): François Bissey
… `pyproject.toml` <!-- ^^^^^ 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? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> - In part split out from sagemath#36489 - Part of sagemath#33577 <!-- 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. - [ ] 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: ... --> - Depends on sagemath#36533 (which changes setup.cfg, merged here) - Depends on sagemath#36885 <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36561 Reported by: Matthias Köppe Reviewer(s): François Bissey
This repairs the installation method described in https://github.com/sagemath/sage#alternative-installation-using-pypi broken by changes in #36400, #36435.
We also expand the tests for this distribution in its tox.ini.
Run
(cd pkgs/sage-conf_pypi && tox -v -v -e py311)
📝 Checklist
⌛ Dependencies