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

Use CVE and CWE roles from Sphinx 8.1 #8457

Merged
merged 1 commit into from
Oct 11, 2024
Merged

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Oct 10, 2024

Changes proposed in this pull request:

  • Sphinx 8.1 adds roles for referencing CVEs (:cve:) and CWEs (:cwe:)
  • https://www.sphinx-doc.org/en/master/changes/index.html
  • Let's use those instead of our own extlinks
  • Update Makefile so we install Pillow for docs builds using the docs extra, so installs Sphinx 8.1+, and avoids defining the docs dependencies in two places

@radarhere
Copy link
Member

@@ -46,7 +46,7 @@ clean:
-rm -rf $(BUILDDIR)/*

install-sphinx:
$(PYTHON) -m pip install --quiet furo olefile sphinx sphinx-copybutton sphinx-inline-tabs sphinxext-opengraph
$(PYTHON) -m pip install -e ..[docs]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this to use an editable install?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Force of habit typing -e!

Which do you prefer?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not fussed. I suppose -e is consistent with our other command in the Makefile that installs an optional dependency.

python3 -m pip install -e .[tests]

@radarhere
Copy link
Member

Sphinx 8 doesn't support Python 3.9. Maybe we should hold off on this until Pillow 12?

@hugovk
Copy link
Member Author

hugovk commented Oct 11, 2024

Note: This changes the CVE URL from cve.org/CVERecord?id=CVE-%s to cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-%s

Thanks, I've opened sphinx-doc/sphinx#13006 to update it in Sphinx.

Sphinx 8 doesn't support Python 3.9. Maybe we should hold off on this until Pillow 12?

Building docs is a different application to the Pillow API's version support policy. And we already upgraded to Sphinx 7.3 (#7985) which also doesn't support Python 3.9 :)

Sphinx's support policy is derived from SPEC 0 meaning they support fewer versions than we do. If we required Sphinx to build on all our supported versions, we'd have to use quite old Sphinx versions.

@radarhere
Copy link
Member

we already upgraded to Sphinx 7.3 (#7985) which also doesn't support Python 3.9 :)

I think it does support Python 3.9.

But that PR did mean that Sphinx didn't support Python 3.8, before we dropped Python 3.8 support from the rest of Pillow in #8183, so your point still stands.

@hugovk
Copy link
Member Author

hugovk commented Oct 11, 2024

But that PR did mean that Sphinx didn't support Python 3.8, before we dropped Python 3.8 support from the rest of Pillow in #8183, so your point still stands.

Yes, this is what I meant 😅

@hugovk hugovk merged commit 3d9c05c into python-pillow:main Oct 11, 2024
47 of 48 checks passed
@hugovk hugovk deleted the sphinx-8.1 branch October 11, 2024 07:58
@hugovk
Copy link
Member Author

hugovk commented Oct 12, 2024

Thanks, I've opened sphinx-doc/sphinx#13006 to update it in Sphinx.

Merged and released in https://github.com/sphinx-doc/sphinx/releases/tag/v8.1.1.

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

Successfully merging this pull request may close these issues.

2 participants