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

Can't extend builtin theme if it doesn't have explicit layout.html #12049

Open
guyer opened this issue Mar 6, 2024 · 22 comments
Open

Can't extend builtin theme if it doesn't have explicit layout.html #12049

guyer opened this issue Mar 6, 2024 · 22 comments

Comments

@guyer
Copy link

guyer commented Mar 6, 2024

Describe the bug

A theme that customizes layout.html cannot inherit from a theme that has an inherited layout.html and does not explicitly have its own, e.g., default, nature, sphinxdoc, or traditional.

How to Reproduce

If I add a custom theme to lumache that inherits from a builtin theme:

diff --git a/docs/source/_themes/lumache/layout.html b/docs/source/_themes/lumache/layout.html
new file mode 100644
index 0000000..7170ae5
--- /dev/null
+++ b/docs/source/_themes/lumache/layout.html
@@ -0,0 +1 @@
+{% extends "!layout.html" %}
diff --git a/docs/source/_themes/lumache/theme.conf b/docs/source/_themes/lumache/theme.conf
new file mode 100644
index 0000000..ffeaaf1
--- /dev/null
+++ b/docs/source/_themes/lumache/theme.conf
@@ -0,0 +1,2 @@
+[theme]
+inherit = sphinxdoc
diff --git a/docs/source/conf.py b/docs/source/conf.py
index 4ea7454..7e23856 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -24,5 +24,6 @@ exclude_patterns = []
 # -- Options for HTML output -------------------------------------------------
 # https://www.sphinx-doc.org/en/master/usage/configuration.html#options-for-html-output
 
-html_theme = 'alabaster'
+html_theme = 'lumache'
+html_theme_path = ['_themes']
 html_static_path = ['_static']

I get

Theme error:
An error happened in rendering the page index.
Reason: RecursionError('maximum recursion depth exceeded')
Running Sphinx v7.1.2
building [mo]: targets for 0 po files that are out of date
writing output... 
building [html]: targets for 1 source files that are out of date
updating environment: [new config] 1 added, 0 changed, 0 removed
reading sources... [100%] index
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
copying assets... copying static files... done
copying extra files... done
done
writing output... [100%] index
Theme error:
An error happened in rendering the page index.
Reason: RecursionError('maximum recursion depth exceeded')

If I change my layout.html to explicitly extend on the inherited layout

diff --git a/docs/source/_themes/lumache/layout.html b/docs/source/_themes/lumache/layout.html
index 7170ae5..a61bb33 100644
--- a/docs/source/_themes/lumache/layout.html
+++ b/docs/source/_themes/lumache/layout.html
@@ -1 +1 @@
-{% extends "!layout.html" %}
+{% extends "sphinxdoc/layout.html" %}

I get

Theme error:
An error happened in rendering the page index.
Reason: TemplateNotFound('sphinxdoc/layout.html')
Running Sphinx v7.1.2
building [mo]: targets for 0 po files that are out of date
writing output... 
building [html]: targets for 1 source files that are out of date
updating environment: [new config] 1 added, 0 changed, 0 removed
reading sources... [100%] index
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
copying assets... copying static files... done
copying extra files... done
done
writing output... [100%] index
Theme error:
An error happened in rendering the page index.
Reason: TemplateNotFound('sphinxdoc/layout.html')

It only works if I have layout.html explicitly extend on the redirected layout.html

diff --git a/docs/source/_themes/lumache/layout.html b/docs/source/_themes/lumache/layout.html
index a61bb33..0f710c9 100644
--- a/docs/source/_themes/lumache/layout.html
+++ b/docs/source/_themes/lumache/layout.html
@@ -1 +1 @@
-{% extends "sphinxdoc/layout.html" %}
+{% extends "basic/layout.html" %}

Environment Information

Platform:              darwin; (macOS-14.3.1-arm64-arm-64bit)
Python version:        3.12.2 | packaged by conda-forge | (main, Feb 16 2024, 20:54:21) [Clang 16.0.6 ])
Python implementation: CPython
Sphinx version:        7.1.2
Docutils version:      0.20.1
Jinja2 version:        3.1.3
Pygments version:      2.17.2

Sphinx extensions

[]

Additional context

Possibly related to #1792, #1794, #1884, #1885

@jayaddison
Copy link
Contributor

Hi @guyer - thanks for writing this report. Can I ask what your objective is and how themes relate to that, to figure out what options are available here?

@guyer
Copy link
Author

guyer commented Mar 7, 2024

@jayaddison Sure. Due to a deeply misguided decision in my organization, I needed to come up with an in-house replacement for ReadTheDocs. Different folks in the organization have used a wide variety of sphinx themes and so I wrote things to basically make a theme that inherits from their preferred theme and makes a few modifications. For instance, I need to graft some organizational headers and footers onto the rendered pages. This has all been working for the projects I tested on, but recently somebody was using sphinxdoc and things were cryptically broken.

All that said, I've figured out a workaround for my project where I walk the theme bases until I find one with a layout.html. It's not pretty, but I think it should work for me.

@jayaddison
Copy link
Contributor

This has all been working for the projects I tested on, but recently somebody was using sphinxdoc and things were cryptically broken.

Ok - I'm a bit confused by this in relation to the original description:

A theme that customizes layout.html cannot inherit from a theme that has an inherited layout.html and does not explicitly have its own, e.g., default, nature, sphinxdoc, or traditional.

Is the sphinxdoc theme somehow different in a way that allows this problem to be replicated for it but not the other themes?

(if so that should help to determine the root cause - maybe you understand and have described it already, but I'm still catching up)

@guyer
Copy link
Author

guyer commented Mar 7, 2024

It's completely clear in my head. Isn't that good enough? 😉

I create a theme that inherits from the user's theme and customizes layout.html. If the user's theme is one of default, nature, sphinxdoc, or traditional, those don't have their own layout.html; they rely on the one in basic or classic.

  • If my layout.html starts with
{% extends "!layout.html" %}

I get RecursionError('maximum recursion depth exceeded').

  • If my layout.html explicitly starts with
{% extends "sphinxdoc/layout.html" %}

I get TemplateNotFound('sphinxdoc/layout.html'). The same happens with default, nature, and traditional.

If the user's theme is any of the other builtin classes, then, e.g.,

  • Explicitly extending that theme's layout
    {% extends "pyramid/layout.html" %}
    is fine.
  • I realize now that using an exclamation point
    {% extends "!layout.html" %}
    still gives a recursion error. I guess this is what the comment in Theming:Templating is about?

    When extending a template in the base theme with the same name, use the theme name as an explicit directory: {% extends "basic/layout.html" %}. From a user templates_path template, you can still use the “exclamation mark” syntax as described in the templating document.

@jayaddison
Copy link
Contributor

Again possibly asking things you've already figured out, but: if you configure the inherit setting to the name of the user theme - if only as a temporary experiment to check - is the documented built as expected for that project?

@guyer
Copy link
Author

guyer commented Mar 7, 2024

Again possibly asking things you've already figured out, but: if you configure the inherit setting to the name of the user theme - if only as a temporary experiment to check - is the documented built as expected for that project?

That's what I'm doing, and it doesn't work. To be explicit, here's what I do:

  • User specifies html_theme = 'sphinxdoc' in their conf.py
  • I create a lumache theme in a _themes directory in the source directory.
  • I change conf.py to say
    html_theme = 'lumache'
    html_theme_path = ['_themes']
  • In _themes/lumache/theme.conf, I specify
    [theme]
    inherit = sphinxdoc
    
  • In _themes/lumache/layout.html, I specify
    {% extends "sphinxdoc/layout.html" %}
  • I get TemplateNotFound('sphinxdoc/layout.html').
  • This only happens if inherit = <base theme> specifies one of the themes that does not have its own layout.html.
  • If I specify inherit = sphinxdoc in _themes/lumache/theme.conf but {% extends "basic/layout.html" %} in _themes/lumache/layout.html, then it works OK.

Alternatively,

  • In _themes/lumache/layout.html, I specify
    {% extends "!layout.html" %}
  • I get RecursionError('maximum recursion depth exceeded').
  • This seems to happen for any value of inherit = <base theme>.

@jayaddison
Copy link
Contributor

Does configuring conf.py with:

[theme]
inherit = lumache

Make any difference?

@guyer
Copy link
Author

guyer commented Mar 8, 2024

Does configuring conf.py with:

[theme]
inherit = lumache

Make any difference?

I don't understand. conf.py is a Python file. That's not Python.

@jayaddison
Copy link
Contributor

Does configuring conf.py with:

[theme]
inherit = lumache

Make any difference?

I don't understand. conf.py is a Python file. That's not Python.

My mistake - that should have been theme.conf.

@guyer
Copy link
Author

guyer commented Mar 8, 2024

No, that doesn't work. How could it? How can a theme inherit itself?

I think we must be talking past each other. The repo I'm talking about is here: https://github.com/guyer/lumache

@jayaddison
Copy link
Contributor

What happened when you tried it?

@guyer
Copy link
Author

guyer commented Mar 8, 2024

Recursion error:
maximum recursion depth exceeded

This can happen with very large or deeply nested source files. You can carefully increase the default Python recursion limit of 1000 in conf.py with e.g.:
    import sys; sys.setrecursionlimit(1500)

@jayaddison
Copy link
Contributor

Ok, thanks. Some of the documentation may need updating.

I'll look at the recursion issue soon. An extremely minimal sample case -- a near-empty conf.py, index.rst, and any other supporting files required to replicate the problem -- would be extremely useful to figure this out. If any of the documentation projects that exhibit the problem are open-source-licensed, I'd recommend starting with one of those, preferably a small one, and narrowing down from there -- that's better than creating a minimal example from scratch, because it'll be more likely to replicate the problem accurately.

@jayaddison
Copy link
Contributor

I'll look at the recursion issue soon.

...(!) and by that I mean the original recursion issue reported - not the experimentation with the inherit theme setting.

@jayaddison jayaddison self-assigned this Mar 8, 2024
@guyer
Copy link
Author

guyer commented Mar 8, 2024

An extremely minimal sample case -- a near-empty conf.py, index.rst, and any other supporting files required to replicate the problem -- would be extremely useful to figure this out.

The repo I pointed you to, https://github.com/guyer/lumache, is exactly that. It's your lumache example (and only from Getting started), plus a bare-bones _themes/lumache/theme.conf and _themes/lumache/layout.html. I don't know how to make it any more minimal or replicable.

@jayaddison
Copy link
Contributor

Ok, I've found some time to catch up on this, and have been able to replicate your findings, including the local workaround to mitigate the problem:

-{% extends "!layout.html" %}
+{% extends "basic/layout.html" %}

...and that explains what you wrote here:

All that said, I've figured out a workaround for my project where I walk the theme bases until I find one with a layout.html. It's not pretty, but I think it should work for me.

Slightly-off-topic: more recent versions do a better job of detecting circular theme inheritance (see 360c7a8#diff-8342bb498a7811cc5902aa78183e3b03ca43983fd66b11ce11f5f52bb64167b2R209).

So the next question is: should lookups to non-existent intermdeiate template files be allowed to succeed? To answer that, I think I'll have to look at some of the code history (perhaps including the purpose of the ! symbol as it relates (or used to relate) to the extends template handling).

@jayaddison
Copy link
Contributor

Slightly-off-topic: more recent versions do a better job of detecting circular theme inheritance (see 360c7a8#diff-8342bb498a7811cc5902aa78183e3b03ca43983fd66b11ce11f5f52bb64167b2R209).

Well, that's a half-truth it seems; those changes aren't yet released (they're in source control, but pending a future release).

So the next question is: should lookups to non-existent intermdeiate template files be allowed to succeed? To answer that, I think I'll have to look at some of the code history (perhaps including the purpose of the ! symbol as it relates (or used to relate) to the extends template handling).

Currently reading: https://www.sphinx-doc.org/en/master/development/templating.html#jinja-sphinx-templating-primer ... to better understand the exclamation-mark symbol as used during template loading. We should link to that section from the relevant part of the theme-development documentation.

@jayaddison
Copy link
Contributor

When using Sphinx's HTML builder, each document is written using the page.html template by default.

The base template for page.html -- which presumably should rarely, if ever, be modified -- basically extends/calls the layout.html template and then contains a placeholder block for the body content of the page (note: the order that the template directives appear in the template file doesn't imply that the results will appear in the same order on the HTML page).

Meanwhile, Sphinx by-design subclasses the default Jinja template loading mechanism and overrides the get_source method with some application-specific logic: in particular it searches the configured templates directory, and any inherited theme directories in turn.

That means that the initial template file-open series will be something like: get_sources(page.html) -> get_sources(layout.html).

  • The former will bubble all the way up to the basic theme automatically.
  • The latter is handled by the local theme - specifically, docs/source/_themes/lumache/layout.html.

The problem is that when docs/source/_themes/lumache/layout.html is evaluated, it's going to call exactly the same get_sources method, and the search path causes it to load exactly the same file again (because in both cases it checks the [_templates, lumache, sphinxdoc, basic] locations -- in that order).

So somehow the logic would need to know "ok, first time you're asked for layout.html, return the local theme file -- but next time, skip that and return the next one".

(loading them in the reverse order isn't the solution here, because we still need to look at the most-specific templates first)

@jayaddison
Copy link
Contributor

Todo: if my working so far is correct, then any use of {% extends "!layout.html" %} in a project-local theme directory would fail due to infinite recursion, regardless of whether the inherited theme has a layout.html file or not; that theory needs to be checked.

@guyer
Copy link
Author

guyer commented Mar 9, 2024

Todo: if my working so far is correct, then any use of {% extends "!layout.html" %} in a project-local theme directory would fail due to infinite recursion, regardless of whether the inherited theme has a layout.html file or not; that theory needs to be checked.

Yes, that is my experience

@jayaddison
Copy link
Contributor

Thank you, @guyer - and thanks for your patience while I learn and confirm the behaviour here.

I have to admit to not being familiar with the internals of theme inheritance and template loading yet, so I'm going to add the "help wanted" label to this issue in case anyone can help. I'll try to resolve it myself over the next few days, and would like to, but assistance could help.

My understanding so far, to summarize for followup is:

  • Intuitively, extending inherited layouts seems to be intended functionality.
  • Infinite recursion occurs when a project-local theme attempts to extend a template.
  • Logic may be needed to discriminate between the 'first-load' of the project-local template and the subsequent 'extend-load' that occurs when the extend directive is evaluated.

@jayaddison
Copy link
Contributor

A combination of both of the changes below allowed me to get things working:

  • Moving the layout.html template into the _templates source subdirectory (as configured in templates_path).
  • Adding the exclamation-mark prefix to the extends filename in the layout.html file.

With those in place, I was able to override the header section of the output HTML.

{% extends "!layout.html" %}
{% block header %}
<div class="header" role="banner">
tremendous
</div>
{% endblock %}

Without the exclamation-mark prefix, the build entered an infinite recursion, similar to when the template file was placed in the _themes/lumache directory.

Without moving the template file, the build entered an infinite recursion regardless of whether the exlamation-mark prefix was used.

As a user, it seems unexpected to have to place a theme-related template in the local project's _templates directory though. Instinctively the theme path is where all the theme-related content should go, especially if there's a plan to extract that into a separate installable Python package at a later time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants