pkgs/sage-conf_pypi: Repair after #36400, #36435#36533
pkgs/sage-conf_pypi: Repair after #36400, #36435#36533vbraun merged 11 commits intosagemath:developfrom
pkgs/sage-conf_pypi: Repair after #36400, #36435#36533Conversation
|
Let's get this in please for 10.2 |
|
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 |
|
Let's try if declaring that in |
|
Still failed. Looking at tox.ini, i see Which |
|
It's going to be the sage-config that comes with the installation of sage-conf in the virtual environment. |
I reinstalled homebrew tox. It still has |
|
Without this PR, I get |
… basepython of .pkg
Yes, same type of failure. |
|
I've made a change and updated the test instructions. |
|
This is the starting part of the log and it is going on. It seems promising! |
|
The is the ending part 🎉 |
|
Testing also with |
|
|
It tests that |
|
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. |
sagemathgh-36135: `sage -fixdistributions` <!-- ^^^^^ 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
sagemathgh-36135: `sage -fixdistributions` <!-- ^^^^^ 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
sagemathgh-36561: `pkgs/sage-conf`: Move metadata from `setup.cfg` to `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
sagemathgh-36561: `pkgs/sage-conf`: Move metadata from `setup.cfg` to `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