QGIS: 2.18.17 -> 3.0.1 plus required libs #38022

coreyoconnor · 2018-03-28T23:10:47Z

I think the wrapper could use some improvements, but it's functional.

Motivation for this change

update QGIS to 3.0.1

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

jtojnar · 2018-03-28T23:25:20Z

pkgs/top-level/python-packages.nix

@@ -10065,6 +10065,27 @@ in {
    };
  };

+  OWSLib = buildPythonPackage rec {


Could you move this to a separate file?

Yes. There is a mix of inline and separate file in python-packages. Is the standard to move to separate file?

In the past having module expressions in this file was okay; nowadays, we are moving them to separate files.

We are moving Python expressions out of pkgs/top-level/python-packages.nix into pkgs/development/python-modules/<module>/default.nix.

Please move the expression there, and call it from pkgs/top-level/python-packages.nix using callPackage ../development/python-modules/<package> { };.

jtojnar · 2018-03-28T23:25:48Z

pkgs/top-level/python-packages.nix

+      sha256 = "0m05225g1sqd2i8r2riaan33953hfni9wjq7n225snhl7klsb5gc";
+    };
+
+    buildInputs = with self; [ dateutil pep8 pillow pyproj pytz pytest pytestcov requests tox ];


Some of these should probably go to checkInputs.

jtojnar · 2018-03-28T23:26:51Z

pkgs/top-level/python-packages.nix

+    meta = {
+      description = "Client programming with Open Geospatial Consortium (OGC) web service.";
+      license = licenses.bsd3;
+      homepage = "http://geopython.github.io/OWSLib/";


URLs need not be quoted.

jtojnar · 2018-03-28T23:26:52Z

pkgs/top-level/python-packages.nix

+    meta = {
+      description = "Client programming with Open Geospatial Consortium (OGC) web service.";
+      license = licenses.bsd3;
+      homepage = "http://geopython.github.io/OWSLib/";


URLs do not need to be quoted.

jtojnar · 2018-03-28T23:27:12Z

pkgs/top-level/qscintilla-pyqt5-widgets.patch

@@ -0,0 +1,12 @@
+diff -u -r orig/Python/configure.py QScintilla_gpl-2.10.3/Python/configure.py


This definitely cannot be in top-level. Should go to pkgs/development/python-modules/qscintilla/.

whoops! editor goof

jtojnar · 2018-03-28T23:38:16Z

pkgs/applications/gis/qgis/default.nix

+  # Build processes depend on the built libraries. Extend
+  # LD_LIBRARY_PATH as required.
+  preConfigure = ''
+    export LD_LIBRARY_PATH=`pwd`/build/output/lib:${stdenv.lib.makeLibraryPath [ openssl ]}$LD_LIBRARY_PATH


Would not adding -DCMAKE_SKIP_BUILD_RPATH=OFF to cmakeFlags remove the need for this hack this?

No idea. That was from the previous QGIS version. I'll try that option and see.

Apparently the preConfigure section is not required any more.

jtojnar · 2018-03-28T23:39:01Z

pkgs/applications/gis/qgis/default.nix

-  preBuild = ''
-    export LD_LIBRARY_PATH=`pwd`/output/lib:${stdenv.lib.makeLibraryPath [ openssl ]}:$LD_LIBRARY_PATH
-  '';
+  nativeBuildInputs = [ cmake ];


Please add ninja when using cmake, it is usually faster than make.

FRidh

We should not yet replace 2.8 by 3.0 because 3.0 is not yet feature complete. Instead, add 3.0 next to 2.8.

FRidh · 2018-03-29T08:23:26Z

pkgs/applications/gis/qgis/default.nix


-  nativeBuildInputs = [ cmake makeWrapper ];
+  pythonBuildInputs = [ python3Packages.qscintilla python3Packages.gdal ] ++


This should go into let ... in expression.

FRidh · 2018-03-29T08:24:42Z

pkgs/applications/gis/qgis/wrapped.nix

+    wrapProgram $out/bin/qgis \
+      --prefix PATH : $program_PATH \
+      --prefix PYTHONPATH : $program_PYTHONPATH \
+      --prefix LD_LIBRARY_PATH : ${stdenv.lib.makeLibraryPath [ openssl ]}


this should be patched instead.

This is probably not necessary anymore. ldd $(nix-build -A qgis)/bin/.qgis-wrapped | grep openssl shows it is linked correctly.

FRidh · 2018-03-29T08:25:02Z

pkgs/applications/gis/qgis/wrapped.nix

+
+    wrapProgram $out/bin/qgis \
+      --prefix PATH : $program_PATH \
+      --prefix PYTHONPATH : $program_PYTHONPATH \


No prefixing, but setting so PYTHONPATH cannot leak inside.

FRidh · 2018-03-29T08:25:33Z

pkgs/top-level/all-packages.nix

@@ -17335,7 +17335,11 @@ with pkgs;

  qemu-riscv = callPackage ../applications/virtualization/qemu/riscv.nix {};

-  qgis = callPackage ../applications/gis/qgis {};
+  qgis-unwrapped = libsForQt5.callPackage ../applications/gis/qgis {


Why expose the unwrapped version? Is it used somewhere?

Exposing the unwrapped version in these cases is easiest way (IMO) to enable an overlay changing the source of the application. EG:

coreyoconnor/nix_configs@c37c85a

FRidh · 2018-03-29T08:25:51Z

pkgs/top-level/python-packages.nix

@@ -10065,6 +10065,27 @@ in {
    };
  };

+  OWSLib = buildPythonPackage rec {


We are moving Python expressions out of pkgs/top-level/python-packages.nix into pkgs/development/python-modules/<module>/default.nix.

Please move the expression there, and call it from pkgs/top-level/python-packages.nix using callPackage ../development/python-modules/<package> { };.

FRidh · 2018-03-29T08:26:01Z

pkgs/top-level/python-packages.nix

+  OWSLib = buildPythonPackage rec {
+    pname = "OWSLib";
+    version = "0.16.0";
+    name = "${pname}-${version}";


The name attribute is added by buildPython* and should therefore be removed.

jtojnar · 2018-03-29T08:35:38Z

We should not yet replace 2.8 by 3.0 because 3.0 is not yet feature complete.

Do you have a source for that? I only found

This is the first release in the 3.x series. It comes with tons of new features and under-the-hood updates. As such, we do not expect it to be as reliable as the 2.18 LTR just yet.

https://qgis.org/en/site/forusers/visualchangelog30/index.html

FRidh · 2018-03-29T08:40:43Z

Its actually the plugins I meant to refer to. Plugins written in 2 are not compatible with 3. As they are a major feature of QGIS we should not be dropping 2 yet. Before 18.09 we can revisit this choice.

coreyoconnor · 2018-03-29T17:32:08Z

That explains the mixed messaging I've seen online. The version number and some messaging indicates the QGIS 3 is released. Other messaging (EG: loading screen) states "early adopter".

I just figured it was inconsistency haha. The plugin situation explains that. I'll re-add the 2 derivation.

This uses the recommended configuration for python 3 and qt5. The config used for python 2 and qt 4 is unchanged from before.

coreyoconnor · 2018-03-30T02:07:55Z

I re-added QGIS 2.18 as well as resolved the noted issues. Other changes with latest commits:

Qscintilla is 2.9 not 2.10 as in the original commits. 2.10 has a different library naming convention (_qtN suffix) which would change the interface for any existing qscintilla dependents. I don't know if that would matter so I'll defer the upgrade till later.
partitioned the python package changes into separate commits.

FRidh

Why is the wrapper needed? Can't this be done in the installPhase or fixupPhase?

Of course it is more convenient when designing the wrappers when not needing to rebuild the whole thing.

FRidh · 2018-03-30T05:51:10Z

pkgs/applications/gis/qgis/3.0.nix

+with lib;
+let
+  pythonBuildInputs = with python3Packages;
+    [ python3Packages.qscintilla gdal jinja2 numpy psycopg2 pygments pyqt5 sip OWSLib six ];


repeating python3Packages

Not a repeat. The "with" does not cause the "qscintilla" to be shadowed. Without the extra python3Packages the "qscintilla" passed is not the one in python3Packages but the C++ library one.

FRidh · 2018-03-30T05:53:07Z

pkgs/development/python-modules/OWSLib/default.nix

+  buildInputs = [ dateutil pep8 pillow pyproj pytz requests tox ];
+  checkInputs = [ pytest pytestcov ];
+
+  doCheck = false;


Why are the tests disabled? Include a comment in the expression explaining why the tests are disabled.

No tests run and the build fails because no test run. I figured this was a bug in the python build infrastructure.

FRidh · 2018-03-30T05:53:26Z

pkgs/development/python-modules/OWSLib/default.nix

+    sha256 = "0m05225g1sqd2i8r2riaan33953hfni9wjq7n225snhl7klsb5gc";
+  };
+
+  buildInputs = [ dateutil pep8 pillow pyproj pytz requests tox ];


cehckInputs?

Already resolved. Not sure why this diff is showing up for you.

coreyoconnor · 2018-03-30T06:13:26Z

The wrapper is required to enable extension of the qgis python environment by users without requiring a rebuild of the full application. EG: to add additional deps for plugins for instance.

FRidh · 2018-03-30T06:19:15Z

pkgs/applications/gis/qgis/wrapped.nix

+let
+  # the python packages that should be included in the PYTHONPATH
+  pythonInputs = with python3Packages;
+    qgis3-unwrapped.pythonBuildInputs ++ [ chardet dateutil pyyaml pytz requests urllib3 ];


Why should these few be included in the wrapper, but not in the build? Are they needed for specific plugins?

Moved to build. The reference errors these resolved only showed up at runtime.
I would expect these are transitive dependencies for packages missing respective propagatedBuildInputs?

They are not hard dependencies otherwise the builds of the dependents would fail. Either upstream has forgotten to list their full dependencies, or they require "extras" of certain packages.
https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-extras-optional-features-with-their-own-dependencies

coreyoconnor · 2018-03-30T07:04:19Z

Amended to resolve current CRs.

FRidh · 2018-03-30T07:38:57Z

pkgs/applications/gis/qgis/wrapped.nix

+      --prefix PATH : $program_PATH \
+      --set PYTHONPATH $program_PYTHONPATH
+
+    # desktop link


The following seems irrelevant for the wrapper and thus better placed in the original derivation. Or am I missing something?

coreyoconnor · 2018-03-30T16:36:41Z

Times up! no further changes will be submitted.

mpickering · 2018-04-12T20:47:53Z

This looks like a great start. I was blocked on this as some dependencies were not building for darwin. If they are resolved I can pick it up.

das-g · 2018-05-11T11:06:48Z

I successfully built QGIS 3 from this branch on NixOS 18.03.

How do we proceed? Shall I attempt to resolve the merge conflicts with master?

Times up! no further changes will be submitted.

What does that mean? Was your effort on this time-boxed @coreyoconnor or are you referring to NixOS release 18.03?

das-g · 2018-06-24T21:49:35Z

QGIS 3.2 Bonn has been released. Time to revive this PR?

coreyoconnor · 2018-06-27T22:32:34Z

Hi! The time I had for this original PR ran out a while ago. I will not have a chance for a few weeks (sadly) to really dig into this again. Anyone should feel free to take the changes thus far and carry on, of course.

I do have a separate branch with these changes rebased against HEAD. Might be able to get this updated accordingly but besides that... a few weeks it is.

coreyoconnor · 2018-11-19T16:50:00Z

Clearing old PRs.

das-g · 2019-03-02T02:37:19Z

New PR for QGIS 3.4.5 (=the new LTR version and now currently only LTR version): #56486

coreyoconnor requested a review from FRidh as a code owner March 28, 2018 23:10

GrahamcOfBorg added 6.topic: python 8.has: clean-up 8.has: package (new) 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Mar 28, 2018

jtojnar reviewed Mar 28, 2018

View reviewed changes

FRidh requested changes Mar 29, 2018

View reviewed changes

coreyoconnor added 3 commits March 29, 2018 16:21

pythonPackages.pyqt5: 5.10 -> 5.10.1

81504a2

pythonPackages.OWSLib: init at 0.16.0

fd1aa24

pythonPackages.qscintilla: Enable for python 3.

f43d3bd

This uses the recommended configuration for python 3 and qt5. The config used for python 2 and qt 4 is unchanged from before.

coreyoconnor force-pushed the qgis-3.0.1 branch 5 times, most recently from 0294449 to 5e3ce30 Compare March 30, 2018 00:56

FRidh requested changes Mar 30, 2018

View reviewed changes

FRidh reviewed Mar 30, 2018

View reviewed changes

qgis: add 3.0.1.

73ca16c

coreyoconnor force-pushed the qgis-3.0.1 branch from 5e3ce30 to 73ca16c Compare March 30, 2018 06:31

FRidh reviewed Mar 30, 2018

View reviewed changes

mpickering mentioned this pull request Apr 12, 2018

version bump request: QGIS 3 #35669

Closed

lsix mentioned this pull request May 29, 2018

qgis: 2.18.17 -> 2.18.20 #41195

Merged

8 tasks

coreyoconnor closed this Nov 19, 2018

lsix mentioned this pull request Jan 28, 2019

pythonPackages.qscintilla: enable for python3 #54834

Merged

10 tasks

das-g mentioned this pull request Mar 2, 2019

qgis: 2.18.28 -> 3.4.5 #56486

Merged

10 tasks

		@@ -0,0 +1,12 @@
		diff -u -r orig/Python/configure.py QScintilla_gpl-2.10.3/Python/configure.py


		nativeBuildInputs = [ cmake makeWrapper ];
		pythonBuildInputs = [ python3Packages.qscintilla python3Packages.gdal ] ++

QGIS: 2.18.17 -> 3.0.1 plus required libs #38022

QGIS: 2.18.17 -> 3.0.1 plus required libs #38022

Conversation

coreyoconnor commented Mar 28, 2018 • edited

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FRidh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtojnar commented Mar 29, 2018

FRidh commented Mar 29, 2018

coreyoconnor commented Mar 29, 2018

coreyoconnor commented Mar 30, 2018

FRidh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coreyoconnor commented Mar 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coreyoconnor commented Mar 30, 2018

Choose a reason for hiding this comment

coreyoconnor commented Mar 30, 2018

mpickering commented Apr 12, 2018

das-g commented May 11, 2018 • edited

das-g commented Jun 24, 2018

coreyoconnor commented Jun 27, 2018

coreyoconnor commented Nov 19, 2018

das-g commented Mar 2, 2019

coreyoconnor commented Mar 28, 2018 •

edited

das-g commented May 11, 2018 •

edited