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

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.

---

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(+)

index 457712355676..b7154cf60c1d 100755
@@ -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

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

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

Yeah, agreed.

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

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

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

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

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

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.

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

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

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.

---

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(+)

index 4392b3cb4020..efab9b14a9f5 100644
@@ -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

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

---

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(-)

index 8138d69a6987..6b40ac1806bb 100644
@@ -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

The files included by kernel-include should be added as build dependencies, in order for sphinx-build to rebuild the corresponding docs if any changes at the included file happens.

---

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_include.py | 3 +++
 1 file changed, 3 insertions(+)

index f523aa68a36b..abe768088377 100755
@@ -59,6 +59,7 @@ class KernelInclude(Include):
     u"""KernelInclude (``kernel-include``) directive"""
 
     def run(self):
+        env = self.state.document.settings.env
         path = os.path.realpath(
             os.path.expandvars(self.arguments[0]))
 

@@ -70,6 +71,8 @@ class KernelInclude(Include):
 
         self.arguments[0] = path
 
+        env.note_dependency(os.path.abspath(path))
+
         #return super(KernelInclude, self).run() # won't work, see HINTs in _run()
         return self._run()
 

2.35.1