Sphinx pre v3 -- removing support

Sphinx pre v3 -- removing support

[Adam Turner]

Hi,

I was pointed in the direction of this mailing list by Jani Nikula in
[1]_, who said:

> Thanks for the ping. I was heavily involved in the early days of
> converting the kernel to use Sphinx, but I haven't closely followed
> the recent developments. Basically I think I'd also be inclined to
> push for much higher minimum Sphinx version requirements than what
> the kernel currently has. The minimum at the moment is v1.7.9 
> (or v2.4.4 for PDF). It's difficult to maintain support for a wide
> range of Sphinx versions. Perhaps the best bet would be to mail the
> kernel documentation list at linux-doc@vger.kernel.org and Cc 
> Jonathan Corbet corbet@lwn.net to try to reach an understanding on
> the recommended minimum version and version ranges that makes sense
> for both parties to support. HTH.

This email is an attempt to do that. 

From Sphinx's perspective, we'd like to remove long-deprecated code.
What is a good solution here for both sides? The intertial option is
for us to delay the deprecation by another major version (removal is
currently scheduled for Sphinx 6 (2023-05), and we are currently 
releasing a major version every May.

Jani reports that you still require Sphinx 1.7.9 -- I have no 
investment in the documentation development of the kernel, but he
rightly notes that is quite an old version -- released 3 years and 9
months ago.

Please would you let me know if there is anything required on our
(Sphinx's) end that would let us drop the "pre v3" support gracefully.

A

Thanks,
Adam

_[1]: https://github.com/sphinx-doc/sphinx/pull/10471#discussion_r888962744

Re: Sphinx pre v3 -- removing support

[Jonathan Corbet]

Adam Turner <aaturnerpython@outlook.com> writes:

> Hi,
>
> I was pointed in the direction of this mailing list by Jani Nikula in
> [1]_, who said:
>
>> Thanks for the ping. I was heavily involved in the early days of
>> converting the kernel to use Sphinx, but I haven't closely followed
>> the recent developments. Basically I think I'd also be inclined to
>> push for much higher minimum Sphinx version requirements than what
>> the kernel currently has. The minimum at the moment is v1.7.9 
>> (or v2.4.4 for PDF). It's difficult to maintain support for a wide
>> range of Sphinx versions. Perhaps the best bet would be to mail the
>> kernel documentation list at linux-doc@vger.kernel.org and Cc 
>> Jonathan Corbet corbet@lwn.net to try to reach an understanding on
>> the recommended minimum version and version ranges that makes sense
>> for both parties to support. HTH.
>
> This email is an attempt to do that. 
>
> From Sphinx's perspective, we'd like to remove long-deprecated code.
> What is a good solution here for both sides? The intertial option is
> for us to delay the deprecation by another major version (removal is
> currently scheduled for Sphinx 6 (2023-05), and we are currently 
> releasing a major version every May.
>
> Jani reports that you still require Sphinx 1.7.9 -- I have no 
> investment in the documentation development of the kernel, but he
> rightly notes that is quite an old version -- released 3 years and 9
> months ago.
>
> Please would you let me know if there is anything required on our
> (Sphinx's) end that would let us drop the "pre v3" support gracefully.

We've been meaning to raise the minimum version for a bit.  Going to v3
might be a bit of a stretch, though.  I still do most of my test builds
with 2.4.3 just because Sphinx got so....much........slower with 3.0.
I've not yet had a chance to try out 5.0 to see if that helps things,
that's on my list to do soon.

Thanks,

jon

Re: Sphinx pre v3 -- removing support

[Adam Turner]

> We've been meaning to raise the minimum version for a bit.  Going to v3
> might be a bit of a stretch, though.  I still do most of my test builds
> with 2.4.3 just because Sphinx got so....much........slower with 3.0.
> I've not yet had a chance to try out 5.0 to see if that helps things,
> that's on my list to do soon.

Sphinx 5.0 should be faster, I changed the logic to cache and reuse
the core publisher objects [1]_. I haven't compared to 2.4.3 though.

A

_[1]: https://github.com/sphinx-doc/sphinx/pull/10337

Re: Sphinx pre v3 -- removing support

[Akira Yokosawa]

Hi Adam,

Please find a couple of inline questions below from a casual user of
Sphinx via kernel documentation build tools.

On Fri, 3 Jun 2022 14:13:27 +0000,
Adam Turner wrote:
> Hi,
> 
> I was pointed in the direction of this mailing list by Jani Nikula in
> [1]_, who said:
> 
>> Thanks for the ping. I was heavily involved in the early days of
>> converting the kernel to use Sphinx, but I haven't closely followed
>> the recent developments. Basically I think I'd also be inclined to
>> push for much higher minimum Sphinx version requirements than what
>> the kernel currently has. The minimum at the moment is v1.7.9 
>> (or v2.4.4 for PDF). It's difficult to maintain support for a wide
>> range of Sphinx versions. Perhaps the best bet would be to mail the
>> kernel documentation list at linux-doc@vger.kernel.org and Cc 
>> Jonathan Corbet corbet@lwn.net to try to reach an understanding on
>> the recommended minimum version and version ranges that makes sense
>> for both parties to support. HTH.
> 
> This email is an attempt to do that. 
> 
> From Sphinx's perspective, we'd like to remove long-deprecated code.
> What is a good solution here for both sides? The intertial option is
> for us to delay the deprecation by another major version (removal is
> currently scheduled for Sphinx 6 (2023-05), and we are currently 
> releasing a major version every May.

So, can we assume that there won't be any backward-incompatible
behavior changes in Sphinx due to the removal of those long-deprecated
code?

Or do you mean that after the release of Sphinx 6, pre v3 Sphinx
will be removed in the PyPI repository?

These questions might be too obvious for you but I have no idea what
you mean by "removing support".

       Thanks, Akira

> 
> Jani reports that you still require Sphinx 1.7.9 -- I have no 
> investment in the documentation development of the kernel, but he
> rightly notes that is quite an old version -- released 3 years and 9
> months ago.
> 
> Please would you let me know if there is anything required on our
> (Sphinx's) end that would let us drop the "pre v3" support gracefully.
> 
> A
> 
> Thanks,
> Adam
> 
> _[1]: https://github.com/sphinx-doc/sphinx/pull/10471#discussion_r888962744
> 

Re: Sphinx pre v3 -- removing support

[Matthew Wilcox]

On Fri, Jun 03, 2022 at 08:21:39AM -0600, Jonathan Corbet wrote:
> Adam Turner <aaturnerpython@outlook.com> writes:
> 
> > Hi,
> >
> > I was pointed in the direction of this mailing list by Jani Nikula in
> > [1]_, who said:
> >
> >> Thanks for the ping. I was heavily involved in the early days of
> >> converting the kernel to use Sphinx, but I haven't closely followed
> >> the recent developments. Basically I think I'd also be inclined to
> >> push for much higher minimum Sphinx version requirements than what
> >> the kernel currently has. The minimum at the moment is v1.7.9 
> >> (or v2.4.4 for PDF). It's difficult to maintain support for a wide
> >> range of Sphinx versions. Perhaps the best bet would be to mail the
> >> kernel documentation list at linux-doc@vger.kernel.org and Cc 
> >> Jonathan Corbet corbet@lwn.net to try to reach an understanding on
> >> the recommended minimum version and version ranges that makes sense
> >> for both parties to support. HTH.
> >
> > This email is an attempt to do that. 
> >
> > From Sphinx's perspective, we'd like to remove long-deprecated code.
> > What is a good solution here for both sides? The intertial option is
> > for us to delay the deprecation by another major version (removal is
> > currently scheduled for Sphinx 6 (2023-05), and we are currently 
> > releasing a major version every May.
> >
> > Jani reports that you still require Sphinx 1.7.9 -- I have no 
> > investment in the documentation development of the kernel, but he
> > rightly notes that is quite an old version -- released 3 years and 9
> > months ago.
> >
> > Please would you let me know if there is anything required on our
> > (Sphinx's) end that would let us drop the "pre v3" support gracefully.
> 
> We've been meaning to raise the minimum version for a bit.  Going to v3
> might be a bit of a stretch, though.  I still do most of my test builds
> with 2.4.3 just because Sphinx got so....much........slower with 3.0.
> I've not yet had a chance to try out 5.0 to see if that helps things,
> that's on my list to do soon.

We'd need to coordinate with kernel.org's automated build of the
documentation.  I believe Konstantin handles that.  With pip, I imagine
he can install whatever version is needed.

There's a bug I've been meaning to track down & report where _some_ links
are broken when building with the Sphinx natively installed on my system
(Debian 4.3.2-1).  I haven't bothered because (a) life is short and (b)
it's not affecting the kernel.org build.  If we're going to ask
kernel.org to move to a newer version of Sphinx, we should make sure
that the links won't be broken on whatever version we pick.

An example:
<span class="kt"><span class="pre">void</span></span><span class="w"> </span><span class="p"><span class="pre">*</span></span><span class="sig-name descname"><span class="n"><span class="pre">kmap_local_folio</span></span></span><span class="sig-paren">(</span><span class="k"><span class="pre">struct</span></span><span class="w"> </span><a class="reference internal" href="#c.kmap_local_folio" title="folio"><span class="n"><span class="pre">folio</span></span></a><span class="w"> </span><span class="p"><span class="pre">*</span></span><span class="n"><span class="pre">folio</span></span>, <span class="n"><span class="pre">size_t</span></span><span class="w"> </span><span class="n"><span class="pre">offset</span></span><span class="sig-paren">)</span><a class="headerlink" href="#c.kmap_local_folio" title="Permalink to this definition">¶</a><br /></dt>

Other than that being a big pile of html, that <a href> around 'folio'
should be a link to struct folio and not back to the c.kmap_local_folio
anchor.

I appreciate this is not a great bug report, but I find the entire
build system beyond my comprehension.

Re: Sphinx pre v3 -- removing support

[Adam Turner]

>> From Sphinx's perspective, we'd like to remove long-deprecated code.
>> What is a good solution here for both sides? The intertial option is
>> for us to delay the deprecation by another major version (removal is
>> currently scheduled for Sphinx 6 (2023-05), and we are currently 
>> releasing a major version every May.

> So, can we assume that there won't be any backward-incompatible
> behavior changes in Sphinx due to the removal of those long-deprecated
> code?

I'm referring to removing support for the "c_allow_pre_v3", 
"c_warn_on_allowed_pre_v3", configuration options [1]_, and the 
associated support for still parsing the pre v3 syntax in the C 
domain [2]_. This means that pre v3 syntax in reStructuredText files
would not work with Sphinx 6 onwards.

> Or do you mean that after the release of Sphinx 6, pre v3 Sphinx
> will be removed in the PyPI repository?

No releases will be removed from PyPI, but if pre v3 syntax is still
used, Sphinx 6.0 would fail to properly parse it.

> These questions might be too obvious for you but I have no idea what
> you mean by "removing support".

A

_[1]: https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-c_allow_pre_v3
_[2]: https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#the-c-domain

Re: Sphinx pre v3 -- removing support

[Adam Turner]

> There's a bug I've been meaning to track down & report where _some_ links
> are broken when building with the Sphinx natively installed on my system
> (Debian 4.3.2-1).  I haven't bothered because (a) life is short and (b)
> it's not affecting the kernel.org build.  If we're going to ask
> kernel.org to move to a newer version of Sphinx, we should make sure
> that the links won't be broken on whatever version we pick.

> An example:
> <span class="kt"><span class="pre">void</span></span><span class="w"> </span><span class="p"><span class="pre">*</span></span><span class="sig-name descname"><span class="n"><span class="pre">kmap_local_folio</span></span></span><span class="sig-paren">(</span><span class="k"><span class="pre">struct</span></span><span class="w"> </span><a class="reference internal" href="#c.kmap_local_folio" title="folio"><span class="n"><span class="pre">folio</span></span></a><span class="w"> </span><span class="p"><span class="pre">*</span></span><span class="n"><span class="pre">folio</span></span>, <span class="n"><span class="pre">size_t</span></span><span class="w"> </span><span class="n"><span class="pre">offset</span></span><span class="sig-paren">)</span><a class="headerlink" href="#c.kmap_local_folio" title="Permalink to this definition">¶</a><br /></dt>

> Other than that being a big pile of html, that <a href> around 'folio'
> should be a link to struct folio and not back to the c.kmap_local_folio
> anchor.

> I appreciate this is not a great bug report, but I find the entire
> build system beyond my comprehension.

Do you have the reST source behind this rendered HTML? I can then try
and find a minimal reproducer and see if we can fix within Sphinx.

A

Re: Sphinx pre v3 -- removing support

[Matthew Wilcox]

On Fri, Jun 03, 2022 at 03:30:29PM +0000, Adam Turner wrote:
> > There's a bug I've been meaning to track down & report where _some_ links
> > are broken when building with the Sphinx natively installed on my system
> > (Debian 4.3.2-1).  I haven't bothered because (a) life is short and (b)
> > it's not affecting the kernel.org build.  If we're going to ask
> > kernel.org to move to a newer version of Sphinx, we should make sure
> > that the links won't be broken on whatever version we pick.
> 
> > An example:
> > <span class="kt"><span class="pre">void</span></span><span class="w"> </span><span class="p"><span class="pre">*</span></span><span class="sig-name descname"><span class="n"><span class="pre">kmap_local_folio</span></span></span><span class="sig-paren">(</span><span class="k"><span class="pre">struct</span></span><span class="w"> </span><a class="reference internal" href="#c.kmap_local_folio" title="folio"><span class="n"><span class="pre">folio</span></span></a><span class="w"> </span><span class="p"><span class="pre">*</span></span><span class="n"><span class="pre">folio</span></span>, <span class="n"><span class="pre">size_t</span></span><span class="w"> </span><span class="n"><span class="pre">offset</span></span><span class="sig-paren">)</span><a class="headerlink" href="#c.kmap_local_folio" title="Permalink to this definition">¶</a><br /></dt>
> 
> > Other than that being a big pile of html, that <a href> around 'folio'
> > should be a link to struct folio and not back to the c.kmap_local_folio
> > anchor.
> 
> > I appreciate this is not a great bug report, but I find the entire
> > build system beyond my comprehension.
> 
> Do you have the reST source behind this rendered HTML? I can then try
> and find a minimal reproducer and see if we can fix within Sphinx.

Alas, I don't.  I don't even know if this is a Sphinx bug or if it's a
bug in one of the kernel addons.  It's generated by scripts/kernel-doc
from:

/**
 * kmap_local_folio - Map a page in this folio for temporary usage
 * @folio: The folio containing the page.
 * @offset: The byte offset within the folio which identifies the page.
 *
...
 */
static inline void *kmap_local_folio(struct folio *folio, size_t offset);

I see an intermediate file in
Documentation/output/_sources/vm/highmem.rst.txt
but that doesn't include the output from scripts/kernel-doc.

This is why I've been reluctant to report it; I lack enough
understanding to be useful :-(

Re: Sphinx pre v3 -- removing support

[Konstantin Ryabitsev]

On Fri, Jun 03, 2022 at 04:22:32PM +0100, Matthew Wilcox wrote:
> We'd need to coordinate with kernel.org's automated build of the
> documentation.  I believe Konstantin handles that.  With pip, I imagine
> he can install whatever version is needed.

Correct. We currently have:

    $ pip show Sphinx
    Name: Sphinx
    Version: 2.4.5

In the setup, I have it being installed as Sphinx~=2.0 with a comment:

    # Don't use Sphinx-3.x until upstream is happy using it 

I'm happy to upgrade that to a newer version if that condition no longer
applies.

-K

Re: Sphinx pre v3 -- removing support

[Jani Nikula]

On Fri, 03 Jun 2022, Adam Turner <aaturnerpython@outlook.com> wrote:
>>> From Sphinx's perspective, we'd like to remove long-deprecated code.
>>> What is a good solution here for both sides? The intertial option is
>>> for us to delay the deprecation by another major version (removal is
>>> currently scheduled for Sphinx 6 (2023-05), and we are currently 
>>> releasing a major version every May.
>
>> So, can we assume that there won't be any backward-incompatible
>> behavior changes in Sphinx due to the removal of those long-deprecated
>> code?
>
> I'm referring to removing support for the "c_allow_pre_v3", 
> "c_warn_on_allowed_pre_v3", configuration options [1]_, and the 
> associated support for still parsing the pre v3 syntax in the C 
> domain [2]_. This means that pre v3 syntax in reStructuredText files
> would not work with Sphinx 6 onwards.
>
>> Or do you mean that after the release of Sphinx 6, pre v3 Sphinx
>> will be removed in the PyPI repository?
>
> No releases will be removed from PyPI, but if pre v3 syntax is still
> used, Sphinx 6.0 would fail to properly parse it.

And that's the crux of the problem. From kernel POV I'd very much prefer
not setting an upper bound for the Sphinx version. I think it's
important to be able to build the documentation using the latest Sphinx,
and gradually iron out the inevitable quirks that arise.

However, if you decide to drop support for pre v3 syntax in Sphinx v6,
and we decide to stick to being able to use pre v3 Sphinx, we can't move
forward to newer versions until we bump the lower bound for the Sphinx
version to v3+. (Or we need to hack around Sphinx version differences in
kernel, but I think that would be best avoided.)

BR,
Jani.


>
>> These questions might be too obvious for you but I have no idea what
>> you mean by "removing support".
>
> A
>
> _[1]: https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-c_allow_pre_v3
> _[2]: https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#the-c-domain
>

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: Sphinx pre v3 -- removing support

[Adam Turner]

>> No releases will be removed from PyPI, but if pre v3 syntax is still
>> used, Sphinx 6.0 would fail to properly parse it.

> And that's the crux of the problem. From kernel POV I'd very much prefer
> not setting an upper bound for the Sphinx version. I think it's
> important to be able to build the documentation using the latest Sphinx,
> and gradually iron out the inevitable quirks that arise.

> However, if you decide to drop support for pre v3 syntax in Sphinx v6,
> and we decide to stick to being able to use pre v3 Sphinx, we can't move
> forward to newer versions until we bump the lower bound for the Sphinx
> version to v3+. (Or we need to hack around Sphinx version differences in
> kernel, but I think that would be best avoided.)

I don't want to be in the position of knowingly breaking the
documentation tooling for the kernel. A strawman of a compromise
would be for us (Sphinx) to delay the removal to Sphinx 7.0, and the
kernel to increase the minimum to Sphinx 3.1 (required for
".. c:namespace::"). That would enable the kernel to gradually update
the syntax over a longer period, as I believe you won't be able to 
use the v3 syntax currently.

Equally, Jonathan said he was hesitant to increase the minimum to 
Sphinx 3, so perhaps that wouldn't work.

A

Re: Sphinx pre v3 -- removing support

[Jonathan Corbet]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> In the setup, I have it being installed as Sphinx~=2.0 with a comment:
>
>     # Don't use Sphinx-3.x until upstream is happy using it 
>
> I'm happy to upgrade that to a newer version if that condition no longer
> applies.

Upstream is "happy" in that it renders the documentation just fine and
we've dealt with the incompatibilities for our modules.  My only
complaint is the speed, but presumably your automated system has no
trouble being patient :)

jon

Re: Sphinx pre v3 -- removing support

[Konstantin Ryabitsev]

On Fri, Jun 03, 2022 at 10:00:42AM -0600, Jonathan Corbet wrote:
> > I'm happy to upgrade that to a newer version if that condition no longer
> > applies.
> 
> Upstream is "happy" in that it renders the documentation just fine and
> we've dealt with the incompatibilities for our modules.  My only
> complaint is the speed, but presumably your automated system has no
> trouble being patient :)

Indeed. So, should I go to 3.x, 4.x, or go all new and shiny with 5.x?

-K

Re: Sphinx pre v3 -- removing support

[Akira Yokosawa]

[+Cc: Mauro]

On Fri, 3 Jun 2022 15:54:33 +0000,
Adam Turner wrote:
>>> No releases will be removed from PyPI, but if pre v3 syntax is still
>>> used, Sphinx 6.0 would fail to properly parse it.
> 
>> And that's the crux of the problem. From kernel POV I'd very much prefer
>> not setting an upper bound for the Sphinx version. I think it's
>> important to be able to build the documentation using the latest Sphinx,
>> and gradually iron out the inevitable quirks that arise.
> 
>> However, if you decide to drop support for pre v3 syntax in Sphinx v6,
>> and we decide to stick to being able to use pre v3 Sphinx, we can't move
>> forward to newer versions until we bump the lower bound for the Sphinx
>> version to v3+. (Or we need to hack around Sphinx version differences in
>> kernel, but I think that would be best avoided.)

I might not be grasping the full context here, but I think the main script of
kernel documentation tool ./scripts/kernel-doc (a perl script) changes its
behavior depending on the target Sphinx version.

Its help text says:

>    Output format modifiers

>       reStructuredText only

>        -sphinx-version VERSION

>                Use the ReST C domain dialect compatible with a specific Sphinx

>                Version.


>
>                If not specified, kernel-doc will auto-detect using the

>                sphinx-build version found on PATH.

So it looks to me like it is already compatible with Sphinx 3.1 and later.

> 
> I don't want to be in the position of knowingly breaking the
> documentation tooling for the kernel. A strawman of a compromise
> would be for us (Sphinx) to delay the removal to Sphinx 7.0, and the
> kernel to increase the minimum to Sphinx 3.1 (required for
> ".. c:namespace::").

Yes, ".. c:namespace::" is actively used in userspace-api documentation.

FYI, see a recent reply from Mauro WRT support of kernel documentation
with different versions of Sphinx at:

  https://lore.kernel.org/linux-doc/20220521114629.6ee9fc06@coco.lan/

        Thanks, Akira


>                       That would enable the kernel to gradually update
> the syntax over a longer period, as I believe you won't be able to 
> use the v3 syntax currently.
> 
> Equally, Jonathan said he was hesitant to increase the minimum to 
> Sphinx 3, so perhaps that wouldn't work.
> 
> A

Re: Sphinx pre v3 -- removing support

[Jani Nikula]

On Sat, 04 Jun 2022, Akira Yokosawa <akiyks@gmail.com> wrote:
> [+Cc: Mauro]
>
> On Fri, 3 Jun 2022 15:54:33 +0000,
> Adam Turner wrote:
>>>> No releases will be removed from PyPI, but if pre v3 syntax is still
>>>> used, Sphinx 6.0 would fail to properly parse it.
>> 
>>> And that's the crux of the problem. From kernel POV I'd very much prefer
>>> not setting an upper bound for the Sphinx version. I think it's
>>> important to be able to build the documentation using the latest Sphinx,
>>> and gradually iron out the inevitable quirks that arise.
>> 
>>> However, if you decide to drop support for pre v3 syntax in Sphinx v6,
>>> and we decide to stick to being able to use pre v3 Sphinx, we can't move
>>> forward to newer versions until we bump the lower bound for the Sphinx
>>> version to v3+. (Or we need to hack around Sphinx version differences in
>>> kernel, but I think that would be best avoided.)
>
> I might not be grasping the full context here, but I think the main script of
> kernel documentation tool ./scripts/kernel-doc (a perl script) changes its
> behavior depending on the target Sphinx version.

That doesn't change my opinion that it would be best avoided! ;)

BR,
Jani.

>
> Its help text says:
>
>>    Output format modifiers
>
>>       reStructuredText only
>
>>        -sphinx-version VERSION
>
>>                Use the ReST C domain dialect compatible with a specific Sphinx
>
>>                Version.
>
>
>>
>>                If not specified, kernel-doc will auto-detect using the
>
>>                sphinx-build version found on PATH.
>
> So it looks to me like it is already compatible with Sphinx 3.1 and later.
>
>> 
>> I don't want to be in the position of knowingly breaking the
>> documentation tooling for the kernel. A strawman of a compromise
>> would be for us (Sphinx) to delay the removal to Sphinx 7.0, and the
>> kernel to increase the minimum to Sphinx 3.1 (required for
>> ".. c:namespace::").
>
> Yes, ".. c:namespace::" is actively used in userspace-api documentation.
>
> FYI, see a recent reply from Mauro WRT support of kernel documentation
> with different versions of Sphinx at:
>
>   https://lore.kernel.org/linux-doc/20220521114629.6ee9fc06@coco.lan/
>
>         Thanks, Akira
>
>
>>                       That would enable the kernel to gradually update
>> the syntax over a longer period, as I believe you won't be able to 
>> use the v3 syntax currently.
>> 
>> Equally, Jonathan said he was hesitant to increase the minimum to 
>> Sphinx 3, so perhaps that wouldn't work.
>> 
>> A

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: Sphinx pre v3 -- removing support

[Jonathan Corbet]

Adam Turner <aaturnerpython@outlook.com> writes:

>> We've been meaning to raise the minimum version for a bit.  Going to v3
>> might be a bit of a stretch, though.  I still do most of my test builds
>> with 2.4.3 just because Sphinx got so....much........slower with 3.0.
>> I've not yet had a chance to try out 5.0 to see if that helps things,
>> that's on my list to do soon.
>
> Sphinx 5.0 should be faster, I changed the logic to cache and reuse
> the core publisher objects [1]_. I haven't compared to 2.4.3 though.

So ... here are a few "make htmldocs" builds that I ran

  VERSION  BUILD TIME
  2.4.3     5:24
  4.1.2    10:51
  4.5.0    10:57
  5.0.1    11:17

So 5.0 has actually regressed from 4.5 in terms of speed, and takes more
than twice the time that 2.4.3 takes.  These slow builds are really
painful when I'm working through the docs patch queue and have to build
things frequently; they are also a disincentive for other developers to
build the docs and make sure they haven't broken anything.

(Interestingly, total CPU time is ~16:45 for 2.4.3, and ~21:45 for the
later versions.  So latter-day Sphinx does use more CPU, but it also
seems to have lost some parallelism.)

I hope you understand why that makes me reluctant to move the minimum
version up to 3.0.  But, looking further at this conversation, I don't
think we need to at this point.  You said:

> I'm referring to removing support for the "c_allow_pre_v3", 
> "c_warn_on_allowed_pre_v3", configuration options [1]_, and the 
> associated support for still parsing the pre v3 syntax in the C 
> domain [2]_. This means that pre v3 syntax in reStructuredText files
> would not work with Sphinx 6 onwards.

Having looked through the all-too-numerous warnings emitted by our docs
build, I don't see any that look like they would be tied to pre-v3
C-domain syntax.  I *believe* that you could remove that support and it
wouldn't bother us, unless there's something I'm missing?

I truly appreciate your contacting us about this - thanks!

jon

Re: Sphinx pre v3 -- removing support

[Jonathan Corbet]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> On Fri, Jun 03, 2022 at 10:00:42AM -0600, Jonathan Corbet wrote:
>> > I'm happy to upgrade that to a newer version if that condition no longer
>> > applies.
>> 
>> Upstream is "happy" in that it renders the documentation just fine and
>> we've dealt with the incompatibilities for our modules.  My only
>> complaint is the speed, but presumably your automated system has no
>> trouble being patient :)
>
> Indeed. So, should I go to 3.x, 4.x, or go all new and shiny with 5.x?

Unless there is some subtle problem in the rendered docs that I'm not
seeing, I don't think that there is any reason to go with the older
releases.  5.0.1 seems to build things just fine, so I'd say go with the
shiniest you can get.

Thanks,

jon

Re: Sphinx pre v3 -- removing support

[Jonathan Corbet]

Matthew Wilcox <willy@infradead.org> writes:

> There's a bug I've been meaning to track down & report where _some_ links
> are broken when building with the Sphinx natively installed on my system
> (Debian 4.3.2-1).  I haven't bothered because (a) life is short and (b)
> it's not affecting the kernel.org build.  If we're going to ask
> kernel.org to move to a newer version of Sphinx, we should make sure
> that the links won't be broken on whatever version we pick.
>
> An example:
> <span class="kt"><span class="pre">void</span></span><span class="w"> </span><span class="p"><span class="pre">*</span></span><span class="sig-name descname"><span class="n"><span class="pre">kmap_local_folio</span></span></span><span class="sig-paren">(</span><span class="k"><span class="pre">struct</span></span><span class="w"> </span><a class="reference internal" href="#c.kmap_local_folio" title="folio"><span class="n"><span class="pre">folio</span></span></a><span class="w"> </span><span class="p"><span class="pre">*</span></span><span class="n"><span class="pre">folio</span></span>, <span class="n"><span class="pre">size_t</span></span><span class="w"> </span><span class="n"><span class="pre">offset</span></span><span class="sig-paren">)</span><a class="headerlink" href="#c.kmap_local_folio" title="Permalink to this definition">¶</a><br /></dt>
>
> Other than that being a big pile of html, that <a href> around 'folio'
> should be a link to struct folio and not back to the c.kmap_local_folio
> anchor.

This is almost certainly our bug, not something in Sphinx.

You can see what our kerneldoc script is generating with a simple:

  scripts/kernel-doc include/linux/highmem.h

Within the output, you'll find the markup for the function in question:

.. c:function:: void * kmap_local_folio (struct folio *folio, size_t offset)

   Map a page in this folio for temporary usage

I am thinking that our automarkup module is getting confused by the
'struct folio' in the prototype there; will try to dig further shortly.

Thanks,

jon

Re: Sphinx pre v3 -- removing support

[Mauro Carvalho Chehab]

Em Fri, 03 Jun 2022 22:05:20 +0300
Jani Nikula <jani.nikula@linux.intel.com> escreveu:

> On Sat, 04 Jun 2022, Akira Yokosawa <akiyks@gmail.com> wrote:
> > [+Cc: Mauro]

Thanks!

> >
> > On Fri, 3 Jun 2022 15:54:33 +0000,
> > Adam Turner wrote:  
> >>>> No releases will be removed from PyPI, but if pre v3 syntax is still
> >>>> used, Sphinx 6.0 would fail to properly parse it.  
> >>   
> >>> And that's the crux of the problem. From kernel POV I'd very much prefer
> >>> not setting an upper bound for the Sphinx version. I think it's
> >>> important to be able to build the documentation using the latest Sphinx,
> >>> and gradually iron out the inevitable quirks that arise.  
> >>   
> >>> However, if you decide to drop support for pre v3 syntax in Sphinx v6,
> >>> and we decide to stick to being able to use pre v3 Sphinx, we can't move
> >>> forward to newer versions until we bump the lower bound for the Sphinx
> >>> version to v3+. (Or we need to hack around Sphinx version differences in
> >>> kernel, but I think that would be best avoided.)  
> >
> > I might not be grasping the full context here, but I think the main script of
> > kernel documentation tool ./scripts/kernel-doc (a perl script) changes its
> > behavior depending on the target Sphinx version.  
> 
> That doesn't change my opinion that it would be best avoided! ;)

Em Fri, 3 Jun 2022 15:27:18 +0000
Adam Turner <aaturnerpython@outlook.com> escreveu:

> I'm referring to removing support for the "c_allow_pre_v3", 
> "c_warn_on_allowed_pre_v3", configuration options [1]_, and the 
> associated support for still parsing the pre v3 syntax in the C 
> domain [2]_. This means that pre v3 syntax in reStructuredText files
> would not work with Sphinx 6 onwards.

If all that it is scheduled for Sphinx 6 is the removal of the
old C domain, this shouldn't be a problem. The kernel-doc has
long gone support to output tags with both pre and post v3 syntaxes.

We also changed the automarkup plugin to allow using v3 C domain
tags when compiling against pre-v3.

Tests required, of course.

-

From my side, there are two points to consider when changing
the minimal release:

- Supporting a version that can build docs 2x faster sounds
  very interesting;
- it would also be interesting to support the native Sphinx
  version that comes with the latest LTS releases,
  As I suspect that bots may benefit from a long-term distros,
  and use the distro-provided signed packages on servers.

  Looking at LTS, what we have is:

	- RHEL 9.0/CentOS 9.0:
	  https://centos.pkgs.org/9-stream/centos-crb-x86_64/python3-sphinx-latex-3.4.3-7.el9.noarch.rpm.html
	  Sphinx 3.4.3
	- Debian 11:
	  Sphinx 3.4.3
	  https://packages.debian.org/bullseye/python3-sphinx
	- Suse 15 SP4:
	  https://scc.suse.com/packages?name=SUSE%20Linux%20Enterprise%20Server&version=15.4&arch=x86_64&query=python3-sphinx&module=
	  Have have both Sphinx 4.2.0 and Sphinx 2.3.1	

  From LTS perspective, it sounds doable to setup the minimal
  version to 3.4, but we would need to adjust the scripts to
  select a different package on Suse, as calling:

	./scripts/sphinx-pre-install --no-virtualenv

  Would recommend installing python3-sphinx package there,
  meaning Sphinx 2.3.1.

So, IMO, we have a couple of alternatives:

1. Change minimal requirement to 2.3:
	- No changes required at sphinx-pre-install's logic;
	- all latest LTS will be supported;
	- pdf will still require a newer version than 2.3.
	- allow "fast builds" with Sphinx < 3;

2. Change minimal requirement to 2.4:
	- no need to check for an specific version for PDF;
	- allow "fast builds" using Sphinx < 3;
	- Changes needed at sphinx-pre-install on Suse logic;

3. Change minimal requirement to 3.4:
	- We can drop backward-compatible logic from kernel-doc
	  and automarkup;
	- all latest LTS will be supported;
	- Changes needed at sphinx-pre-install on Suse logic;
	- No "fast build" suing Sphinx < 3.

On a side note, while Kernel documentation builds with 3.0,
it is not really properly supported, as there are troubles
on C domain there (lots of warnings and broken cross references
are generated there). So, I would avoid setting the minimal 
requirement to 3.0.

Regards,
Mauro

Re: Sphinx pre v3 -- removing support

[Konstantin Ryabitsev]

On Fri, Jun 03, 2022 at 03:50:29PM -0600, Jonathan Corbet wrote:
> > Indeed. So, should I go to 3.x, 4.x, or go all new and shiny with 5.x?
> 
> Unless there is some subtle problem in the rendered docs that I'm not
> seeing, I don't think that there is any reason to go with the older
> releases.  5.0.1 seems to build things just fine, so I'd say go with the
> shiniest you can get.

Okay, https://docs.kernel.org/ is now built with Sphinx-5.0.1. At least, the
timestamps on the files are telling me so, because I can't see any other
changes. :)

-K

Re: Sphinx pre v3 -- removing support

[Matthew Wilcox]

On Mon, Jun 13, 2022 at 11:40:59AM -0400, Konstantin Ryabitsev wrote:
> On Fri, Jun 03, 2022 at 03:50:29PM -0600, Jonathan Corbet wrote:
> > > Indeed. So, should I go to 3.x, 4.x, or go all new and shiny with 5.x?
> > 
> > Unless there is some subtle problem in the rendered docs that I'm not
> > seeing, I don't think that there is any reason to go with the older
> > releases.  5.0.1 seems to build things just fine, so I'd say go with the
> > shiniest you can get.
> 
> Okay, https://docs.kernel.org/ is now built with Sphinx-5.0.1. At least, the
> timestamps on the files are telling me so, because I can't see any other
> changes. :)

Can confirm the bug I mentioned is now present on at least
https://www.kernel.org/doc/html/latest/core-api/mm-api.html#c.folio_add_wait_queue

What should be the link to 'struct folio' is instead a link to
folio_add_wait_queue.

Re: Sphinx pre v3 -- removing support

[Adam Turner]

> Can confirm the bug I mentioned is now present on at least
> https://www.kernel.org/doc/html/latest/core-api/mm-api.html#c.folio_add_wait_queue

> What should be the link to 'struct folio' is instead a link to
> folio_add_wait_queue.

From a skim of the "view source" link, I think it is probably a problem in the kernel-doc directive, offending lines copied below:

.. code-block:: restructuredtext

   Filemap
   -------

   .. kernel-doc:: mm/filemap.c
      :export:

Happy to take a look on the Sphinx side for any regressions we've introduced, though I don't know anything about the kernel's documentation tooling so can't help from that side.

A

Re: Sphinx pre v3 -- removing support

[Matthew Wilcox]

On Mon, Jun 13, 2022 at 04:16:12PM +0000, Adam Turner wrote:
> > Can confirm the bug I mentioned is now present on at least
> > https://www.kernel.org/doc/html/latest/core-api/mm-api.html#c.folio_add_wait_queue
> 
> > What should be the link to 'struct folio' is instead a link to
> > folio_add_wait_queue.
> 
> >From a skim of the "view source" link, I think it is probably a problem in the kernel-doc directive, offending lines copied below:
> 
> .. code-block:: restructuredtext
> 
>    Filemap
>    -------
> 
>    .. kernel-doc:: mm/filemap.c
>       :export:
> 
> Happy to take a look on the Sphinx side for any regressions we've introduced, though I don't know anything about the kernel's documentation tooling so can't help from that side.

Jon's pretty sure it's our bug,

https://lore.kernel.org/linux-doc/87tu91ieiw.fsf@meer.lwn.net/

Our tooling is an unholy mess of perl and python, so it may take some
time to track down.

Re: Sphinx pre v3 -- removing support

[Mauro Carvalho Chehab]

Em Fri, 3 Jun 2022 15:27:18 +0000
Adam Turner <aaturnerpython@outlook.com> escreveu:

> >> From Sphinx's perspective, we'd like to remove long-deprecated code.
> >> What is a good solution here for both sides? The intertial option is
> >> for us to delay the deprecation by another major version (removal is
> >> currently scheduled for Sphinx 6 (2023-05), and we are currently 
> >> releasing a major version every May.  
> 
> > So, can we assume that there won't be any backward-incompatible
> > behavior changes in Sphinx due to the removal of those long-deprecated
> > code?  
> 
> I'm referring to removing support for the "c_allow_pre_v3", 
> "c_warn_on_allowed_pre_v3", configuration options [1]_, and the 
> associated support for still parsing the pre v3 syntax in the C 
> domain [2]_. This means that pre v3 syntax in reStructuredText files
> would not work with Sphinx 6 onwards.
> 
> > Or do you mean that after the release of Sphinx 6, pre v3 Sphinx
> > will be removed in the PyPI repository?  
> 
> No releases will be removed from PyPI, but if pre v3 syntax is still
> used, Sphinx 6.0 would fail to properly parse it.

Adam,

Despite the performance issues with Sphinx > 2.x.x, there is another
reason why the default is to use 2.4.4 version. Currently, building
the docs with any version newer than that will cause 11 false-positives
warnings:

	    Documentation/driver-api/usb/usb:164: ./drivers/usb/core/message.c:967: WARNING: Duplicate C declaration, also defined at driver-api/usb/gadget:783.
Declaration is '.. c:function:: int usb_string (struct usb_device *dev, int index, char *buf, size_t size)'.
    Documentation/driver-api/usb/usb.rst:967: WARNING: Duplicate C declaration, also defined at driver-api/usb/gadget:783.
Declaration is '.. c:struct:: usb_string'.
    Documentation/driver-api/miscellaneous:48: ./drivers/pwm/core.c:599: WARNING: Duplicate C declaration, also defined at driver-api/miscellaneous:240.
Declaration is '.. c:function:: int pwm_capture (struct pwm_device *pwm, struct pwm_capture *result, unsigned long timeout)'.
    Documentation/driver-api/surface_aggregator/client-api:25: ./drivers/platform/surface/aggregator/controller.c:1689: WARNING: Duplicate C declaration, also defined at driver-api/surface_aggregator/client-api:105.
Declaration is '.. c:function:: int ssam_request_sync (struct ssam_controller *ctrl, const struct ssam_request *spec, struct ssam_response *rsp)'.
    Documentation/driver-api/80211/mac80211:109: ./include/net/mac80211.h:4933: WARNING: Duplicate C declaration, also defined at driver-api/80211/mac80211:1041.
Declaration is '.. c:function:: void ieee80211_tx_status (struct ieee80211_hw *hw, struct sk_buff *skb)'.
    Documentation/gpu/amdgpu/driver-core:163: ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:735: WARNING: Duplicate C declaration, also defined at gpu/amdgpu/driver-core:93.
Declaration is '.. c:function:: void amdgpu_vm_tlb_seq_cb (struct dma_fence *fence, struct dma_fence_cb *cb)'.
    Documentation/gpu/drm-kms:360: ./drivers/gpu/drm/drm_fourcc.c:298: WARNING: Duplicate C declaration, also defined at gpu/drm-kms:36.
Declaration is '.. c:function:: const struct drm_format_info * drm_format_info (u32 format)'.
    Documentation/gpu/drm-kms:459: ./drivers/gpu/drm/drm_modeset_lock.c:392: WARNING: Duplicate C declaration, also defined at gpu/drm-kms:49.
Declaration is '.. c:function:: int drm_modeset_lock (struct drm_modeset_lock *lock, struct drm_modeset_acquire_ctx *ctx)'.
    Documentation/gpu/drm-uapi:357: ./drivers/gpu/drm/drm_ioctl.c:917: WARNING: Duplicate C declaration, also defined at gpu/drm-uapi:70.
Declaration is '.. c:function:: bool drm_ioctl_flags (unsigned int nr, unsigned int *flags)'.
    Documentation/gpu/rfc/i915_scheduler:138: ./include/uapi/drm/i915_drm.h:3: WARNING: Duplicate C declaration, also defined at gpu/driver-uapi:2101.
Declaration is '.. c:struct:: i915_context_engines_parallel_submit'.
    Documentation/gpu/rfc/i915_scheduler.rst:3: WARNING: Duplicate C declaration, also defined at gpu/driver-uapi:2101.
Declaration is '.. c:struct:: i915_context_engines_parallel_submit'.

Basically, on 11 places inside the Kernel we use the same name for
functions and for struct (or enum), as this is perfectly fine on C.

Yet, even having different tags at the C domain after Sphinx 3.x, it
still doesn't place them on separate namespaces. Those are all caused
by this bug, whose fixes are yet to be merged:

	https://github.com/sphinx-doc/sphinx/pull/8313

Is this planned to be solved during Sphinx 5.x development cycle?

Regards,
Mauro