CI docbuild: Do not rebuild sagelib from scratch #36473

mkoeppe · 2023-10-17T04:58:55Z

Reworking #35652 as discussed in #36469 (comment)

Also applying the changes made in #35866 (cosmetic improvements to the Actions logs of doc-build.yml) to doc-build-pdf.yml

📝 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

kwankyu · 2023-10-17T05:53:07Z

.github/workflows/doc-build.yml

@@ -124,7 +124,7 @@ jobs:
          set -ex
          export SAGE_USE_CDNS=yes
          mv /sage/local/share/doc/sage/html/en/.git /sage/.git-doc
-          make doc-clean doc-uninstall sagelib-clean && git clean -fx src/sage
+          make doc-clean doc-uninstall
          mkdir -p /sage/local/share/doc/sage/html/en/ && mv /sage/.git-doc /sage/local/share/doc/sage/html/en/.git
          ./config.status && make doc-html


Not relevant here, but I wonder what /sage/local/share/doc/sage/html/en/.git is, and why it is important to preserve this across doc-clean doc-uninstall.

This is how we keep track of the changes in the built documentation, compared to what is contained in the Docker image.

Ah, I see. You use this information to create "changes" file list (as seen below) of the doc.

By the way, the "changes" list seems not working properly...

the "changes" list seems not working properly...

This is from a recent change to CDN use for mathjax -- see the diff: https://deploy-preview-36473--sagemath-tobias.netlify.app/html.diff

I've opened #36475 for fixing this

github-actions · 2023-10-17T05:53:48Z

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

kwankyu · 2023-10-17T06:20:16Z

Now the doc build is very fast. Very nice.

kwankyu

LGTM.

kwankyu · 2023-10-17T06:23:12Z

Thanks to Tobias too!

tobiasdiez · 2023-10-17T06:24:48Z

Currently make doc html reinstalls sage_conf and sage_lib. Would it be easy to extract this to a new job to make the output easier to understand?

tobiasdiez · 2023-10-17T10:16:59Z

Another suggestion for improvement: use continue-on-error: true on the incremental step. Then a failure of this step doesn't cause the whole job/workflow to fail.

mkoeppe · 2023-10-17T17:36:32Z

Currently make doc html reinstalls sage_conf and sage_lib.
Would it be easy to extract this to a new job to make the output easier to understand?

They are essentially noisy no-ops, but we could use make sagemath_doc_html-build-deps in a previous step and make sagemath_doc_html-no-deps instead of make doc-html.
Let's keep this in mind for #36446

(Edit: Now done as part of #36475)

mkoeppe · 2023-10-17T17:38:03Z

Another suggestion for improvement: use continue-on-error: true on the incremental step. Then a failure of this step doesn't cause the whole job/workflow to fail.

Good idea, but obsolete if #36446 works well enough.

…changes   From sagemath#36473 (comment)    ### 📝 Checklist     - [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  - Depends on sagemath#36473 (merged here) - Depends on sagemath#36442 (merged here)  URL: sagemath#36475 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee

@kwankyu

…arate jobs for pyright, build, modularized tests, long tests    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   ### 📝 Checklist     - [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  - Depends on sagemath#36938 (merged here) - Depends on sagemath#36951 (merged here) - Depends on sagemath#37351 (merged here) - Depends on sagemath#37750 (merged here)  URL: sagemath#36498 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe

@kwankyu

…arate jobs for pyright, build, modularized tests, long tests    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   ### 📝 Checklist     - [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  - Depends on sagemath#36938 (merged here) - Depends on sagemath#36951 (merged here) - Depends on sagemath#37351 (merged here) - Depends on sagemath#37750 (merged here)  URL: sagemath#36498 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe

@kwankyu

…arate jobs for pyright, build, modularized tests, long tests    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   ### 📝 Checklist     - [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  - 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)  URL: sagemath#36498 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe

CI docbuild: Do not rebuild sagelib from scratch

fe5d8f2

mkoeppe added the c: scripts label Oct 17, 2023

mkoeppe self-assigned this Oct 17, 2023

mkoeppe mentioned this pull request Oct 17, 2023

Revert to non-incremental docbuild #36469

Closed

5 tasks

mkoeppe added the s: needs review label Oct 17, 2023

mkoeppe requested review from kwankyu and tobiasdiez October 17, 2023 05:28

mkoeppe mentioned this pull request Oct 17, 2023

CI: Merge workflows into build.yml #36446

Closed

5 tasks

kwankyu reviewed Oct 17, 2023

View reviewed changes

kwankyu approved these changes Oct 17, 2023

View reviewed changes

kwankyu added s: positive review and removed s: needs review labels Oct 17, 2023

mkoeppe mentioned this pull request Oct 17, 2023

.github/workflows/doc-build.yml: Repair display of changes #36475

Merged

5 tasks

vbraun merged commit 4260e44 into sagemath:develop Oct 21, 2023
33 of 41 checks passed

github-actions bot removed the s: positive review label Oct 21, 2023

mkoeppe added this to the sage-10.2 milestone Oct 21, 2023

mkoeppe mentioned this pull request Oct 21, 2023

CI build, doc-build: Run containers explicitly, separate jobs for pyright, build, modularized tests, long tests #36498

Merged

5 tasks

mkoeppe mentioned this pull request Oct 22, 2023

CI docbuild: Run containers explicitly with docker run, docker exec #36508

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI docbuild: Do not rebuild sagelib from scratch #36473

CI docbuild: Do not rebuild sagelib from scratch #36473

mkoeppe commented Oct 17, 2023 •

edited

kwankyu Oct 17, 2023

mkoeppe Oct 17, 2023

kwankyu Oct 17, 2023 •

edited

kwankyu Oct 17, 2023

mkoeppe Oct 17, 2023

mkoeppe Oct 17, 2023

github-actions bot commented Oct 17, 2023

kwankyu commented Oct 17, 2023

kwankyu left a comment

kwankyu commented Oct 17, 2023

tobiasdiez commented Oct 17, 2023 •

edited

tobiasdiez commented Oct 17, 2023

mkoeppe commented Oct 17, 2023 •

edited

mkoeppe commented Oct 17, 2023

CI docbuild: Do not rebuild sagelib from scratch #36473

CI docbuild: Do not rebuild sagelib from scratch #36473

Conversation

mkoeppe commented Oct 17, 2023 • edited

📝 Checklist

⌛ Dependencies

kwankyu Oct 17, 2023

Choose a reason for hiding this comment

mkoeppe Oct 17, 2023

Choose a reason for hiding this comment

kwankyu Oct 17, 2023 • edited

Choose a reason for hiding this comment

kwankyu Oct 17, 2023

Choose a reason for hiding this comment

mkoeppe Oct 17, 2023

Choose a reason for hiding this comment

mkoeppe Oct 17, 2023

Choose a reason for hiding this comment

github-actions bot commented Oct 17, 2023

kwankyu commented Oct 17, 2023

kwankyu left a comment

Choose a reason for hiding this comment

kwankyu commented Oct 17, 2023

tobiasdiez commented Oct 17, 2023 • edited

tobiasdiez commented Oct 17, 2023

mkoeppe commented Oct 17, 2023 • edited

mkoeppe commented Oct 17, 2023

mkoeppe commented Oct 17, 2023 •

edited

kwankyu Oct 17, 2023 •

edited

tobiasdiez commented Oct 17, 2023 •

edited

mkoeppe commented Oct 17, 2023 •

edited