2022-03-25 18:27:07

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v2 0/4] Better handle dependencies on Sphinx extensions

Sphinx has its own way to identify the need of rebuilding the documentation.
It means that extensions need to use an internal API in order to notify about
the need to consider other files.

The kerneldoc.py extension already does that, maintainers_include.py doesn't
need (as it uses an API that internally does that), and kfigure.py does it on a
different way. So, those are already safe.

However, other extensions don't notify nor implement their own checks,
so, when a file that was parsed by them is changed, the corresponding
documentation won't be rebuilt.

This series add support for it for ABI, features and kernel-include.

Mauro Carvalho Chehab (4):
scripts/get_feat.pl: allow output the parsed file names
docs: kernel_abi.py: add sphinx build dependencies
docs: kernel_feat.py: add build dependencies
docs: kernel_include.py: add sphinx build dependencies

Documentation/sphinx/kernel_abi.py | 4 ++++
Documentation/sphinx/kernel_feat.py | 20 ++++++++++++++++++--
Documentation/sphinx/kernel_include.py | 3 +++
scripts/get_feat.pl | 11 +++++++++++
4 files changed, 36 insertions(+), 2 deletions(-)

--
2.35.1



2022-03-25 18:55:50

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v2 3/4] docs: kernel_feat.py: add build dependencies

Ensure that the feature files will be regenerated if any
changes happen at the Documentation/features files that were
processed by gen_feat.pl.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v2 0/4] at: https://lore.kernel.org/all/[email protected]/

Documentation/sphinx/kernel_feat.py | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/sphinx/kernel_feat.py b/Documentation/sphinx/kernel_feat.py
index 8138d69a6987..6b40ac1806bb 100644
--- a/Documentation/sphinx/kernel_feat.py
+++ b/Documentation/sphinx/kernel_feat.py
@@ -33,6 +33,7 @@ u"""

import codecs
import os
+import re
import subprocess
import sys

@@ -82,7 +83,7 @@ class KernelFeat(Directive):

env = doc.settings.env
cwd = path.dirname(doc.current_source)
- cmd = "get_feat.pl rest --dir "
+ cmd = "get_feat.pl rest --enable-fname --dir "
cmd += self.arguments[0]

if len(self.arguments) > 1:
@@ -102,7 +103,22 @@ class KernelFeat(Directive):
shell_env["srctree"] = srctree

lines = self.runCmd(cmd, shell=True, cwd=cwd, env=shell_env)
- nodeList = self.nestedParse(lines, fname)
+
+ line_regex = re.compile("^#define FILE (\S+)$")
+
+ out_lines = ""
+
+ for line in lines.split("\n"):
+ match = line_regex.search(line)
+ if match:
+ fname = match.group(1)
+
+ # Add the file to Sphinx build dependencies
+ env.note_dependency(os.path.abspath(fname))
+ else:
+ out_lines += line + "\n"
+
+ nodeList = self.nestedParse(out_lines, fname)
return nodeList

def runCmd(self, cmd, **kwargs):
--
2.35.1

2022-03-25 19:07:50

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v2 1/4] scripts/get_feat.pl: allow output the parsed file names

Such output could be helpful while debugging it, but its main
goal is to tell kernel_feat.py about what files were used
by the script. Thie way, kernel_feat.py can add those as
documentation dependencies.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v2 0/4] at: https://lore.kernel.org/all/[email protected]/

scripts/get_feat.pl | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/scripts/get_feat.pl b/scripts/get_feat.pl
index 457712355676..b7154cf60c1d 100755
--- a/scripts/get_feat.pl
+++ b/scripts/get_feat.pl
@@ -13,6 +13,7 @@ my $man;
my $debug;
my $arch;
my $feat;
+my $enable_fname;

my $basename = abs_path($0);
$basename =~ s,/[^/]+$,/,;
@@ -31,6 +32,7 @@ GetOptions(
'arch=s' => \$arch,
'feat=s' => \$feat,
'feature=s' => \$feat,
+ "enable-fname" => \$enable_fname,
man => \$man
) or pod2usage(2);

@@ -95,6 +97,10 @@ sub parse_feat {
return if ($file =~ m,($prefix)/arch-support.txt,);
return if (!($file =~ m,arch-support.txt$,));

+ if ($enable_fname) {
+ printf "#define FILE %s\n", abs_path($file);
+ }
+
my $subsys = "";
$subsys = $2 if ( m,.*($prefix)/([^/]+).*,);

@@ -580,6 +586,11 @@ Output features for a single specific feature.
Changes the location of the Feature files. By default, it uses
the Documentation/features directory.

+=item B<--enable-fname>
+
+Prints the file name of the feature files. This can be used in order to
+track dependencies during documentation build.
+
=item B<--debug>

Put the script in verbose mode, useful for debugging. Can be called multiple
--
2.35.1

2022-03-25 19:42:11

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v2 2/4] docs: kernel_abi.py: add sphinx build dependencies

Ensure that Sphinx-build will handle the files parsed by
get_abi.pl as dependencies. This way, if they are touched,
the ABI output will be regenerated.

Reported-by: Hans de Goede <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v2 0/4] at: https://lore.kernel.org/all/[email protected]/

Documentation/sphinx/kernel_abi.py | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/sphinx/kernel_abi.py b/Documentation/sphinx/kernel_abi.py
index 4392b3cb4020..efab9b14a9f5 100644
--- a/Documentation/sphinx/kernel_abi.py
+++ b/Documentation/sphinx/kernel_abi.py
@@ -128,6 +128,7 @@ class KernelCmd(Directive):
return out

def nestedParse(self, lines, fname):
+ env = self.state.document.settings.env
content = ViewList()
node = nodes.section()

@@ -154,6 +155,9 @@ class KernelCmd(Directive):
self.do_parse(content, node)
content = ViewList()

+ # Add the file to Sphinx build dependencies
+ env.note_dependency(os.path.abspath(f))
+
f = new_f

# sphinx counts lines from 0
--
2.35.1

2022-03-25 20:25:25

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] scripts/get_feat.pl: allow output the parsed file names

Mauro Carvalho Chehab <[email protected]> writes:

> Such output could be helpful while debugging it, but its main
> goal is to tell kernel_feat.py about what files were used
> by the script. Thie way, kernel_feat.py can add those as
> documentation dependencies.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>

So I think these are worth getting into 5.18, but I do have one question:

> @@ -95,6 +97,10 @@ sub parse_feat {
> return if ($file =~ m,($prefix)/arch-support.txt,);
> return if (!($file =~ m,arch-support.txt$,));
>
> + if ($enable_fname) {
> + printf "#define FILE %s\n", abs_path($file);
> + }
> +

Why do you output the file names in this format? This isn't input to
the C preprocessor, so the #define just seems strange. What am I
missing here?

Thanks,

jon

2022-03-25 23:48:47

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] scripts/get_feat.pl: allow output the parsed file names

Em Fri, 25 Mar 2022 13:19:28 -0600
Jonathan Corbet <[email protected]> escreveu:

> Mauro Carvalho Chehab <[email protected]> writes:
>
> > Such output could be helpful while debugging it, but its main
> > goal is to tell kernel_feat.py about what files were used
> > by the script. Thie way, kernel_feat.py can add those as
> > documentation dependencies.
> >
> > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
>
> So I think these are worth getting into 5.18,

Yeah, agreed.

> but I do have one question:
>
> > @@ -95,6 +97,10 @@ sub parse_feat {
> > return if ($file =~ m,($prefix)/arch-support.txt,);
> > return if (!($file =~ m,arch-support.txt$,));
> >
> > + if ($enable_fname) {
> > + printf "#define FILE %s\n", abs_path($file);
> > + }
> > +
>
> Why do you output the file names in this format? This isn't input to
> the C preprocessor, so the #define just seems strange. What am I
> missing here?

Well, I didn't think much about that... I just ended using a way that is
already used on get_abi.pl, and was originally imported from kernel-doc :-)

It could be using whatever other tag, but I would keep those three scripts
using a similar markup string for file names and line numbers:

scripts/get_abi.pl:
printf "#define LINENO %s%s#%s\n\n", $prefix, $file[0], $data{$what}->{line_no};

scripts/kernel-doc:
print "#define LINENO " . $lineno . "\n";

Thanks,
Mauro

2022-03-25 23:57:40

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] scripts/get_feat.pl: allow output the parsed file names

Em Sat, 26 Mar 2022 00:21:09 +0100
Mauro Carvalho Chehab <[email protected]> escreveu:

> Em Fri, 25 Mar 2022 13:19:28 -0600
> Jonathan Corbet <[email protected]> escreveu:
>
> > Mauro Carvalho Chehab <[email protected]> writes:
> >
> > > Such output could be helpful while debugging it, but its main
> > > goal is to tell kernel_feat.py about what files were used
> > > by the script. Thie way, kernel_feat.py can add those as
> > > documentation dependencies.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> >
> > So I think these are worth getting into 5.18,
>
> Yeah, agreed.
>
> > but I do have one question:
> >
> > > @@ -95,6 +97,10 @@ sub parse_feat {
> > > return if ($file =~ m,($prefix)/arch-support.txt,);
> > > return if (!($file =~ m,arch-support.txt$,));
> > >
> > > + if ($enable_fname) {
> > > + printf "#define FILE %s\n", abs_path($file);
> > > + }
> > > +
> >
> > Why do you output the file names in this format? This isn't input to
> > the C preprocessor, so the #define just seems strange. What am I
> > missing here?
>
> Well, I didn't think much about that... I just ended using a way that is
> already used on get_abi.pl, and was originally imported from kernel-doc :-)
>
> It could be using whatever other tag, but I would keep those three scripts
> using a similar markup string for file names and line numbers:
>
> scripts/get_abi.pl:
> printf "#define LINENO %s%s#%s\n\n", $prefix, $file[0], $data{$what}->{line_no};
>
> scripts/kernel-doc:
> print "#define LINENO " . $lineno . "\n";

Btw, maybe we could replace them tree with a Sphinx comment, like:

get_feat.pl:
.. FILE <file_name>
kernel-doc:
.. LINE <line_number>
get_abi.pl:
.. FILE_LINE <file_name>:<line_number>

(or something similar)

Just let me know what you prefer and I can take care of the needed
changes on this patch and, if it is the case, writing the extra patches
in order to use the same model on kernel-doc and get_abi.pl.

Thanks,
Mauro

2022-03-26 00:42:03

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] scripts/get_feat.pl: allow output the parsed file names

Mauro Carvalho Chehab <[email protected]> writes:

> Btw, maybe we could replace them tree with a Sphinx comment, like:
>
> get_feat.pl:
> .. FILE <file_name>
> kernel-doc:
> .. LINE <line_number>
> get_abi.pl:
> .. FILE_LINE <file_name>:<line_number>
>
> (or something similar)
>
> Just let me know what you prefer and I can take care of the needed
> changes on this patch and, if it is the case, writing the extra patches
> in order to use the same model on kernel-doc and get_abi.pl.

If it were just me, I'd just put "FILE <name>" or something simple.

I don't really have a strong opinion on the matter though; it's not like
people have to actually look at these things. I was mostly curious as
to why you'd done it that way. I can take the original patches or any
of the variants above; just let me know which you like best and we'll
get this done.

Thanks,

jon

2022-03-26 21:47:26

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] scripts/get_feat.pl: allow output the parsed file names

Em Fri, 25 Mar 2022 18:02:09 -0600
Jonathan Corbet <[email protected]> escreveu:

> Mauro Carvalho Chehab <[email protected]> writes:
>
> > Btw, maybe we could replace them tree with a Sphinx comment, like:
> >
> > get_feat.pl:
> > .. FILE <file_name>
> > kernel-doc:
> > .. LINE <line_number>
> > get_abi.pl:
> > .. FILE_LINE <file_name>:<line_number>
> >
> > (or something similar)
> >
> > Just let me know what you prefer and I can take care of the needed
> > changes on this patch and, if it is the case, writing the extra patches
> > in order to use the same model on kernel-doc and get_abi.pl.
>
> If it were just me, I'd just put "FILE <name>" or something simple.

This won't be a problem here, but I would prefer to use something that
would have zero chances of causing issues at kernel-doc and on other
similar scripts that we might end needing. Using a valid ReST tag like
starting it with ".." should fulfill such goal.

A side effect is that the output of the script would be a valid ReST file,
either with or without file/line information, which sounds a good idea
on my eyes.

> I don't really have a strong opinion on the matter though; it's not like
> people have to actually look at these things.

Yes. That's why I ended using #define, the actual meta-tag won't matter
for people.

> I was mostly curious as
> to why you'd done it that way. I can take the original patches or any
> of the variants above; just let me know which you like best and we'll
> get this done.

Ok. I'll resubmit the patch changing to the comments tag. I'll add
patches also for kernel-doc and get_abi.pl for all of them to use
the same meta-tag.

Thanks,
Mauro