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
maint(docs): use xelatex for latex docs build (1.7 branch) #20309
Conversation
The PDF docs build was failing with ! Fatal fontspec error: "cannot-use-pdftex" ! ! The fontspec package requires either XeTeX or LuaTeX. sympy#20307 This commit changes the release script to use xelatex instead of pdflatex.
✅ Hi, I am the SymPy bot (v161). I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.7. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
Codecov Report
@@ Coverage Diff @@
## 1.7 #20309 +/- ##
=============================================
+ Coverage 75.710% 75.719% +0.008%
=============================================
Files 674 674
Lines 174162 174162
Branches 41098 41098
=============================================
+ Hits 131859 131874 +15
+ Misses 36557 36538 -19
- Partials 5746 5750 +4 |
make | ||
# Build with xelatex because pdflatex can not be used with fontspec. | ||
# https://github.com/sympy/sympy/issues/20307 | ||
export LATEXMKOPTS='-xelatex -silent' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The option -silent
is not necessary for documentation release builds because it only fixes the issues with logs for travis tests.
But -xelatex
option can be bound to the makefile itself if there is possible way to do so, However, I'd be against using string substitutions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think modifying the Makefile is better than having it not work unless you set an environment variable. Although if we had make pdf
that ran the other Makefile we could just set this variable in the top-level Makefile (not substitutions necessary).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, it may be possible to set this via a latexmkrc file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm open to any suggestions. For now I'm waiting to see if the release script passes with this change.
What I don't understand is why this fontspec issue has come up now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make latex
generates another Makefile that builds the pdf. Prior to the commit I referenced, the Makefile was automatically modified with sed to use xelatex, but after @sylee957's PR, the sed was removed and you have to set an environment variable manually.
We really should figure out how to get the release script running on a CI so that we can avoid breaking it. It may also help to somehow share the scripts used on Travis and the release script so that updates to one also change the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main manual step involved in running the release script us fixing the authors in .mailmap and AUTHORS.
Sorting that out would mean that every PR from a newer contributor has to update those files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you run make latex
it says that make latexpdf
does this, but that doesn't exist in the Makefile. We should probably update our Makefile to the latest standard sphinx Makefile (i.e., what you'd get from sphinx-quickstart in the latest version of Sphinx). We also include a lot of custom stuff, so this might not be straightforward.
This didn't work:
|
I think for xonsh it's |
That's what I'm trying in #20316 |
The PDF docs build was failing with
#20307
This commit changes the release script to use xelatex instead of
pdflatex.
References to other Issues or PRs
Brief description of what is fixed or changed
Other comments
Release Notes