Re: [RFC v2 18/38] docs: sphinx/kernel_abi: use AbiParser directly

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Tue, 28 Jan 2025 15:37:25 -0700
Jonathan Corbet <corbet@lwn.net> escreveu:

> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
> 
> > Instead of running get_abi.py script, import AbiParser class and
> > handle messages directly there using an interactor. This shold save some
> > memory, as there's no need to exec python inside the Sphinx python
> > extension.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> >  Documentation/sphinx/kernel_abi.py | 26 +++++++++++++++-----------
> >  scripts/get_abi.py                 |  2 +-
> >  2 files changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/Documentation/sphinx/kernel_abi.py b/Documentation/sphinx/kernel_abi.py
> > index fc7500fad119..93d537d8cb6c 100644
> > --- a/Documentation/sphinx/kernel_abi.py
> > +++ b/Documentation/sphinx/kernel_abi.py
> > @@ -42,6 +42,11 @@ from docutils.parsers.rst import directives, Directive
> >  from sphinx.util.docutils import switch_source_input
> >  from sphinx.util import logging
> >  
> > +srctree = os.path.abspath(os.environ["srctree"])
> > +sys.path.insert(0, os.path.join(srctree, "scripts"))
> > +
> > +from get_abi import AbiParser  
> 
> I have to admit that I don't like this bit of messing around with the
> path.  And importing things out of scripts/ seems ... inelegant.
> 
> I take it you still want to be able to run get_abi.py standalone even
> after it's directly integrated into Sphinx?  

Yes, because calling it via command line provides:

1. a way to test the parser and check the results;
2. a search utility;
3. the undefined symbol verification.

Btw, if you look at the other Sphinx modules, they do exactly the same:
they execute code from scripts/. The only difference here is that,
instead of loading the a perl/python/shell engine and running the entire
script from there, it is importing just a class.

> In this case, it might be
> nicer to have the common library functionality in its own module that
> can be imported into both sphinx and the standalone command. 

This would be possible too: place the classes on a common lib dir and
then import it from both command line and Sphinx extensions.

If we're willing to do that, then perhaps we can have separate files
for each different class, as this could make it easier to maintain.

> That still
> leaves open the question of where that module lives
> (Documentation/sphinx perhaps?) and how the Python path gets set up
> correctly...

I guess the command line at scripts/ could use something like this to
get the library location (untested) and add to the import search PATH:

	import os

	python_lib_dir="some/location"

	scriptdir = os.path.dirname(os.path.realpath(__file__))

	sys.path.insert(0, os.path.join(srctree, f"../{python_lib_dir}"))

	from abi_parser import abiParser

Now, I'm not sure if the best location for python libraries would
be at Documentation/sphinx, as we may end needing other python
libraries with time and not all would be used by Sphinx.

In short: I would be more inclined to place them on 
a new lib directory (tools/lib? tools/py_lib? scripts/lib?).

See, with the content of this series, if we split files per each class,
it would mean 3 files:

  1. abi_parser.py, containing AbiParser class (plus ABI_DIR const);
     (this is the only class used by Documentation/sphinx extensions)
  2. abi_regex.py, containing AbiRegex class;
  3. abi_symbols.py, containing SystemSymbols class.

Now, if we're going on this direction, it may also make sense to split
the command line classes/functions into 4 (or 5 files) for argparse
argument definition and command run code. If we do that, it means that
other files will be stored somewhere:

  4. abi_cmd_rest.py: AbiRest and AbiValidate classes for the rest 
     and validate arguments (I would likely place both at the same file,
     as the code is similar - but it could also be split on two separate
     files);
  5. abi_cmd_search.py: AbiSearch - for the search arguments;
  6. abi_cmd_undefined.py: AbiUndefined - for the undocumented symbol check 
     arguments;

Finally, there is the one under scripts/:

  7. get_abi.py: with the main function

For (1), Documentation/sphinx could make sense, but (2) to (6) are
used only by the command line tool. Placing them at Documentation/ 
seems weird. Well, nothing prevents having them at scripts/, IMHO, things
would become more organized if we place the Python files with 0644
flags elsewhere.

Thanks,
Mauro