-
-
Notifications
You must be signed in to change notification settings - Fork 661
Update conda-lock files, rename macOS conda-lock files to match CI Conda, remove unexplained duplicate lock files #37998
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
Conversation
Documentation preview for this PR (built with commit b560f4d; changes) is ready! 🎉 |
|
|
That's with |
|
@gmou3 Here we have run in a strange symbol clash from the Cythonization of GraphicMatroid. See https://stackoverflow.com/questions/22240973/major-and-minor-macros-defined-in-sys-sysmacros-h-pulled-in-by-iterator; would you be able to work around it? |
Shouldn't the CI then work on this ticket if this ticket fixes it? Because it seems like @mkoeppe canceled the run. Should it be this way? |
@vbraun This fixes the "Build&Test using conda": See the green checkmarks. |
@vbraun I've now updated the branch protection rule to make the "Expected -- Waiting for status to be reported" disappear (for conda) |
Thanks, Volker. |
Tests for the 9 platforms running at https://github.com/mkoeppe/sage/actions/runs/9154171460 |
sagemathgh-38033: Avoid conflict with macro `minor` <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Author: @gmou3 Reviewer: @mkoeppe - Split out from sagemath#37998 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [ ] 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 and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#38033 Reported by: Matthias Köppe Reviewer(s):
sagemathgh-38034: `sage.libs.giac`: Compile with std=c++11 <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Fixes sagemath#37998 (comment) - Split out from sagemath#37998 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [ ] 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 and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#38034 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
sagemathgh-38034: `sage.libs.giac`: Compile with std=c++11 <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Fixes sagemath#37998 (comment) - Split out from sagemath#37998 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [ ] 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 and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#38034 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
@vbraun Let's get this in please. |
Labels are being manipulated again here. |
I suppose that @tobiasdiez is being blocked by you so he cannot state what he would like to change in this PR? Apart from that, it would seem to me that he's entitled to set a PR to "needs work" and request changes. Tobias, maybe you can try to share what you would like to change by some other means? |
He previously communicated in the thread https://groups.google.com/g/sage-devel/c/tw6J8vU7IY0 |
Apparently he did not feel that his remarks were addressed sufficiently but I can only speculate. |
sagemathgh-38034: `sage.libs.giac`: Compile with std=c++11 <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Fixes sagemath#37998 (comment) - Split out from sagemath#37998 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [ ] 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 and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#38034 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee
… macOS x86_64 ('macos-13')
Confirmed. "Test run before this commit" has the same failures but in the baseline set from this PR merged as a CI Fix. Tobias' argument (no upgrade that introduces new failures) is sound but based on false facts. |
Is this
new? |
As we are here, may I ask (not a thing to do here): Could we put all these environment-xxx files into, say
in the developer guide? |
some restructuring like this could be a good idea, but not for this PR |
I think this is another sporadic failure, which I've probably seen before. |
Another question. This is also not to be done here: It seems that the script |
It was said at the time that maybe one day these lock files would be updated automatically by a GH Actions workflow. |
Regarding
As far as I can see, there is nothing "oriented by github actions" in naming lock files. But the comment may be to the old version of this PR... |
Yes, I thought something like it. Perhaps |
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.
I am positive to this PR. We need this PR, ASAP to fix conda ci.
Thanks. |
The renaming is done so that the fix here can take effect on PR runs via the "CI Fix" mechanism (as changes to the workflow files themselves cannot take effect).
And update by running
.github/workflows/conda-lock-update.py
for all architectures.Workflow matrix variables now contain
-latest
, to add tests for the macOS x86_64 platform (runner name:macos-13
)Removes some unexplained duplications (both
arm
andarm64
files for macOS, bothaarch
andaarch64
for Linux)Fixes CI Conda is broken because
macos-latest
is now Apple Silicon #37997Author: @mkoeppe, @gmou3
📝 Checklist
⌛ Dependencies
minor
#38033 (split out from here)sage.libs.giac
: Compile with std=c++11 #38034 (split out from here)