Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not assign html_theme_path #9654

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/user/feature-flags.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ This helps users to pin dependencies on conda and to improve build time.

``DONT_OVERWRITE_SPHINX_CONTEXT``: :featureflags:`DONT_OVERWRITE_SPHINX_CONTEXT`

``SKIP_SPHINX_HTML_THEME_PATH``: :featureflags:`SKIP_SPHINX_HTML_THEME_PATH`
benjaoming marked this conversation as resolved.
Show resolved Hide resolved

When using sphinx-rtd-theme, ``html_theme_path`` is defined automatically in ``conf.py`` for older versions of Sphinx.
All projects created after January 2023 and projects using Sphinx 6+ skips the definition and do not need this feature flag.

If you need to explicitly block ``html_theme_path`` from being defined,
please state *why* you need this feature flag enabled,
as we will have to consider this a potential bug.

``DONT_CREATE_INDEX``: :featureflags:`DONT_CREATE_INDEX`

When Read the Docs detects that your project doesn't have an ``index.md`` or ``README.rst``,
Expand Down
7 changes: 6 additions & 1 deletion readthedocs/doc_builder/backends/sphinx.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,12 @@ def get_config_params(self):
'dont_overwrite_sphinx_context': self.project.has_feature(
Feature.DONT_OVERWRITE_SPHINX_CONTEXT,
),
'docsearch_disabled': self.project.has_feature(Feature.DISABLE_SERVER_SIDE_SEARCH),
"docsearch_disabled": self.project.has_feature(
Feature.DISABLE_SERVER_SIDE_SEARCH
),
"skip_html_theme_path": self.project.has_feature(
Feature.SKIP_SPHINX_HTML_THEME_PATH
),
}

finalize_sphinx_context_data.send(
Expand Down
24 changes: 23 additions & 1 deletion readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,37 @@ using_rtd_theme = (
) or 'html_theme' not in globals()
)
if using_rtd_theme:
theme = importlib.import_module('{{ html_theme_import }}')
html_theme = '{{ html_theme }}'
html_style = None
html_theme_options = {}

# Legacy behavior for html_theme_path:
benjaoming marked this conversation as resolved.
Show resolved Hide resolved
# It is suspected that old builds with sphinx_rtd_theme required an extra
# definition of html_theme_path in order to work.
# This behavior is seen documented for old releases of sphinx_rtd_theme.
# An example of old instructions:
# https://github.com/readthedocs/sphinx_rtd_theme/blob/abd951a9cd98851269b8b5f5e59b1b4d29b1416c/README.rst
# Discussion of issue:
# https://github.com/readthedocs/readthedocs.org/pull/9654
#
# We need to isolate the old legacy behavior because it causes Sphinx
# to skip calling the theme's setup(app) so its initialization never happens.
# This legacy behavior will gradually be sliced out until its deprecated and removed.
# Skipped for Sphinx 6+
# Skipped by Feature flag SKIP_SPHINX_HTML_THEME_PATH
# Skipped by all new projects since SKIP_SPHINX_HTML_THEME_PATH's introduction (jan 2023)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agjohnson I believe we should also do this, just want to make sure that you have seen it. New projects won't follow strange old install guides, and they are likely on later versions of Sphinx.

if (
using_rtd_theme
and version_info < (6,0)
and not {{ skip_html_theme_path|yesno:"True,False" }}
):
theme = importlib.import_module('{{ html_theme_import }}')
if 'html_theme_path' in globals():
html_theme_path.append(theme.get_html_theme_path())
else:
html_theme_path = [theme.get_html_theme_path()]

# Define websupport2_base_url and websupport2_static_url
if globals().get('websupport2_base_url', False):
websupport2_base_url = '{{ api_host }}/websupport'
websupport2_static_url = '{{ settings.STATIC_URL }}'
Expand Down
7 changes: 7 additions & 0 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1807,6 +1807,7 @@ def add_features(sender, **kwargs):
# may be added by other packages
ALLOW_DEPRECATED_WEBHOOKS = "allow_deprecated_webhooks"
DONT_OVERWRITE_SPHINX_CONTEXT = "dont_overwrite_sphinx_context"
SKIP_SPHINX_HTML_THEME_PATH = "skip_sphinx_html_theme_path"
MKDOCS_THEME_RTD = "mkdocs_theme_rtd"
API_LARGE_DATA = "api_large_data"
DONT_SHALLOW_CLONE = "dont_shallow_clone"
Expand Down Expand Up @@ -1859,6 +1860,12 @@ def add_features(sender, **kwargs):
'Do not overwrite context vars in conf.py with Read the Docs context',
),
),
(
SKIP_SPHINX_HTML_THEME_PATH,
_(
"On Sphinx < 6, do not define html_theme_path",
benjaoming marked this conversation as resolved.
Show resolved Hide resolved
),
),
(
MKDOCS_THEME_RTD,
_('Use Read the Docs theme for MkDocs as default theme'),
Expand Down