All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
@ 2022-03-29  6:07 Akira Yokosawa
  2022-03-29 13:01 ` Bagas Sanjaya
  2022-03-29 15:31 ` Jonathan Corbet
  0 siblings, 2 replies; 21+ messages in thread
From: Akira Yokosawa @ 2022-03-29  6:07 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: Mauro Carvalho Chehab, linux-doc, Akira Yokosawa

jinja2 release 3.1.0 (March 24, 2022) broke Sphinx<4.0.
This looks like the result of deprecating Python 3.6.
It has been tested against Sphinx 4.3.0 and later later.

Setting an upper limit of <3.1 to junja2 can unbreak Sphinx<4.0
including Sphinx 2.4.4.

Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-doc@vger.kernel.org
---
Or we can bump the requirement to Sphinx>=4.0.
Thoughts?

        Thanks Akira
--
 Documentation/sphinx/requirements.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/sphinx/requirements.txt b/Documentation/sphinx/requirements.txt
index 9a35f50798a6..2c573541ab71 100644
--- a/Documentation/sphinx/requirements.txt
+++ b/Documentation/sphinx/requirements.txt
@@ -1,2 +1,4 @@
+# jinja2>=3.1 is not compatible with Sphinx<4.0
+jinja2<3.1
 sphinx_rtd_theme
 Sphinx==2.4.4

base-commit: 9df072c73b9891e44f7f59f3b7c8f852b4485e80
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29  6:07 [PATCH] docs: sphinx/requirements: Limit jinja2<3.1 Akira Yokosawa
@ 2022-03-29 13:01 ` Bagas Sanjaya
  2022-03-29 14:00   ` Jonathan Corbet
  2022-03-29 14:01   ` Akira Yokosawa
  2022-03-29 15:31 ` Jonathan Corbet
  1 sibling, 2 replies; 21+ messages in thread
From: Bagas Sanjaya @ 2022-03-29 13:01 UTC (permalink / raw)
  To: Akira Yokosawa, Jonathan Corbet; +Cc: Mauro Carvalho Chehab, linux-doc

On 29/03/22 13.07, Akira Yokosawa wrote:
> diff --git a/Documentation/sphinx/requirements.txt b/Documentation/sphinx/requirements.txt
> index 9a35f50798a6..2c573541ab71 100644
> --- a/Documentation/sphinx/requirements.txt
> +++ b/Documentation/sphinx/requirements.txt
> @@ -1,2 +1,4 @@
> +# jinja2>=3.1 is not compatible with Sphinx<4.0
> +jinja2<3.1
>   sphinx_rtd_theme
>   Sphinx==2.4.4
> 

I see that we had already pinned the exact Sphinx version to 2.4.4 (or am
I read the requirements wrong?), so this only matters when people use Sphinx
from distribution packages, rather than using virtualenv as recommended.

-- 
An old man doll... just what I always wanted! - Clara

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29 13:01 ` Bagas Sanjaya
@ 2022-03-29 14:00   ` Jonathan Corbet
  2022-03-29 14:08     ` Akira Yokosawa
  2022-03-29 14:01   ` Akira Yokosawa
  1 sibling, 1 reply; 21+ messages in thread
From: Jonathan Corbet @ 2022-03-29 14:00 UTC (permalink / raw)
  To: Bagas Sanjaya, Akira Yokosawa; +Cc: Mauro Carvalho Chehab, linux-doc

Bagas Sanjaya <bagasdotme@gmail.com> writes:

> On 29/03/22 13.07, Akira Yokosawa wrote:
>> diff --git a/Documentation/sphinx/requirements.txt b/Documentation/sphinx/requirements.txt
>> index 9a35f50798a6..2c573541ab71 100644
>> --- a/Documentation/sphinx/requirements.txt
>> +++ b/Documentation/sphinx/requirements.txt
>> @@ -1,2 +1,4 @@
>> +# jinja2>=3.1 is not compatible with Sphinx<4.0
>> +jinja2<3.1
>>   sphinx_rtd_theme
>>   Sphinx==2.4.4
>> 
>
> I see that we had already pinned the exact Sphinx version to 2.4.4 (or am
> I read the requirements wrong?), so this only matters when people use Sphinx
> from distribution packages, rather than using virtualenv as recommended.

We have been suggesting 2.4.4 simply because it's much faster than the
later releases, but it's not a requirement.

Thanks,

jon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29 13:01 ` Bagas Sanjaya
  2022-03-29 14:00   ` Jonathan Corbet
@ 2022-03-29 14:01   ` Akira Yokosawa
  1 sibling, 0 replies; 21+ messages in thread
From: Akira Yokosawa @ 2022-03-29 14:01 UTC (permalink / raw)
  To: Bagas Sanjaya, Jonathan Corbet; +Cc: Mauro Carvalho Chehab, linux-doc

On Tue, 29 Mar 2022 20:01:46 +0700,
Bagas Sanjaya wrote:
> On 29/03/22 13.07, Akira Yokosawa wrote:
>> diff --git a/Documentation/sphinx/requirements.txt b/Documentation/sphinx/requirements.txt
>> index 9a35f50798a6..2c573541ab71 100644
>> --- a/Documentation/sphinx/requirements.txt
>> +++ b/Documentation/sphinx/requirements.txt
>> @@ -1,2 +1,4 @@
>> +# jinja2>=3.1 is not compatible with Sphinx<4.0
>> +jinja2<3.1
>>   sphinx_rtd_theme
>>   Sphinx==2.4.4
>>
> 
> I see that we had already pinned the exact Sphinx version to 2.4.4 (or am
> I read the requirements wrong?), so this only matters when people use Sphinx
> from distribution packages, rather than using virtualenv as recommended.

On the contrary!

This matters when you do fresh-install of Sphinx 2.4.4 using virtualenv.
Please try it without the limit of inja2<3.1.
If you can't reproduce the breakage, I might be missing something
something important.

I guess you had your Sphinx installed before jinja2 3.1.0
was released (March 24, 2022), didn't you?

        Thanks, Akira


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29 14:00   ` Jonathan Corbet
@ 2022-03-29 14:08     ` Akira Yokosawa
  0 siblings, 0 replies; 21+ messages in thread
From: Akira Yokosawa @ 2022-03-29 14:08 UTC (permalink / raw)
  To: Jonathan Corbet, Bagas Sanjaya; +Cc: Mauro Carvalho Chehab, linux-doc

On Tue, 29 Mar 2022 08:00:53 -0600,
Jonathan Corbet wrote:
> Bagas Sanjaya <bagasdotme@gmail.com> writes:
> 
>> On 29/03/22 13.07, Akira Yokosawa wrote:
>>> diff --git a/Documentation/sphinx/requirements.txt b/Documentation/sphinx/requirements.txt
>>> index 9a35f50798a6..2c573541ab71 100644
>>> --- a/Documentation/sphinx/requirements.txt
>>> +++ b/Documentation/sphinx/requirements.txt
>>> @@ -1,2 +1,4 @@
>>> +# jinja2>=3.1 is not compatible with Sphinx<4.0
>>> +jinja2<3.1
>>>   sphinx_rtd_theme
>>>   Sphinx==2.4.4
>>>
>>
>> I see that we had already pinned the exact Sphinx version to 2.4.4 (or am
>> I read the requirements wrong?), so this only matters when people use Sphinx
>> from distribution packages, rather than using virtualenv as recommended.
> 
> We have been suggesting 2.4.4 simply because it's much faster than the
> later releases, but it's not a requirement.

There was a time when Sphinx 3.x.x was memory hungry and slower than 2.4.4.
Nowadays, Sphinx 4.x.x is getting much better and actually is much faster
than 2.4.4 in my experience.  Might depend on the underlying Python
release, though.

Sphinx 4.5.0 released yesterday works just fine.

        Thanks, Akira

> 
> Thanks,
> 
> jon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29  6:07 [PATCH] docs: sphinx/requirements: Limit jinja2<3.1 Akira Yokosawa
  2022-03-29 13:01 ` Bagas Sanjaya
@ 2022-03-29 15:31 ` Jonathan Corbet
  2022-03-29 15:36   ` Randy Dunlap
                     ` (2 more replies)
  1 sibling, 3 replies; 21+ messages in thread
From: Jonathan Corbet @ 2022-03-29 15:31 UTC (permalink / raw)
  To: Akira Yokosawa; +Cc: Mauro Carvalho Chehab, linux-doc, Akira Yokosawa

Akira Yokosawa <akiyks@gmail.com> writes:

> jinja2 release 3.1.0 (March 24, 2022) broke Sphinx<4.0.
> This looks like the result of deprecating Python 3.6.
> It has been tested against Sphinx 4.3.0 and later later.

*Sigh*.  I wish this stuff didn't feel like such a house of cards
sometimes... 

> Setting an upper limit of <3.1 to junja2 can unbreak Sphinx<4.0
> including Sphinx 2.4.4.
>
> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-doc@vger.kernel.org
> ---
> Or we can bump the requirement to Sphinx>=4.0.
> Thoughts?

It's probably time to consider a bump there, but that is a bigger one
than I would want to do at this point.  So I'll just fast-track this
patch in; dropping it into the stable updates probably makes sense too.

Thanks,

jon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29 15:31 ` Jonathan Corbet
@ 2022-03-29 15:36   ` Randy Dunlap
  2022-03-29 23:51     ` Akira Yokosawa
  2022-03-30  0:25   ` Mauro Carvalho Chehab
  2022-03-30  1:29   ` [PATCH] docs: sphinx/requirements: Limit jinja2<3.1 Akira Yokosawa
  2 siblings, 1 reply; 21+ messages in thread
From: Randy Dunlap @ 2022-03-29 15:36 UTC (permalink / raw)
  To: Jonathan Corbet, Akira Yokosawa; +Cc: Mauro Carvalho Chehab, linux-doc



On 3/29/22 08:31, Jonathan Corbet wrote:
> Akira Yokosawa <akiyks@gmail.com> writes:
> 
>> jinja2 release 3.1.0 (March 24, 2022) broke Sphinx<4.0.
>> This looks like the result of deprecating Python 3.6.
>> It has been tested against Sphinx 4.3.0 and later later.
> 
> *Sigh*.  I wish this stuff didn't feel like such a house of cards
> sometimes... 

ack.

>> Setting an upper limit of <3.1 to junja2 can unbreak Sphinx<4.0
>> including Sphinx 2.4.4.
>>
>> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>> Cc: linux-doc@vger.kernel.org
>> ---
>> Or we can bump the requirement to Sphinx>=4.0.
>> Thoughts?
> 
> It's probably time to consider a bump there, but that is a bigger one
> than I would want to do at this point.  So I'll just fast-track this
> patch in; dropping it into the stable updates probably makes sense too.

Yeah, some of us don't have Python4 installed...

-- 
~Randy

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29 15:36   ` Randy Dunlap
@ 2022-03-29 23:51     ` Akira Yokosawa
  2022-03-30  0:07       ` Randy Dunlap
  0 siblings, 1 reply; 21+ messages in thread
From: Akira Yokosawa @ 2022-03-29 23:51 UTC (permalink / raw)
  To: Randy Dunlap, Jonathan Corbet; +Cc: Mauro Carvalho Chehab, linux-doc

On Tue, 29 Mar 2022 08:36:23 -0700,
Randy Dunlap wrote:
> 
> On 3/29/22 08:31, Jonathan Corbet wrote:
>> Akira Yokosawa <akiyks@gmail.com> writes:
>>
>>> jinja2 release 3.1.0 (March 24, 2022) broke Sphinx<4.0.
>>> This looks like the result of deprecating Python 3.6.
>>> It has been tested against Sphinx 4.3.0 and later later.
>>
>> *Sigh*.  I wish this stuff didn't feel like such a house of cards
>> sometimes... 
> 
> ack.
> 
>>> Setting an upper limit of <3.1 to junja2 can unbreak Sphinx<4.0
>>> including Sphinx 2.4.4.
>>>
>>> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>>> Cc: linux-doc@vger.kernel.org
>>> ---
>>> Or we can bump the requirement to Sphinx>=4.0.
>>> Thoughts?
>>
>> It's probably time to consider a bump there, but that is a bigger one
>> than I would want to do at this point.  So I'll just fast-track this
>> patch in; dropping it into the stable updates probably makes sense too.
> 
> Yeah, some of us don't have Python4 installed...

You can run Sphinx 4.5.0 with Python>=3.6.
(jinja2>3.1 requires Python>=3.7, though.)

A requirement list for installing latest Sphinx for kerneldoc would be
a single line of:

----
sphinx_rtd_theme
----

Or for Python 3.6,

----
jinja2<3.1
sphinx_rtd_theme
----

should work (tested with Python 3.6.9 on Ubuntu 18.04).
That is, give or take the occasional breakages caused by a conflict
between updated dependent packages or updated dependencies.

We can't be safe just by pinning Sphinx version.  Old Sphinx can't
know of breakages by future updates in its dependencies.
To avoid this, we need to pin every dependent package.
I'm not sure this is the right thing to do, though...

        Thanks, Akira

> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29 23:51     ` Akira Yokosawa
@ 2022-03-30  0:07       ` Randy Dunlap
  0 siblings, 0 replies; 21+ messages in thread
From: Randy Dunlap @ 2022-03-30  0:07 UTC (permalink / raw)
  To: Akira Yokosawa, Jonathan Corbet; +Cc: Mauro Carvalho Chehab, linux-doc



On 3/29/22 16:51, Akira Yokosawa wrote:
> On Tue, 29 Mar 2022 08:36:23 -0700,
> Randy Dunlap wrote:
>>
>> On 3/29/22 08:31, Jonathan Corbet wrote:
>>> Akira Yokosawa <akiyks@gmail.com> writes:
>>>
>>>> jinja2 release 3.1.0 (March 24, 2022) broke Sphinx<4.0.
>>>> This looks like the result of deprecating Python 3.6.
>>>> It has been tested against Sphinx 4.3.0 and later later.
>>>
>>> *Sigh*.  I wish this stuff didn't feel like such a house of cards
>>> sometimes... 
>>
>> ack.
>>
>>>> Setting an upper limit of <3.1 to junja2 can unbreak Sphinx<4.0
>>>> including Sphinx 2.4.4.
>>>>
>>>> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
>>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>>>> Cc: linux-doc@vger.kernel.org
>>>> ---
>>>> Or we can bump the requirement to Sphinx>=4.0.
>>>> Thoughts?
>>>
>>> It's probably time to consider a bump there, but that is a bigger one
>>> than I would want to do at this point.  So I'll just fast-track this
>>> patch in; dropping it into the stable updates probably makes sense too.
>>
>> Yeah, some of us don't have Python4 installed...
> 
> You can run Sphinx 4.5.0 with Python>=3.6.
> (jinja2>3.1 requires Python>=3.7, though.)
> 
> A requirement list for installing latest Sphinx for kerneldoc would be
> a single line of:
> 
> ----
> sphinx_rtd_theme
> ----
> 
> Or for Python 3.6,
> 
> ----
> jinja2<3.1
> sphinx_rtd_theme
> ----
> 
> should work (tested with Python 3.6.9 on Ubuntu 18.04).
> That is, give or take the occasional breakages caused by a conflict
> between updated dependent packages or updated dependencies.
> 
> We can't be safe just by pinning Sphinx version.  Old Sphinx can't
> know of breakages by future updates in its dependencies.
> To avoid this, we need to pin every dependent package.
> I'm not sure this is the right thing to do, though...

Hi Akira,
Thanks for the tips.
However, I think that I misread Sphinx.4+ as Python4. :(

-- 
~Randy

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29 15:31 ` Jonathan Corbet
  2022-03-29 15:36   ` Randy Dunlap
@ 2022-03-30  0:25   ` Mauro Carvalho Chehab
  2022-03-30 14:59     ` Akira Yokosawa
  2022-03-30  1:29   ` [PATCH] docs: sphinx/requirements: Limit jinja2<3.1 Akira Yokosawa
  2 siblings, 1 reply; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2022-03-30  0:25 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: Akira Yokosawa, linux-doc

Hi Jon,

Em Tue, 29 Mar 2022 09:31:43 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> Akira Yokosawa <akiyks@gmail.com> writes:
> 
> > jinja2 release 3.1.0 (March 24, 2022) broke Sphinx<4.0.
> > This looks like the result of deprecating Python 3.6.
> > It has been tested against Sphinx 4.3.0 and later later.  
> 
> *Sigh*.  I wish this stuff didn't feel like such a house of cards
> sometimes... 
> 
> > Setting an upper limit of <3.1 to junja2 can unbreak Sphinx<4.0
> > including Sphinx 2.4.4.
> >
> > Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Cc: linux-doc@vger.kernel.org
> > ---
> > Or we can bump the requirement to Sphinx>=4.0.
> > Thoughts?  
> 
> It's probably time to consider a bump there, but that is a bigger one
> than I would want to do at this point. 

Doing a bump at Documentation/sphinx/requirements.txt doesn't mean that people 
using have older versions would need to upgrade, as we don't need to bump the
minimal requirement at Documentation/conf.py, nor the versions suggested by 
scripts/sphinx-pre-install.

So, I don't see a problem on setting it to use sphinx==4.3.2 (or some other 
version).

Yet, I would keep using a known-to-be-good version, instead of letting
pip to just get the latest one.

We need to verify both PDF and html generation, though, as I remember
that some 4.x versions had/(have?) issues with the C domain and duplicate
symbols detection.

Also, it could be worth to check the build time with 2.4.4 and with
whatever newer version we stick.

> So I'll just fast-track this
> patch in; dropping it into the stable updates probably makes sense too.

Agreed.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-29 15:31 ` Jonathan Corbet
  2022-03-29 15:36   ` Randy Dunlap
  2022-03-30  0:25   ` Mauro Carvalho Chehab
@ 2022-03-30  1:29   ` Akira Yokosawa
  2022-03-30 19:44     ` Jonathan Corbet
  2 siblings, 1 reply; 21+ messages in thread
From: Akira Yokosawa @ 2022-03-30  1:29 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: Mauro Carvalho Chehab, linux-doc

On Tue, 29 Mar 2022 09:31:43 -0600,
Jonathan Corbet wrote:
> Akira Yokosawa <akiyks@gmail.com> writes:
> 
>> jinja2 release 3.1.0 (March 24, 2022) broke Sphinx<4.0.
>> This looks like the result of deprecating Python 3.6.
>> It has been tested against Sphinx 4.3.0 and later later.
Oh, I made a staccato here.
Thank you for amending it.
> 
> *Sigh*.  I wish this stuff didn't feel like such a house of cards
> sometimes... 
> 
>> Setting an upper limit of <3.1 to junja2 can unbreak Sphinx<4.0
>> including Sphinx 2.4.4.
>>
>> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>> Cc: linux-doc@vger.kernel.org
>> ---
>> Or we can bump the requirement to Sphinx>=4.0.
>> Thoughts?
> 
> It's probably time to consider a bump there, but that is a bigger one
> than I would want to do at this point.  So I'll just fast-track this
> patch in; dropping it into the stable updates probably makes sense too.

This can be applied cleanly for Linux>=5.15.
For 5.10.x, there is a dependent commit:

    37397b092e7f ("docs: sphinx-requirements: Move sphinx_rtd_theme to top")

Linux 5.4.x needs another one:

    d5afc9640a6d ("docs: update recommended Sphinx version to 2.4.4")

... and so on.

How far do you want to backport this?

        Thanks, Akira
> 
> Thanks,
> 
> jon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-30  0:25   ` Mauro Carvalho Chehab
@ 2022-03-30 14:59     ` Akira Yokosawa
  2022-03-30 17:07       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 21+ messages in thread
From: Akira Yokosawa @ 2022-03-30 14:59 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-doc, Jonathan Corbet, Akira Yokosawa

Hi Mauro,

On Wed, 30 Mar 2022 02:25:34 +0200,
Mauro Carvalho Chehab wrote:
[...]
> We need to verify both PDF and html generation, though, as I remember
> that some 4.x versions had/(have?) issues with the C domain and duplicate
> symbols detection.

Can you elaborate on the issue you observed?
In which document did you see it?

Verification of generated HTML and PDF can be hard.

Different Sphinx might generate slightly different .html or .tex
files with no visible effect in the final rendering.  Hmm...

Do you have an idea for automated regression testing?

> 
> Also, it could be worth to check the build time with 2.4.4 and with
> whatever newer version we stick.

I agree. I'll see what I can do.

        Thanks, Akira

> 
>> So I'll just fast-track this
>> patch in; dropping it into the stable updates probably makes sense too.
> 
> Agreed.
> 
> Thanks,
> Mauro

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-30 14:59     ` Akira Yokosawa
@ 2022-03-30 17:07       ` Mauro Carvalho Chehab
  2022-03-31 14:32         ` Akira Yokosawa
  0 siblings, 1 reply; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2022-03-30 17:07 UTC (permalink / raw)
  To: Akira Yokosawa; +Cc: linux-doc, Jonathan Corbet

Em Wed, 30 Mar 2022 23:59:05 +0900
Akira Yokosawa <akiyks@gmail.com> escreveu:

> Hi Mauro,
> 
> On Wed, 30 Mar 2022 02:25:34 +0200,
> Mauro Carvalho Chehab wrote:
> [...]
> > We need to verify both PDF and html generation, though, as I remember
> > that some 4.x versions had/(have?) issues with the C domain and duplicate
> > symbols detection.  
> 
> Can you elaborate on the issue you observed?
> In which document did you see it?

Sorry, it was on Sphinx 3.x, although the most complete fix got
merged on 4.0, I guess. This patch is related to it:

	b34b86d7a418 ("docs: conf.py: fix c:function support with Sphinx 3.x")

Basically, the Sphinx maintainer for the C domain rewrote the code,
causing all references generated by kernel-doc to be broken, and
almost all references at the media docs as well. Before the changes,
there were just one domain for C code references, used for functions,
structs, enums, etc. After the change, each one requires a different
tag. The kerneldoc script has gained support for Sphinx version when
such issue was addressed.

Another consequence of such change is that you can't have more than
one "read()" function inside the entire Kernel. While this makes
sense on userspace, It doesn't at Kernelspace, as different subsystems
may handle read/write/ioctl/... syscalls on their particular ways.
So, building docs were causing warnings about duplicated symbols.

There were some changes that went on 4.x to fix it, when 
".. c:namespace::" got merged. I don't remember when it was added.

> Verification of generated HTML and PDF can be hard.
> 
> Different Sphinx might generate slightly different .html or .tex
> files with no visible effect in the final rendering.  Hmm...

That was not the case on that time. We had to stick to Sphinx up to
version 2.4 for a couple of Kernel release cycles, in order to fix, as the
changes weren't trivial.

> Do you have an idea for automated regression testing?

Except for doing periodic html and pdf builds and reporting build errors,
no.

For html, perhaps some regression testing could be done by using pandoc
to convert html back into ReST (or to some other format) and compare if the
output from the same source with different Sphinx versions are identical 
(or similar enough). On such case, I would get rid of using read the docs
style, using the simplest possible one. That's easier said than done, though,
as such conversion could produce errors due to problems on pandoc - or some
minor differences - So, whomever would be running such tests would likely
need to add something in order to handle similar but different outputs and
exclude false-positives.

> 
> > 
> > Also, it could be worth to check the build time with 2.4.4 and with
> > whatever newer version we stick.  
> 
> I agree. I'll see what I can do.
> 
>         Thanks, Akira
> 
> >   
> >> So I'll just fast-track this
> >> patch in; dropping it into the stable updates probably makes sense too.  
> > 
> > Agreed.
> > 
> > Thanks,
> > Mauro  



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-30  1:29   ` [PATCH] docs: sphinx/requirements: Limit jinja2<3.1 Akira Yokosawa
@ 2022-03-30 19:44     ` Jonathan Corbet
  0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Corbet @ 2022-03-30 19:44 UTC (permalink / raw)
  To: Akira Yokosawa; +Cc: Mauro Carvalho Chehab, linux-doc

Akira Yokosawa <akiyks@gmail.com> writes:

>> It's probably time to consider a bump there, but that is a bigger one
>> than I would want to do at this point.  So I'll just fast-track this
>> patch in; dropping it into the stable updates probably makes sense too.
>
> This can be applied cleanly for Linux>=5.15.
> For 5.10.x, there is a dependent commit:
>
>     37397b092e7f ("docs: sphinx-requirements: Move sphinx_rtd_theme to top")
>
> Linux 5.4.x needs another one:
>
>     d5afc9640a6d ("docs: update recommended Sphinx version to 2.4.4")
>
> ... and so on.
>
> How far do you want to backport this?

Ah...thanks for pointing that out.  I don't think it's worth dragging in
other commits, so 5.15 is it, I guess.

jon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1
  2022-03-30 17:07       ` Mauro Carvalho Chehab
@ 2022-03-31 14:32         ` Akira Yokosawa
  2022-05-21  7:58           ` "WARNING: Duplicate C declaration" from recent Sphinx (was Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1) Akira Yokosawa
  0 siblings, 1 reply; 21+ messages in thread
From: Akira Yokosawa @ 2022-03-31 14:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-doc, Jonathan Corbet, Akira Yokosawa

On Wed, 30 Mar 2022 19:07:24 +0200,
Mauro Carvalho Chehab wrote:
> Em Wed, 30 Mar 2022 23:59:05 +0900
> Akira Yokosawa <akiyks@gmail.com> escreveu:
> 
>> Hi Mauro,
>>
>> On Wed, 30 Mar 2022 02:25:34 +0200,
>> Mauro Carvalho Chehab wrote:
>> [...]
>>> We need to verify both PDF and html generation, though, as I remember
>>> that some 4.x versions had/(have?) issues with the C domain and duplicate
>>> symbols detection.  
>>
>> Can you elaborate on the issue you observed?
>> In which document did you see it?
> 
> Sorry, it was on Sphinx 3.x, although the most complete fix got
> merged on 4.0, I guess. This patch is related to it:
> 
> 	b34b86d7a418 ("docs: conf.py: fix c:function support with Sphinx 3.x")
> 
> Basically, the Sphinx maintainer for the C domain rewrote the code,
> causing all references generated by kernel-doc to be broken, and
> almost all references at the media docs as well. Before the changes,
> there were just one domain for C code references, used for functions,
> structs, enums, etc. After the change, each one requires a different
> tag. The kerneldoc script has gained support for Sphinx version when
> such issue was addressed.
> 
> Another consequence of such change is that you can't have more than
> one "read()" function inside the entire Kernel. While this makes
> sense on userspace, It doesn't at Kernelspace, as different subsystems
> may handle read/write/ioctl/... syscalls on their particular ways.
> So, building docs were causing warnings about duplicated symbols.
> 
> There were some changes that went on 4.x to fix it, when 
> ".. c:namespace::" got merged. I don't remember when it was added.

Thank you for the detailed explanation.

So I compared logs from "make SPHINXDIRS=driver-api htmldocs" with
Sphinx 2.4.4 and 4.5.0 on current docs-next.

There are 8 more lines in the log from 4.5.0 than from 2.4.4, give
or take minor format differences.

Here are those extra 8 lines (long lines are kept):

----
/wk/Documentation/driver-api/usb/usb.rst:967: WARNING: Duplicate C declaration, also defined at usb/gadget:775.
Declaration is '.. c:struct:: usb_string'.
/wk/Documentation/driver-api/miscellaneous:48: ./drivers/pwm/core.c:679: WARNING: Duplicate C declaration, also defined at miscellaneous:305.
Declaration is '.. c:function:: int pwm_capture (struct pwm_device *pwm, struct pwm_capture *result, unsigned long timeout)'.
/wk/Documentation/driver-api/surface_aggregator/client-api:25: ./drivers/platform/surface/aggregator/controller.c:1689: WARNING: Duplicate C declaration, also defined at 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)'.
/wk/Documentation/driver-api/80211/mac80211:109: ./include/net/mac80211.h:4811: WARNING: Duplicate C declaration, also defined at 80211/mac80211:1024.
Declaration is '.. c:function:: void ieee80211_tx_status (struct ieee80211_hw *hw, struct sk_buff *skb)'.
----

So those "WARNING: Duplicate C declaration" messages are what you
mentioned earlier, aren't they?

I'm looking at Documentation/driver-api/usb/usb.rst:967 and
usb/gadget.rst:775, but can't figure out what is/are wrong.

How am I supposed to interpret those messages?
Are those line counts of intermediate files or some such?

> 
>> Verification of generated HTML and PDF can be hard.
>>
>> Different Sphinx might generate slightly different .html or .tex
>> files with no visible effect in the final rendering.  Hmm...
> 
> That was not the case on that time. We had to stick to Sphinx up to
> version 2.4 for a couple of Kernel release cycles, in order to fix, as the
> changes weren't trivial.
> 
>> Do you have an idea for automated regression testing?
> 
> Except for doing periodic html and pdf builds and reporting build errors,
> no.

Ah, by "PDF and html generation", you meant build errors.
Now I understand what you said.

> 
> For html, perhaps some regression testing could be done by using pandoc
> to convert html back into ReST (or to some other format) and compare if the
> output from the same source with different Sphinx versions are identical 
> (or similar enough). On such case, I would get rid of using read the docs
> style, using the simplest possible one. That's easier said than done, though,
> as such conversion could produce errors due to problems on pandoc - or some
> minor differences - So, whomever would be running such tests would likely
> need to add something in order to handle similar but different outputs and
> exclude false-positives.

This sounds hard if not impossible...

        Thanks, Akira

> 
>>
>>>
>>> Also, it could be worth to check the build time with 2.4.4 and with
>>> whatever newer version we stick.  
>>
>> I agree. I'll see what I can do.
>>
>>         Thanks, Akira
>>
>>>   
>>>> So I'll just fast-track this
>>>> patch in; dropping it into the stable updates probably makes sense too.  
>>>
>>> Agreed.
>>>
>>> Thanks,
>>> Mauro  
> 
> 
> 
> Thanks,
> Mauro

^ permalink raw reply	[flat|nested] 21+ messages in thread

* "WARNING: Duplicate C declaration" from recent Sphinx (was Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1)
  2022-03-31 14:32         ` Akira Yokosawa
@ 2022-05-21  7:58           ` Akira Yokosawa
  2022-05-21  9:46             ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 21+ messages in thread
From: Akira Yokosawa @ 2022-05-21  7:58 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-doc, Jonathan Corbet, Akira Yokosawa

On Thu, 31 Mar 2022 23:32:41 +0900,
Akira Yokosawa wrote:
> On Wed, 30 Mar 2022 19:07:24 +0200,
> Mauro Carvalho Chehab wrote:
>> Em Wed, 30 Mar 2022 23:59:05 +0900
>> Akira Yokosawa <akiyks@gmail.com> escreveu:
>>
>>> Hi Mauro,
>>>
>>> On Wed, 30 Mar 2022 02:25:34 +0200,
>>> Mauro Carvalho Chehab wrote:
>>> [...]
>>>> We need to verify both PDF and html generation, though, as I remember
>>>> that some 4.x versions had/(have?) issues with the C domain and duplicate
>>>> symbols detection.  
>>>
>>> Can you elaborate on the issue you observed?
>>> In which document did you see it?
>>
>> Sorry, it was on Sphinx 3.x, although the most complete fix got
>> merged on 4.0, I guess. This patch is related to it:
>>
>> 	b34b86d7a418 ("docs: conf.py: fix c:function support with Sphinx 3.x")
>>
>> Basically, the Sphinx maintainer for the C domain rewrote the code,
>> causing all references generated by kernel-doc to be broken, and
>> almost all references at the media docs as well. Before the changes,
>> there were just one domain for C code references, used for functions,
>> structs, enums, etc. After the change, each one requires a different
>> tag. The kerneldoc script has gained support for Sphinx version when
>> such issue was addressed.
>>
>> Another consequence of such change is that you can't have more than
>> one "read()" function inside the entire Kernel. While this makes
>> sense on userspace, It doesn't at Kernelspace, as different subsystems
>> may handle read/write/ioctl/... syscalls on their particular ways.
>> So, building docs were causing warnings about duplicated symbols.
>>
>> There were some changes that went on 4.x to fix it, when 
>> ".. c:namespace::" got merged. I don't remember when it was added.
> 
> Thank you for the detailed explanation.
> 
> So I compared logs from "make SPHINXDIRS=driver-api htmldocs" with
> Sphinx 2.4.4 and 4.5.0 on current docs-next.
> 
> There are 8 more lines in the log from 4.5.0 than from 2.4.4, give
> or take minor format differences.
> 
> Here are those extra 8 lines (long lines are kept):
> 
> ----
> /wk/Documentation/driver-api/usb/usb.rst:967: WARNING: Duplicate C declaration, also defined at usb/gadget:775.
> Declaration is '.. c:struct:: usb_string'.
> /wk/Documentation/driver-api/miscellaneous:48: ./drivers/pwm/core.c:679: WARNING: Duplicate C declaration, also defined at miscellaneous:305.
> Declaration is '.. c:function:: int pwm_capture (struct pwm_device *pwm, struct pwm_capture *result, unsigned long timeout)'.
> /wk/Documentation/driver-api/surface_aggregator/client-api:25: ./drivers/platform/surface/aggregator/controller.c:1689: WARNING: Duplicate C declaration, also defined at 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)'.
> /wk/Documentation/driver-api/80211/mac80211:109: ./include/net/mac80211.h:4811: WARNING: Duplicate C declaration, also defined at 80211/mac80211:1024.
> Declaration is '.. c:function:: void ieee80211_tx_status (struct ieee80211_hw *hw, struct sk_buff *skb)'.
> ----
> 
> So those "WARNING: Duplicate C declaration" messages are what you
> mentioned earlier, aren't they?
> 

So, I think I have figured out what causes those "WARNING: Duplicate
C declaration".

When you have kernel-doc comments for both struct and function
of the same name, recent Sphinx emits this warning.

Although Sphinx versions 1.7.9 and 2.4.4 don't complain, the result
is the same with Sphinx 3.x and 4.x (with the fix to kernel-doc Mauro
mentioned above).

I have no idea which version of Sphinx is employed for building pages at
https://www.kernel.org/doc/html/latest/driver-api/80211/mac80211.html,
but the cross reference to the ieee80211_tx_status() function in the
description of ieee80211_rx_ni() points to struct ieee80211_tx_status,
which is not an expected behavior.

In this case, it seems to me that both the struct and function
kernel-doc comments are included by the kernel-doc directive

.. kernel-doc:: include/net/mac80211.h
   :functions:
	ieee80211_rx_status
        [...]

at Documentation/driver-api/80211/mac80211.rst:109.

As the :functions: option is identical to :identifiers:, both of
kernel-doc comments in mac80211.h, namely:

    include/net/mac80211.h:1148: * struct ieee80211_tx_status - extended tx status info for rate control

    include/net/mac80211.h:4813: * ieee80211_tx_status - transmit status callback

are extracted by the kerneldoc extension (or the kernel-doc script).

Mauro, does your earlier comment:
>> Another consequence of such change is that you can't have more than
>> one "read()" function inside the entire Kernel. 

apply to those struct and functions of the identical name?

I just want to know what is the expected behavior in this case.

Note: Line counts in this mail are those of v5.18-rc7.

        Thanks, Akira

[...]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: "WARNING: Duplicate C declaration" from recent Sphinx (was Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1)
  2022-05-21  7:58           ` "WARNING: Duplicate C declaration" from recent Sphinx (was Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1) Akira Yokosawa
@ 2022-05-21  9:46             ` Mauro Carvalho Chehab
  2022-05-21  9:59               ` Akira Yokosawa
  2022-05-22  0:57               ` Akira Yokosawa
  0 siblings, 2 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2022-05-21  9:46 UTC (permalink / raw)
  To: Akira Yokosawa; +Cc: linux-doc, Jonathan Corbet

Em Sat, 21 May 2022 16:58:45 +0900
Akira Yokosawa <akiyks@gmail.com> escreveu:

> On Thu, 31 Mar 2022 23:32:41 +0900,
> Akira Yokosawa wrote:
> > On Wed, 30 Mar 2022 19:07:24 +0200,
> > Mauro Carvalho Chehab wrote:  
> >> Em Wed, 30 Mar 2022 23:59:05 +0900
> >> Akira Yokosawa <akiyks@gmail.com> escreveu:
> >>  
> >>> Hi Mauro,
> >>>
> >>> On Wed, 30 Mar 2022 02:25:34 +0200,
> >>> Mauro Carvalho Chehab wrote:
> >>> [...]  
> >>>> We need to verify both PDF and html generation, though, as I remember
> >>>> that some 4.x versions had/(have?) issues with the C domain and duplicate
> >>>> symbols detection.    
> >>>
> >>> Can you elaborate on the issue you observed?
> >>> In which document did you see it?  
> >>
> >> Sorry, it was on Sphinx 3.x, although the most complete fix got
> >> merged on 4.0, I guess. This patch is related to it:
> >>
> >> 	b34b86d7a418 ("docs: conf.py: fix c:function support with Sphinx 3.x")
> >>
> >> Basically, the Sphinx maintainer for the C domain rewrote the code,
> >> causing all references generated by kernel-doc to be broken, and
> >> almost all references at the media docs as well. Before the changes,
> >> there were just one domain for C code references, used for functions,
> >> structs, enums, etc. After the change, each one requires a different
> >> tag. The kerneldoc script has gained support for Sphinx version when
> >> such issue was addressed.
> >>
> >> Another consequence of such change is that you can't have more than
> >> one "read()" function inside the entire Kernel. While this makes
> >> sense on userspace, It doesn't at Kernelspace, as different subsystems
> >> may handle read/write/ioctl/... syscalls on their particular ways.
> >> So, building docs were causing warnings about duplicated symbols.
> >>
> >> There were some changes that went on 4.x to fix it, when 
> >> ".. c:namespace::" got merged. I don't remember when it was added.  
> > 
> > Thank you for the detailed explanation.
> > 
> > So I compared logs from "make SPHINXDIRS=driver-api htmldocs" with
> > Sphinx 2.4.4 and 4.5.0 on current docs-next.
> > 
> > There are 8 more lines in the log from 4.5.0 than from 2.4.4, give
> > or take minor format differences.
> > 
> > Here are those extra 8 lines (long lines are kept):
> > 
> > ----
> > /wk/Documentation/driver-api/usb/usb.rst:967: WARNING: Duplicate C declaration, also defined at usb/gadget:775.
> > Declaration is '.. c:struct:: usb_string'.
> > /wk/Documentation/driver-api/miscellaneous:48: ./drivers/pwm/core.c:679: WARNING: Duplicate C declaration, also defined at miscellaneous:305.
> > Declaration is '.. c:function:: int pwm_capture (struct pwm_device *pwm, struct pwm_capture *result, unsigned long timeout)'.
> > /wk/Documentation/driver-api/surface_aggregator/client-api:25: ./drivers/platform/surface/aggregator/controller.c:1689: WARNING: Duplicate C declaration, also defined at 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)'.
> > /wk/Documentation/driver-api/80211/mac80211:109: ./include/net/mac80211.h:4811: WARNING: Duplicate C declaration, also defined at 80211/mac80211:1024.
> > Declaration is '.. c:function:: void ieee80211_tx_status (struct ieee80211_hw *hw, struct sk_buff *skb)'.
> > ----
> > 
> > So those "WARNING: Duplicate C declaration" messages are what you
> > mentioned earlier, aren't they?
> >   
> 
> So, I think I have figured out what causes those "WARNING: Duplicate
> C declaration".

Basically there are two places defining the same function. This could
either be:

1. because the same header/c file is included on multiple places with
   kernel-doc directives;
2. because both *.c and *.h files declare the same function and both
   are included via kernel-doc directives;
3. because they use different namespaces;
4. because they're documenting system calls.

For (1) and (2) the solution is to fix the kernel-doc includes and/or
the header/c files;

For (3) and (4) the solution is to define a c namespace via
	.. c:namespace:: foo
meta-tags.

> When you have kernel-doc comments for both struct and function
> of the same name, recent Sphinx emits this warning.

Yes.

> 
> Although Sphinx versions 1.7.9 and 2.4.4 don't complain, the result
> is the same with Sphinx 3.x and 4.x (with the fix to kernel-doc Mauro
> mentioned above).

True, it doesn't complain, but the generated documents have issues.

> I have no idea which version of Sphinx is employed for building pages at
> https://www.kernel.org/doc/html/latest/driver-api/80211/mac80211.html,
> but the cross reference to the ieee80211_tx_status() function in the
> description of ieee80211_rx_ni() points to struct ieee80211_tx_status,
> which is not an expected behavior.
> 
> In this case, it seems to me that both the struct and function
> kernel-doc comments are included by the kernel-doc directive
> 
> .. kernel-doc:: include/net/mac80211.h
>    :functions:
> 	ieee80211_rx_status
>         [...]
> 
> at Documentation/driver-api/80211/mac80211.rst:109.
> 
> As the :functions: option is identical to :identifiers:, both of
> kernel-doc comments in mac80211.h, namely:
> 
>     include/net/mac80211.h:1148: * struct ieee80211_tx_status - extended tx status info for rate control
> 
>     include/net/mac80211.h:4813: * ieee80211_tx_status - transmit status callback
> 
> are extracted by the kerneldoc extension (or the kernel-doc script).

The Kernel-doc extension should create two separate references for newer Kernels,
depending on the version.

With older versions of Sphinx, it generates:

	$ ./scripts/kernel-doc -sphinx-version 2.1 include/net/mac80211.h|grep "c:.*ieee80211_tx_status\b"
	.. c:type:: struct ieee80211_tx_status
	.. c:function:: void ieee80211_tx_status (struct ieee80211_hw *hw, struct sk_buff *skb)
	.. c:function:: void ieee80211_tx_status_ext (struct ieee80211_hw *hw, struct ieee80211_tx_status *status)

Here, there's just a single namespace, so both function and type will be
considered as the same thing. No warnings are generated, though.

Versions 3.1 and above:

	$ ./scripts/kernel-doc -sphinx-version 3.1 include/net/mac80211.h|grep "c:.*ieee80211_tx_status\b"
	.. c:struct:: ieee80211_tx_status
	.. c:function:: void ieee80211_tx_status (struct ieee80211_hw *hw, struct sk_buff *skb)
	.. c:function:: void ieee80211_tx_status_ext (struct ieee80211_hw *hw, struct ieee80211_tx_status *status)

This works since version 3.0, but only on version 4.0 namespace tags
started to work.

As far as I know:

Sphinx < 3: there's a single namespace. It doesn't check duplicated
refs. So, cross-references there will be plain broken on symbols with
identical names.

Sphinx 3.0: Although it uses different tags, there's still a single
namespace. It will warn about duplicate symbols. Building docs with
such version will generate lots of warnings that should not be fixed.

This is a version that we don't support well.

Sphinx 3.1 and above: structs, enums, functions, typedefs, etc have their
own separate namespaces. So, it is possible to have struct with the same
name as a function.

Yet, it will complain about duplicated symbols for system calls. I guess
we added a hack somethere to avoid too much noise on versions between
3.1 and 4.0.

Sphinx 4.0 and above: it is now possible to add a namespace. This allows
fixing things like read() system calls that have different meanings on
different subsystems.

On other words, only with Sphinx 4.0 and above, the cross-references
for C domain symbols should all be OK.

> 
> Mauro, does your earlier comment:
> >> Another consequence of such change is that you can't have more than
> >> one "read()" function inside the entire Kernel.   
> 
> apply to those struct and functions of the identical name?
> 
> I just want to know what is the expected behavior in this case.

Yes, that's the case for versions < 4.0. On 4.0, we need to specify a
c namespace to document them.

You can se such things if you do a:

	$ git grep c:namespace Documentation/userspace-api/

The media uAPI documentation has separate documentation for syscalls,
depending on being CEC, V4L or one of the DVB APIs.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: "WARNING: Duplicate C declaration" from recent Sphinx (was Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1)
  2022-05-21  9:46             ` Mauro Carvalho Chehab
@ 2022-05-21  9:59               ` Akira Yokosawa
  2022-05-22  0:57               ` Akira Yokosawa
  1 sibling, 0 replies; 21+ messages in thread
From: Akira Yokosawa @ 2022-05-21  9:59 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-doc, Jonathan Corbet, Akira Yokosawa

On 2022/05/21 18:46,
Mauro Carvalho Chehab wrote:
> Em Sat, 21 May 2022 16:58:45 +0900
> Akira Yokosawa <akiyks@gmail.com> escreveu:
> 
>> On Thu, 31 Mar 2022 23:32:41 +0900,
>> Akira Yokosawa wrote:
>>> On Wed, 30 Mar 2022 19:07:24 +0200,
>>> Mauro Carvalho Chehab wrote:  
>>>> Em Wed, 30 Mar 2022 23:59:05 +0900
>>>> Akira Yokosawa <akiyks@gmail.com> escreveu:
>>>>  
>>>>> Hi Mauro,
>>>>>
>>>>> On Wed, 30 Mar 2022 02:25:34 +0200,
>>>>> Mauro Carvalho Chehab wrote:
>>>>> [...]  
>>>>>> We need to verify both PDF and html generation, though, as I remember
>>>>>> that some 4.x versions had/(have?) issues with the C domain and duplicate
>>>>>> symbols detection.    
>>>>>
>>>>> Can you elaborate on the issue you observed?
>>>>> In which document did you see it?  
>>>>
>>>> Sorry, it was on Sphinx 3.x, although the most complete fix got
>>>> merged on 4.0, I guess. This patch is related to it:
>>>>
>>>> 	b34b86d7a418 ("docs: conf.py: fix c:function support with Sphinx 3.x")
>>>>
>>>> Basically, the Sphinx maintainer for the C domain rewrote the code,
>>>> causing all references generated by kernel-doc to be broken, and
>>>> almost all references at the media docs as well. Before the changes,
>>>> there were just one domain for C code references, used for functions,
>>>> structs, enums, etc. After the change, each one requires a different
>>>> tag. The kerneldoc script has gained support for Sphinx version when
>>>> such issue was addressed.
>>>>
>>>> Another consequence of such change is that you can't have more than
>>>> one "read()" function inside the entire Kernel. While this makes
>>>> sense on userspace, It doesn't at Kernelspace, as different subsystems
>>>> may handle read/write/ioctl/... syscalls on their particular ways.
>>>> So, building docs were causing warnings about duplicated symbols.
>>>>
>>>> There were some changes that went on 4.x to fix it, when 
>>>> ".. c:namespace::" got merged. I don't remember when it was added.  
>>>
>>> Thank you for the detailed explanation.
>>>
>>> So I compared logs from "make SPHINXDIRS=driver-api htmldocs" with
>>> Sphinx 2.4.4 and 4.5.0 on current docs-next.
>>>
>>> There are 8 more lines in the log from 4.5.0 than from 2.4.4, give
>>> or take minor format differences.
>>>
>>> Here are those extra 8 lines (long lines are kept):
>>>
>>> ----
>>> /wk/Documentation/driver-api/usb/usb.rst:967: WARNING: Duplicate C declaration, also defined at usb/gadget:775.
>>> Declaration is '.. c:struct:: usb_string'.
>>> /wk/Documentation/driver-api/miscellaneous:48: ./drivers/pwm/core.c:679: WARNING: Duplicate C declaration, also defined at miscellaneous:305.
>>> Declaration is '.. c:function:: int pwm_capture (struct pwm_device *pwm, struct pwm_capture *result, unsigned long timeout)'.
>>> /wk/Documentation/driver-api/surface_aggregator/client-api:25: ./drivers/platform/surface/aggregator/controller.c:1689: WARNING: Duplicate C declaration, also defined at 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)'.
>>> /wk/Documentation/driver-api/80211/mac80211:109: ./include/net/mac80211.h:4811: WARNING: Duplicate C declaration, also defined at 80211/mac80211:1024.
>>> Declaration is '.. c:function:: void ieee80211_tx_status (struct ieee80211_hw *hw, struct sk_buff *skb)'.
>>> ----
>>>
>>> So those "WARNING: Duplicate C declaration" messages are what you
>>> mentioned earlier, aren't they?
>>>   
>>
>> So, I think I have figured out what causes those "WARNING: Duplicate
>> C declaration".
> 
> Basically there are two places defining the same function. This could
> either be:
> 
> 1. because the same header/c file is included on multiple places with
>    kernel-doc directives;
> 2. because both *.c and *.h files declare the same function and both
>    are included via kernel-doc directives;
> 3. because they use different namespaces;
> 4. because they're documenting system calls.
> 
> For (1) and (2) the solution is to fix the kernel-doc includes and/or
> the header/c files;
> 
> For (3) and (4) the solution is to define a c namespace via
> 	.. c:namespace:: foo
> meta-tags.
> 
>> When you have kernel-doc comments for both struct and function
>> of the same name, recent Sphinx emits this warning.
> 
> Yes.
> 
>>
>> Although Sphinx versions 1.7.9 and 2.4.4 don't complain, the result
>> is the same with Sphinx 3.x and 4.x (with the fix to kernel-doc Mauro
>> mentioned above).
> 
> True, it doesn't complain, but the generated documents have issues.
> 
>> I have no idea which version of Sphinx is employed for building pages at
>> https://www.kernel.org/doc/html/latest/driver-api/80211/mac80211.html,
>> but the cross reference to the ieee80211_tx_status() function in the
>> description of ieee80211_rx_ni() points to struct ieee80211_tx_status,
>> which is not an expected behavior.
>>
>> In this case, it seems to me that both the struct and function
>> kernel-doc comments are included by the kernel-doc directive
>>
>> .. kernel-doc:: include/net/mac80211.h
>>    :functions:
>> 	ieee80211_rx_status
>>         [...]
>>
>> at Documentation/driver-api/80211/mac80211.rst:109.
>>
>> As the :functions: option is identical to :identifiers:, both of
>> kernel-doc comments in mac80211.h, namely:
>>
>>     include/net/mac80211.h:1148: * struct ieee80211_tx_status - extended tx status info for rate control
>>
>>     include/net/mac80211.h:4813: * ieee80211_tx_status - transmit status callback
>>
>> are extracted by the kerneldoc extension (or the kernel-doc script).
> 
> The Kernel-doc extension should create two separate references for newer Kernels,
> depending on the version.
> 
> With older versions of Sphinx, it generates:
> 
> 	$ ./scripts/kernel-doc -sphinx-version 2.1 include/net/mac80211.h|grep "c:.*ieee80211_tx_status\b"
> 	.. c:type:: struct ieee80211_tx_status
> 	.. c:function:: void ieee80211_tx_status (struct ieee80211_hw *hw, struct sk_buff *skb)
> 	.. c:function:: void ieee80211_tx_status_ext (struct ieee80211_hw *hw, struct ieee80211_tx_status *status)
> 
> Here, there's just a single namespace, so both function and type will be
> considered as the same thing. No warnings are generated, though.
> 
> Versions 3.1 and above:
> 
> 	$ ./scripts/kernel-doc -sphinx-version 3.1 include/net/mac80211.h|grep "c:.*ieee80211_tx_status\b"
> 	.. c:struct:: ieee80211_tx_status
> 	.. c:function:: void ieee80211_tx_status (struct ieee80211_hw *hw, struct sk_buff *skb)
> 	.. c:function:: void ieee80211_tx_status_ext (struct ieee80211_hw *hw, struct ieee80211_tx_status *status)
> 
> This works since version 3.0, but only on version 4.0 namespace tags
> started to work.
> 
> As far as I know:
> 
> Sphinx < 3: there's a single namespace. It doesn't check duplicated
> refs. So, cross-references there will be plain broken on symbols with
> identical names.
> 
> Sphinx 3.0: Although it uses different tags, there's still a single
> namespace. It will warn about duplicate symbols. Building docs with
> such version will generate lots of warnings that should not be fixed.
> 
> This is a version that we don't support well.
> 
> Sphinx 3.1 and above: structs, enums, functions, typedefs, etc have their
> own separate namespaces. So, it is possible to have struct with the same
> name as a function.
> 
> Yet, it will complain about duplicated symbols for system calls. I guess
> we added a hack somethere to avoid too much noise on versions between
> 3.1 and 4.0.
> 
> Sphinx 4.0 and above: it is now possible to add a namespace. This allows
> fixing things like read() system calls that have different meanings on
> different subsystems.
> 
> On other words, only with Sphinx 4.0 and above, the cross-references
> for C domain symbols should all be OK.
> 
>>
>> Mauro, does your earlier comment:
>>>> Another consequence of such change is that you can't have more than
>>>> one "read()" function inside the entire Kernel.   
>>
>> apply to those struct and functions of the identical name?
>>
>> I just want to know what is the expected behavior in this case.
> 
> Yes, that's the case for versions < 4.0. On 4.0, we need to specify a
> c namespace to document them.
> 
> You can se such things if you do a:
> 
> 	$ git grep c:namespace Documentation/userspace-api/
> 
> The media uAPI documentation has separate documentation for syscalls,
> depending on being CEC, V4L or one of the DVB APIs.

Mauro, many thanks for the quick and detailed explanation.
I think it will take a little while for me to digest all of this, but
when I do, I'll try and see if I can fix the offending docs by proper
uses of namespaces for Sphinx >= 4.0.

        Thanks, Akira

> 
> Thanks,
> Mauro

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: "WARNING: Duplicate C declaration" from recent Sphinx (was Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1)
  2022-05-21  9:46             ` Mauro Carvalho Chehab
  2022-05-21  9:59               ` Akira Yokosawa
@ 2022-05-22  0:57               ` Akira Yokosawa
  2022-05-22  5:07                 ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 21+ messages in thread
From: Akira Yokosawa @ 2022-05-22  0:57 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-doc, Jonathan Corbet

On Sat, 21 May 2022 11:46:29 +0200,
Mauro Carvalho Chehab wrote:
[...]
> 
> Sphinx 4.0 and above: it is now possible to add a namespace. This allows
> fixing things like read() system calls that have different meanings on
> different subsystems.
> 
> On other words, only with Sphinx 4.0 and above, the cross-references
> for C domain symbols should all be OK.

So, I noticed there is a PR at https://github.com/sphinx-doc/sphinx/pull/8313
which is still open.

This PR is supposed to resolve "WARNING: Duplicate C declaration"
due to struct and function with the same name, isn't it?

Are you sure the issue is resolved in Sphinx 4.0 and later?

        Thanks, Akira


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: "WARNING: Duplicate C declaration" from recent Sphinx (was Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1)
  2022-05-22  0:57               ` Akira Yokosawa
@ 2022-05-22  5:07                 ` Mauro Carvalho Chehab
  2022-05-22  9:28                   ` Akira Yokosawa
  0 siblings, 1 reply; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2022-05-22  5:07 UTC (permalink / raw)
  To: Akira Yokosawa; +Cc: linux-doc, Jonathan Corbet

Em Sun, 22 May 2022 09:57:45 +0900
Akira Yokosawa <akiyks@gmail.com> escreveu:

> On Sat, 21 May 2022 11:46:29 +0200,
> Mauro Carvalho Chehab wrote:
> [...]
> > 
> > Sphinx 4.0 and above: it is now possible to add a namespace. This allows
> > fixing things like read() system calls that have different meanings on
> > different subsystems.
> > 
> > On other words, only with Sphinx 4.0 and above, the cross-references
> > for C domain symbols should all be OK.  
> 
> So, I noticed there is a PR at https://github.com/sphinx-doc/sphinx/pull/8313
> which is still open.

Are you sure? I always believed that this (or a variant of it) got 
merged on 4.0.

> 
> This PR is supposed to resolve "WARNING: Duplicate C declaration"
> due to struct and function with the same name, isn't it?
> 
> Are you sure the issue is resolved in Sphinx 4.0 and later?

You need to ping Sphinx C domain maintainer to be sure. This was
the author of the PR by the time I looked into it, but I'm not
tracking Sphinx development, so things might have changed.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: "WARNING: Duplicate C declaration" from recent Sphinx (was Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1)
  2022-05-22  5:07                 ` Mauro Carvalho Chehab
@ 2022-05-22  9:28                   ` Akira Yokosawa
  0 siblings, 0 replies; 21+ messages in thread
From: Akira Yokosawa @ 2022-05-22  9:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-doc, Jonathan Corbet, Akira Yokosawa

On Sun, 22 May 2022 07:07:07 +0200,
Mauro Carvalho Chehab wrote:
> Em Sun, 22 May 2022 09:57:45 +0900
> Akira Yokosawa <akiyks@gmail.com> escreveu:
> 
>> On Sat, 21 May 2022 11:46:29 +0200,
>> Mauro Carvalho Chehab wrote:
>> [...]
>>>
>>> Sphinx 4.0 and above: it is now possible to add a namespace. This allows
>>> fixing things like read() system calls that have different meanings on
>>> different subsystems.
>>>
>>> On other words, only with Sphinx 4.0 and above, the cross-references
>>> for C domain symbols should all be OK.  
>>
>> So, I noticed there is a PR at https://github.com/sphinx-doc/sphinx/pull/8313
>> which is still open.
> 
> Are you sure? I always believed that this (or a variant of it) got 
> merged on 4.0.

Hmm, I'm afraid I don't think it has ever did.

In the discussion of the PR, you commented on 15 Oct 2020 at
https://github.com/sphinx-doc/sphinx/pull/8313#issuecomment-708977457:

> On a quick test, the remaining warnings I was getting with the Linux
> Kernel docs with Sphinx 3.x have gone after this patch set.
> 
> Thanks!
>
> I'm looking forward to see those patches applied to 3.2.2 ;-)

So you might have thought that the issue was already resolved.

> 
>>
>> This PR is supposed to resolve "WARNING: Duplicate C declaration"
>> due to struct and function with the same name, isn't it?
>>
>> Are you sure the issue is resolved in Sphinx 4.0 and later?
> 
> You need to ping Sphinx C domain maintainer to be sure. This was
> the author of the PR by the time I looked into it, but I'm not
> tracking Sphinx development, so things might have changed.

Later on in the discussion, @jakobandersen reported the status of
the PR on 2 Apr 2021 at:
https://github.com/sphinx-doc/sphinx/pull/8313#issuecomment-812408069.

If I understand the complex comment correctly, there is another PR at
https://github.com/sphinx-doc/sphinx/pull/8929 as a prerequisite
to PR 8313.

PR 8929 now belongs to the 5.x milestone (missed 5.0.0). PR 8313 has never
had a milestone assigned.  So it looks to me like things are progressing
very slowly and we need to wait another release or two at least.

Of course, all of this is just my interpretation without knowing any
of Sphinx development and might be wrong in many aspects.

        Thanks, Akira

> 
> Thanks,
> Mauro

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-05-22  9:28 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-29  6:07 [PATCH] docs: sphinx/requirements: Limit jinja2<3.1 Akira Yokosawa
2022-03-29 13:01 ` Bagas Sanjaya
2022-03-29 14:00   ` Jonathan Corbet
2022-03-29 14:08     ` Akira Yokosawa
2022-03-29 14:01   ` Akira Yokosawa
2022-03-29 15:31 ` Jonathan Corbet
2022-03-29 15:36   ` Randy Dunlap
2022-03-29 23:51     ` Akira Yokosawa
2022-03-30  0:07       ` Randy Dunlap
2022-03-30  0:25   ` Mauro Carvalho Chehab
2022-03-30 14:59     ` Akira Yokosawa
2022-03-30 17:07       ` Mauro Carvalho Chehab
2022-03-31 14:32         ` Akira Yokosawa
2022-05-21  7:58           ` "WARNING: Duplicate C declaration" from recent Sphinx (was Re: [PATCH] docs: sphinx/requirements: Limit jinja2<3.1) Akira Yokosawa
2022-05-21  9:46             ` Mauro Carvalho Chehab
2022-05-21  9:59               ` Akira Yokosawa
2022-05-22  0:57               ` Akira Yokosawa
2022-05-22  5:07                 ` Mauro Carvalho Chehab
2022-05-22  9:28                   ` Akira Yokosawa
2022-03-30  1:29   ` [PATCH] docs: sphinx/requirements: Limit jinja2<3.1 Akira Yokosawa
2022-03-30 19:44     ` Jonathan Corbet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.