[PATCH v2 0/4] Better handle dependencies on Sphinx extensions

[PATCH v2 0/4] Better handle dependencies on Sphinx extensions

[Mauro Carvalho Chehab]

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

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

[Mauro Carvalho Chehab]

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 <mchehab@kernel.org>
---

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/cover.1648216561.git.mchehab@kernel.org/

 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

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

[Jonathan Corbet]

Mauro Carvalho Chehab <mchehab@kernel.org> 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 <mchehab@kernel.org>

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

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

[Mauro Carvalho Chehab]

Em Fri, 25 Mar 2022 13:19:28 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> Mauro Carvalho Chehab <mchehab@kernel.org> 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 <mchehab@kernel.org>  
> 
> 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

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

[Mauro Carvalho Chehab]

Em Sat, 26 Mar 2022 00:21:09 +0100
Mauro Carvalho Chehab <mchehab@kernel.org> escreveu:

> Em Fri, 25 Mar 2022 13:19:28 -0600
> Jonathan Corbet <corbet@lwn.net> escreveu:
> 
> > Mauro Carvalho Chehab <mchehab@kernel.org> 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 <mchehab@kernel.org>    
> > 
> > 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

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

[Jonathan Corbet]

Mauro Carvalho Chehab <mchehab@kernel.org> 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

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

[Mauro Carvalho Chehab]

Em Fri, 25 Mar 2022 18:02:09 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> Mauro Carvalho Chehab <mchehab@kernel.org> 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

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

[Mauro Carvalho Chehab]

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 <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---

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/cover.1648216561.git.mchehab@kernel.org/

 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

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

[Mauro Carvalho Chehab]

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 <mchehab@kernel.org>
---

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/cover.1648216561.git.mchehab@kernel.org/

 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

[PATCH v2 4/4] docs: kernel_include.py: add sphinx build dependencies

[Mauro Carvalho Chehab]

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.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---

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/cover.1648216561.git.mchehab@kernel.org/

 Documentation/sphinx/kernel_include.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/sphinx/kernel_include.py b/Documentation/sphinx/kernel_include.py
index f523aa68a36b..abe768088377 100755
--- a/Documentation/sphinx/kernel_include.py
+++ b/Documentation/sphinx/kernel_include.py
@@ -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