Skip to content
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: Update Linux platforms / runners, remove ubuntu-trusty, opensuse-{15.3,15.4} #37351

Merged
merged 9 commits into from
Apr 12, 2024

Conversation

mkoeppe
Copy link
Member

@mkoeppe mkoeppe commented Feb 15, 2024

We increase parallelization according to the upgraded Linux runners on GH Actions (4 CPUs) and add some newer platforms.

fedora-40 fails because we reject GCC 14, that's now tracked in #37353.

Update (Apr 2024):

  • ubuntu-trusty reaches EOL, we drop it. Also remove some past-EOL OpenSUSE Leap; see Meta-ticket: Remove support for outdated distros #32074
  • also reducing the list of systems tested with --enable-system-site-packages because there is no interest in doing the porting work
  • updating the default platform (used by the incremental build workflows) from ubuntu-focal to ubuntu-jammy.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • 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

Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reduce the scope before applying the blocker label.

.github/workflows/build.yml Show resolved Hide resolved
@@ -25,6 +25,7 @@ on:
"ubuntu-jammy",
"ubuntu-lunar",
"ubuntu-mantic",
"ubuntu-noble",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove some old systems to keep the maintenance effort and the cpu time (relatively) constant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we will continue to test what is supported.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean supported by SageMath or the Ubuntu organization?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

If the latter, I am not sure we should worry about "pro support". But maybe we have a policy about that that I am not aware of?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saraedum I mean supported by SageMath.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saraedum for reference, what is supported by SageMath and why we drop support is tracked in #32074.

.github/workflows/docker.yml Show resolved Hide resolved
@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 18, 2024

Stop this nonsense, Tobias.

@mkoeppe mkoeppe requested a review from kwankyu February 19, 2024 07:20
@tobiasdiez
Copy link
Contributor

Removing blocker as there are problems with other PRs, see #37428 for more details.

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 23, 2024

Removing blocker as there are problems with other PRs, see #37428 for more details.

That PR has absolutely no details.

Mass removal of the "blocker" label on 9 PRs is not OK. @roed314 @jhpalmieri @williamstein

And even when removing "blocker" is necessary in a case to fix a problem that it causes on other PRs, the mildest form of label change should be used -- reducing to "critical".

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 25, 2024

@vbraun please merge

@vbraun
Copy link
Member

vbraun commented Feb 25, 2024

I don't see anything here that should be a blocker (but if somebody reviews it in time then I'll merge it of course)

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 26, 2024

@vbraun Just a bit hard to see whether the release is ready when the CI is broken, that's all. And of course we want to be able to point users to the list of supported platforms when the release is made, without having to guess.

@mkoeppe mkoeppe changed the title CI: Update Linux platforms / runners CI: Update Linux platforms / runners, remove ubuntu-trusty, opensuse-{15.3,15.4} Apr 1, 2024
@@ -111,18 +111,11 @@ jobs:
["ubuntu-jammy",
"ubuntu-lunar",
"ubuntu-mantic",
"debian-bullseye",
"debian-bookworm",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orlitzky The changes in this file reduce the platforms tested with --enable-system-site-packages. Is this change OK with you?
(See https://github.com/sagemath/sage/actions/runs/8501570896 for the tests with 10.4.beta1)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes no difference to me. Having e.g. debian-bullseye in the CI is useful only to people who want to use system packages on Debian bullseye. I don't think we have any -- not developers, anyway.

Copy link

github-actions bot commented Apr 6, 2024

Documentation preview for this PR (built with commit bce5774; changes) is ready! 🎉

@tornaria
Copy link
Contributor

tornaria commented Apr 7, 2024

Documentation preview for this PR (built with commit bce5774; changes) is ready! 🎉

If you are annoyed by this useless bot spam, please 👍 on #37740.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 7, 2024

Let's get this in please.

@saraedum Not sure if this should still be "disputed". By passage of time, we are targeting a different release, and I've made numerous changes to the branch since.

@saraedum
Copy link
Member

saraedum commented Apr 7, 2024

@mkoeppe I think this is currently how our rules work. If a PR is disputed, it remains disputed. That's maybe something to change but I think this is not for this PR to discuss.

Current Votes (2-2)

👍 @mkoeppe (author)
👍 @kwankyu #37351 (comment)
👎 @tobiasdiez #37351 (comment)
👎 @dimpase #37351 (comment)

@roed314
Copy link
Contributor

roed314 commented Apr 10, 2024

+1 from me.

@mkoeppe mkoeppe requested a review from jhpalmieri April 10, 2024 04:12
@mkoeppe mkoeppe mentioned this pull request Apr 10, 2024
5 tasks
@jhpalmieri
Copy link
Member

+1. Looks fine to me.

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 10, 2024

Thanks @roed314 @jhpalmieri.

This brings the tally to 4-2, setting to "positive review".

@vbraun vbraun merged commit 33b15fb into sagemath:develop Apr 12, 2024
14 of 15 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone Apr 12, 2024
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 18, 2024
…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
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 20, 2024
…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
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 25, 2024
…plex to 7, papilo to 2.2, pyscipopt to 5, onetbb to 2021.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". -->

https://www.scipopt.org/doc-9.0.0/html/RN90.php

- [x] scipopt/SCIP-SDP#12
- [ ] scipopt/soplex#35
- [x] needs pyscipopt update

### 📝 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.
- [ ] 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. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->

- Depends on sagemath#37237
- Depends on sagemath#37351
- Depends on sagemath#36970
- Depends on sagemath#37392
    
URL: sagemath#37494
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request May 2, 2024
…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
@mkoeppe mkoeppe deleted the ci_linux_2024_02 branch May 9, 2024 07:39
vbraun pushed a commit to vbraun/sage that referenced this pull request May 24, 2024
    
<!-- ^^^^^
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 -->
For this upgrade we tighten the check for GMP so that 6.2.1 is accepted
but 6.2.0 is rejected. For example `ubuntu-focal-standard` brings 6.2.0,
we accept it, and then FLINT complains:
https://github.com/mkoeppe/flint2/actions/runs/7718792281/job/2104079370
6#step:11:5395

We also change the upper bound on system `flint` accepted by
`configure`. Closes sagemath#37838.

<!-- 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: ...
-->
- Depends on sagemath#37484
- Depends on sagemath#37495 (merged and reverted here)
- Depends on sagemath#37570 (merged here)
- Depends on sagemath#37351 (merged here)
- Depends on sagemath#37726 (merged here to resolve merge conflict)

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#37203
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: scripts disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ p: critical / 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants