Message-ID: <20250819030239.41a2e97f@foz.lan>
Date: Tue, 19 Aug 2025 03:02:39 +0200
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Akira Yokosawa <akiyks@...il.com>
Cc: Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 00/11] Fix PDF doc builds on major distros

Em Tue, 19 Aug 2025 08:26:31 +0900
Akira Yokosawa <akiyks@...il.com> escreveu:

> On Mon, 18 Aug 2025 11:07:58 -0600, Jonathan Corbet wrote:
> > Akira Yokosawa <akiyks@...il.com> writes:
> >   
> >> Ah, I have finally understood what 5/11 is trying to do.
> >>
> >> Its changelog mainly talks about an issue you saw after adding options
> >> to xindy in that same commit, and you added
> >>
> >>    \newfontfamily\headingfont{DejaVu Serif}
> >>
> >> to resolve it.
> >>
> >> Current changelog didn't make sense at all for me.
> >>
> >> Can you please reword it and make it easier to follow?
> >>
> >> With that, feel free to add my
> >>
> >> Reviewed-by: Akira Yokosawa <akiyks@...il.com>  
> > 
> > So, if I have managed to understand this conversation, this reword is
> > all we need to get this series merged..?  
> 
> Well, after some thoughts on the conversation took place on xindy,
> I think I have to withdraw my Reviewed-by: tag.
> 
> I was the one who was totally confused.
> 
> Please disregard it.
> 
> Mauro, I can't review on 5/11 unless you provide me exact steps to reproduce
> the font discovery issue you said you have observed under debian at 4/11 of
> this series.  That is, without assuming your other series of build-wrapper.

See below.

> The build-wrapper should be upper compatible with the current way of
> running sub-make, without any change in conf.py.

The build-wrapper series doesn't make any changes on conf.py:

	 $ git diff pdfdocs..sphinx-build-wrapper --stat

	 .pylintrc                                |   2 +-
	 Documentation/Makefile                   | 142 +++-------
	 Documentation/sphinx/parallel-wrapper.sh |  33 ---
	 scripts/jobserver-exec                   |  88 ++-----
	 scripts/lib/jobserver.py                 | 149 +++++++++++
	 scripts/sphinx-build-wrapper             | 767 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
	 scripts/sphinx-pre-install               |  14 +-
	 7 files changed, 994 insertions(+), 201 deletions(-)

It also doesn't change the build logic: it just moves it to the
script. The only difference was with regards to serializing the build,
as I forgot to take it into account when I wrote it. However, I sent
already a patch addressing it.

In summary, all the sphinx-build-wrapper series do is:

- move code from Documentation/Makefile into a python script;
- get rid of a shell script;
- split scripts/jobserver-exec into an exec and a library;
- change the output of doc build to produce a summary at the end,
  returning an error code only if one or more PDF files were not
  built;
- allow calling the script directly without requiring a makefile;
- add a couple of optional command line parameters to help debugging.

On the other hand, the pdf series diffstat is:

$ git diff netlink_v10..pdfdocs --stat
 Documentation/Makefile     |   4 ++--
 Documentation/conf.py      | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------
 scripts/sphinx-pre-install |  41 ++++++++++++++++++++++++++++++++---------
 3 files changed, 96 insertions(+), 55 deletions(-)

The Makefile change is actually a fix:

	diff --git a/Documentation/Makefile b/Documentation/Makefile
	index 820f07e0afe6..2ed334971acd 100644
	--- a/Documentation/Makefile
	+++ b/Documentation/Makefile
	@@ -63,2 +63,2 @@ endif #HAVE_LATEXMK
	-PAPEROPT_a4     = -D latex_paper_size=a4
	-PAPEROPT_letter = -D latex_paper_size=letter
	+PAPEROPT_a4     = -D latex_elements.papersize=a4paper
	+PAPEROPT_letter = -D latex_elements.papersize=letterpaper

So, the entire series do:

1. Fix paper size define, as latex_paper_size doesn´t exist since
   Sphinx 1.x. It got replaced by latex_elements.papersize;

2. Broken package dependencies related to PDF builds on several
   distros inside sphinx-pre-install;

3. It properly construct the name of pdfdocs to be built when
   SPHINXDIRS is used;

4. It prevents the usage of T1 fontenc fonts, which could be
   caused by either one of those two reasons:

	a) \sphinxhyphen{}
	b) index build

I got those when checking what packages were required on some
distros (Debian, Ubuntu, Mageia, openMandriva, Gentoo).

Depending on the distro, the Sphinx version and the installed
packages, the *.tex file ends adding:

	\usepackage[T1]{fontenc}

This is incompatible with xelatex with utf-8 fonts as T1 is not UTF-8
ready.

with (a), \sphinxhyphen{} logic - together with babel, ends
requiring pzdr.tfm.

To do the conf.py, I did some changes for it to better align
with:

	https://www.sphinx-doc.org/en/master/latex.html#module-latex

As the original settings were written for Sphinx 1.4.1 and
had very little fixes since then.

The changes are:

1. Added:
	"fontenc": ""

   to prevent a possible default of having:
	r'\usepackage[T1]{fontenc}'

2. Place font settings at the right place for more modern Spinx versions.
   If you look there, font specs shall be under "fontpkg", not under
   "preamble":

+    'fontpkg': dedent(r'''
+        \usepackage{fontspec}
+        \setmainfont{DejaVu Serif}
+        \setsansfont{DejaVu Sans}
+        \setmonofont{DejaVu Sans Mono}

-    # Additional stuff for the LaTeX preamble.
-    "preamble": """
-        % Use some font with UTF-8 support with XeLaTeX
-        \\usepackage{fontspec}
-        \\setsansfont{DejaVu Sans}
-        \\setromanfont{DejaVu Serif}
-        \\setmonofont{DejaVu Sans Mono}
-    """,
 }

3. Added a fix where some LaTeX modules were trying to use T1 fontenc:

        \newfontfamily\headingfont{DejaVu Serif}

4. I used dedent() to remove weird whitespaces at the beginning inside
   .tex files - no functional changes - this is just cosmetic at the
   output;

5. I moved preamble from a separate part of conf.py:

+    "preamble": dedent(r"""
+        % Load kerneldoc specific LaTeX settings
+        \input{kerneldoc-preamble.sty}
+    """),

-# Load kerneldoc specific LaTeX settings
-latex_elements["preamble"] += """
-        % Load kerneldoc specific LaTeX settings
-        \\input{kerneldoc-preamble.sty}
-"""

6. I solved an issue were xindy was trying to include T1 fontenc.
   while here, I also added a parameter recommended at sphinx Latex
   configuration doc:

+    "passoptionstopackages": dedent(r"""
+        \PassOptionsToPackage{svgnames}{xcolor}
+        % Avoid encoding troubles when creating indexes
+        \PassOptionsToPackage{xindy}{language=english,codepage=utf8,noautomatic}
+    """),

(the xcolor came from the documentation)

The above doesn't load xindy; it just ensures that codepage=utf8 is used
if it is used;

7. some distros didn't build pdf due to the lack of an index file.
   So, I added this:

+    'printindex': r'\footnotesize\raggedright\printindex',

which is also suggested at
	 https://www.sphinx-doc.org/en/master/latex.html#module-latex

So, basically, the changes at conf.py better align it with Sphinx
documentation, and were experimentally tested: I didn't find
any regressions on it, and it fixed PDF builds for several distros.

Did you find any regressions?

> I don't think this is too much to ask. 

Actually, it is... finding a way for you to reproduce it would 
require me to start from scratch and redo everything again,
hoping that I'll do exactly the same way. It took me 2 entire
weeks to do it the first time. 

I very much prefer dropping patch 05/11 and keep PDF broken
than spending 2 weeks redoing it.

> Moving both the goal post

What do you mean by "goal post"?

The pdfdocs series is there just to make sure that pdf builds
will work on most distros, as currently they don't build on
several of them.

> and the build script at the same time is the wrong thing to do
> in my opinion.

Well, we can postpone the PDF series - or at least the patches
you find problematic.

The build script cleanup is important, though as it affects
a series I have afterwards addressing some build issues 
affecting only the media subsystem, which is the only user
of kernel-include tag.

Thanks,
Mauro

Powered by blists - more mailing lists