-
-
Notifications
You must be signed in to change notification settings - Fork 482
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
CI: Merge open blocker PRs in all CI workflows + other improvements #36338
Conversation
…w macos.yml, update macOS, Xcode versions
0c0b7bd
to
32b9f5a
Compare
32b9f5a
to
e4025c1
Compare
1f12c3c
to
d7c07c0
Compare
e4105ab
to
37c96cf
Compare
… is not needed in the container
37c96cf
to
3949cc7
Compare
are we not reinventing the wheel here, by any chance? |
Yes, @dimpase, I have considered many options, including Merge Queues. But moving to using Merge Queues is not actionable for the project; the release manager would have to adopt a changed workflow. As I wrote in #36338 (comment):
|
I don't see how a merge queue with blockers (or positively reviewed blockers) is different in functionality from this PR. |
Well, Merge Queues are attached to a branch protection rule for a branch. A PR can only be merged in one branch, namely its target branch. So doing anything like this would interfere with the release manager's work. |
@dimpase You may want to try out merge queues with https://github.com/dimpase/primecountpy/pulls ... |
I don't think it's a repo which needs them. |
But it's harmless to play with them there. |
Thanks for the benevolent action, @vbraun! |
<!-- Please provide a concise, informative and self-explanatory title. --> <!-- Don't put issue numbers in the title. Put it in the Description below. --> <!-- For example, instead of "Fixes sagemath#12345", use "Add a new method to multiply two integers" --> ### 📚 Description Follow-up on sagemath#35169. The container `sage-docker-fedora-31-maximal-with-targets` used for the PDF docbuild turned out to be not reliable. Here we replace it by `ubuntu-focal-standard-with-targets` and install texlive in it. We also copy over the incremental build from doc-build.yml and the method to get CI fixes from blocker tickets from sagemath#36338. This workflow is currently disabled in sagemath/sage. Example run: https://github.com/mkoeppe/sage/actions/runs/6318741659/job/17158468016 <!-- 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". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x ]`. --> - [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#36338 - Depends on sagemath#36348 <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#35373 Reported by: Matthias Köppe Reviewer(s): Dima Pasechnik
…conda/centos/archlinux system packages <!-- ^^^^^ 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 --> Fixes breakage from sagemath#36338 seen in https://github.com/sagemath/sage/acti ons/runs/6332061118/job/17206153694: The 2nd stage (`...-with-targets`) does not find the container from the 1st stage (`...-with-targets-pre`) because they are keyed to the commit sha. Fixed here by keying it to the sha before merging the CI fixes. Also fixing the following failures: - `conda-forge-minimal` (as seen in https://github.com/sagemath/sage/act ions/runs/6207962004/job/16858998561); failed because the python3 depcheck packages were missing. Fixed now - check with `tox -e docker- conda-forge-minimal -- config.status` - `centos-stream-9-python3.9-minimal` (https://github.com/sagemath/sage/ actions/runs/6207962004/job/16858997494), `almalinux-9-python3.11-minimal` (https://github.com/sagemath/sage/actio ns/runs/6207962004/job/16858997663) and `-standard`; fixes sagemath#34786 - `archlinux-standard` - libgiac was missing in at least one run Also adding a few more uses of GH Action output groups for convenience. Tests running at https://github.com/mkoeppe/sage/actions/workflows/ci- linux.yml <!-- 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#36358 Reported by: Matthias Köppe Reviewer(s): Dima Pasechnik
<!-- Please provide a concise, informative and self-explanatory title. --> <!-- Don't put issue numbers in the title. Put it in the Description below. --> <!-- For example, instead of "Fixes #12345", use "Add a new method to multiply two integers" --> ### 📚 Description Follow-up on #35169. The container `sage-docker-fedora-31-maximal-with-targets` used for the PDF docbuild turned out to be not reliable. Here we replace it by `ubuntu-focal-standard-with-targets` and install texlive in it. We also copy over the incremental build from doc-build.yml and the method to get CI fixes from blocker tickets from #36338. This workflow is currently disabled in sagemath/sage. Example run: https://github.com/mkoeppe/sage/actions/runs/6318741659/job/17158468016 <!-- 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 #12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x ]`. --> - [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 - #12345: short description why this is a dependency - #34567: ... --> - Depends on #36338 - Depends on #36348 <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #35373 Reported by: Matthias Köppe Reviewer(s): Dima Pasechnik
…ntos/archlinux system packages <!-- ^^^^^ 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 #1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Fixes breakage from #36338 seen in https://github.com/sagemath/sage/acti ons/runs/6332061118/job/17206153694: The 2nd stage (`...-with-targets`) does not find the container from the 1st stage (`...-with-targets-pre`) because they are keyed to the commit sha. Fixed here by keying it to the sha before merging the CI fixes. Also fixing the following failures: - `conda-forge-minimal` (as seen in https://github.com/sagemath/sage/act ions/runs/6207962004/job/16858998561); failed because the python3 depcheck packages were missing. Fixed now - check with `tox -e docker- conda-forge-minimal -- config.status` - `centos-stream-9-python3.9-minimal` (https://github.com/sagemath/sage/ actions/runs/6207962004/job/16858997494), `almalinux-9-python3.11-minimal` (https://github.com/sagemath/sage/actio ns/runs/6207962004/job/16858997663) and `-standard`; fixes #34786 - `archlinux-standard` - libgiac was missing in at least one run Also adding a few more uses of GH Action output groups for convenience. Tests running at https://github.com/mkoeppe/sage/actions/workflows/ci- linux.yml <!-- 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 #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 - #12345: short description why this is a dependency - #34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #36358 Reported by: Matthias Köppe Reviewer(s): Dima Pasechnik
…arate jobs for pyright, build, modularized tests, long tests <!-- ^^^^^ 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? --> Running the containers explicitly, instead of using the declarative `container:` feature of GH Actions gives us more control: - we can create more space on the host if necessary; we just scraped by an out of space condition in sagemath#36473 / sagemath#36469 (comment) - we can run some operations outside of the container but in the same job; this will make the separate "Get CI fixes" jobs unnecessary, addressing the cosmetic concerns from sagemath#36338 (comment), sagemath#36349 - it enables caching between the various workflows (as first discussed in sagemath#36446). We split out static type checking with pyright into its separate workflow: - **pyright.yml**: As static checking does not need a build of the Sage library, for PRs that do not make any changes to packages, there's nothing to build, and the workflow gives a fast turnaround just after 10 minutes. For applying the CI fixes from blocker tickets, this workflow uses the technique favored in sagemath#36349. The workflow **build.yml** first launches a job: - **test-new:** It builds incrementally (using a tox-generated `Dockerfile` and https://github.com/marketplace/actions/build-and-push- docker-images) and does the quick incremental test. This completes within 10 to 20 minutes when there's no change. After this is completed, more jobs are launched: - **test-mod:** It again builds incrementally and tests a modularized distribution. Later (with more from sagemath#35095), more jobs will be added to this matrix job for other distributions. - **test-long:** It again builds incrementally and runs the long test. The workflows **doc-build.yml** and **doc-build-pdf.yml** again build incrementally and then build the documentation. The diffing code for the HTML documentation now runs in the host, not the container, which simplifies things. (To show that diffing still works, we include a small change to the Sage library.) Splitting the workflow jobs implements @kwankyu's suggestion from: - sagemath#35652 (comment) (Fixes sagemath#35814) The steps "again builds incrementally" actually use the GH Actions cache, https://docs.docker.com/build/ci/github-actions/cache/#cache- backend-api. When nothing is cached and the 3 workflows are launched in parallel, they may each build the same thing. But when there's congestion that leads to the workflows to be run serially, the 2nd and 3rd workflow will be able to use the cache from the 1st workflow. This elasticity may be helpful in reducing congestion. There is a rather small per-project limit of 10 GB for this cache, so we'll have to see how effectively caching works in practice. See https://github.com/sagemath/sage/actions/caches <!-- 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: ... --> - Depends on sagemath#36938 (merged here) - Depends on sagemath#36951 (merged here) - Depends on sagemath#37351 (merged here) - Depends on sagemath#37750 (merged here) <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36498 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe
…arate jobs for pyright, build, modularized tests, long tests <!-- ^^^^^ 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? --> Running the containers explicitly, instead of using the declarative `container:` feature of GH Actions gives us more control: - we can create more space on the host if necessary; we just scraped by an out of space condition in sagemath#36473 / sagemath#36469 (comment) - we can run some operations outside of the container but in the same job; this will make the separate "Get CI fixes" jobs unnecessary, addressing the cosmetic concerns from sagemath#36338 (comment), sagemath#36349 - it enables caching between the various workflows (as first discussed in sagemath#36446). We split out static type checking with pyright into its separate workflow: - **pyright.yml**: As static checking does not need a build of the Sage library, for PRs that do not make any changes to packages, there's nothing to build, and the workflow gives a fast turnaround just after 10 minutes. For applying the CI fixes from blocker tickets, this workflow uses the technique favored in sagemath#36349. The workflow **build.yml** first launches a job: - **test-new:** It builds incrementally (using a tox-generated `Dockerfile` and https://github.com/marketplace/actions/build-and-push- docker-images) and does the quick incremental test. This completes within 10 to 20 minutes when there's no change. After this is completed, more jobs are launched: - **test-mod:** It again builds incrementally and tests a modularized distribution. Later (with more from sagemath#35095), more jobs will be added to this matrix job for other distributions. - **test-long:** It again builds incrementally and runs the long test. The workflows **doc-build.yml** and **doc-build-pdf.yml** again build incrementally and then build the documentation. The diffing code for the HTML documentation now runs in the host, not the container, which simplifies things. (To show that diffing still works, we include a small change to the Sage library.) Splitting the workflow jobs implements @kwankyu's suggestion from: - sagemath#35652 (comment) (Fixes sagemath#35814) The steps "again builds incrementally" actually use the GH Actions cache, https://docs.docker.com/build/ci/github-actions/cache/#cache- backend-api. When nothing is cached and the 3 workflows are launched in parallel, they may each build the same thing. But when there's congestion that leads to the workflows to be run serially, the 2nd and 3rd workflow will be able to use the cache from the 1st workflow. This elasticity may be helpful in reducing congestion. There is a rather small per-project limit of 10 GB for this cache, so we'll have to see how effectively caching works in practice. See https://github.com/sagemath/sage/actions/caches <!-- 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: ... --> - Depends on sagemath#36938 (merged here) - Depends on sagemath#36951 (merged here) - Depends on sagemath#37351 (merged here) - Depends on sagemath#37750 (merged here) <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36498 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe
…arate jobs for pyright, build, modularized tests, long tests <!-- ^^^^^ 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? --> Running the containers explicitly, instead of using the declarative `container:` feature of GH Actions gives us more control: - we can create more space on the host if necessary; we just scraped by an out of space condition in sagemath#36473 / sagemath#36469 (comment) - we can run some operations outside of the container but in the same job; this will make the separate "Get CI fixes" jobs unnecessary, addressing the cosmetic concerns from sagemath#36338 (comment), sagemath#36349 - it enables caching between the various workflows (as first discussed in sagemath#36446). We split out static type checking with pyright into its separate workflow: - **pyright.yml**: As static checking does not need a build of the Sage library, for PRs that do not make any changes to packages, there's nothing to build, and the workflow gives a fast turnaround just after 10 minutes. For applying the CI fixes from blocker tickets, this workflow uses the technique favored in sagemath#36349. The workflow **build.yml** first launches a job: - **test-new:** It builds incrementally (using a tox-generated `Dockerfile` and https://github.com/marketplace/actions/build-and-push- docker-images) and does the quick incremental test. This completes within 10 to 20 minutes when there's no change. After this is completed, more jobs are launched: - **test-mod:** It again builds incrementally and tests a modularized distribution. Later (with more from sagemath#35095), more jobs will be added to this matrix job for other distributions. - **test-long:** It again builds incrementally and runs the long test. The workflows **doc-build.yml** and **doc-build-pdf.yml** again build incrementally and then build the documentation. The diffing code for the HTML documentation now runs in the host, not the container, which simplifies things. (To show that diffing still works, we include a small change to the Sage library.) Splitting the workflow jobs implements @kwankyu's suggestion from: - sagemath#35652 (comment) (Fixes sagemath#35814) The steps "again builds incrementally" actually use the GH Actions cache, https://docs.docker.com/build/ci/github-actions/cache/#cache- backend-api. When nothing is cached and the 3 workflows are launched in parallel, they may each build the same thing. But when there's congestion that leads to the workflows to be run serially, the 2nd and 3rd workflow will be able to use the cache from the 1st workflow. This elasticity may be helpful in reducing congestion. There is a rather small per-project limit of 10 GB for this cache, so we'll have to see how effectively caching works in practice. See https://github.com/sagemath/sage/actions/caches <!-- 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: ... --> - Depends on sagemath#36938 (merged here) - Depends on sagemath#36951 (merged here) - Depends on sagemath#37351 (merged here) - Depends on sagemath#37750 (merged here) - Depends on sagemath#37277 (merged here) <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36498 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe
The new script
.ci/merge-fixes.sh
uses the GitHub CLI to retrieve open PRs with the "blocker" label from sagemath/sage and tries to merge them into the tested branch, ignoring any PRs that give merge failures.We call this script in all CI workflows. Thus,
will take effect in the CI workflows of all PRs.
This can be seen in the runs of the workflows of the present PR (section "Merge CI fixes from sagemath/sage").
Additionally,
debian-trixie
, which failed because a system package is no longer availablemacos-conda-maximal
by removing some fictitious distros/conda.txt files📝 Checklist
⌛ Dependencies