Created attachment 209843 [details] Patch QGIS to 3.10.1 and overcome the QMap problems This patch updates to the newest QGIS version 3.10.1. For some months now, the build process of graphics/qgis 3.8.2 had problems while using the QT5 QMap() function with large list like for proj6[1]. Compiling such long QMap() list with clang takes several hours instead of some seconds or minutes[2], often it breaks the build. It turns out, that building that QMap() code with clang takes exhaustive memory[3], for which a nice patch already exists on OpenBSD[4]. While this OpenBSD patch builds fine on amd64, it needs another patch on i386 (usage of '-O1' instead the clang specific patch '-mllvm -inline-threshold=128'). After we found a solution for the QMap problem, I had to wait for the release of QGIS 3.10.1 because of some tricky problems with proj6 / gdal combinations[5], I wanted to avoid. [1] https://github.com/qgis/QGIS/blob/master/src/core/qgscoordinatereferencesystem_legacy.h#L23 [2] http://osgeo-org.1560.x6.nabble.com/QGIS-Developer-QGIS-3-8-0-on-FreeBSD-build-takes-several-hours-td5408676.html [3] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241687 [4]https://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/geo/qgis/patches/patch-src_core_CMakeLists_txt?rev=1.22&content-type=text/x-cvsweb-markup&hideattic=1 [5] https://www.mail-archive.com/qgis-developer@lists.osgeo.org/msg48670.html ============================================================================= bug #241096 and bug #241687 should be obsolete after this patch is committed! ============================================================================= Changes of the port: - Update from 3.8.2 to 3.10.1 - Allow using default clang again - Update several Python dependencies - Add textproc/py-nltk, math/py-pandas, sysutils/py-psutil, graphics/py-pyrsgis, x11-toolkits/py-qt5-quick for Python extensions - Add USES=gl and USE_GL=gl - Add USE_PYQT=quickwidgets - Add several CMAKE ARGS - Add files/patch-src_core_CMakeLists.txt for QMap problem - Change above patch into '-O1' for i386 - Update pkg-plist The patch is tested on Poudriere (11.3i/a, 12.0i/a, and HEADi/a), 'portlint -AC' seems happy.
Build info is available at https://gitlab.com/swills/freebsd-ports/pipelines/102126441
Thanks Rainer, One comment about SIP. Normally with https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=223409 you don't have to configure the variable `-DQSCI_SIP_DIR` (not tested)
(In reply to Loïc Bartoletti from comment #2) Hi Loïc, I am using `-DQSCI_SIP_DIR` because of the subdir 'Qsci'. But again, I tested to build graphics/qgis with and without setting this variable. The build logs are exactly the same, so it is likely that you are right here ;) Should I remove the variable in this patch to 3.10.1 already, or is it sufficient to do it with the next update?
A few comments related to the LLVM+QMap issues: * Upstream's also patched src/core/CMakeLists.txt to add -mllvm -inline-threshold=128, isn't it better to import that patch and reference the upstream commit? * The part in the Makefile changing those flags to -O1, how about referencing bug 241687 in the comment and checking __FreeBSD_version to decide whether to apply it? IIUC, 11.3 is the oldest supported release, and it already includes LLVM 8.0 and does not need the change to -O1; 12.0 is broken, but 12.1 is not, as the latter also has LLVM 8.0. Per https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/versions-12.html, I guess something like __FreeBSD_version >= 1200000 && __FreeBSD_version < 1200506 would be enough, and it also makes it easier to drop this check once 12.0 is no longer supported.
(In reply to Raphael Kubo da Costa from comment #4) Hi Raphael, Thanks for your response. Q1: In the patch of this PR 'files/patch-src_core_CMakeLists.txt' is included already[1]. Q2: In several tests (on Poudriere) it turns out, that patching[1] again with '-O1' is only needed on FreeBSD i386, independend from LLVM version. The memory problem (with i386) also occurs on 13.0-CURRENT, also with newest clang9. On some boxes, this only happens with small RAM memory (<= 8 GB). So, for the moment, it seems best to use [1] in general and change it to '-O1' for i386. Hope, this is ok for you. For a next QGIS release, which should include [1] in the source, I will rework the port, of course. [1] https://bugs.freebsd.org/bugzilla/attachment.cgi?id=209843&action=diff#qgis/files/patch-src_core_CMakeLists.txt_sec1
> Q1: In the patch of this PR 'files/patch-src_core_CMakeLists.txt' is included already[1]. Yeah, I meant either referencing the upstream commit before the actual patch or backporting the whole commit with the commit message included. What I generally do in ports or KDE's is fetch/adjust the upstream patch ( https://github.com/qgis/QGIS/commit/5ad7f6497f26383429177ec720c493a60dbfebd0.patch) and call the patch "patch-git_5ad7f649" so it's easier to identify this is temporary and coming from upstream. > Q2: In several tests (on Poudriere) it turns out, that patching[1] again with '-O1' is only needed on FreeBSD i386, independend from LLVM version. The memory problem (with i386) also occurs on 13.0-CURRENT, also with newest clang9. On some boxes, this only happens with small RAM memory (<= 8 GB). So, for the moment, it seems best to use [1] in general and change it to '-O1' for i386. Got it, thanks!
(In reply to Raphael Kubo da Costa from comment #6) To Q1: Ok, now I got it. Yes, your approach is much more elegant and I will do it next time this way. For now (version 3.10.1) I tend to retain it like it is already, because the next minor release of QGIS (3.10.2) is only a few weeks aways. Please feel free to contradict, if needed.
OK, are we ready to land then? I guess since the -DQSCI_SIP_DIR part was already there it doesn't need to be removed in this commit?
(In reply to Raphael Kubo da Costa from comment #8) For me, it would be fine so far. If nobody else complains ...
A commit references this bug: Author: rakuco Date: Fri Dec 20 16:40:11 UTC 2019 New revision: 520519 URL: https://svnweb.freebsd.org/changeset/ports/520519 Log: Update to 3.10.1. For some months now, the build process of graphics/qgis 3.8.2 had problems while using the QT5 QMap() function with large list like for proj6[1]. Compiling such long QMap() list with clang takes several hours instead of some seconds or minutes[2], often it breaks the build. It turns out that building that QMap() code with clang takes exhaustive memory[3], for which a nice patch already exists on OpenBSD[4]. While this OpenBSD patch builds fine on amd64, it needs another patch on i386 (usage of '-O1' instead the clang specific patch '-mllvm -inline-threshold=128'). After we found a solution for the QMap problem, it was necessary to wait for the release of QGIS 3.10.1 because of some tricky problems with proj6 / gdal combinations[5] that should be avoided. [1] https://github.com/qgis/QGIS/blob/master/src/core/qgscoordinatereferencesystem_legacy.h#L23 [2] http://osgeo-org.1560.x6.nabble.com/QGIS-Developer-QGIS-3-8-0-on-FreeBSD-build-takes-several-hours-td5408676.html [3] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241687 [4] https://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/geo/qgis/patches/patch-src_core_CMakeLists_txt?rev=1.22&content-type=text/x-cvsweb-markup&hideattic=1 [5] https://www.mail-archive.com/qgis-developer@lists.osgeo.org/msg48670.html Changes in the port: - Update from 3.8.2 to 3.10.1 - Allow using default clang again - Update several Python dependencies - Add textproc/py-nltk, math/py-pandas, sysutils/py-psutil, graphics/py-pyrsgis, x11-toolkits/py-qt5-quick for Python extensions - Add USES=gl and USE_GL=gl - Add USE_PYQT=quickwidgets - Add several CMAKE ARGS - Add files/patch-src_core_CMakeLists.txt for QMap problem - Change above patch into '-O1' for i386 - Update pkg-plist PR: 242557 PR: 241687 Submitted by: Rainer Hurling <rhurlin@gwdg.de> (maintainer) Changes: head/graphics/qgis/Makefile head/graphics/qgis/distinfo head/graphics/qgis/files/patch-src_core_CMakeLists.txt head/graphics/qgis/pkg-plist
Committed at last, thank you. I'd like to leave bug 241687 open so that we can track whatever fixes need to land into the LLVM port to remove the workaround.