CI: Add PDF doc building #35169
Closed
CI: Add PDF doc building #35169
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## develop #35169 +/- ##
===========================================
- Coverage 88.62% 88.61% -0.01%
===========================================
Files 2148 2148
Lines 398855 398855
===========================================
- Hits 353480 353459 -21
- Misses 45375 45396 +21 see 27 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
This run failed with |
|
It builds |
tobiasdiez
reviewed
Feb 22, 2023
tobiasdiez
changed the title
|
That would of course also be a solution, but I didn't want to make the time to HTML deployment longer. |
|
How long would the pdf docbuild alone take? The html docbuild is already taken quite some time, adding a bit more shouldn't matter that much... |
|
About 30 minutes wall time, according to the log: |
|
Then I would prefer to add it directly in the docbuild workflow. |
|
Half an hour is pretty long when people are making edits to the documentation, I'd say |
|
The current workflow already takes 1.5h, so it's not like that you get realtime feedback. The main build also takes 2h or more, so both workflows would be finished at about the same time. |
|
I don't think this should be triggered automatically. It's a slow workflow, and only rarely pdf building gets broken (assuming html building works). |
|
The workflow is not slower than "build". And since we were upgraded to GH Teams, we have many more parallel runners. |
really? it's an extra 30min to 1h of wall clock to making html docs - as it would pull in a good chunk of TeXLive, and as it is something to be run (for quite a while) after html docs have been built. |
|
Yes, really. Except currently it's building incrementally from an ancient image and so it recompiles more than the other workflows (Build&Test, Build documentation). |
|
OK - let's see how it goes. If it proves to be a CI bottleneck we should turn it off by default, then. |
|
Can the results from the PDF build be uploaded to netify, just like it's done for html? |
|
I still think it should be just an added step in the (html) docbuild workflow. Then you can also easily upload it to netlify. |
|
I don't think there's value in making the pdfs available. All that matters is that it builds. |
|
you still want to know whether an iffy character you used in a docstring and broke pdf building, looks correct after a workaround |
|
Let's get this in please. |
|
OK, pdf upload may be done in another PR |
tobiasdiez
suggested changes
Mar 24, 2023
There was a problem hiding this comment.
The added build is failing (after the merge at least).
Moreover, it will never be possible to upload the pdf to netlify from a second workflow (since the publish workflow can only be triggered by one workflow). So this is a serious design flaw and should be fixed. Finally, the uploaded artefact is 500mb big. Are these only the doc pdfs? I cannot check since the download fails for me at a couple of mb. But this is another point why it would be important to upload the pdfs to netlify since no one will download 500mb to just check a symbol in the compiled pdf.
|
The only purpose of this workflow is to flag an error before it hits Volker's queue. |
|
"serious design flaw" - no |
|
It does fail, with a strange tex error. |
dimpase
reviewed
Mar 25, 2023
|
I chose this outdated one because it is close to one of @vbraun's buildbot configurations. |
|
Blast from the past... Fedora 30 has been dead for almost 3 years. |
|
Documentation preview for this PR is ready! 🎉 |
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 1, 2023
<!-- 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
vbraun
pushed a commit
that referenced
this pull request
Oct 8, 2023
<!-- 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📚 Description
We add another workflow that is run on every PR: building PDF docs.
📝 Checklist
⌛ Dependencies