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

Simplify and sort packages we rely on #1135

Merged
merged 2 commits into from Jan 17, 2024

Conversation

DelazJ
Copy link
Contributor

@DelazJ DelazJ commented Mar 18, 2023

  • Lock readthedocs theme version instead of sphinx
  • Remove sphinx from the list, as part of dependencies for many others
  • Unlock sphinx-intl version, and avoid build warning message due to deprecated process

@strk
Copy link
Contributor

strk commented Oct 13, 2023

This PR has been pending for 7 months now, who has the power of merging ? What's the policy here ? (I don't seem to have that ability, despite being a "QGIS committer")

@rduivenvoorde
Copy link
Contributor

rduivenvoorde commented Oct 13, 2023

@strk several peeps can (including @DelazJ himself). My issue with pulling is that to test/use this I have to create/upload the build images again, which is more a hassle (too me) then what this PR wins (I think).

So unless there is an issue which get fixed with this, I prefer to lock the version (untill there is a (security) or other issue with those versions.

@strk you could also get merge rights if you want. And also note https://blog.qgis.org/2023/10/03/call-for-proposals-qgis-website-overhaul-2023-2024/

@agiudiceandrea
Copy link
Contributor

@DelazJ @rduivenvoorde it looks like the actions are failing https://github.com/qgis/QGIS-Website/actions/runs/7517566690/job/20463880317 due to not updated Sphinx version.

Sphinx version error:
The sphinxcontrib.applehelp extension used by this project needs at least Sphinx v5.0; it therefore cannot be built with this version.
...
en.log:15:Running Sphinx v4.5.0

@DelazJ
Copy link
Contributor Author

DelazJ commented Jan 14, 2024

Hi,
Yes I didn't finally merge because of the website overhaul and didn't want to "break things". Also because the building process/github actions of QGIS website became somehow obscure to me.
Richard, new Sphinx and ReadTheDocs versions brought some new features and fixes, at least on rendering and translation management. Also keeping sphinx 4.5 undoubtedly will lead to a technical debt, and I'm afraid we reached it. Starting from today our github actions to build docs will fail (see in website, in docs) as some libraries were split from Sphinx and now require a newer version (see sphinx-doc/sphinx#4971 and e.g. sphinx-doc/sphinxcontrib-applehelp#15).
This doesn't affect virtual env that already exist but will bite new contributors trying to build locally, and our github actions as the venv is recreated at every run.
Options I see:

  • pin all the concerned dependencies in our REQUIREMENTS. txt: everything will work as before but.. this reminds me when we were using Sphinx 1.x and the hassle to upgrade and move to ReadTheDocs years ago
  • use an updated requirements in the github actions (and not use the REQUIREMENTS.txt in them) : CI builds will pass but people can't build locally. Also there is no guarantee that the rendering will be the same in github artifacts vs our official build
  • update REQUIREMENTS.txt to newer versions AND update on server: IMHO this the most sustainable option and the one that is worth the efforts. I just don't have idea what newer version we could pin (I stopped to follow these projects some time ago)

@DelazJ
Copy link
Contributor Author

DelazJ commented Jan 14, 2024

Oups, just notice your message @agiudiceandrea (took me time to write mine and I didn't scroll the page in the meantime 😃 )

@rduivenvoorde
Copy link
Contributor

@DelazJ @agiudiceandrea I can have a look this evening, and try to build a new Docker image on the server first.

Is the requirements.txt file uptodate now for that? Should this work now?

@DelazJ
Copy link
Contributor Author

DelazJ commented Jan 15, 2024

Is the requirements.txt file uptodate now for that? Should this work now?

Thanks @rduivenvoorde and no, this PR is not really up-to-date! I find too many limitations for testing in this repo IMHO so I played a bit in the docs repo and you should be good with qgis/QGIS-Documentation#8782 or qgis/QGIS-Documentation#8783 (the latter has my preference). I'm not really well informed on these locales issues so... it works but if anything cleaner exists... (and you probably would need to update the locales in the docker also?)

@rduivenvoorde
Copy link
Contributor

rduivenvoorde commented Jan 15, 2024

@DelazJ Hi H, I cannot follow the different PR's, and it is not clear enough to me what exactly we are fixing or trying to accomplish.

FYI https://github.com/qgis/QGIS-Sysadmin/blob/master/docker/sphinx/Dockerfile-html#L12
when creating an image to build html (for both site and docs), we pull requirements.txt from QGIS-Website, so all version pinnings, upgrades, additions etc etc should be done there.
If we need something in the container, it should be added to the Dockerfiles in QGIS-Sysadmin

Maybe it is better to do this together? Tuesday or Wednesday evening? Or maybe friday afternoon or so?

* Lock sphinx version to 7.2.6
* Unlock sphinx-intl version, and avoid build warning message due to deprecated process
Python 3.7 is no longer supported by new releases of Sphinx
@DelazJ
Copy link
Contributor Author

DelazJ commented Jan 16, 2024

Hello Richard,
Sorry! There are indeed many things in play and I agree they may not be clear at first sight.
Forget changes in the docs repo (I will close them). This PR should be good now; the main issue we try to fix is to install and execute in various contexts (local machine, QGIS Server, github actions) a newer Sphinx version, for the reasons exposed earlier.
Things are green here again. Every necessary changes are in the REQUIREMENTS.txt and in the Makefile and both files are available when running from github actions, docker, and personal computer. So I don't think there is any changes we need to specifically add to the dockerfile anymore. We should be good with just pulling the new image (is this done manually by you or triggered automatically at next build time?)

Anyway, I can be available wednesday evening.

If you need more details

When you run make xxx with new version of Sphinx, you (I couldn't find why) get an issue with locale

Traceback (most recent call last):
File "/usr/local/bin/sphinx-build", line 8, in
sys.exit(main())
File "/usr/local/lib/python3.10/dist-packages/sphinx/cmd/build.py", line 326, in main
locale.setlocale(locale.LC_ALL, '')
File "/usr/lib/python3.10/locale.py", line 620, in setlocale
return _setlocale(category, locale)
locale.Error: unsupported locale setting

The workaround I've been suggested is to setup LC_ALL variable to C.UTF-8 (on Linux). After a number of tests, I finally found the way to insert it in the MakeFile (I've been doing my tests in the docs repository modifying every files in which we were calling a make command while, ironically, that setup already existed in the website Makefile 💢 ).

QGIS-Website/Makefile

Lines 20 to 21 in f8dc464

# needed for python2 -> python3 migration?
export LC_ALL=C.UTF-8

Hope that clarifies...
I still couldn't find how new macOS users can build docs and website locally... but not a show-stopper IMHO

@rduivenvoorde
Copy link
Contributor

Anyway, I can be available wednesday evening.

At 20:00 ? Ping me on Matrix?

@DelazJ
Copy link
Contributor Author

DelazJ commented Jan 17, 2024

OK

@rduivenvoorde rduivenvoorde merged commit 66fc35f into qgis:master Jan 17, 2024
1 check passed
@DelazJ DelazJ deleted the simplifyWebsiteREQ branch January 18, 2024 08:41
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.

None yet

4 participants