Facilitate Sphinx documentation from all parts of the tree

RESOLVED FIXED in mozilla29

Status

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: gps, Assigned: gps)

Tracking

Trunk
mozilla29
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Assignee

Description

6 years ago
In bug 917988 we added Sphinx docs for build system documentation. I've heard rumblings that other groups would like in on the party.

I was on a plane today and worked on expanding the Sphinx docs to be generic and work with docs defined in any tree. Will submit a patch in a few minutes.
Assignee

Comment 1

6 years ago
And here's my vision.

I moved the generic Sphinx docs foo into tools/docs. I added some
moz.build symbols for declaring directories to include in the Sphinx doc
tree and for defining directories containing Python packages.

I also added a mode to read moz.build files via os.walk() instead of
using the internal traversal data (TIERS, DIRS) from moz.build files.
glandium had expressed interest in doing this, so I went ahead and
implemented it for this patch.

I originally had a config file listing all the directories defining
Sphinx doc dirs and Python packages. But, that violates the principle of
having everything for a particular "module" being defined in a single
directory. We have moz.build files for defining metadata, let's use
them. Although, if you want to use a separate file for defining Sphinx
metadata so we can keep moz.build files build config specific, we can
os.walk for that instead. But, I anticipate that over time Sphinx docs
will be part of the build, so I imagine they'll make it into moz.build
eventually. But YAGNI applies.
Attachment #833299 - Flags: review?(mh+mozilla)
Assignee

Updated

6 years ago
Assignee: nobody → gps
Status: NEW → ASSIGNED
Assignee

Comment 2

6 years ago
I made it so you don't need to edit index.rst when adding new doc trees.

I made the performance of os.walk much better by skipping ignored directories and ignoring .hg and .git.

I added docs on how everything works.
Attachment #833299 - Attachment is obsolete: true
Attachment #833299 - Flags: review?(mh+mozilla)
Attachment #833368 - Flags: review?(mh+mozilla)
Assignee

Updated

6 years ago
Blocks: 939866
Comment on attachment 833368 [details] [diff] [review]
Allow Sphinx docs to come from all over the tree, v2

Review of attachment 833368 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/docs/conf.py
@@ -1,1 @@
> -# This Source Code Form is subject to the terms of the Mozilla Public

Please hg move this file.

::: python/moz.build
@@ +7,5 @@
> +SPHINX_PYTHON_PACKAGE_DIRS += [
> +    ('codegen',),
> +    ('mozbuild/mozbuild', 'test'),
> +    ('mozbuild/mozpack', 'test'),
> +    ('mozversioncontrol/mozversioncontrol',),

The syntax is really awkward and without reading sandbox_symbols.py, there's no way to guess what it means and why one would need tuples even without something to exclude. Can't we just say that we skip 'test' directories everywhere, and people wanting to use sphinx have to be aware of that?

::: python/mozbuild/mozbuild/frontend/reader.py
@@ +622,5 @@
> +        data from executed moz.build files to drive traversal.
> +
> +        This is a generator of Sandbox instances.
> +        """
> +        # FUTURE we should have an API for walking topsrcdir.

Future should probably use python AST to find all assignments to DIRS instead. Same applies for SPHINX*, by the way, because you're currently relying on the fact that they're at a convenient place, and that running the rest of the moz.build doesn't have side effects (which is why you currently have to modify some of the moz.builds).

@@ +625,5 @@
> +        """
> +        # FUTURE we should have an API for walking topsrcdir.
> +        ignores = [
> +            '.hg',
> +            '.git',

I think you should just ignore everything starting with a ., which using FileFinder from mozpack would do for you.

::: tools/docs/conf.py
@@ +27,5 @@
> +project = u'Mozilla Source Tree'
> +year = datetime.now().year
> +
> +# Grab the version from the source tree's milestone.
> +with open(os.path.join(mozilla_dir, 'config', 'milestone.txt'), 'rt') as fh:

Followup fodder: port milestone.pl to python and use that.

::: tools/docs/index.rst
@@ +28,5 @@
> +--------------------
> +
> +To add new documentation, define the ``SPHINX_TREES`` and
> +``SPHINX_PYTHON_PACKAGE_DIRS`` variables in ``moz.build`` files in
> +thre tree and documentation will automatically get picked up.

the

@@ +45,5 @@
> +   e.g. ``SPHINX_TREES['featureX'] = 'docs'``. This says *the ``docs``
> +   directory under the current directory should be installed into the
> +   Sphinx documentation tree under ``/featureX``*.
> +4. If you have Python packages you would like to generate Python API
> +   documentation for, you can use ``SPHINX_PYTHON_PACAKGE_DIRS`` to

PACKAGE

@@ +47,5 @@
> +   Sphinx documentation tree under ``/featureX``*.
> +4. If you have Python packages you would like to generate Python API
> +   documentation for, you can use ``SPHINX_PYTHON_PACAKGE_DIRS`` to
> +   declare directories containing Python packages. e.g.
> +   ``SPHINX_PYTHON_PACKAGE_DIRS += ('mozpackage',)``.

This doesn't match what's in python/moz.build.
Attachment #833368 - Flags: review?(mh+mozilla) → feedback+
Assignee

Updated

6 years ago
Depends on: 941245
Assignee

Comment 4

6 years ago
(In reply to Mike Hommey [:glandium] from comment #3)
> ::: python/mozbuild/mozbuild/frontend/reader.py
> @@ +622,5 @@
> > +        data from executed moz.build files to drive traversal.
> > +
> > +        This is a generator of Sandbox instances.
> > +        """
> > +        # FUTURE we should have an API for walking topsrcdir.
> 
> Future should probably use python AST to find all assignments to DIRS
> instead. Same applies for SPHINX*, by the way, because you're currently
> relying on the fact that they're at a convenient place, and that running the
> rest of the moz.build doesn't have side effects (which is why you currently
> have to modify some of the moz.builds).

Using AST to find all DIRS is interesting. Could be difficult if a variable (as opposed to a string literal) is assigned to a *DIRS.

moz.build evaluation should be free of side-effects other than populating an in-memory data structure. There is a reason why we strive to have sandboxing between individual moz.build files as much as possible. export() is a notable exception (which I didn't care much for when we introduced it for this reason).
Assignee

Comment 5

6 years ago
Addressed review comments modulo directory traversal foo, which I think
is out of scope. We can always change how directory traversal works in a
followup patch.
Attachment #8335614 - Flags: review?(mh+mozilla)
Assignee

Updated

6 years ago
Attachment #833368 - Attachment is obsolete: true
Comment on attachment 8335614 [details] [diff] [review]
Allow Sphinx docs to come from all over the tree

Review of attachment 8335614 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/docs/conf.py
@@ -1,1 @@
> -# This Source Code Form is subject to the terms of the Mozilla Public

Please hg move this file instead of hg rm/hg add.

::: python/mozbuild/mozbuild/frontend/reader.py
@@ +636,5 @@
> +        # Ignore object directories.
> +        ignores |= {p for p in os.listdir(self.topsrcdir) if p.startswith('obj')}
> +
> +        ignores = {os.path.normpath(os.path.join(self.topsrcdir, p))
> +            for p in ignores}

cf. comments on the FileFinder patch.

You're also not ignoring test directories besides the mozbuild one.
Attachment #8335614 - Flags: review?(mh+mozilla) → feedback+
Assignee

Comment 7

6 years ago
Fixed up review comments and rebased on top of new FileFinder ignore
API.
Attachment #8344532 - Flags: review?(mh+mozilla)
Assignee

Updated

6 years ago
Attachment #8335614 - Attachment is obsolete: true
Assignee

Comment 8

6 years ago
Upgraded to Sphinx theme 0.4, which makes the page title and other
breadcrumbs report proper values (not MDN specific things).
Attachment #8345039 - Flags: review?(mh+mozilla)
Assignee

Updated

6 years ago
Attachment #8344532 - Attachment is obsolete: true
Attachment #8344532 - Flags: review?(mh+mozilla)
Comment on attachment 8345039 [details] [diff] [review]
Allow Sphinx docs to come from all over the tree

Review of attachment 8345039 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/frontend/reader.py
@@ +629,5 @@
> +        # for DIRS references in the AST, even if a directory is added behind
> +        # a conditional. For now, just walk the filesystem and intelligently
> +        # exclude.
> +        ignore = {
> +            'python/mozbuild/mozbuild/test',

The comment about other test directories still applies. Maybe **/test?

@@ +633,5 @@
> +            'python/mozbuild/mozbuild/test',
> +        }
> +
> +        # Ignore object directories.
> +        ignore |= {p for p in os.listdir(self.topsrcdir) if p.startswith('obj')}

How about adding 'obj*' to ignore, instead?
Attachment #8345039 - Flags: review?(mh+mozilla) → review+
Assignee

Updated

6 years ago
Depends on: 948251
https://hg.mozilla.org/mozilla-central/rev/0eec37596a6d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Assignee

Updated

6 years ago
Blocks: 949906
Assignee

Updated

6 years ago
Blocks: 922191
Blocks: 986719

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.