-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Build: implement build.jobs.create_environment
and build.jobs.install
#11551
Comments
does anything speak against always creating and running from a venv? so the |
you could consider always 'activating' the venv docker-style: # this env var is exported when you activate a venv
ENV VIRTUAL_ENV="/usr/share/python3/app"
# this PATH prepend is performed when you activate a venv
ENV PATH="${VIRTUAL_ENV}/bin:${PATH}"
# create the venv, it's already activated by the commands above
RUN python3.12 -m venv ${VIRTUAL_ENV}
# that's it. and now (illustrative) your build.jobs.install and onwards
# pip points at /usr/share/python3/app/bin/pip which installs stuff to the venv
RUN pip install uv
# uv points at /usr/share/python3/app/bin/uv which installs stuff to the venv
RUN uv pip install -r docs.txt
# sphinx-build points at /usr/share/python3/app/bin/sphinx-build
RUN sphinx-build ... |
So, if a user overrides the create_environment step, all other commands won't work, as they depend on the If we ask users to override all other steps if create_enviroment is overridden, not sure if there is a difference in just letting users run any commands (as build.commands does). We could ask users to always create an environment on those paths, but then we will likely need to provide examples for each tool on how to create an environment on a given path (if they support this easily), and/or maybe having them call the commands like this example https://docs.readthedocs.io/en/stable/build-customization.html#install-dependencies-with-poetry And all that is for Python based projects, I see that #11216 is also linked, users will need to override all steps when using a different tool like node, and it will also be weird for those projects having to have a Do we still want to go in this direction? Looks like we still have some things to decide |
I think that's the compromise/contract the user assumes when overriding these jobs, and that's fine to me for now. On the other hand, I think we can explore a different approach here as a kind of replacement of the version: 2
build:
os: ubuntu-24.04
tools:
python: latest
apt_packages:
- tree
jobs:
install:
- pip install requirements.txt
build:
html:
- sphinx-build -j auto -E -b html docs/ $READTHEDOCS_OUTPUT/html Note that I'm not defining Footnotes |
I think both of the these approaches make sense, but I generally think it's fine if folks break their build overriding stuff as Manuel said. I'm not 100% sure about a generic builder, but it does seem like a nice option for folks who want a bit more structure. I think this is what @agjohnson has been advocating for instead of |
Yeah, overall I most interested in |
I guess the question is what we want to achieve with this: Allow users to use another package managerThis won't be easy to do for users, since all of our commands expect a virtual env to be created in a specific path. And uv for example has a proper interface to run commands (is there a way to even run those commands without using uv?). Other package managers don't even create a Python virtual env (node). We install some additional "core" deps using pip We run the sphinx/mkdocs commands using Allow users to do custom things but still be able to install apt-packagesI guess this depends on the first point, if there is a lot of friction for users to change the package manager, this won't matter much. Commands run in a structured mannerNot sure if I understand why this is relevant for us, at the end we care only about the output docs, the process that was followed to generate those aren't relevant for the final output. Other use cases?... My conclusion is that if we add support for this, in the current state, users will need to do a lot of work and workarounds in order to use their tools, and they will be using these tools in a special manner, not like they were designed to (like to run a command using Using build.commands is just more explicit for me, users use their tools as they will normally use them locally, no new workarounds to learn, only output their docs in the given directories. The only problem is that they can't install apt-packages, but that's something we can solve there without having to implement a new feature (just allow sudo, or just install the listed apt-packages).
Overriding the environment creation and installation step for me is still for advanced users, as mentioned, they won't be using this tools like they are used to. If we still want to go with this direction, there are some things we need to decide, and documentation to write for the workarounds needed for the tools we want to support. |
The more structure we have, the better for the platform. As an example, we don't want to run the PDF build on PR builder to speed up the preview. Eventually, this could also be configurable from the Admin -- that will be possible if we have a structured build. I see the proposal we discussed useful for these different use cases:
In my opinion, the proposed solution covers all the use cases you described but adding structure to the build process, which as I mentioned, it has some current benefits but also leaves the door open to potential features in the future. |
I don't agree with this. Users wanting to use builds:
# https://docs.astral.sh/uv/getting-started/
jobs:
create_environment:
- curl -LsSf https://astral.sh/uv/install.sh | sh
- uv python install 3.12 # not needed if `build.tools.python: 3.12` is declared
install:
- uv pip install -r requirements.txt
build:
html:
- uv run sphinx -b html ...
pdf: # won't be ran on PR builds
- uv run sphinx -b pdf ...
I want to eventually migrate off the build process from the application and give users the ability to do whatever they want and having root access; but we are not there yet. Even if we were close to that, it still won't be a replacement of the structured build process that |
In your example, you are also overriding the build steps, which is not mentioned in this initial implementation. In case we want to also implement the customization of the build, there are some other questions, like what to do with the |
That is what we expect to implement immediately after we implement Implementing first |
I'll echo what @humitos is pointing out here as well, I think we're on the same page here. This pattern might have been more obvious starting with The main goal is to provide more touch points on I'd go one step further and say there might be a future where even |
Indeed. It feels like there are still bits to decide around how this integrates with the existing config options. I think we may need a design doc here, or a more specif details about how this feature integrates with the existing config. |
On our call today, we discussed moving forward here. It sounds like the validation might be complex, and we will need to figure out exactly what that looks like. If it's easy, we're going to support a generic builder (no Another big question is what to do with |
We've been discussing multiple times a way to override pre-defined jobs. It seems we have reached a good timeframe where we can prioritize this work now that we are enabling addons by default on October 7th.
I propose to start with
build.jobs.create_environment
andbuild.jobs.install
so people can start using any package manager they want: Poetry, uv, pixi, pdm, etc.Usage example for
uv
Important things to consider
uv pip install sphinx
, how do we runsphinx-build
? is it going to be in the path already?build.jobs.create_environment
is overwritten?build.jobs.create_enviroment
does in the example?Related issues
build.commands
shall support custom package installation (build.apt_packages
) #9599build.jobs
could havepre_upload
andpost_upload
jobs #9139The text was updated successfully, but these errors were encountered: