search

gramineproject / graphene

Graphene / Graphene-SGX - a library OS for Linux multi-process applications, with Intel SGX support
https://grapheneproject.io
GNU Lesser General Public License v3.0
771 stars 260 forks source link

[Docs] conf.py: Check the version of Sphinx to avoid invoking deprecated method #2601

Closed xymeng16 closed 3 years ago

xymeng16 commented 3 years ago

This PR is related to building the documentation.

In the setup() function of conf.py, app.add_stylesheet() is used to add a CSS file. But this only works for Sphinx under version 1.8 as Sphinx has renamed app.add_stylesheet() into app.add_css_file() since version 1.8. Hence I added a version check before invoking that to avoid that such an error happens.

How to test?

Change the directory into Documentation/, and use make html.

This change is Reviewable

xymeng16 commented 3 years ago

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…
Hmm, but we have `sphinx==1.8.0` in our `requirements.txt`? So, I'm not sure if we should add IFs for other versions, I don't think we support them. CC: @woju

Oh, thanks for your reply. I have the sphinx installed in my environment so I ignored the requirements.txt. So if applicable, please ignore and close this PR.

(FYI, at least sphinx 4.1.2 and 4.0.0.b2 work based on my tests.)

woju commented 3 years ago

I'm unconvinced. We can't update sphinx at least until 22.04, because 20.04 still has sphinx 1.8 and we will need deb-packaged sphinx to build manpages in our debs (its not only for rtfd.io, where we could have greater latitude). When we'll be eventually upgrading, we'll just rename the function, without this conditional. So unless I miss something, this brings nothing but maintenance. If anything, I would prefer a comment # after sphinx $VERSION, rename this to .add_css().

dimakuv commented 3 years ago

Thanks for the PR, @xymeng16, but based on input from @mkow and @woju looks like we should close it.