On 06/02/2024 09:49, Jani Nikula wrote:
On Mon, 05 Feb 2024, Vegard Nossum <vegard.nossum@xxxxxxxxxx> wrote:
It probably doesn't matter a whole lot what we actually pass here,
but the .rst being processed seems most appropriate to me.
But it's not the actual .rst file being parsed here. It's something
originating from some oher file and processed in between. The kernel-doc
extension takes care to map the parsing errors in source code comments
to the right source file and line, which is where the problem is, not in
the .rst file.
The line numbers in the error messages will be adjusted according to the
ViewList. So I don't think you'll get messages that actually point at
line where the directive is either.
Please experiment with some errors injected to see what the output will
be.
Thanks for the comment, also see my related response to Mauro here:
<https://lore.kernel.org/all/3fce153d-37c5-4c4d-a4b3-44e376daa095@xxxxxxxxxx/>
TL;DR: there are several different issues in both kernel_abi.py and
kernel_feat.py related to error reporting, and it's on my TODO.
High-level explanation:
Both scripts register an rST directive which call out to a Perl script
(scripts/get_{abi,feat}.py, respectively). The Perl script generates a
mixture of rST syntax, some of which it generates itself on the fly and
some of which comes from auxiliary files (the Documentation/ABI/** and
Documentation/features/** files, respectively).
Now, the two scripts have diverged a little bit in their implementation
of exactly how they do this.
kernel_abi.py (get_abi.pl) is the one I would consider "more correct",
since it actually attempts to attribute the source lines correctly, by
taking the "top-level" filename as the 'fname' parameter to
nestedParse() and updating the current file/line number whenever it
encounters '.. LINENO:' from the Perl script's output.
What you say is true -- when there is a warning at the "top level"
(before any '.. LINENO:' directive has been encountered), kernel_abi.py
does not report the actual .rst file being parsed, it reports the
location of the '.. kernel-abi:' directive instead. This is what I
changed in this patch (2/8) -- I changed it from being
self.arguments[0], which is the argument to the .. kernel-abi:
directive, in most cases something like ABI/stable or ABI/obsolete,
which is not even an .rst file, to being the .rst file that contains the
directive. My rationale is that this is more helpful than just being
pointed to something that isn't even an .rst file.
Now, about kernel_feat.py (get_feat.pl), it has similar logic, but
worse, in a way: get_feat.pl outputs all its '.. FILE:' lines at the top
of its output, followed by the rest of the generated .rst content. So
there isn't really much point in trying to attribute these lines to the
"correct" file -- yes, we should probably fix get_feat.pl here. Anyway,
the script doesn't parse the '.. FILE:' directives in the same way, it
just attributes ALL the generated lines to self.arguments[0] (or
doc.current_source after this patch). The only thing it actually uses
those directives for is to tell Sphinx about dependencies so the output
is rebuilt if one of the sources change.
The bottom line is that this reporting has NEVER fully worked as intended.
I was attempting to push things in the right direction with this patch
series, but I didn't want to do too much at once since that makes it
even harder for the patches to get past the Great (Review) Filter.
Vegard
