Allow build.commands without build.tools (#10281)

* Adjust tests

* Update validation to allow build.commands without build.tools

* Remove comments, adds a negative test about re: "`build.jobs` can't be used without using `build.os`"

* Rename BuildWithTools => BuildWithOs

* Not legacy, still supported!

* Make test case command not confuse, it should make sense

* Adds a note to remember to create the folders before adding content

* Improve a docs sentence

* Use $READTHEDOCS_OUTPUT in examples

* Update docs/user/build-customization.rst

Co-authored-by: Manuel Kaufmann <[email protected]>

---------

Co-authored-by: Manuel Kaufmann <[email protected]>

Author: benjaoming

docs/user/build-customization.rst

@@ -360,14 +360,25 @@ Where to put files
 ~~~~~~~~~~~~~~~~~~
 
 It is your responsibility to generate HTML and other formats of your documentation using :ref:`config-file/v2:build.commands`.
-The contents of the ``_readthedocs/<format>/`` directory will be hosted as part of your documentation.
+The contents of the ``$READTHEDOCS_OUTPUT/<format>/`` directory will be hosted as part of your documentation.
+
+We store the the base folder name ``_readthedocs/`` in the environment variable ``$READTHEDOCS_OUTPUT`` and encourage that you use this to generate paths.
 
 Supported :ref:`formats <downloadable-documentation:accessing offline formats>` are published if they exist in the following directories:
 
-* ``_readthedocs/html/`` (required)
-* ``_readthedocs/htmlzip/``
-* ``_readthedocs/pdf/``
-* ``_readthedocs/epub/``
+* ``$READTHEDOCS_OUTPUT/html/`` (required)
+* ``$READTHEDOCS_OUTPUT/htmlzip/``
+* ``$READTHEDOCS_OUTPUT/pdf/``
+* ``$READTHEDOCS_OUTPUT/epub/``
+
+.. note::
+
+   Remember to create the folders before adding content to them.
+   You can ensure that the output folder exists by adding the following command:
+
+   .. code-block:: console
+
+       mkdir -p $READTHEDOCS_OUTPUT/html/
 
 Search support
 ~~~~~~~~~~~~~~

readthedocs/config/config.py

@@ -19,7 +19,7 @@
     Build,
     BuildJobs,
     BuildTool,
-    BuildWithTools,
+    BuildWithOs,
     Conda,
     Mkdocs,
     Python,
@@ -274,7 +274,7 @@ def validate(self):
 
     @property
     def using_build_tools(self):
-        return isinstance(self.build, BuildWithTools)
+        return isinstance(self.build, BuildWithOs)
 
     @property
     def is_using_conda(self):
@@ -781,11 +781,7 @@ def validate_conda(self):
             conda['environment'] = validate_path(environment, self.base_path)
         return conda
 
-    # NOTE: I think we should rename `BuildWithTools` to `BuildWithOs` since
-    # `os` is the main and mandatory key that makes the diference
-    #
-    # NOTE: `build.jobs` can't be used without using `build.os`
-    def validate_build_config_with_tools(self):
+    def validate_build_config_with_os(self):
         """
         Validates the build object (new format).
 
@@ -799,9 +795,10 @@ def validate_build_config_with_tools(self):
         tools = {}
         with self.catch_validation_error('build.tools'):
             tools = self.pop_config('build.tools')
-            validate_dict(tools)
-            for tool in tools.keys():
-                validate_choice(tool, self.settings['tools'].keys())
+            if tools:
+                validate_dict(tools)
+                for tool in tools.keys():
+                    validate_choice(tool, self.settings["tools"].keys())
 
         jobs = {}
         with self.catch_validation_error("build.jobs"):
@@ -824,13 +821,11 @@ def validate_build_config_with_tools(self):
             commands = self.pop_config("build.commands", default=[])
             validate_list(commands)
 
-        if not tools:
+        if not (tools or commands):
             self.error(
-                key='build.tools',
+                key="build.tools",
                 message=(
-                    'At least one tools of [{}] must be provided.'.format(
-                        ' ,'.join(self.settings['tools'].keys())
-                    )
+                    "At least one item should be provided in 'tools' or 'commands'"
                 ),
                 code=CONFIG_REQUIRED,
             )
@@ -856,12 +851,13 @@ def validate_build_config_with_tools(self):
                 build["commands"].append(validate_string(command))
 
         build['tools'] = {}
-        for tool, version in tools.items():
-            with self.catch_validation_error(f'build.tools.{tool}'):
-                build['tools'][tool] = validate_choice(
-                    version,
-                    self.settings['tools'][tool].keys(),
-                )
+        if tools:
+            for tool, version in tools.items():
+                with self.catch_validation_error(f"build.tools.{tool}"):
+                    build["tools"][tool] = validate_choice(
+                        version,
+                        self.settings["tools"][tool].keys(),
+                    )
 
         build['apt_packages'] = self.validate_apt_packages()
         return build
@@ -914,8 +910,8 @@ def validate_build(self):
         raw_build = self._raw_config.get('build', {})
         with self.catch_validation_error('build'):
             validate_dict(raw_build)
-        if 'os' in raw_build:
-            return self.validate_build_config_with_tools()
+        if "os" in raw_build or "commands" in raw_build or "tools" in raw_build:
+            return self.validate_build_config_with_os()
         return self.validate_old_build_config()
 
     def validate_apt_package(self, index):
@@ -1335,7 +1331,7 @@ def build(self):
                 )
                 for tool, version in build['tools'].items()
             }
-            return BuildWithTools(
+            return BuildWithOs(
                 os=build['os'],
                 tools=tools,
                 jobs=BuildJobs(**build["jobs"]),

readthedocs/config/models.py

@@ -35,7 +35,7 @@ def __init__(self, **kwargs):
         super().__init__(**kwargs)
 
 
-class BuildWithTools(Base):
+class BuildWithOs(Base):
 
     __slots__ = ("os", "tools", "jobs", "apt_packages", "commands")
 

readthedocs/config/tests/test_config.py

@@ -35,7 +35,7 @@
 from readthedocs.config.models import (
     Build,
     BuildJobs,
-    BuildWithTools,
+    BuildWithOs,
     Conda,
     PythonInstall,
     PythonInstallRequirements,
@@ -1051,7 +1051,7 @@ def test_new_build_config(self):
         )
         build.validate()
         assert build.using_build_tools
-        assert isinstance(build.build, BuildWithTools)
+        assert isinstance(build.build, BuildWithOs)
         assert build.build.os == 'ubuntu-20.04'
         assert build.build.tools['python'].version == '3.9'
         full_version = settings.RTD_DOCKER_BUILD_SETTINGS['tools']['python']['3.9']
@@ -1086,7 +1086,10 @@ def test_new_build_config_conflict_with_build_python_version(self):
             build.validate()
         assert excinfo.value.key == 'python.version'
 
-    def test_commands_build_config(self):
+    def test_commands_build_config_tools_and_commands_valid(self):
+        """
+        Test that build.tools and build.commands are valid together.
+        """
         build = self.get_build_config(
             {
                 "build": {
@@ -1097,9 +1100,27 @@ def test_commands_build_config(self):
             },
         )
         build.validate()
-        assert isinstance(build.build, BuildWithTools)
+        assert isinstance(build.build, BuildWithOs)
         assert build.build.commands == ["pip install pelican", "pelican content"]
 
+    def test_build_jobs_without_build_os_is_invalid(self):
+        """
+        build.jobs can't be used without build.os
+        """
+        build = self.get_build_config(
+            {
+                "build": {
+                    "tools": {"python": "3.8"},
+                    "jobs": {
+                        "pre_checkout": ["echo pre_checkout"],
+                    },
+                },
+            },
+        )
+        with raises(InvalidConfig) as excinfo:
+            build.validate()
+        assert excinfo.value.key == "build.os"
+
     def test_commands_build_config_invalid_command(self):
         build = self.get_build_config(
             {
@@ -1124,20 +1145,23 @@ def test_commands_build_config_invalid_no_os(self):
         )
         with raises(InvalidConfig) as excinfo:
             build.validate()
-        assert excinfo.value.key == "build.commands"
+        assert excinfo.value.key == "build.os"
 
-    def test_commands_build_config_invalid_no_tools(self):
+    def test_commands_build_config_valid(self):
+        """It's valid to build with just build.os and build.commands."""
         build = self.get_build_config(
             {
                 "build": {
                     "os": "ubuntu-22.04",
-                    "commands": ["pip install pelican", "pelican content"],
+                    "commands": ["echo 'hello world' > _readthedocs/html/index.html"],
                 },
             },
         )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == "build.tools"
+        build.validate()
+        assert isinstance(build.build, BuildWithOs)
+        assert build.build.commands == [
+            "echo 'hello world' > _readthedocs/html/index.html"
+        ]
 
     @pytest.mark.parametrize("value", ["", None, "pre_invalid"])
     def test_jobs_build_config_invalid_jobs(self, value):
@@ -1196,7 +1220,7 @@ def test_jobs_build_config(self):
             },
         )
         build.validate()
-        assert isinstance(build.build, BuildWithTools)
+        assert isinstance(build.build, BuildWithOs)
         assert isinstance(build.build.jobs, BuildJobs)
         assert build.build.jobs.pre_checkout == ["echo pre_checkout"]
         assert build.build.jobs.post_checkout == ["echo post_checkout"]