qgis / QGIS-Website

QGIS-Website
130 stars 285 forks source link

Simplify and sort packages we rely on #1135

Closed DelazJ closed 5 months ago

DelazJ commented 1 year ago
strk commented 8 months ago

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 commented 8 months ago

@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 commented 5 months ago

@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 commented 5 months ago

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 https://github.com/sphinx-doc/sphinx/issues/4971 and e.g. https://github.com/sphinx-doc/sphinxcontrib-applehelp/pull/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:

DelazJ commented 5 months ago

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

rduivenvoorde commented 5 months ago

@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 commented 5 months ago

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 https://github.com/qgis/QGIS-Documentation/pull/8782 or https://github.com/qgis/QGIS-Documentation/pull/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 commented 5 months ago

@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?

DelazJ commented 5 months ago

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 💢 ). https://github.com/qgis/QGIS-Website/blob/f8dc464f781133cf3b456243be530a28e85f8d5e/Makefile#L20-L21 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 commented 5 months ago

Anyway, I can be available wednesday evening.

At 20:00 ? Ping me on Matrix?

DelazJ commented 5 months ago

OK