Build: remove conda_append_core_requirements feature flag (#11847)

We have had this flag set for a while now.
readthedocs · Dec 23, 2024 · ac87f67 · ac87f67
1 parent 65b835e
commit ac87f67
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 91 deletions.
diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py
@@ -239,9 +239,8 @@ def conda_bin_name(self):
         return self.config.python_interpreter
 
     def setup_base(self):
-        if self.project.has_feature(Feature.CONDA_APPEND_CORE_REQUIREMENTS):
-            self._append_core_requirements()
-            self._show_environment_yaml()
+        self._append_core_requirements()
+        self._show_environment_yaml()
 
         self.build_env.run(
             self.conda_bin_name(),
@@ -357,48 +356,12 @@ def _get_core_requirements(self):
         return pip_requirements, conda_requirements
 
     def install_core_requirements(self):
-        """Install basic Read the Docs requirements into the Conda env."""
-
-        if self.project.has_feature(Feature.CONDA_APPEND_CORE_REQUIREMENTS):
-            # Skip install core requirements since they were already appended to
-            # the user's ``environment.yml`` and installed at ``conda env
-            # create`` step.
-            return
-
-        pip_requirements, conda_requirements = self._get_core_requirements()
-        # Install requirements via ``conda install`` command if they were
-        # not appended to the ``environment.yml`` file.
-        cmd = [
-            self.conda_bin_name(),
-            "install",
-            "--yes",
-            "--quiet",
-            "--name",
-            self.version.slug,
-        ]
-        cmd.extend(conda_requirements)
-        self.build_env.run(
-            *cmd,
-            cwd=self.checkout_path,
-            # TODO: on tests I found that we are not passing ``bin_path`` here
-            # for some reason.
-        )
+        """
+        Skip installing requirements.
 
-        # Install requirements via ``pip install``
-        pip_cmd = [
-            self.venv_bin(filename="python"),
-            "-m",
-            "pip",
-            "install",
-            "-U",
-            "--no-cache-dir",
-        ]
-        pip_cmd.extend(pip_requirements)
-        self.build_env.run(
-            *pip_cmd,
-            bin_path=self.venv_bin(),
-            cwd=self.checkout_path,  # noqa - no comma here in py27 :/
-        )
+        Skip installing core requirements, since they were already appended to
+        the user's ``environment.yml`` and installed at ``conda env create`` step.
+        """
 
     def install_requirements_file(self, install):
         # as the conda environment was created by using the ``environment.yml``

diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py
@@ -1878,7 +1878,6 @@ def add_features(sender, **kwargs):
     # Feature constants - this is not a exhaustive list of features, features
     # may be added by other packages
     API_LARGE_DATA = "api_large_data"
-    CONDA_APPEND_CORE_REQUIREMENTS = "conda_append_core_requirements"
     RECORD_404_PAGE_VIEWS = "record_404_page_views"
     DISABLE_PAGEVIEWS = "disable_pageviews"
     RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header"
@@ -1909,10 +1908,6 @@ def add_features(sender, **kwargs):
             API_LARGE_DATA,
             _("Build: Try alternative method of posting large data"),
         ),
-        (
-            CONDA_APPEND_CORE_REQUIREMENTS,
-            _("Conda: Append Read the Docs core requirements to environment.yml file"),
-        ),
         (
             RECORD_404_PAGE_VIEWS,
             _("Proxito: Record 404s page views."),

diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py
@@ -1552,8 +1552,13 @@ def test_requirements_from_config_file_installed(self, load_yaml_config):
             ]
         )
 
+    @mock.patch("readthedocs.core.utils.filesystem.assert_path_is_inside_docroot")
     @mock.patch("readthedocs.doc_builder.director.load_yaml_config")
-    def test_conda_config_calls_conda_command(self, load_yaml_config):
+    def test_conda_config_calls_conda_command(
+        self, load_yaml_config, assert_path_is_inside_docroot
+    ):
+        # While testing, we are unsure if temporary test files exist in the docroot.
+        assert_path_is_inside_docroot.return_value = True
         load_yaml_config.return_value = get_build_config(
             {
                 "version": 2,
@@ -1588,34 +1593,19 @@ def test_conda_config_calls_conda_command(self, load_yaml_config):
                 mock.call("asdf", "global", "python", python_version),
                 mock.call("asdf", "reshim", "python", record=False),
                 mock.call(
-                    "conda",
-                    "env",
-                    "create",
-                    "--quiet",
-                    "--name",
-                    self.version.slug,
-                    "--file",
+                    "cat",
                     "environment.yaml",
                     cwd=mock.ANY,
-                    bin_path=mock.ANY,
                 ),
                 mock.call(
                     "conda",
-                    "install",
-                    "--yes",
+                    "env",
+                    "create",
                     "--quiet",
                     "--name",
                     self.version.slug,
-                    "sphinx",
-                    cwd=mock.ANY,
-                ),
-                mock.call(
-                    mock.ANY,
-                    "-m",
-                    "pip",
-                    "install",
-                    "-U",
-                    "--no-cache-dir",
+                    "--file",
+                    "environment.yaml",
                     cwd=mock.ANY,
                     bin_path=mock.ANY,
                 ),
@@ -1654,8 +1644,13 @@ def test_conda_config_calls_conda_command(self, load_yaml_config):
             ],
         )
 
+    @mock.patch("readthedocs.core.utils.filesystem.assert_path_is_inside_docroot")
     @mock.patch("readthedocs.doc_builder.director.load_yaml_config")
-    def test_python_mamba_commands(self, load_yaml_config):
+    def test_python_mamba_commands(
+        self, load_yaml_config, assert_path_is_inside_docroot
+    ):
+        # While testing, we are unsure if temporary test files exist in the docroot.
+        assert_path_is_inside_docroot.return_value = True
         load_yaml_config.return_value = get_build_config(
             {
                 "version": 2,
@@ -1683,6 +1678,11 @@ def test_python_mamba_commands(self, load_yaml_config):
                 mock.call("asdf", "install", "python", "mambaforge-4.10.3-10"),
                 mock.call("asdf", "global", "python", "mambaforge-4.10.3-10"),
                 mock.call("asdf", "reshim", "python", record=False),
+                mock.call(
+                    "cat",
+                    "environment.yaml",
+                    cwd=mock.ANY,
+                ),
                 mock.call(
                     "mamba",
                     "env",
@@ -1695,26 +1695,7 @@ def test_python_mamba_commands(self, load_yaml_config):
                     bin_path=None,
                     cwd=mock.ANY,
                 ),
-                mock.call(
-                    "mamba",
-                    "install",
-                    "--yes",
-                    "--quiet",
-                    "--name",
-                    "latest",
-                    "sphinx",
-                    cwd=mock.ANY,
-                ),
-                mock.call(
-                    mock.ANY,
-                    "-m",
-                    "pip",
-                    "install",
-                    "-U",
-                    "--no-cache-dir",
-                    bin_path=mock.ANY,
-                    cwd=mock.ANY,
-                ),
+                mock.call("test", "-x", "_build/html", cwd=mock.ANY, record=False),
             ]
         )