Skip to content

Remove temp fix for Sphinx _static file bug #2709

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

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

pushfoo
Copy link
Member

@pushfoo pushfoo commented May 30, 2025

TL;DR: Remove temp fix for Sphinx _static bug + add --jobs flag for ./make.py html and ./make.py serve

Performance?

Warning

This bumps min developer Py to 3.11 and it needs testing to ensure performance issues are solved!

In-depth test instructions are located in a comment below. Current test status:

  • Windows
  • Mac
  • GitHub CI (~ 5m 6s)
  • Linux
    • Debian (~ 10m, may be due disk load on / old hardware)
    • Ubuntu
    • Arch

We can continue using the temp fix until we're sure this PR's good.

Wait, What Changed in Sphinx 8.2.X?

Seems like:

  1. Faster partial rebuilds
  2. Much slower first builds

I've brought the first-time build down by adding a --jobs / -j flag pass-through to ./make.py. It seems to restore perf (now under 10 min, was 45+ minutes), but Sphinx's parallelism might not work correctly on Windows:

Windows is not supported. Note that not all parts and not all builders of Sphinx can be parallelized.

What's Changed

  • Remove util/sphinx_static_file_temp_fix and invocation in conf.py
  • Upgrade to Sphinx 8.2.3 to include fix for temp files
  • Add mention of requiring Python 3.11 as comments in pyproject.toml
  • Update Python versions for CI doc build (readthedocs + GitHub)
  • Explain the contents of the dev deps

pushfoo added 2 commits May 30, 2025 15:15
* Remove util/sphinx_static_file_temp_fix and invocation in conf.py

* Upgrade to Sphinx 8.2.3 to include fix for temp files

* Add mention of requiring Python 3.11 as comments in pyproject.toml

* Update Python versions for CI doc build (readthedocs + GitHub)

* Explain the contents of the dev deps
@pushfoo
Copy link
Member Author

pushfoo commented May 30, 2025

Test instructions

Please report:

  • Your OS
  • Your Python version
  • Build time
  • Whether static copying works
  • (If slower than 5 mins) Disk type (SSD, HDD, etc)

To get started, first make sure you have Python >= 3.11 in an activated virtual environment.

Time a clean build

"Does it run fast enough on clean build?"

  1. git fetch
  2. git checkout remove-copy-tempfix
  3. pip install -I -e .[dev]
  4. ./make.py clean
  5. Time execution of first build:
    • For Linux (and likely Mac): time ./make.py html
    • For Windows, the PowerShell may be Measure-Command { python make.py html } (PowerShell doc)

Ensure static file copy works

i.e. "Does Sphinx now handle CSS changes correctly?"

  1. ./make.py serve
  2. Make a trivial edit to one of these files
    • Example: * { background: red; blue; }
  3. Make sure to save it
  4. Watch for sphinx-autobuild to detect the change
  5. Once done, refresh the brower
  6. If the change does not visually show up, inspect the sources in your browser dev tools

@pushfoo
Copy link
Member Author

pushfoo commented May 30, 2025

OS: Debian
Python: 3.13
Build time:
real    9m29.949s
user    30m14.665s
sys     0m18.968s

Static copying works?: Yes

@eruvanos
Copy link
Member

eruvanos commented Jun 3, 2025

This changes breaks installation via uv:

Creating virtual environment at: .venv
  × No solution found when resolving dependencies for split
  │ (python_full_version == '3.10.*'):
  ╰─▶ Because the requested Python version (>=3.10) does not satisfy
      Python>=3.11 and sphinx==8.2.3 depends on Python>=3.11, we can conclude
      that sphinx==8.2.3 cannot be used.
      And because arcade[dev] depends on sphinx==8.2.3 and your project
      requires arcade[dev], we can conclude that your project's requirements
      are unsatisfiable.

      hint: The `requires-python` value (>=3.10) includes Python versions
      that are not supported by your dependencies (e.g., sphinx==8.2.3 only
      supports >=3.11). Consider using a more restrictive `requires-python`
      value (like >=3.11).

@pushfoo
Copy link
Member Author

pushfoo commented Jun 3, 2025

How did we have anything work in the past?

  • [dev] required 3.10
  • everyday use required only 3.9

@eruvanos
Copy link
Member

eruvanos commented Jun 3, 2025

I always had to change the required python to 3.10 and remember to not commit it to github.
I would love to not restart this again 😔

@einarf
Copy link
Member

einarf commented Jun 3, 2025

Dev dependencies could be moved out of the toml if needed (or maybe just doc reqs). The issues it creates for uv is pretty serious and we shouldn't break it.

@eruvanos
Copy link
Member

eruvanos commented Jun 4, 2025

do we really require 3.11 for docs? Is there no easy way around it? :/

@pushfoo
Copy link
Member Author

pushfoo commented Jun 4, 2025

TL;DR: We can make things easier at the cost of tech debt, but we've still gotta deal with Sphinx :(

I always had to change the required python to 3.10 and remember to not commit it to github.
I would love to not restart this again 😔

I think einarf and I have both have ideas to make it better.

The "Easy" Way?

Is there no easy way around it? :/

I'll see what I can do to improve our ./make.py.

This ./make.py script I wrote for another project suggests I can shim in a no-typer mode.

But first, we need to:

  1. remove old build output modes
    • They're commented out right now
    • We could replace them with a comment linking the commit that had them
  2. Enhancement: Add time reporting for make.py #2710

Why do all that?

It makes room for nice things!

They could include:

  1. local patch handing
  2. ./make.py interactive-install 😎?
  3. helping with a docs/requirements.txt like pyglet's old-school setup approach

I don't like the last approach as much, but it should work.

... The Non-Easy Way?

TL;DR: Moving away first from ReST, then from Sphinx. But that's a mega-project of its own.

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

Successfully merging this pull request may close these issues.

3 participants