Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
s-makin opened this issue Mar 7, 2024 · 3 comments
Open

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

s-makin opened this issue Mar 7, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@s-makin
Copy link
Contributor

s-makin commented Mar 7, 2024

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 ru-fu added the bug Something isn't working label Mar 7, 2024
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 12, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 13, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 13, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 13, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 13, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/starter-pack that referenced this issue Mar 13, 2024
Changing the build job to `pre_install` for the `build_requirements.py`
script means that the build machine will already be fully set up, so
that you can use pip to install any packages that are required in
`custom_conf.py` and will otherwise give errors.

This is required to work around canonical#197.

Signed-off-by: Ruth Fuchss <[email protected]>
@ru-fu
Copy link
Contributor

ru-fu commented Mar 13, 2024

Sorry for the noise here. 😉
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 ?

ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 13, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 13, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/lxd that referenced this issue Mar 13, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
tomponline pushed a commit to tomponline/lxd that referenced this issue Mar 13, 2024
Manually install gitpython, because it must be available to
generate the requirements.txt file, which is before the venv is
created.

Issue for this is here:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Ruth Fuchss <[email protected]>
(cherry picked from commit 5db8be4)
@dviererbe
Copy link
Contributor

dviererbe commented Mar 13, 2024

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:

  • Solution 1: two rounds of dependency installation:
    1. we introduce a conf_requirements.txt file which just contains dependencies needed for custom_conf.py and will be installed before build_requirements.py runs
    2. build_requirements.py will build a build_requirements.txt based on custom_conf.py which then gets installed
  • Solution 2: Have a 2nd custom_conf.py file that gets called after pip install from conf.py
  • Solution 3: Have a static config file (e.g. json, yaml, etc.) that is used to build requirements.txt
  • "Solution" 4: We do not support that use case.

question: @ru-fu What do you think?

simondeziel added a commit to simondeziel/lxd that referenced this issue Mar 13, 2024
Only source SPHINXENV file once and consistently use pip while working around
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Simon Deziel <[email protected]>
simondeziel added a commit to simondeziel/lxd that referenced this issue Mar 13, 2024
Consistently use pip and link to the upstream bug:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Simon Deziel <[email protected]>
simondeziel added a commit to simondeziel/lxd that referenced this issue Mar 13, 2024
And link to the upstream bug:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Simon Deziel <[email protected]>
tomponline pushed a commit to tomponline/lxd that referenced this issue Mar 13, 2024
And link to the upstream bug:
canonical/sphinx-docs-starter-pack#197

Signed-off-by: Simon Deziel <[email protected]>
(cherry picked from commit 1d3b87f)
@ru-fu
Copy link
Contributor

ru-fu commented Mar 13, 2024

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:

  • Solution 1 seems to be the easiest. What I don't like about it is that it can be confusing what you need to add into the conf_requirements.txt file, and that you might need to add the same tools to several files. But we can document that, and maybe automatically include the conf_requirements.txt file into the generated requirements.txt file.
  • Solution 2 would be very confusing imo. How do you decide what to put where?
  • How does solution 3 differ from just having a static requirements.txt file?
  • Definitely not solution 4, since we have several projects that need custom code in custom_conf.py.

One idea I had (and which is funnily quite similar to your solution 2, but doesn't seem as confusing to me 😆 ) 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 ...

ru-fu added a commit to ru-fu/starter-pack that referenced this issue Jun 4, 2024
See canonical#197
If projects include additional Python packages in custom_conf.py,
those must be installed before running the build_requirements.py
script.
This change makes it possible to just specify such packages in
the ADDPREREQS variable.

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/starter-pack that referenced this issue Jun 11, 2024
See canonical#197
If projects include additional Python packages in custom_conf.py,
those must be installed before running the build_requirements.py
script.
This change makes it possible to just specify such packages in
the ADDPREREQS variable.

Signed-off-by: Ruth Fuchss <[email protected]>
ru-fu added a commit to ru-fu/starter-pack that referenced this issue Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants