Improvements to qgis and grass packaging, introduce qgis-ltr derivation#150286
Improvements to qgis and grass packaging, introduce qgis-ltr derivation#150286mpickering wants to merge 10 commits intoNixOS:masterfrom
Conversation
This is used for processing lidar point clouds.
Grass is a runtime dependency which qgis attempts to find on the path.
This feature is on by default with normal distributions so we should also enable it.
a8bbf12 to
ca2311b
Compare
|
I applied the review suggestions now. Thanks. |
|
As a quick note, I've been working on fixing qscintilla on darwin (#150595) -- the one thing that the PR currently regresses is qgis on linux. I still need to test how it works with this new expression for QGIS and what other changes it might need! |
|
To follow up: qgis on master only needs an updated |
| qt3d | ||
| pdal | ||
| zstd | ||
| ] ++ lib.optional withGrass grass |
There was a problem hiding this comment.
grass is currently broken on darwin, due to #126101. Suggest withGrass -> (!stdenv.isDarwin && withGrass) in all cases for now.
There was a problem hiding this comment.
Upon further thought, it's not totally clear to me why grass should fundamentally be dependent on webkitgtk. I suppose this is still a stopgap to get qgis to even build on darwin, while it might be possible to go deeper into grass/wxwidgets/etc in parallel to figure out how darwin can still keep grass even while leaving out webkitgtk while it's broken.
There was a problem hiding this comment.
It turns out it's actually a pretty small fix: wxwidgets just shouldn't ever build webkitgtk while webkitgtk is broken (
). I'll submit a PR for that so that no workaround here is needed.There was a problem hiding this comment.
See #150908 for the wxwidgets fix. There's still a few other issues with grass, so if those can't get resolved then it still may be better to disable grass to get QGIS to still build at all.
In addition, pdal also doesn't work on darwin, due to its dependence on https://github.com/asmaloney/libE57Format, which currently is linux only (see asmaloney/libE57Format#48). I'm not very versed in how static versus dynamic libraries play a role in nix, so I'm not sure what is needed to make that derivation cross-platform compatible. Until then, pdal should probably be disabled and excluded as a build input for darwin.
There was a problem hiding this comment.
Additionally with grass, even after the changes mentioned above and with #150908 applied, the build fails on darwin, even though it stops hitting any nix packaging incompatibility errors.
Specifically, the run ends with
Started compilation: Thu Dec 16 01:56:47 UTC 2021
--
Errors in:
/private/tmp/nix-build-grass.drv-2/source/lib/cairodriver
/private/tmp/nix-build-grass.drv-2/source/lib/display
/private/tmp/nix-build-grass.drv-2/source/display/d.barscale
/private/tmp/nix-build-grass.drv-2/source/display/d.geodesic
/private/tmp/nix-build-grass.drv-2/source/display/d.graph
/private/tmp/nix-build-grass.drv-2/source/display/d.grid
/private/tmp/nix-build-grass.drv-2/source/display/d.his
/private/tmp/nix-build-grass.drv-2/source/display/d.histogram
/private/tmp/nix-build-grass.drv-2/source/display/d.info
/private/tmp/nix-build-grass.drv-2/source/display/d.labels
/private/tmp/nix-build-grass.drv-2/source/display/d.legend
/private/tmp/nix-build-grass.drv-2/source/display/d.legend.vect
/private/tmp/nix-build-grass.drv-2/source/display/d.linegraph
/private/tmp/nix-build-grass.drv-2/source/display/d.mon
/private/tmp/nix-build-grass.drv-2/source/display/d.northarrow
/private/tmp/nix-build-grass.drv-2/source/display/d.path
/private/tmp/nix-build-grass.drv-2/source/display/d.profile
/private/tmp/nix-build-grass.drv-2/source/display/d.rast
/private/tmp/nix-build-grass.drv-2/source/display/d.rast.arrow
/private/tmp/nix-build-grass.drv-2/source/display/d.rast.num
/private/tmp/nix-build-grass.drv-2/source/display/d.rhumbline
/private/tmp/nix-build-grass.drv-2/source/display/d.text
/private/tmp/nix-build-grass.drv-2/source/display/d.rgb
/private/tmp/nix-build-grass.drv-2/source/display/d.vect
/private/tmp/nix-build-grass.drv-2/source/display/d.fontlist
/private/tmp/nix-build-grass.drv-2/source/display/d.vect.chart
/private/tmp/nix-build-grass.drv-2/source/display/d.where
/private/tmp/nix-build-grass.drv-2/source/display/d.vect.thematic
/private/tmp/nix-build-grass.drv-2/source/display/d.font
/private/tmp/nix-build-grass.drv-2/source/display/d.erase
/private/tmp/nix-build-grass.drv-2/source/display/d.colortable
/private/tmp/nix-build-grass.drv-2/source/vector/v.label
/private/tmp/nix-build-grass.drv-2/source/vector/v.label.sa
/private/tmp/nix-build-grass.drv-2/source/misc/m.nviz.script
/private/tmp/nix-build-grass.drv-2/source/visualization/ximgview
--
In case of errors please change into the directory with error and run 'make'.
If you get multiple errors, you need to deal with them in the order they
appear in the error log. If you get an error building a library, you will
also get errors from anything which uses the library.
--
Finished compilation: Thu Dec 16 02:09:58 UTC 2021
make: *** [Makefile:70: default] Error 1
Digging deeper into the log around the specific errors, they show things like the following, suggesting that it's trying to build for X11:
-L/nix/store/iaahycli6sk1qj97ccmbri5zb4zacxif-freetype-2.11.0/lib -lz -lcairo -lfontconfig -lfreetype -L/usr/X11/lib -lX11
ld: library not found for -lX11
clang-7: �[0;1;31merror: �[0mlinker command failed with exit code 1 (use -v to see invocation)�[0m
make[3]: *** [../../include/Make/Shlib.make:10: /private/tmp/nix-build-grass.drv-2/source/dist.x86_64-apple-darwin21.1.0/lib/libgrass_cairodriver.7.8.dylib] Error 1
make[3]: Leaving directory '/private/tmp/nix-build-grass.drv-2/source/lib/cairodriver'
make[3]: Entering directory '/private/tmp/nix-build-grass.drv-2/source/lib/bitmap'
The output during configure shows the following, suggesting that it's not able to recognize that it's on macOS, which may be why it's trying to build with X.
configure: WARNING: unrecognized options: --with-proj-lib
checking build system type... x86_64-apple-darwin21.1.0
checking host system type... x86_64-apple-darwin21.1.0
checking for gcc... clang
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether clang accepts -g... yes
checking for clang option to accept ISO C89... none needed
checking for executable suffix... no
checking for full floating-point support... yes
checking for pwd... /nix/store/ylblikfcniagmbnhckp3fakjkbxqy4zl-coreutils-9.0/bin/pwd
checking for source directory... "/private/tmp/nix-build-grass.drv-2/source"
checking for build directory... "/private/tmp/nix-build-grass.drv-2/source"
checking for git... no
checking for MacOSX App... "no"
checking for MacOSX architectures... no
checking for MacOSX SDK... no
checking how to build libraries... shared
checking for additional include dirs...
checking for additional library dirs...
| buildInputs = [ flex bison zlib proj gdal libtiff libpng fftw sqlite cairo | ||
| readline ffmpeg makeWrapper wxGTK30 netcdf geos postgresql libmysqlclient blas | ||
| libLAS proj-datumgrid zstd pdal wrapGAppsHook ] | ||
| ++ (with python3Packages; [ python python-dateutil wxPython_4_1 numpy ]); |
There was a problem hiding this comment.
Can wxPython_4_1 either be switched to wxPython_4_0 or put with logic similar to this:
++ (with python3Packages; [ python python-dateutil numpy ] ++
lib.optional stdenv.isDarwin wxPython_4_0 ++
lib.optional stdenv.isLinux wxPython_4_1);
For reasons not entirely clear to me with #150908, wxGTK30 builds on darwin x86, but wxGTK31 fails with the error that --enable-universal-binaries (found here) isn't a valid option. I don't totally understand why, since it also exists in 3.0.
| "--with-readline" | ||
| "--with-wxwidgets" | ||
| "--with-netcdf" | ||
| "--with-pdal" |
There was a problem hiding this comment.
On darwin, pdal should also similarly be disabled here pending a resolution to asmaloney/libE57Format#48
| six | ||
| ]; | ||
| in mkDerivation rec { | ||
| version = "3.16.14"; |
| ]; | ||
| in mkDerivation rec { | ||
| version = "3.16.14"; | ||
| version = "3.22.1"; |
|
When applied on top of the #150595 branch, the following patch successfully completes a make on darwin for However, it does still fail on final wrap/install, as the libraries do not link correctly and The only section that can't apply cleanly to master is the |
|
@sikmir are there any other issues you see with this PR? I'd be happy to include the version bump in a subsequent PR that wraps up all the darwin build fixes I've got going in this WIP patch noted above. It'd be great to get this PR stabilized in main so that there'll be plenty of time to work out issues with ltr versus pr as the next QGIS release cycle works its way through in advance of 22.05 -- and in a very self-interested way, to ensure that darwin has a chance to fix all its build issues in advance of that too! |
|
Anyone fancy merging this? |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
| # unpackPhase | ||
|
|
There was a problem hiding this comment.
| # unpackPhase |
| --set PYTHONPATH $program_PYTHONPATH | ||
| ''; | ||
|
|
||
| meta = qgis-ltr-unwrapped.meta; |
There was a problem hiding this comment.
| meta = qgis-ltr-unwrapped.meta; | |
| inherit (qgis-ltr-unwrapped) meta; |
| postFixup = '' | ||
| # unpackPhase | ||
| # grass has to be availble on the command line even though we baked in | ||
| # the path at build time using GRASS_PREFIX | ||
| wrapProgram $out/bin/qgis \ | ||
| --prefix PATH : ${lib.makeBinPath (lib.optional withGrass grass)} | ||
| ''; |
There was a problem hiding this comment.
| postFixup = '' | |
| # unpackPhase | |
| # grass has to be availble on the command line even though we baked in | |
| # the path at build time using GRASS_PREFIX | |
| wrapProgram $out/bin/qgis \ | |
| --prefix PATH : ${lib.makeBinPath (lib.optional withGrass grass)} | |
| ''; | |
| postFixup = lib.optionalString withGrass '' | |
| # grass has to be availble on the command line even though we baked in | |
| # the path at build time using GRASS_PREFIX | |
| wrapProgram $out/bin/qgis \ | |
| --prefix PATH : ${lib.makeBinPath [ grass ]} | |
| ''; |
We do not want to empty wrap it.
| repo = "QGIS"; | ||
| rev = "final-${lib.replaceStrings [ "." ] [ "_" ] version}"; | ||
| sha256 = "sha256-3FUGSBdlhJhhpTPtYuzKOznsC7PJV3kRL9Il2Yryi1Q="; | ||
| sha256 = "sha256:0hpbbv84lh1m7vrxv8d8x5kxgxcf0dydsvr3r2brgv3b40lpavd4"; |
There was a problem hiding this comment.
| sha256 = "sha256:0hpbbv84lh1m7vrxv8d8x5kxgxcf0dydsvr3r2brgv3b40lpavd4"; | |
| sha256 = "0hpbbv84lh1m7vrxv8d8x5kxgxcf0dydsvr3r2brgv3b40lpavd4"; |
| postFixup = '' | ||
| # unpackPhase | ||
| # grass has to be availble on the command line even though we baked in | ||
| # the path at build time using GRASS_PREFIX | ||
| wrapProgram $out/bin/qgis \ | ||
| --prefix PATH : ${lib.makeBinPath (lib.optional withGrass grass)} | ||
| ''; |
There was a problem hiding this comment.
| postFixup = '' | |
| # unpackPhase | |
| # grass has to be availble on the command line even though we baked in | |
| # the path at build time using GRASS_PREFIX | |
| wrapProgram $out/bin/qgis \ | |
| --prefix PATH : ${lib.makeBinPath (lib.optional withGrass grass)} | |
| ''; | |
| postFixup = lib.optionalString withGrass '' | |
| # grass has to be availble on the command line even though we baked in | |
| # the path at build time using GRASS_PREFIX | |
| wrapProgram $out/bin/qgis \ | |
| --prefix PATH : ${lib.makeBinPath [ grass ]} | |
| ''; |
| withMultimedia = true; | ||
| }; | ||
|
|
||
| pyqt5_with_qtlocation = self.pyqt5.override { | ||
| withLocation = true; | ||
| }; |
There was a problem hiding this comment.
We shouldn't introduce multiple variants of pyqt5 because it will create strange bugs when a python environment contains multiple variants of the same package. This override should be done in the package outside of pythonPackages.
| "-DCMAKE_SKIP_BUILD_RPATH=OFF" | ||
| "-DWITH_3D=True" | ||
| "-DPYQT5_SIP_DIR=${python3Packages.pyqt5}/${python3Packages.python.sitePackages}/PyQt5/bindings" | ||
| "-DQSCI_SIP_DIR=${python3Packages.qscintilla-qt5}/share/sip/PyQt5" |
There was a problem hiding this comment.
| "-DQSCI_SIP_DIR=${python3Packages.qscintilla-qt5}/share/sip/PyQt5" | |
| "-DQSCI_SIP_DIR=${python3Packages.qscintilla-qt5}/${python3Packages.python.sitePackages}/PyQt5/bindings" |
Per #150595, now merged
| "-DWITH_3D=True" | ||
| "-DWITH_PDAL=TRUE" | ||
| "-DPYQT5_SIP_DIR=${python3Packages.pyqt5_with_qtlocation}/${python3Packages.python.sitePackages}/PyQt5/bindings" | ||
| "-DQSCI_SIP_DIR=${python3Packages.qscintilla-qt5}/share/sip/PyQt5" |
There was a problem hiding this comment.
This line has now changed on master:
|
@mpickering let me know if you'd like any help here -- if useful and you're too busy, I can try to set up a PR to your own repo to incorporate these changes into this branch |
Thanks @willcohen , this branch works perfectly for me locally and I don't intend to change it anymore so if you want to help that would be great. |
|
#156747 incorporates all outstanding review comments from this branch, rebased to master. Once that PR is merged I can separately submit the additional patches that I'd been working on in-conversation to fix the build on darwin. |
|
@mpickering i think this can be closed with the merge of #156747! |
Motivation for this change
This patch fixes some various issues with grass and qgis packaging.
Upgrade grass to 7.8.6
Upgrade grass build to use python3 (A list of packages that grep for "python2" #148779)
Add some missing feature flags to grass to enable some more algorithms.
Add pdal dependency to grass to allow lidar point cloud processing.
Fix the QGIS grass support
Enable the 3D view in QGIS
Add qgis-ltr version and bump the main derivation to 3.22.1.
cc @lsix
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes