Holzhaus / sphinx-multiversion

Sphinx extension for building self-hosted versioned docs.
https://holzhaus.github.io/sphinx-multiversion/
BSD 2-Clause "Simplified" License
148 stars 65 forks source link

Same autodoc content end up in all doc-versions due to calling conf.py one time too many #69

Open Griatch opened 3 years ago

Griatch commented 3 years ago

Summary

Running sphinx-multisession will run conf.py two times per branch/tag, so if the code is imported in conf.py it will be cached by Python and the calling branch's code will be used for all autodocs across all versions.

The setup

Here's how a part of my conf.py looks (simplfied to show the concept):

...
ROOT_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))

print(ROOT_DIR)   # for debug

sys.path.insert(0, ROOT_DIR)

import mypackage
mypackage.custom_init()

...

This is a requirement of mypackage since for sphinx to be able to import and work with mypackage, it must be properly initialized first at least once. Not shown are also that this configures autodoc to build automatic api documentation from mypackage docstrings.

The issue

Let's say I have master and develop branches, with the same content. I then make a docstring change to a function only in the develop branch.

Needless to say, this makes sphinx-multiversion somewhat useless. :)

Analysis

We'll consider three directories:

When printing the ROOT_DIR value it's clear sphinx-multiversion calls sphinx one time too many. If we run it from develop branch,

As seen above, the conf file will always be intialized once under the path you call sphinx-multiversion from. This means that in my conf file, the import of mypackage will be cached by Python and not be reimported when it is then called again later (since that happens in the same subprocess). The result is that the package always appears to the sphinx process as being the develop-branch version and will use the docstrings from that branch.

Solution

Here's how I could rewrite the conf file:

...
ROOT_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
sys.path.insert(0, ROOT_DIR)

modnames = [mname for mname in sys.modules if mname.startswith("mypackage")]
for modname in modnames:
    del sys.modules[modname]

import mypackage
mypackage.custom_init()

...

In other words, one must clean all packages and sub-packages of mypackage manually to get rid of the previous install (just doing a importlib.reload(mypackage) doesn't work if mypackage is a complex package with a lot of sub-packages like in this case). Doing the for-loop does fix the issue, but it will lead to a truckload of warning messages on every build since Python will warn that doing this is not recommended and can lead to strange side effects (and maybe it does, it's hard to verify without going through every docstring).

One could also consider trying to identify which calling args are being used to see if mypackage should be initialized or not. This feels pretty hacky though.

The true reason for this issue is in sphinx-multiversion, in main.py, line 322-323:

        current_argv.extend(
                [
                    *defines,
                    "-D",
                    "smv_current_version={}".format(version_name), 
                     "-c",                            #  <------
                     confdir_absolute,       #  <------
                    data["sourcedir"],
                    data["outputdir"],
                    *args.filenames,
                ]
            )
            logger.debug("Running sphinx-build with args: %r", current_argv)
            cmd = (
                sys.executable,
                *get_python_flags(),
                "-m",
                "sphinx",
                *current_argv,
            )

Commenting out -c confir_absolute resolves this issue because then sphinx-multiversion will only be calling conf.py one time per branch/tag (within each tmp/ dir) and since the branches are each processed in a separate process there is no risk of this bug appearing.

I'd make a PR for it, but it's a trivial fix and I don't know if there is some other reason for the calling location's conf.py to be passed that I'm not aware of.

oesteban commented 2 years ago

I think this is more of an issue of sphinx itself. I have successfully overcome the autodoc problem by forcing the reload of cached modules in the sphinx importer function.

Griatch commented 2 years ago

The fact remains that without -c confdir_absolute this problem goes away. :shrug:

oesteban commented 2 years ago

Sure, I'm not the maintainer. 👍

andypexai commented 4 months ago

I think this is more of an issue of sphinx itself. I have successfully overcome the autodoc problem by forcing the reload of cached modules in the sphinx importer function.

I'm interested in that solution but I don't know quite how to implement that. I can't find an event to make a callback for. This autodoc-process-docstring event is the closest thing I found. Do you have an example somewhere?