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

4.1.0 regression: HTML logo or Favicon specified as file not being found on output #9438

Closed
ViktorHaag opened this issue Jul 12, 2021 · 10 comments · Fixed by #9440
Closed

4.1.0 regression: HTML logo or Favicon specified as file not being found on output #9438

ViktorHaag opened this issue Jul 12, 2021 · 10 comments · Fixed by #9440

Comments

@ViktorHaag
Copy link
Contributor

ViktorHaag commented Jul 12, 2021

Describe the bug

If you specify html_logo or html_favicon values in conf.py as files (not URLS), then they won't be found properly in the generated HTML output unless you place the files in the configuration root of your source.

For fuller explanation, refer to merged #9381.

How to Reproduce

Place a logo for your HTML docs in a subdirectory below the configuration root of your source files (like _static/logofile.png for example).

Build your docs as normal. Your logofile.png will get copied into the output tree's _static subdirectory as expected, but your generated HTML will refer to it as if it were in _static/_static/logofile.png.

Workaround. Placing your project's logo image and favicon file into the configuration root directory so that your html_logo and html_favicon config variables have no file path to the files is a workaround until a fix is merged.

Expected behavior

When referring to your logo or favicon files by file path in configuration, the generated HTML output should seek to find them directly in the _static subdirectory in the output tree as expected.

See attached test.zip project.

test.zip

Your project

See Test project attached in Expected behaviour

Screenshots

No response

OS

MacOS 11.4

Python version

3.9

Sphinx version

4.1.0

Sphinx extensions

No response

Extra tools

No response

Additional context

No response

tronical added a commit to tronical/sphinx that referenced this issue Jul 13, 2021
… conf.py

Merge request sphinx-doc#9381 broke support for local logos/favicons as it
retained the paths that are passed to the template engine. That's wrong
as the actual path will be in _static/.

This should fix sphinx-doc#9438
@tronical
Copy link
Contributor

I've submitted a merge request (#9440) to try to fix this, along with a regression test. Could you try and see if this fixes your setup as well?

@PhilipMay
Copy link

I have the same issue with the 4.1.0
@tronical I tested your fix. It works for me. Thanks.

@ViktorHaag
Copy link
Contributor Author

I've submitted a merge request (#9440) to try to fix this, along with a regression test. Could you try and see if this fixes your setup as well?

Yep -- works with my project. Thanks!

@tk0miya tk0miya added this to the 4.1.1 milestone Jul 13, 2021
tronical added a commit to tronical/sphinx that referenced this issue Jul 13, 2021
… conf.py

Merge request sphinx-doc#9381 broke support for local logos/favicons as it
retained the paths that are passed to the template engine. That's wrong
as the actual path will be in _static/.

This should fix sphinx-doc#9438
@tk0miya
Copy link
Member

tk0miya commented Jul 13, 2021

Fixed by #9440. I'll ship a bugfix release within a day.

@tk0miya tk0miya closed this as completed Jul 13, 2021
@tupui
Copy link

tupui commented Jul 13, 2021

Thank you @tk0miya and @tronical!

@tupui
Copy link

tupui commented Jul 14, 2021

@tk0miya would it be possible for you to yank 4.1.0 on PyPi? This way we don't need to exclude this version from all the requirements around there.

@tk0miya
Copy link
Member

tk0miya commented Aug 1, 2021

IMO, yanking is not needed. If we'll remove all past packages contains bugs or regressions, there are no old packages on PyPI.

@tupui
Copy link

tupui commented Aug 1, 2021

IMO, yanking is not needed. If we'll remove all past packages contains bugs or regressions, there are no old packages on PyPI.

@tk0miya I disagree here as the previous version is breaking a lot of us, which other versions are not doing. You had to re-release another version just for this bug, yanking was introduced especially for that. To circumvent it, we now all need to do this: Sphinx>=3, !=3.1.0, !=4.1.0. IMHO adding more of these is not advisable...

@tk0miya
Copy link
Member

tk0miya commented Aug 1, 2021

I disagree here as the previous version is breaking a lot of us, which other versions are not doing. You had to re-release another version just for this bug, yanking was introduced especially for that.

We release bug fix packages (ex. 4.1.Z) only when the latest stable version (ex. 4.Y.Z) contains critical bugs. It means all bug fix packages should be yanked when newer one is released, right? Is it really the correct usage of the "yank" feature?

Note: 4.1.1 is not only for this bug. It also contains another bug fix.
https://github.com/sphinx-doc/sphinx/milestone/111?closed=1

@tupui
Copy link

tupui commented Aug 1, 2021

The PEP for yanking is here https://www.python.org/dev/peps/pep-0592/. The line is often blurry to say "this release should not be used at all". Here it's just my opinion to yank the release because it broke something which was not supposed to break. In the end it's on you to decide 😃 In any case, I appreciate that you were fast to fix the issue 👍 The rest is details.

andthum added a commit to andthum/mdtools that referenced this issue Aug 12, 2021
Lower the minimum required Sphinx version from 4.0 to 3.0 again.

Note that Sphinx version 4.1.0 does not render the html logo and favicon
due to a bug (sphinx-doc/sphinx#9438).
However, this version is not explicitly excluded from the requirements
file, because pip tries to avoid to install this version automatically.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants