Bug 242557 - graphics/qgis: Update to 3.10.1
Summary: graphics/qgis: Update to 3.10.1
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Raphael Kubo da Costa
URL:
Keywords: buildisok
Depends on:
Blocks:
 
Reported: 2019-12-10 21:02 UTC by Rainer Hurling
Modified: 2019-12-20 16:49 UTC (History)
3 users (show)

See Also:


Attachments
Patch QGIS to 3.10.1 and overcome the QMap problems (21.61 KB, patch)
2019-12-10 21:02 UTC, Rainer Hurling
rhurlin: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rainer Hurling 2019-12-10 21:02:14 UTC
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.
Comment 1 Automation User 2019-12-12 20:50:24 UTC
Build info is available at https://gitlab.com/swills/freebsd-ports/pipelines/102126441
Comment 2 Loïc Bartoletti freebsd_committer 2019-12-13 06:34:48 UTC
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)
Comment 3 Rainer Hurling 2019-12-14 09:25:55 UTC
(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?
Comment 4 Raphael Kubo da Costa freebsd_committer 2019-12-20 12:09:27 UTC
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.
Comment 5 Rainer Hurling 2019-12-20 12:30:08 UTC
(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
Comment 6 Raphael Kubo da Costa freebsd_committer 2019-12-20 12:56:22 UTC
> 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!
Comment 7 Rainer Hurling 2019-12-20 13:08:07 UTC
(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.
Comment 8 Raphael Kubo da Costa freebsd_committer 2019-12-20 13:18:56 UTC
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?
Comment 9 Rainer Hurling 2019-12-20 13:32:10 UTC
(In reply to Raphael Kubo da Costa from comment #8)

For me, it would be fine so far. If nobody else complains ...
Comment 10 commit-hook freebsd_committer 2019-12-20 16:41:06 UTC
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
Comment 11 Raphael Kubo da Costa freebsd_committer 2019-12-20 16:49:29 UTC
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.