Build: implementation of `build.commands` #9150

humitos · 2022-04-27T15:46:31Z

Initial implementation of build.commands as a beta feature to expose to user and collect more use cases.

Build without using any tool

version: 2

build:
  os: ubuntu-20.04
  commands:
    - mkdir output/
    - echo "Hello world! `date`" > output/index.html

  tools:  # <--- this shouldn't be required for these cases where no tools are needed for the build
    python: "3"

(https://github.com/readthedocs/test-builds/blob/build-commands/.readthedocs.yaml)

Build details page

`index.html` file served by El Proxito

Build using `pelican`

version: 2

build:
  os: ubuntu-22.04
  tools:
    python: "3.10"
  commands:
    - pip install pelican[markdown]
    # After installing Python packages with binaries on it, we need to call `asdf reshim`
    # (recreate shims for version of a package)
    # https://github.com/danhper/asdf-python/issues/37
    - asdf reshim
    - pelican --settings docs/pelicanconf.py --output output/ docs/

(https://github.com/readthedocs/test-builds/blob/pelican/.readthedocs.yaml)

Build details page

`index.html` file served by El Proxito

Decisions / Notes

Expose the feature as beta, communicating this in the documentation and also on the build details page when we detect build.commands on the config file
It defines an "implicit contract" where the output/ folder under the repository checkout will be uploaded to S3
No HTML integrations will be done (no flyout, no ads, no search)
No extra formats like ePUB, PDF and HTMLZip will be available
Without the flyout integration, this is probably more useful for single-version projects
build.os and build.tools are mandatory when using build.commands
We should probably make build.tools not required since it's possible to generate HTML without any of these languages as shown in the first example where only echo command is used
asdf requires re-shim the binaries after installing new CLI tools with Python to be able to find them when executing any of these commands. There was an attempt to re-shim automatically at Automatically run reshim? asdf-community/asdf-python#37 but they removed the post_<tool>_<shim> because Revert to using exec when running a shim asdf-vm/asdf#502. This may need some extra research to make the UX better. However, for now, I'm adding asdf reshim to my build.commands. We could execute asdf reshim python after each of the build.commands, for now as well
I didn't update the "Build customization" page, but I'm happy to add a small example there (https://docs.readthedocs.io/en/stable/build-customization.html) if we consider it's good at this point. However, it may require a good amount of re-writing on that page to make the distinction between the two approaches clear

Related #9062 and others linked into that issue.

ericholscher · 2022-04-27T19:27:49Z

Neat how little code this is. Definitely needs a lot more done around the edges to make it nicer, but great that we already have all the basic infrastructure to make it work. 👍

readthedocs/projects/tasks/builds.py

Minimal implementation of `build.commands` as a POC to show how it could work. It's using a `.readthedocs.yaml` similar to this one: ```yaml version: 2 build: os: ubuntu-20.04 commands: - mkdir output/ - echo "Hello world" > output/index.html tools: python: "3" ``` This, of course, is not a good implementation and it's done pretty quick as a way to show what parts of the code are required to be touched. I think this will help with the discussion about how the UX around `build.commands` could work. It defines an "implicit contract" where the `output/` folder under the repository checkout will be uploaded to S3 and no HTML integrations will be done.

Reference #9238

The method `install_build_tools` is not tied to the Python environment, and it's more related to the build's director, instead. I'm moving it to the `BuildDirector` class also because it's needed there when using `build.commands` as the first step to execute to install the `build.tools` specified by the user to build their docs.

…mitos/build-commands

humitos · 2022-05-23T10:38:07Z

I'm asking review for the three of you @ericholscher, @agjohnson, and @stsewd because I think this is an important change that will mold our path moving forward regarding the UX.

…mitos/build-commands

ericholscher

This looks close to me. I think we're close enough to get it out next week for testing. There's lots of additional work to be done in docs, etc. -- but this is enough for now.

docs/user/build-customization.rst

ericholscher · 2022-05-31T21:30:47Z

docs/user/config-file/v2.rst

@@ -473,7 +473,7 @@ Specify a list of commands that Read the Docs will run on the build process.
 When ``build.commands`` is used, none of the :term:`pre-defined build jobs` will be executed.
 (see :doc:`/build-customization` for more details).
 This allows you to run custom commands and control the build process completely.
-The ``output/`` directory (relative to the checkout's path) will be uploaded and hosted by Read the Docs.
+The ``_readthedocs/html`` directory (relative to the checkout's path) will be uploaded and hosted by Read the Docs.


This still feels really small compared to how important the information is, but we can improve it over time.

I agree. This has to be a lot more prominent. However,

I wasn't sure how to make it fit here in a good way

I'm not sure this is the right place for explaining this more. From the config file, we should point users to the "Build customization" page and explain all the things in detail there. Otherwise, we will be overloading the config file reference

I'm thinking we may need a section in the "Build customization" page that list all the considerations required to use build.commands or something like that we can link from here. I feel I'm not coming with a good idea of how to write this, tho 🤷🏼

Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

Tests were failing because it's not possible to use `self.build` at this point.

Execute `asdf python reshim` automatically when we detect that pip/mamba/conda/poetry were called to install a Python package that may install an executable.

Make sure that `config.build.commands` is not empty.

When updating the `Version` object via the API on success build, we were using the same value we received from the API for the `Version` object. This commit uses the `doctype` value from the config file used to build the version. This way, we keep the `Version.documentation_type` updated when there are changes in the config file for a particular version.

humitos · 2022-06-01T11:06:46Z

I updated the PR with all the suggestions that I received and improved the code a little bit. Now, it handles reshiming automatically when it detects a Python package manager installing something and it also uses a 'generic' doctype for this type of documentation (build.commands).

For now, we are using template-based notifications but we plan to change that because none of us like this pattern. However, it's good enough, for now, to move forward and don't block this feature on that.

build.tools is required for now, but I opened an issue to track that and remove that restriction. I'm not doing it in this PR because it involves a good refactor of the ConfigFile object due that it differentiates legacy from the newer pattern based on build.tools instead of build.os.

I'm happy to merge this during this week and deploy it the next one 👍🏼

docs/user/build-customization.rst

readthedocs/doc_builder/director.py

readthedocs/projects/tasks/builds.py

agjohnson

This is looking great. 👍 from me with the last bits of new feedback resolved

readthedocs/doc_builder/exceptions.py

This will allow us to be smarter about _when to reshim_. With this update, it will be handled automatically by `asdf` and we won't require a custom chunk of code in our application: https://github.com/readthedocs/readthedocs.org/blob/a5965129c61b9bcdff2f2098ff7ce7f8c093dc74/readthedocs/doc_builder/director.py#L373-L386 Currently, multi-lines commands that install something with `pip` and immediate after that, inside the multi-line command try to use the executable installed, fail because it's not automatically reshimed. By reshiming at `asdf` level, this case will be solved. Related: readthedocs/readthedocs.org#9150 (comment) Related: asdf-community/asdf-python#136

* asdf: update `asdf` and all its plugins This will allow us to be smarter about _when to reshim_. With this update, it will be handled automatically by `asdf` and we won't require a custom chunk of code in our application: https://github.com/readthedocs/readthedocs.org/blob/a5965129c61b9bcdff2f2098ff7ce7f8c093dc74/readthedocs/doc_builder/director.py#L373-L386 Currently, multi-lines commands that install something with `pip` and immediate after that, inside the multi-line command try to use the executable installed, fail because it's not automatically reshimed. By reshiming at `asdf` level, this case will be solved. Related: readthedocs/readthedocs.org#9150 (comment) Related: asdf-community/asdf-python#136 * Docs: update readme to mention `ubuntu-22.04` and `buildx` * Tests: update version to check

humitos added Needed: design decision A core team decision is required Status: blocked Issue is blocked on another issue labels Apr 27, 2022

ericholscher linked an issue May 10, 2022 that may be closed by this pull request

Build: implement build.commands override of build commands #9062

Closed

4 tasks

ericholscher reviewed May 10, 2022

View reviewed changes

readthedocs/projects/tasks/builds.py Outdated Show resolved Hide resolved

humitos self-assigned this May 11, 2022

humitos mentioned this pull request May 13, 2022

A way to disable installation of default requirements #8208

Closed

humitos removed Status: blocked Issue is blocked on another issue Needed: design decision A core team decision is required labels May 18, 2022

humitos force-pushed the humitos/build-commands branch from 41451f8 to 13b52de Compare May 18, 2022 14:47

humitos added 4 commits May 23, 2022 10:22

Docs: reference for build.commands

e578554

Build: solve problem with sanitized output

5365087

Reference #9238

humitos force-pushed the humitos/build-commands branch from 13b52de to 2286206 Compare May 23, 2022 09:42

humitos added 4 commits May 23, 2022 12:00

Template: add warning note on build details' page

97412b2

Build: fail the build if build.commands without output/ folder

a22e3bd

Test: build.commands test cases

27d1781

Merge branch 'main' of github.com:readthedocs/readthedocs.org into hu…

e55d008

…mitos/build-commands

humitos marked this pull request as ready for review May 23, 2022 10:35

humitos requested review from a team as code owners May 23, 2022 10:35

humitos requested review from ericholscher, agjohnson and stsewd May 23, 2022 10:35

humitos added 4 commits May 23, 2022 12:47

Typo in order of arguments

243fde5

Tests: fix mock patch's path

66b41a8

Tests: compare against "" since we are not returning None

935664a

Docs: mention that integrations aren't supported

958c2ce

humitos added 3 commits May 30, 2022 12:26

Merge branch 'main' of github.com:readthedocs/readthedocs.org into hu…

8b1fc27

…mitos/build-commands

Build: import error

ad4eb4a

Build: return GENERIC doctype when building with build.commands

996c97f

humitos requested review from stsewd, agjohnson and ericholscher May 30, 2022 10:58

ericholscher approved these changes May 31, 2022

View reviewed changes

humitos and others added 3 commits June 1, 2022 10:33

Apply suggestions from code review

1a9bf4f

Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com>

Build: doctype uses insternal config object

d94f339

Tests were failing because it's not possible to use `self.build` at this point.

Build: detect Python commands that require reshiming

472750c

Execute `asdf python reshim` automatically when we detect that pip/mamba/conda/poetry were called to install a Python package that may install an executable.

humitos mentioned this pull request Jun 1, 2022

Config file: make build.tools not required when using build.commands #9280

Closed

humitos added 4 commits June 1, 2022 12:26

Build: config.build.commands is always present

cd6f3c8

Make sure that `config.build.commands` is not empty.

Build: check for commands and emptyness

338fde5

Test: build.commands and Version.documentation_type update

b72906e

stsewd reviewed Jun 1, 2022

View reviewed changes

docs/user/build-customization.rst Outdated Show resolved Hide resolved

readthedocs/doc_builder/director.py Outdated Show resolved Hide resolved

readthedocs/projects/tasks/builds.py Outdated Show resolved Hide resolved

agjohnson approved these changes Jun 2, 2022

View reviewed changes

readthedocs/doc_builder/exceptions.py Outdated Show resolved Hide resolved

humitos added 4 commits June 2, 2022 10:55

Build: update Version.documentation_type properly

4b7c14f

Build: define reshim_commands as set

b5d168a

Docs: minor typo

61bb432

Build: improve error message when no output folder

632eb7c

humitos enabled auto-merge June 2, 2022 09:01

Build: bugfix when working with sets and tuples

9df29dd

humitos merged commit 62effc7 into main Jun 2, 2022

humitos deleted the humitos/build-commands branch June 2, 2022 09:42

humitos mentioned this pull request Jul 21, 2022

Build: remove hacky auto-reshim and use official one #9444

Open

2 tasks

humitos mentioned this pull request Mar 9, 2023

asdf: update asdf and all its plugins readthedocs/readthedocs-docker-images#189

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build: implementation of `build.commands` #9150

Build: implementation of `build.commands` #9150

humitos commented Apr 27, 2022 •

edited

Loading

ericholscher commented Apr 27, 2022

humitos commented May 23, 2022

ericholscher left a comment

ericholscher May 31, 2022

humitos Jun 1, 2022

humitos commented Jun 1, 2022 •

edited

Loading

agjohnson left a comment •

edited

Loading

Build: implementation of build.commands #9150

Build: implementation of build.commands #9150

Conversation

humitos commented Apr 27, 2022 • edited Loading

Build without using any tool

Build details page

index.html file served by El Proxito

Build using pelican

Build details page

index.html file served by El Proxito

Decisions / Notes

ericholscher commented Apr 27, 2022

humitos commented May 23, 2022

ericholscher left a comment

Choose a reason for hiding this comment

ericholscher May 31, 2022

Choose a reason for hiding this comment

humitos Jun 1, 2022

Choose a reason for hiding this comment

humitos commented Jun 1, 2022 • edited Loading

agjohnson left a comment • edited Loading

Choose a reason for hiding this comment

Build: implementation of `build.commands` #9150

Build: implementation of `build.commands` #9150

humitos commented Apr 27, 2022 •

edited

Loading

`index.html` file served by El Proxito

Build using `pelican`

`index.html` file served by El Proxito

humitos commented Jun 1, 2022 •

edited

Loading

agjohnson left a comment •

edited

Loading