Merge branch 'humitos/use-storage-properly-tar' into rel-environments

Author: humitos

common

@@ -1,5 +1,7 @@
 """Endpoint to generate footer HTML."""
 
+import re
+
 from django.conf import settings
 from django.shortcuts import get_object_or_404
 from django.template import loader as template_loader
@@ -142,13 +144,12 @@ def _get_context(self):
         version = self._get_version()
 
         page_slug = self.request.GET.get('page', '')
+        path = ''
         if page_slug and page_slug != 'index':
-            if main_project.documentation_type == 'sphinx_htmldir':
-                path = page_slug + '/'
+            if version.documentation_type == 'sphinx_htmldir':
+                path = re.sub('/index$', '', page_slug) + '/'
             else:
                 path = page_slug + '.html'
-        else:
-            path = ''
 
         context = {
             'project': project,

readthedocs/api/v2/views/footer_views.py

@@ -685,8 +685,6 @@ def validate(self):
         self.validate_doc_types()
         self._config['mkdocs'] = self.validate_mkdocs()
         self._config['sphinx'] = self.validate_sphinx()
-        # TODO: remove later
-        self.validate_final_doc_type()
         self._config['submodules'] = self.validate_submodules()
         self.validate_keys()
 
@@ -965,29 +963,6 @@ def validate_sphinx(self):
 
         return sphinx
 
-    def validate_final_doc_type(self):
-        """
-        Validates that the doctype is the same as the admin panel.
-
-        This a temporal validation, as the configuration file should support per
-        version doctype, but we need to adapt the rtd code for that.
-        """
-        dashboard_doctype = self.defaults.get('doctype', 'sphinx')
-        if self.doctype != dashboard_doctype:
-            error_msg = (
-                'Your project is configured as "{}" in your admin dashboard,'
-            ).format(self.builders_display[dashboard_doctype])
-
-            if dashboard_doctype == 'mkdocs' or not self.sphinx:
-                error_msg += ' but there is no "{}" key specified.'.format(
-                    'mkdocs' if dashboard_doctype == 'mkdocs' else 'sphinx',
-                )
-            else:
-                error_msg += ' but your "sphinx.builder" key does not match.'
-
-            key = 'mkdocs' if self.doctype == 'mkdocs' else 'sphinx.builder'
-            self.error(key, error_msg, code=INVALID_KEYS_COMBINATION)
-
     def validate_submodules(self):
         """
         Validates the submodules key.

readthedocs/config/config.py

@@ -1511,9 +1511,6 @@ def test_sphinx_builder_default(self):
         build.validate()
         build.sphinx.builder == 'sphinx'
 
-    @pytest.mark.skip(
-        'This test is not compatible with the new validation around doctype.',
-    )
     def test_sphinx_builder_ignores_default(self):
         build = self.get_build_config(
             {},
@@ -1686,28 +1683,6 @@ def test_mkdocs_fail_on_warning_check_default(self):
         build.validate()
         assert build.mkdocs.fail_on_warning is False
 
-    def test_validates_different_filetype_mkdocs(self):
-        build = self.get_build_config(
-            {'mkdocs': {}},
-            {'defaults': {'doctype': 'sphinx'}},
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'mkdocs'
-        assert 'configured as "Sphinx Html"' in str(excinfo.value)
-        assert 'there is no "sphinx" key' in str(excinfo.value)
-
-    def test_validates_different_filetype_sphinx(self):
-        build = self.get_build_config(
-            {'sphinx': {}},
-            {'defaults': {'doctype': 'sphinx_htmldir'}},
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'sphinx.builder'
-        assert 'configured as "Sphinx HtmlDir"' in str(excinfo.value)
-        assert 'your "sphinx.builder" key does not match' in str(excinfo.value)
-
     def test_submodule_defaults(self):
         build = self.get_build_config({})
         build.validate()

readthedocs/config/tests/test_config.py

@@ -548,6 +548,7 @@ def __init__(
             record=True,
             environment=None,
             update_on_success=True,
+            start_time=None,
     ):
         super().__init__(project, environment)
         self.version = version
@@ -557,7 +558,7 @@ def __init__(
         self.update_on_success = update_on_success
 
         self.failure = None
-        self.start_time = datetime.utcnow()
+        self.start_time = start_time or datetime.utcnow()
 
     def __enter__(self):
         return self

readthedocs/doc_builder/environments.py

@@ -119,17 +119,9 @@ def pull_cached_environment(self):
                     'msg': msg,
                 }
             )
-            _, tmp_filename = tempfile.mkstemp(suffix='.tar')
             remote_fd = storage.open(filename, mode='rb')
-            with open(tmp_filename, mode='wb') as local_fd:
-                local_fd.write(remote_fd.read())
-
-            with tarfile.open(tmp_filename) as tar:
-                tar.extractall(self.version.project.doc_path)
-
-            # Cleanup the temporary file
-            if os.path.exists(tmp_filename):
-                os.remove(tmp_filename)
+            with tarfile.open(fileobj=remote_fd) as tar:
+                tar.extractall(self.project.doc_path)
 
     def push_cached_environment(self):
         if not self.project.has_feature(feature_id=Feature.CACHED_ENVIRONMENT):
@@ -157,7 +149,8 @@ def push_cached_environment(self):
 
             # Special handling for .cache directory because it's per-project
             path = os.path.join(project_path, '.cache')
-            tar.add(path, arcname='.cache')
+            if os.path.exists(path):
+                tar.add(path, arcname='.cache')
 
         storage = get_storage_class(settings.RTD_BUILD_ENVIRONMENT_STORAGE)()
         with open(tmp_filename, 'rb') as fd:
@@ -473,6 +466,7 @@ def __init__(
         if config is not None:
             self.config = config
         self.task = task
+        self.build_start_time = None
         # TODO: remove this
         self.setup_env = None
 
@@ -577,6 +571,7 @@ def run_setup(self, record=True):
             update_on_success=False,
             environment=self.get_rtd_env_vars(),
         )
+        self.build_start_time = environment.start_time
 
         # TODO: Remove.
         # There is code that still depends of this attribute
@@ -667,6 +662,9 @@ def run_build(self, record):
             build=self.build,
             record=record,
             environment=env_vars,
+
+            # Pass ``start_time`` here to not reset the timer
+            start_time=self.build_start_time,
         )
 
         # Environment used for building code, usually with Docker

readthedocs/projects/tasks.py

@@ -124,7 +124,8 @@ def _serve_docs_nginx(self, request, final_project, version_slug, path, download
         response['X-RTD-Domain'] = request.get_host()
         response['X-RTD-Project'] = final_project.slug
         response['X-RTD-Version'] = version_slug
-        response['X-RTD-Path'] = path
+        # Needed to strip any GET args, etc.
+        response['X-RTD-Path'] = urlparse(path).path
         if hasattr(request, 'rtdheader'):
             response['X-RTD-Version-Method'] = 'rtdheader'
         if hasattr(request, 'subdomain'):

readthedocs/proxito/views/mixins.py

@@ -261,7 +261,7 @@ def get(self, request, proxito_path, template_name='404.html'):
         # If that doesn't work, attempt to serve the 404 of the current version (version_slug)
         # Secondly, try to serve the 404 page for the default version
         # (project.get_default_version())
-        for version_slug_404 in [version_slug, final_project.get_default_version()]:
+        for version_slug_404 in set([version_slug, final_project.get_default_version()]):
             for tryfile in ('404.html', '404/index.html'):
                 storage_root_path = final_project.get_storage_path(
                     type_='html',

readthedocs/proxito/views/serve.py

@@ -781,9 +781,6 @@ def test_sphinx_builder(
 
         get_builder_class.assert_called_with(result)
 
-    @pytest.mark.skip(
-        'This test is not compatible with the new validation around doctype.',
-    )
     @patch('readthedocs.projects.tasks.get_builder_class')
     def test_sphinx_builder_default(
             self, get_builder_class, checkout_path, tmpdir,

readthedocs/rtd_tests/tests/test_config_integration.py

@@ -143,6 +143,60 @@ def test_not_show_edit_on_github(self):
         self.assertIn('View', response.data['html'])
         self.assertNotIn('Edit', response.data['html'])
 
+    def test_index_pages_sphinx_htmldir(self):
+        version = self.pip.versions.get(slug=LATEST)
+        version.documentation_type = 'sphinx_htmldir'
+        version.save()
+
+        # A page with slug 'index' should render like /en/latest/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=index&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/', response.data['html'])
+        self.assertNotIn('/en/latest/index.html', response.data['html'])
+
+        # A page with slug 'foo/index' should render like /en/latest/foo/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=foo/index&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/foo/', response.data['html'])
+        self.assertNotIn('/en/latest/foo.html', response.data['html'])
+        self.assertNotIn('/en/latest/foo/index.html', response.data['html'])
+
+        # A page with slug 'foo/bar' should render like /en/latest/foo/bar/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=foo/bar&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/foo/bar/', response.data['html'])
+        self.assertNotIn('/en/latest/foo/bar.html', response.data['html'])
+        self.assertNotIn('/en/latest/foo/bar/index.html', response.data['html'])
+
+        # A page with slug 'foo/bar/index' should render like /en/latest/foo/bar/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=foo/bar/index&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/foo/bar/', response.data['html'])
+        self.assertNotIn('/en/latest/foo/bar.html', response.data['html'])
+        self.assertNotIn('/en/latest/foo/bar/index.html', response.data['html'])
+
+        # A page with slug 'foo/index/bar' should render like /en/latest/foo/index/bar/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=foo/index/bar&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/foo/index/bar/', response.data['html'])
+        self.assertNotIn('/en/latest/foo/index/bar.html', response.data['html'])
+        self.assertNotIn('/en/latest/foo/index/bar/index.html', response.data['html'])
+
 
 class TestFooterHTML(BaseTestFooterHTML, TestCase):