search

readthedocs / sphinx-notfound-page

Create a custom 404 page with absolute URLs hardcoded
https://sphinx-notfound-page.readthedocs.io/
MIT License
48 stars 32 forks source link

Do not hardcode CSS/JS checksums #226

Closed mitya57 closed 9 months ago

mitya57 commented 9 months ago

These checksums are tied to a particular version of Sphinx, Alabaster or Pygments, and the tests may fail when a different version is used.

Instead, calculate the checksum dynamically during the test.

humitos commented 9 months ago

Can you check why Python 3.8 tests are failing?

mitya57 commented 9 months ago

Oh, it's because the _file_checksum function is in different places in Sphinx 7.1 and Sphinx 7.2.

Maybe I should just inline the checksum calculation code instead of using the private API. Or ask Sphinx developers to make it public.

mitya57 commented 9 months ago

After removing the sanity checks that we don't need for tests, that function has only 6 lines. So I inlined it.

humitos commented 9 months ago

Oh, it's because the _file_checksum function is in different places in Sphinx 7.1 and Sphinx 7.2.

We have other places in the code where we check the Sphinx version and import different things, like:

if sphinx.version_info < (7, 1):
  from ... import ...
else:
  from ... import ...

I'd prefer if we import the correct function instead of in-lining it because if they change the logic, we will have the same problem that this PR is trying to solve: don't hardcode the hashes. If we hardcode the logic, we will have the same problem in the future.

mitya57 commented 9 months ago

Updated the PR. And I have opened issue sphinx-doc/sphinx#11743 asking to make that function public API.

humitos commented 9 months ago

Excellent! Thanks 👍🏼