FIX: use config-inited event to register config

[agoose77]

Fixes #768, obviates need for executablebooks/jupyter-book#2111

I'll restate the rationale here:

The registration order is as follows:

• [[extensions]] sphinx::Application.setup_extension()
• sphinx::Config.init_values()
• <config-inited>
• sphinx::Builder.init()
• [[theme]] sphinx::Registry.load_extension()
• sphinx::Builder.create_template_bridge()
• <builder-inited>

Crucially:

1. Extensions are registered first.
   • Any theme variables set in setup() are subsequently clobbered by sphinx::Config.init_values.
   • setup() is called exclusively once for a theme/extension, so these variables are never set again.
   • Extensions must use e.g. config-inited to set this value before the theme is required.
2. Themes are registered later.
   • Setting theme config directly in setup() ensures that the theme search path (in create_template_bridge is correct.
   • We can't use an event for this because the theme is set-up immediately (in sphinx::Registry.load_extension) before the path is required (in sphinx::Builder.create_template_bridge) i.e. before any further events are dispatched.

What's happening in JB and other users is that they're using SBT as both a theme and an extension, so we start at the top of the registration order. Thus, we need to support both use cases, i.e. set config twice. This is acceptable because due to the registration system we either see

• config set in <config-inited>, but not from setup()
• config set in setup(), after <config-inited> is dispatched

Finally, it should be noted that SBT needs to be in extensions whilst it adds features (the margin directive) that are non-HTML focused.

[agoose77]

Just cleanup!

[agoose77]

@ghisvail, @adam-grant-hendry, @choldgraf I'm pinging you three as the most recent investigators of the issue with SBT's use in extensions.

This PR makes the changes suggested by @adam-grant-hendry in a closed PR, which is to convert the builder-inited listener to a config-inited listener. Above in the PR description I've outlined why I think that's the appropriate solution over removing SBT from extensions, but I'd appreciate some additional input here to ensure I've not missed anything.

If you don't have any bandwidth for this, please let me know! I'll just make a release and we'll deal with the fall-out through normal channels (if there is any!).

[ghisvail]

I am not familiar with the specifics of Sphinx et al.

All I can offer is to test whether things work appropriately with the new release.

[choldgraf]

I thought that for some folks if the config initialization was called twice, it would raise an error. If that's not the case then I think it's fine to just do this twice as long as nobody reports regressions

[agoose77]

@choldgraf to be clear, I don't think this should ever set the config twice. AFAICT either:

1. SBT is only used as a theme, which means config-inited has already triggered by the time we register the listener
2. SBT is both a theme and extension, in which case the app.config in setup() is nearly immediately clobbered.

I'm hoping that isn't a cause of bugs, and I've not seen any evidence as yet to suggest it might be 🤞

[choldgraf]

I don't have much time to do a thorough review but as long as there aren't regressions and folks report expected behavior I'm +1

[frgigr]

Dear all, sorry to reopen this thread, but I'm quite sure this MR breaks the Jupyter-Book compilation. The error message is the following one:

Extension error (sphinx_book_theme):
Handler <function update_general_config at 0x7f09a7474790> for event 'config-inited' threw an exception (exception: update_general_config() takes 1 positional argument but 2 were given)
Traceback (most recent call last):
  File "/home/user/.local/lib/python3.9/site-packages/sphinx/events.py", line 97, in emit
    results.append(listener.handler(self.app, *args))
TypeError: update_general_config() takes 1 positional argument but 2 were given

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/user/.local/lib/python3.9/site-packages/jupyter_book/sphinx.py", line 114, in build_sphinx
    app = Sphinx(
  File "/home/user/.local/lib/python3.9/site-packages/sphinx/application.py", line 257, in __init__
    self.events.emit('config-inited', self.config)
  File "/home/user/.local/lib/python3.9/site-packages/sphinx/events.py", line 108, in emit
    raise ExtensionError(__("Handler %r for event %r threw an exception") %
sphinx.errors.ExtensionError: Handler <function update_general_config at 0x7f09a7474790> for event 'config-inited' threw an exception (exception: update_general_config() takes 1 positional argument but 2 were given)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/user/.local/bin/jupyter-book", line 8, in <module>
    sys.exit(main())
  File "/home/user/.local/lib/python3.9/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/home/user/.local/lib/python3.9/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/home/user/.local/lib/python3.9/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/user/.local/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/user/.local/lib/python3.9/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/user/.local/lib/python3.9/site-packages/jupyter_book/cli/main.py", line 317, in build
    builder_specific_actions(
  File "/home/user/.local/lib/python3.9/site-packages/jupyter_book/cli/main.py", line 528, in builder_specific_actions
    raise RuntimeError(_message_box(msg, color="red", doprint=False)) from result
RuntimeError: 
===============================================================================

There was an error in building your book. Look above for the cause.

===============================================================================

Steps to reproduce:

• Install Jupyter Book: pip install jupyter-book
• Quickly generate a sample book: jupyter-book create mynewbook/
• Build the sample book: jupyter-book build mynewbook/

No issues found when reverting to 1.1.0 version. Thanks in advance and apologies if the culprit is not here!

[agoose77]

@frgigr that's a new bug! Thanks for reporting it, will issue a fix.