canonical / sphinx-docs-starter-pack

A documentation starter-pack
https://canonical-starter-pack.readthedocs-hosted.com/
Other
13 stars 35 forks source link

Circular logic in build_requirements.py and custom_conf.py #197

Open s-makin opened 5 months ago

s-makin commented 5 months ago

I needed to add a custom module (gitpython) to the build requirements for use in a custom scriptlet I added in custom_conf.py. I added the module to the custom_conf.py file as suggested in the starter pack instructions. When I built locally (where I have gitpython installed), everything worked correctly and the requirements.txt file showed the module as expected.

However, it seems that we're running build_requirements.py to generate the requirements file, but the script is also importing the custom_conf.py file which already NEEDS the requirement, which makes the logic somewhat circular. When I later submitted a PR including this custom module + scriptlet, the build couldn't complete as the gitpython module was never installed.

We have implemented a workaround by using a separate script file containing my scriptlet, which I then call from conf.py after build_requirements.py and custom_conf.py, but this is not ideal in the long term since if we intend people to keep automatically up-to-date with the starter pack the conf.py file will eventually get overwritten.

ru-fu commented 5 months ago

Sorry for the noise here. :wink: It took me a while to figure out a workaround. It seems to work to change the build job for the build_requirements.py file to pre_install and then manually pip-install the missing packages before running the script. (See #201 for this change.)

This is only a workaround though and doesn't fix the underlying problem.

Any idea on how to fix this, @dviererbe ?

dviererbe commented 5 months ago

note: If I understand the Ubuntu Pro case correctly, the reason why they need the module in custom_conf.py is to setup some file structure. The code does not call or hook into any sphinx configuration. They just placed the code there, to run code before sphinx builds anything.

suggestion: Therefore the solution to this problem should be to provide hooks that can be customized by downstreams.

thought: Still, this outlines another potential problem. What if they would have actually "configured" something in custom_conf.py and need to install a module?

suggestion: I have some Ideas how to solve that:

question: @ru-fu What do you think?

ru-fu commented 5 months ago

First of all, I don't have a clear picture yet of how this dependency management will fit into the ongoing work of turning the starter pack into an extension. It might be that the problem automatically goes away then - but if it doesn't, we should probably figure out a solution that works in both scenarios.

That aside:

One idea I had (and which is funnily quite similar to your solution 2, but doesn't seem as confusing to me :laughing: ) is to have some way to exclude parts of the custom_conf.py from processing when building the requirements. The reason for that is that we're now running this file twice - which might increase build times and isn't necessary. In LXD, we have code in there to download some Swagger JavaScripts and to generate Markdown files for our man pages. We don't need any of this for building the requirements file, we need it later when actually building the docs. So if we had some part of the file always executed, and another part behind a "if ($variable)", that could work? If we have a way to set the variable accordingly both for local builds and on RTD ...