-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
Improvements to qgis and grass packaging, introduce qgis-ltr derivation #150286
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. |
@@ -96,27 +101,40 @@ in mkDerivation rec { | |||
qscintilla | |||
qtserialport | |||
qtxmlpatterns | |||
qt3d | |||
pdal | |||
zstd | |||
] ++ lib.optional withGrass grass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out it's actually a pretty small fix: wxwidgets just shouldn't ever build webkitgtk while webkitgtk is broken (
++ optional withWebKit webkitgtk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -37,6 +38,7 @@ stdenv.mkDerivation rec { | |||
"--with-readline" | |||
"--with-wxwidgets" | |||
"--with-netcdf" | |||
"--with-pdal" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On darwin
, pdal
should also similarly be disabled here pending a resolution to asmaloney/libE57Format#48
six | ||
]; | ||
in mkDerivation rec { | ||
version = "3.16.14"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.16.15 is out
sip_4 | ||
owslib | ||
six | ||
]; | ||
in mkDerivation rec { | ||
version = "3.16.14"; | ||
version = "3.22.1"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.22.2 is out
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# unpackPhase |
--set PYTHONPATH $program_PYTHONPATH | ||
''; | ||
|
||
meta = qgis-ltr-unwrapped.meta; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
pname = "qgis-unwrapped"; | ||
|
||
src = fetchFromGitHub { | ||
owner = "qgis"; | ||
repo = "QGIS"; | ||
rev = "final-${lib.replaceStrings [ "." ] [ "_" ] version}"; | ||
sha256 = "sha256-3FUGSBdlhJhhpTPtYuzKOznsC7PJV3kRL9Il2Yryi1Q="; | ||
sha256 = "sha256:0hpbbv84lh1m7vrxv8d8x5kxgxcf0dydsvr3r2brgv3b40lpavd4"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"-DQSCI_SIP_DIR=${python3Packages.qscintilla-qt5}/share/sip/PyQt5" | |
"-DQSCI_SIP_DIR=${python3Packages.qscintilla-qt5}/${python3Packages.python.sitePackages}/PyQt5/bindings" |
Per #150595, now merged
"-DPYQT5_SIP_DIR=${python3Packages.pyqt5}/${python3Packages.python.sitePackages}/PyQt5/bindings" | ||
"-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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line has now changed on master:
"-DQSCI_SIP_DIR=${python3Packages.qscintilla-qt5}/${python3Packages.python.sitePackages}/PyQt5/bindings" |
@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 = true
set 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.sh
to update generated release notes