python: Add Rtree library #36760

bgamari · 2018-03-10T16:37:56Z

Motivation for this change

This library is used by another package which I am working on packaging.

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.

FRidh

Please check the general and Python-specific contributing guidelines.

bgamari · 2018-03-10T18:07:31Z

Oh dear, yes, I clearly failed here. Sorry about that.

Let's try this again.

bgamari · 2018-03-10T21:35:02Z

Alright, I believe this should be ready for review.

Thanks @FRidh!

dotlambda

Your commit message should start with pythonPackages.rtree

dotlambda · 2018-03-17T10:48:30Z

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

+  propagatedBuildInputs = [ libspatialindex ];
+  patchPhase = ''
+    substituteInPlace rtree/core.py --replace \
+      "find_library('spatialindex_c')" "'${libspatialindex}/lib/libspatialindex_c.so.4'"


Not very future-proof because of the .4 suffix.

Also please use ${stdenv.hostPlatform.extensions.sharedLibrary}

dotlambda · 2018-03-17T10:48:43Z

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

+  name = "${pname}-${version}";
+  src = fetchPypi {
+    inherit pname version;
+    sha256 = null;


Oh dear, good catch.

dotlambda · 2018-03-17T10:49:05Z

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

+    inherit pname version;
+    sha256 = null;
+  };
+  doCheck = false; # appears to be broken


More detail please. Did you try fixing the failures?

dotlambda · 2018-03-17T10:49:22Z

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

+buildPythonPackage rec {
+  pname = "Rtree";
+  version = "0.8.3";
+  name = "${pname}-${version}";


Leave this line out.

dotlambda · 2018-03-17T10:49:31Z

pkgs/top-level/python-packages.nix

@@ -21125,6 +21125,8 @@ EOF
  wsproto = callPackage ../development/python-modules/wsproto { };

  h11 = callPackage ../development/python-modules/h11 { };
+
+  Rtree = callPackage ../development/python-modules/Rtree { inherit (pkgs) libspatialindex; };


s/Rtree/rtree/g

Really? The library is actually called Rtree and I see plenty of other packages whose names reflect their actual capitalization.

I prefer lowercase, but I think it's up to you.
However, plaese try to stick with the alphabetical ordering of python-packages.nix.

bgamari · 2018-04-03T14:48:00Z

I believe I have fixed all of the noted issues save the test failures. Unfortunately the failures appear to be due to memory unsafe issues that I was unable to reproduce on under Debian with the same versions of libspatialindex and Rtree. For instance, valgrind says (one of many failures),

==10453== Invalid read of size 4
==10453==    at 0x4EF1DFA: _PyObject_Free (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4FBADC3: _sre_SRE_Pattern_findall (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4EEC8AF: _PyCFunction_FastCallDict (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4F5DD71: call_function (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4F60926: _PyEval_EvalFrameDefault (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4F5CEBE: _PyFunction_FastCall (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4F5DC54: call_function (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4F60926: _PyEval_EvalFrameDefault (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4F5D8A5: _PyEval_EvalCodeWithName (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4F5DA4E: fast_function (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4F5DC54: call_function (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)
==10453==    by 0x4F60926: _PyEval_EvalFrameDefault (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0)

Nevertheless, I was able to use Rtree without any trouble in an application, so it's not clear to me that this should hold up merging.

dotlambda · 2018-04-03T15:33:57Z

@GrahamcOfBorg build python2.pkgs.Rtree python3.pkgs.Rtree

GrahamcOfBorg · 2018-04-03T15:34:23Z

Success on x86_64-linux (full log)

Attempted: python2.pkgs.Rtree, python3.pkgs.Rtree

Partial log (click to expand)

Successfully installed Rtree-0.8.3
/build/Rtree-0.8.3
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/wlfbkxskz804izli29fs6qdc251ls4kd-python3.6-Rtree-0.8.3
strip is /nix/store/fzcs0fn6bb04m82frhlb78nc03ny3w55-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/wlfbkxskz804izli29fs6qdc251ls4kd-python3.6-Rtree-0.8.3/lib
patching script interpreter paths in /nix/store/wlfbkxskz804izli29fs6qdc251ls4kd-python3.6-Rtree-0.8.3
checking for references to /build in /nix/store/wlfbkxskz804izli29fs6qdc251ls4kd-python3.6-Rtree-0.8.3...
/nix/store/y10ybjlpizqjml5i62m3691nw5q2xk6j-python2.7-Rtree-0.8.3
/nix/store/wlfbkxskz804izli29fs6qdc251ls4kd-python3.6-Rtree-0.8.3

GrahamcOfBorg · 2018-04-03T15:35:09Z

Success on aarch64-linux (full log)

Attempted: python2.pkgs.Rtree, python3.pkgs.Rtree

Partial log (click to expand)

Successfully installed Rtree-0.8.3
/build/Rtree-0.8.3
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/z6fypbjx82a963ij8mlz49bika858582-python3.6-Rtree-0.8.3
strip is /nix/store/3zq400fri5dv7d30lpxlqm2v9y1iis6j-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/z6fypbjx82a963ij8mlz49bika858582-python3.6-Rtree-0.8.3/lib
patching script interpreter paths in /nix/store/z6fypbjx82a963ij8mlz49bika858582-python3.6-Rtree-0.8.3
checking for references to /build in /nix/store/z6fypbjx82a963ij8mlz49bika858582-python3.6-Rtree-0.8.3...
/nix/store/f5m1hb5m5x4n06lxfaa0bai7xfgvnlhx-python2.7-Rtree-0.8.3
/nix/store/z6fypbjx82a963ij8mlz49bika858582-python3.6-Rtree-0.8.3

This is a commonly-used package for spatial indexing. Strangely enough, the tests are broken due to memory unsafety that I was unable to reproduce under Debian. For instance, when run with Python 3.6 valgrind reports, ==10453== Invalid read of size 4 ==10453== at 0x4EF1DFA: _PyObject_Free (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4FE7D96: _PyFaulthandler_Fini (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4F8438D: Py_FinalizeEx (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4F9F398: Py_Main (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x400BDF: main (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/bin/python3.6) ==10453== Address 0x67b1020 is 352 bytes inside a block of size 640 free'd ==10453== at 0x4C2DD6B: free (in /nix/store/6z028lfnxyhh8dlngpm6zrkwqxmbglj4-valgrind-3.13.0/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==10453== by 0x4EDDC4A: free_keys_object (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4EDEE63: dict_dealloc (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4EECF59: module_clear (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4FA062B: collect (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4FA13A0: _PyGC_CollectNoFail (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4F79CED: PyImport_Cleanup (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4F8436F: Py_FinalizeEx (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4F9F398: Py_Main (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x400BDF: main (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/bin/python3.6) ==10453== ==10453== Conditional jump or move depends on uninitialised value(s) ==10453== at 0x4EF1E03: _PyObject_Free (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4EFEF2D: type_dealloc (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4EFD009: subtype_dealloc (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4FA063E: collect (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4FA13A0: _PyGC_CollectNoFail (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4F79DF8: PyImport_Cleanup (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4F8436F: Py_FinalizeEx (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x4F9F398: Py_Main (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/lib/libpython3.6m.so.1.0) ==10453== by 0x400BDF: main (in /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/bin/python3.6) ==10453== ...

bgamari requested a review from FRidh as a code owner March 10, 2018 16:37

GrahamcOfBorg added the 6.topic: python label Mar 10, 2018

FRidh requested changes Mar 10, 2018

View reviewed changes

FRidh added the 2.status: work-in-progress label Mar 10, 2018

bgamari force-pushed the Rtree branch 2 times, most recently from cc50e62 to 7b6f5ac Compare March 10, 2018 18:07

bgamari force-pushed the Rtree branch 3 times, most recently from b4fcb56 to f5b440f Compare March 10, 2018 21:23

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Mar 10, 2018

dotlambda requested changes Mar 17, 2018

View reviewed changes

bgamari force-pushed the Rtree branch from f5b440f to 7dc5f20 Compare April 3, 2018 14:36

bgamari force-pushed the Rtree branch 2 times, most recently from 35e4b20 to f56999c Compare April 3, 2018 14:50

dotlambda force-pushed the Rtree branch from f56999c to 6bc4747 Compare April 4, 2018 16:38

dotlambda merged commit b948487 into NixOS:master Apr 4, 2018

marcus7070 mentioned this pull request Mar 13, 2021

python3Packages.Rtree: fix build #116128

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

python: Add Rtree library #36760

python: Add Rtree library #36760

bgamari commented Mar 10, 2018 •

edited

FRidh left a comment

bgamari commented Mar 10, 2018

bgamari commented Mar 10, 2018

dotlambda left a comment

dotlambda Mar 17, 2018

dotlambda Mar 17, 2018

dotlambda Mar 17, 2018

bgamari Mar 19, 2018

dotlambda Mar 17, 2018

dotlambda Mar 17, 2018

dotlambda Mar 17, 2018

bgamari Mar 19, 2018

dotlambda Mar 19, 2018

bgamari commented Apr 3, 2018

dotlambda commented Apr 3, 2018

GrahamcOfBorg commented Apr 3, 2018

GrahamcOfBorg commented Apr 3, 2018

python: Add Rtree library #36760

python: Add Rtree library #36760

Conversation

bgamari commented Mar 10, 2018 • edited

Motivation for this change

Things done

FRidh left a comment

Choose a reason for hiding this comment

bgamari commented Mar 10, 2018

bgamari commented Mar 10, 2018

dotlambda 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

Choose a reason for hiding this comment

bgamari commented Apr 3, 2018

dotlambda commented Apr 3, 2018

GrahamcOfBorg commented Apr 3, 2018

GrahamcOfBorg commented Apr 3, 2018

bgamari commented Mar 10, 2018 •

edited