920638 - Integrate moz.build symbols into Sphinx documentation

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

The docstrings in sandbox_symbols.py describing the symbols in the moz.build sandbox are restructured text. Now that we have Sphinx, we should find a way to integrate them into the Sphinx docs.

FWIW, the reason there is a test to cover docstring formatting is to ensure the docstrings can be formatted consistently without breaking |mach mozbuild-reference| and other downstream consumers relying on them being valid restructured text.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Integrate moz.build symbols and Python API docs into Sphinx

This should mostly be a rubber stamp review. But, there are a few
changes in python/mozbuild that warrant review.

Essentially this patch does 3 things:

1) Adds an --api-docs mode to |mach build-docs| which will generate .rst
API docs for Mozilla's in-tree Python packages
2) Adds newly generated .rst files to version control.
3) Integrates the mozbuild sandbox symbols into Sphinx

#3 is the most complicated. I had to write a Sphinx extension to import
the symbols. I could have introduced a manual step where we need to
update an .rst file every time we update sandbox_symbols.py, but that's
tedious and won't get done. The extension turned out to be rather
trivial.

There's lots of moving around code from the mozbuild-reference mach
command module into a new mozbuild.sphinx module. I also adjusted the
ReST in sandbox_symbols.py to be warning free. It renders oh so nicely
now.

There are a few dozen formatting errors for docstrings in the Python
modules. I hope to one day see that number go down to 0. Should be a
good first bug for somebody.

Comment 2

[Michael Shal [:mshal]]

Comment on attachment 813340 [details] [diff] [review]
Integrate moz.build symbols and Python API docs into Sphinx

> @CommandProvider
>-class MozbuildFileCommands(object):
>+class MozbuildFileCommands(MachCommandBase):
>     @Command('mozbuild-reference', category='build-dev',
>         description='View reference documentation on mozbuild files.')
>     @CommandArgument('symbol', default=None, nargs='*',
>         help='Symbol to view help on. If not specified, all will be shown.')
>     @CommandArgument('--name-only', '-n', default=False, action='store_true',
>         help='Print symbol names only.')
>     def reference(self, symbol, name_only=False):
>+        # mozbuild.sphinx imports some Sphinx modules, so we need to be sure
>+        # the optional Sphinx package is installed.
>+        self._activate_virtualenv()
>+        self.virtualenv_manager.install_pip_package('Sphinx==1.1.3')
>+
>+        from mozbuild.sphinx import (
>+            format_module,
>+            function_reference,
>+            special_reference,
>+            variable_reference,
>+        )
>+
>+        import mozbuild.frontend.sandbox_symbols as m

Is it good form to import as a single character name? We don't seem to do that anywhere else.

Also, what is the rationale for checking in files that are generated automatically? In general I don't like it since it makes it confusing what is actually edit-able by a developer, and what needs to be reviewed by a reviewer. But since it seems these files are already in the tree, I don't see this as a reason to block this patch.

Comment 3

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/1190ae280cec

I don't like auto-generated files in the tree either. But doing this right would require some code to aggregate static files with auto-generated files and I figured it was easier to ignore the problem for now. Followup bug.