Migrate pdf-doc build to meson #40597
Conversation
|
Documentation preview for this PR (built with commit 4487fc2; changes) is ready! 🎉 |
|
this still doesn't reenable live doc, no? remark: it doesn't speed up the build (understandable, since most of the time are spent on running lualatex) |
I didn't touch the live docs here, and leave this to #40586.
There could be some speed gains if we convince sphinx to export a dep file (maybe similar to https://gitlab.com/qemu-project/qemu/-/blob/master/docs/sphinx/depfile.py?ref_type=heads) so that meson can watch the source files for each doc target and then incrementally rebuild the docs. That should be faster than sphinx' incrementally rebuild logic. |
|
that may not be necessary, the bottleneck is still in latexmk. TeX's dependency tracker is quite fickle, I don't think we will ever be able to make a meson replacement for latexmk. But either way, likely out of scope here. (latexmk's incremental tracking is not 100% correct either, but better than nothing.) |
|
Right, the idea with depfile was also more for the html build. Honestly, I don't care about the pdf. I think the website is fully sufficient and only a handful of people are actually looking at those pdfs. The only real purpose is to catch latex errors... |
sagemathgh-40597: Migrate pdf-doc build to meson <!-- ^ 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". --> Migrate the pdf-doc building to meson. For this, a bug in the builder is fixed that prevented running Make against the sphinx-generated files when invoked via meson. Along the way, the output in case of an error of the make script is improved. Generated pdfs can be found at https://github.com/sagemath/sage/actions/ runs/17005745386/artifacts/3779209846. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] 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 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#40597 Reported by: Tobias Diez Reviewer(s):
<!-- ^ 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". --> Migrate the pdf-doc building to meson. For this, a bug in the builder is fixed that prevented running Make against the sphinx-generated files when invoked via meson. Along the way, the output in case of an error of the make script is improved. Generated pdfs can be found at https://github.com/sagemath/sage/actions/ runs/17005745386/artifacts/3779209846. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] 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 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#40597 Reported by: Tobias Diez Reviewer(s):
<!-- ^ 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". --> Migrate the pdf-doc building to meson. For this, a bug in the builder is fixed that prevented running Make against the sphinx-generated files when invoked via meson. Along the way, the output in case of an error of the make script is improved. Generated pdfs can be found at https://github.com/sagemath/sage/actions/ runs/17005745386/artifacts/3779209846. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] 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 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#40597 Reported by: Tobias Diez Reviewer(s):
|
maybe you should merge latest develop first (PDF build only starts failing there). |
pyproject.toml
Outdated
| docs-pdf = [ | ||
| "pkg:generic/latexmk", | ||
| "pkg:generic/lualatex", | ||
| ] |
There was a problem hiding this comment.
is free_fonts and xindy not needed to be included here?
| @@ -265,13 +265,37 @@ def pdf(self): | |||
| with open(tex_file, 'w') as f: | |||
| f.write(ref) | |||
|
|
|||
| make_target = "cd '%s' && $MAKE %s && mv -f *.pdf '%s'" | |||
| error_message = "failed to run $MAKE %s in %s" | |||
| make_cmd = os.environ.get('MAKE', 'make') | |||
There was a problem hiding this comment.
can the user set MAKE='make -j5'? if so the command below would fail.
(it's encouraged that they set MAKEFLAGS=-j5 instead, but still…)
tools/update-conda.py
Outdated
| @@ -177,6 +178,7 @@ def get_dependencies(pyproject_toml: Path, python: str, platform: str) -> set[st | |||
| "sagemath_giac", | |||
| "pynormaliz", # due to https://github.com/sagemath/sage/issues/40214 | |||
| "latte-integrale", # due to https://github.com/sagemath/sage/issues/40216 | |||
| "pkg:generic/lualatex", # texlive-core doesn't include lualatex | |||
There was a problem hiding this comment.
will this by default include texlive into the conda environment? Since most users/developers of sage don't want to build documentation and tex live is very heavy, I'd rather not.
There was a problem hiding this comment.
Good point. I've now uncommented the info in pyproject.toml so that it doesn't get included in the conda env (or any other env that reads the "external" info for that matter). But it's still there for people to look at it.
|
All I can say is the component that builds pdf on new pull request is working well, but what step have you taken to ensure that the things that need/does not need to be executed on new push to develop branch are also working? |
latexmk needs to be told to ignore luatex-cache - which it apparently shares across a number of processes, so it's a potential source of race conditions. See e.g. https://tex.stackexchange.com/questions/347980/how-to-make-latexmk-ignore-certain-input-files-as-dependencies without it I keep getting |
|
What I don't understand is where to put |
|
@user202729 Thanks for the review. I've addressed your comments now.
There is nothing special about the "push to develop" situation. The pdf docs are built in the same way and published as github action output. That output is currently not used at all (except if someone downloads it manually). @dimpase These parallelization issues look very much unrelated to the meson migration. I don't understand why they occur now, except if for some reason the old make build actually run the pdf compilation in serial, but meson now does it in parallel. |
try running also, how are you running it (since apparently the way it's ran on the CI at this point doesn't cause that error)? |
this issue is probably due to the recent bump of sphinx to 8.2.3, and the fact that more and more people are moving to TeXLive 2024. I have an impression that sphinx project neglects the LaTeX builds. E.g. their use of luatex85.sty is something LaTeX gurus frown upon: And |
|
I am now trying this on top of #39030, and I guess it will suffer the same shortcoming - absence of a way to clean up docs, but leave the rest (sagelib) in consistent state, ready for re-doing |
|
@tobiasdiez OK, this works on top of #39030, so how about merging this into #39030 ? |
Okay, done. Could you then please review this PR, so that it doesn't delay #39030? |
|
How about just closing it and concentrating on #39030 ? reviewing this alone seems meaningless to me |
|
good news: the build mostly works. bad news… the build occasionally fails. https://github.com/sagemath/sage/actions/runs/17277815071/job/49038947643#step:8:1870 where line 61 is https://github.com/sagemath/sage/actions/runs/17272791004/job/49021777269 happened again in https://github.com/sagemath/sage/actions/runs/17294516739/job/49089360398#step:8:6657 looks tough to debug. Might be related to the LuaTeX font cache mentioned by dimpase above. Maybe one can just clean-up and rerun on failure or something. |
<!-- ^ 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". --> Replace the old setuptools-based build by the new meson-based one in sage-the-distro. Delete most of the old stuff that is no longer needed now. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] 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 and checked the documentation preview. ### ⌛ Dependencies - sagemath#40133 - sagemath#39973 - sagemath#40071 - sagemath#40597 <!-- 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#39030 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik, François Bissey, Tobias Diez
sagemathgh-39030: Use meson in sage-the-distro <!-- ^ 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". --> Replace the old setuptools-based build by the new meson-based one in sage-the-distro. Delete most of the old stuff that is no longer needed now. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] 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 and checked the documentation preview. ### ⌛ Dependencies - sagemath#40133 - sagemath#39973 - sagemath#40071 - sagemath#40597 <!-- 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#39030 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik, François Bissey, Tobias Diez
Migrate the pdf-doc building to meson. For this, a bug in the builder is fixed that prevented running Make against the sphinx-generated files when invoked via meson. Along the way, the output in case of an error of the make script is improved.
Generated pdfs can be found at https://github.com/sagemath/sage/actions/runs/17005745386/artifacts/3779209846.
📝 Checklist
⌛ Dependencies