Implement sphinx key from v2 config (#4482)

* Test conf.py default value

* Fix conf.py path

* Don't skip tests

* Expose config object in builders

* Use doctype from config

* Pass python_env in tests

* Fix old test

* Implement sphinx key

* Implement fail_on_warning

* Skip tests for mkdocs

* Fix raising wrong exception

* Fix tests

* Remove search from tests

* Return the absolute path

* Check for None

* Use the config file

* Fix test

* Fix test

* Remove repetition

* Break and fix test

readthedocs/config/config.py

@@ -526,6 +526,17 @@ def build(self):
     def doctype(self):
         return self.defaults['doctype']
 
+    @property
+    def sphinx(self):
+        config_file = self.defaults['sphinx_configuration']
+        if config_file is not None:
+            config_file = os.path.join(self.base_path, config_file)
+        return Sphinx(
+            builder=self.doctype,
+            configuration=config_file,
+            fail_on_warning=False,
+        )
+
 
 class BuildConfigV2(BuildConfigBase):
 
@@ -955,7 +966,8 @@ def get_configuration_class(version):
         version = int(version)
         return configurations_class[version]
     except (KeyError, ValueError):
-        raise ConfigError(
-            'Invalid version of the configuration file',
+        raise InvalidConfig(
+            'version',
             code=VERSION_INVALID,
+            error_message='Invalid version of the configuration file',
         )

readthedocs/doc_builder/backends/sphinx.py

@@ -41,9 +41,14 @@ class BaseSphinx(BaseBuilder):
 
     def __init__(self, *args, **kwargs):
         super(BaseSphinx, self).__init__(*args, **kwargs)
+        self.config_file = self.config.sphinx.configuration
         try:
+            if not self.config_file:
+                self.config_file = self.project.conf_file(self.version.slug)
             self.old_artifact_path = os.path.join(
-                self.project.conf_dir(self.version.slug), self.sphinx_build_dir)
+                os.path.dirname(self.config_file),
+                self.sphinx_build_dir
+            )
         except ProjectConfigurationError:
             docs_dir = self.docs_dir()
             self.old_artifact_path = os.path.join(
@@ -69,7 +74,7 @@ def get_config_params(self):
         # TODO this should be handled better in the theme
         conf_py_path = os.path.join(
             os.path.sep,
-            self.version.get_conf_py_path(),
+            self.config_file,
             '',
         )
         remote_version = self.version.commit_name
@@ -151,15 +156,16 @@ def get_config_params(self):
 
     def append_conf(self, **__):
         """Find or create a ``conf.py`` with a rendered ``doc_builder/conf.py.tmpl`` appended"""
-        try:
-            self.version.get_conf_py_path()
-        except ProjectConfigurationError:
+        if self.config_file is None:
             master_doc = self.create_index(extension='rst')
             self._write_config(master_doc=master_doc)
 
         try:
-            outfile_path = self.project.conf_file(self.version.slug)
-            outfile = codecs.open(outfile_path, encoding='utf-8', mode='a')
+            self.config_file = (
+                self.config_file or
+                self.project.conf_file(self.version.slug)
+            )
+            outfile = codecs.open(self.config_file, encoding='utf-8', mode='a')
         except (ProjectConfigurationError, IOError):
             trace = sys.exc_info()[2]
             six.reraise(
@@ -183,7 +189,7 @@ def append_conf(self, **__):
         self.run(
             'cat',
             os.path.relpath(
-                outfile_path,
+                self.config_file,
                 self.project.checkout_path(self.version.slug),
             ),
             cwd=self.project.checkout_path(self.version.slug),
@@ -199,6 +205,8 @@ def build(self):
         ]
         if self._force:
             build_command.append('-E')
+        if self.config.sphinx.fail_on_warning:
+            build_command.append('-W')
         build_command.extend([
             '-b',
             self.sphinx_builder,
@@ -210,8 +218,10 @@ def build(self):
             self.sphinx_build_dir,
         ])
         cmd_ret = self.run(
-            *build_command, cwd=project.conf_dir(self.version.slug),
-            bin_path=self.python_env.venv_bin())
+            *build_command,
+            cwd=os.path.dirname(self.config_file),
+            bin_path=self.python_env.venv_bin()
+        )
         return cmd_ret.successful
 
 
@@ -353,7 +363,7 @@ class PdfBuilder(BaseSphinx):
 
     def build(self):
         self.clean()
-        cwd = self.project.conf_dir(self.version.slug)
+        cwd = os.path.dirname(self.config_file)
 
         # Default to this so we can return it always.
         self.run(

readthedocs/doc_builder/base.py

@@ -46,6 +46,7 @@ def __init__(self, build_env, python_env, force=False):
         self.python_env = python_env
         self.version = build_env.version
         self.project = build_env.project
+        self.config = python_env.config
         self._force = force
         self.target = self.project.artifact_path(
             version=self.version.slug, type_=self.type)

readthedocs/doc_builder/config.py

@@ -31,7 +31,10 @@ def load_yaml_config(version):
     python_version = 3 if project.python_interpreter == 'python3' else 2
     allow_v2 = project.has_feature(Feature.ALLOW_V2_CONFIG_FILE)
     try:
-        sphinx_configuration = version.get_conf_py_path()
+        sphinx_configuration = path.join(
+            version.get_conf_py_path(),
+            'conf.py'
+        )
     except ProjectConfigurationError:
         sphinx_configuration = None
 

readthedocs/projects/tasks.py

@@ -701,7 +701,7 @@ def build_docs(self):
 
     def build_docs_html(self):
         """Build HTML docs."""
-        html_builder = get_builder_class(self.project.documentation_type)(
+        html_builder = get_builder_class(self.config.doctype)(
             build_env=self.build_env,
             python_env=self.python_env,
         )

readthedocs/rtd_tests/tests/test_config_integration.py

@@ -232,7 +232,6 @@ def test_requirements_file(self, load_config):
         self.assertEqual(config.python.requirements, '__init__.py')
 
 
-@pytest.mark.skip
 @pytest.mark.django_db
 @mock.patch('readthedocs.projects.models.Project.checkout_path')
 class TestLoadConfigV2(object):
@@ -290,12 +289,10 @@ def test_report_using_invalid_version(self, checkout_path, tmpdir):
 
     @pytest.mark.parametrize('config', [{}, {'formats': []}])
     @patch('readthedocs.projects.models.Project.repo_nonblockinglock', new=MagicMock())
-    @patch('readthedocs.doc_builder.backends.sphinx.SearchBuilder.build')
     @patch('readthedocs.doc_builder.backends.sphinx.HtmlBuilder.build')
     @patch('readthedocs.doc_builder.backends.sphinx.HtmlBuilder.append_conf')
     def test_build_formats_default_empty(
-            self, append_conf, html_build, search_build,
-            checkout_path, config, tmpdir):
+            self, append_conf, html_build, checkout_path, config, tmpdir):
         """
         The default value for formats is [], which means no extra
         formats are build.
@@ -304,22 +301,26 @@ def test_build_formats_default_empty(
         self.create_config_file(tmpdir, config)
 
         update_docs = self.get_update_docs_task()
+        python_env = Virtualenv(
+            version=self.version,
+            build_env=update_docs.build_env,
+            config=update_docs.config
+        )
+        update_docs.python_env = python_env
         outcomes = update_docs.build_docs()
 
         # No extra formats were triggered
         assert outcomes['html']
-        assert outcomes['search']
         assert not outcomes['localmedia']
         assert not outcomes['pdf']
         assert not outcomes['epub']
 
     @patch('readthedocs.projects.models.Project.repo_nonblockinglock', new=MagicMock())
     @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.build_docs_class')
-    @patch('readthedocs.doc_builder.backends.sphinx.SearchBuilder.build')
     @patch('readthedocs.doc_builder.backends.sphinx.HtmlBuilder.build')
     @patch('readthedocs.doc_builder.backends.sphinx.HtmlBuilder.append_conf')
     def test_build_formats_only_pdf(
-            self, append_conf, html_build, search_build, build_docs_class,
+            self, append_conf, html_build, build_docs_class,
             checkout_path, tmpdir):
         """
         Only the pdf format is build.
@@ -328,11 +329,17 @@ def test_build_formats_only_pdf(
         self.create_config_file(tmpdir, {'formats': ['pdf']})
 
         update_docs = self.get_update_docs_task()
+        python_env = Virtualenv(
+            version=self.version,
+            build_env=update_docs.build_env,
+            config=update_docs.config
+        )
+        update_docs.python_env = python_env
+
         outcomes = update_docs.build_docs()
 
         # Only pdf extra format was triggered
         assert outcomes['html']
-        assert outcomes['search']
         build_docs_class.assert_called_with('sphinx_pdf')
         assert outcomes['pdf']
         assert not outcomes['localmedia']
@@ -642,6 +649,35 @@ def test_sphinx_configuration_default(
         append_conf.assert_called_once()
         move.assert_called_once()
 
+    @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.move')
+    @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.append_conf')
+    @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.run')
+    def test_sphinx_configuration_default(
+            self, run, append_conf, move, checkout_path, tmpdir):
+        """Should be default to find a conf.py file."""
+        checkout_path.return_value = str(tmpdir)
+
+        apply_fs(tmpdir, {'conf.py': ''})
+        self.create_config_file(tmpdir, {})
+        self.project.conf_py_file = ''
+        self.project.save()
+
+        update_docs = self.get_update_docs_task()
+        config = update_docs.config
+        python_env = Virtualenv(
+            version=self.version,
+            build_env=update_docs.build_env,
+            config=config
+        )
+        update_docs.python_env = python_env
+
+        update_docs.build_docs_html()
+
+        args, kwargs = run.call_args
+        assert kwargs['cwd'] == str(tmpdir)
+        append_conf.assert_called_once()
+        move.assert_called_once()
+
     @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.move')
     @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.append_conf')
     @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.run')
@@ -716,6 +752,7 @@ def test_sphinx_fail_on_warning(
         append_conf.assert_called_once()
         move.assert_called_once()
 
+    @pytest.mark.skip
     @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.move')
     @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.append_conf')
     @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.run')
@@ -754,6 +791,7 @@ def test_mkdocs_configuration(
         append_conf.assert_called_once()
         move.assert_called_once()
 
+    @pytest.mark.skip
     @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.move')
     @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.append_conf')
     @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.run')