Python-kivy: init at 1.10.0 #29305

vanschelven · 2017-09-13T09:12:44Z

Motivation for this change

Kivy is an Open source Python library for rapid development of applications
that make use of innovative user interfaces, such as multi-touch apps.

Most of Kivy's dependencies are optional, so defining a good "default" set of useful dependencies was a bit of a challenge. The present set of dependencies works well for me, but more dependencies, or even options for configuring the package, may be required in the future.

There is also some discussion at kivy/kivy#5367

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
- Linux
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 · 2017-09-13T09:16:05Z

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

+buildPythonPackage rec {
+  name = "Kivy-1.10.0";
+
+  src = pkgs.fetchurl {


FRidh · 2017-09-13T09:17:56Z

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

+
+  propagatedBuildInputs = with self; [
+    pillow
+    pkgs.mtdev


Why is this library propagated?

This is a mistake; I thought that would be helpful, but the actual fix was found later (https://groups.google.com/forum/#!topic/nix-devel/CPQKpM3u1EY), and I never downgraded to non-propagated. Will fix.

FRidh · 2017-09-13T09:19:03Z

pkgs/top-level/python-packages.nix

@@ -11176,6 +11176,29 @@ in {

  keyring = callPackage ../development/python-modules/keyring { };

+  kivy = callPackage ../development/python-modules/kivy { inherit stdenv buildPythonPackage maintainers platforms licenses pkgs self ; };
+
+  kivygarden = buildPythonPackage rec {


in separate file

FRidh · 2017-09-13T09:19:41Z

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

+# assert stdenv.lib.versionOlder self.cython.version "0.25.3";
+
+buildPythonPackage rec {
+  name = "Kivy-1.10.0";


can you include pname, version, and ${pname}-${version} as is now more commonly done.

FRidh · 2017-09-13T09:19:51Z

pkgs/top-level/python-packages.nix

+    # The sole reason for this package is that it is a (questionable) dependency of Kivy
+    name = "kivy-garden-0.1.4";
+
+    src = pkgs.fetchurl {


FRidh · 2017-09-13T09:19:57Z

pkgs/top-level/python-packages.nix

+
+  kivygarden = buildPythonPackage rec {
+    # The sole reason for this package is that it is a (questionable) dependency of Kivy
+    name = "kivy-garden-0.1.4";


same with pname and version

FRidh · 2017-09-13T09:20:25Z

pkgs/top-level/python-packages.nix

+      sha256 = "0wkcpr2zc1q5jb0bi7v2dgc0vs5h1y7j42mviyh764j2i0kz8mn2";
+    };
+
+    buildInputs = with self; [ requests ];


Is this not needed during runtime? Or is it needed for the tests? In case of the latter, use checkInputs.

It's needed during runtime.

kivy-garden is a "not invented here" package manager for kivy components, and as such uses requests to fetch those components.

The real problem is that such a thing exists in the first place, and that it is an explicit dependency of Kivy. I'll talk with the kivy people about both of those issues.

In the meantime, given the fact that the present PR makes kivy-garden available, it should do so in a non-broken way, so I'll promote this to a propagatedBuildInput

@FRidh

As per the comments by @FRidh (once no more questions/remarks remain, I'll squash the commits into a single one)

FRidh · 2017-09-13T19:43:48Z

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

@@ -0,0 +1,101 @@
+{ stdenv, buildPythonPackage, fetchPypi, maintainers, platforms, licenses, pkgs, self }:


parameters are supposed to be individual functions or derivations, and not sets. Exceptions are stdenv or lib.

I'm having some trouble implementing this advice, and associated advice on given repeatedly below on how to remove the associated explicit parameter-setting on the calling side. Could you show me how it's done?

FRidh · 2017-09-13T20:10:38Z

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

+
+  buildInputs = with self; [
+    pkgconfig
+    cython


these two can be nativeBuildInputs if I am correct.

I am not confident that I understand the difference (see https://groups.google.com/forum/#!topic/nix-devel/Ad1b0RShe6k) and all other Python packages that I could find do in fact use buildInputs here, so I'm a bit wary to proceed with this tip. Feel free to correct me if you're sure about this behavior though.

FRidh · 2017-09-13T20:11:31Z

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

+    kivygarden  # See https://github.com/kivy/kivy/issues/5367 suggestion I-2
+    requests  # See https://github.com/kivy/kivy/issues/5367 suggestion I-3
+    docutils # See https://github.com/kivy/kivy/issues/5367 question Q-5
+    pygments # See https://github.com/kivy/kivy/issues/5367 question Q-6


Are these 4 only needed during build time? Or during runtime? If the latter -> propagatedBuildInputs

Regarding docutils: purely build time.

Regarding the rest: setup.py refuses to install without these packages present, but they are fundamentally optional and (in the case of kivygarden especially) it's undesirable to pollute the runtime environment too much with their presence. If someone e.g. want the pygments functionality at runtime, it's better to have them install this explicitly than to "pull in the world"

In short, I stand with the decision to have them be build time dependencies, but it's a bit of a judgement call.

FRidh · 2017-09-13T20:11:44Z

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

+    # https://kivy.org/docs/installation/installation.html
+    # https://kivy.org/docs/installation/installation-linux.html
+
+    pkgs.mesa


get rid of pkgs

FRidh · 2017-09-13T20:12:12Z

pkgs/development/python-modules/kivy/kivy-garden.nix

@@ -0,0 +1,25 @@
+# The sole reason for this package is that it is a (questionable) dependency of Kivy
+{ buildPythonPackage, fetchPypi, maintainers, platforms, licenses, pkgs, self }:


same as the other

FRidh · 2017-09-13T20:12:33Z

pkgs/development/python-modules/kivy/kivy-garden.nix

+    homepage    = "https://pypi.python.org/pypi/kivy-garden";
+    license     = licenses.mit;
+    maintainers = with maintainers; [ vanschelven ];
+    platforms   = platforms.unix;  # Can only test linux; in principle other platforms are supported


buildPythonPackage already sets platforms

FRidh · 2017-09-13T20:12:45Z

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

+    homepage    = "https://pypi.python.org/pypi/kivy";
+    license     = licenses.mit;
+    maintainers = with maintainers; [ vanschelven ];
+    platforms   = platforms.unix;  # I can only test linux; in principle other platforms are supported


buildPythonPackage already sets platforms

FRidh · 2017-09-13T20:13:09Z

pkgs/top-level/python-packages.nix

@@ -11176,6 +11176,10 @@ in {

  keyring = callPackage ../development/python-modules/keyring { };

+  kivy = callPackage ../development/python-modules/kivy { inherit stdenv buildPythonPackage fetchPypi maintainers platforms licenses pkgs self ; };


callPackage will populate the arguments automatically

vanschelven · 2017-09-27T09:17:34Z

@FRidh I've responded in the above remarking that I'm not confident enough in my knowledge of Nix packaging to push forwards from here. Any chance you could implement your own remarks/requests so that we can push this PR forward and get it accepted?

vanschelven · 2017-12-20T11:32:08Z

I'm closing this pull request; it no longer works against the current version of master, and updating is non-trivial at the moment. I might reopen a similar request at a later point in time.

Python-kivy: init at 1.10.0

993bdd8

vanschelven requested a review from FRidh as a code owner September 13, 2017 09:12

FRidh requested changes Sep 13, 2017

View reviewed changes

FRidh added the 6.topic: python label Sep 13, 2017

Kivy: cleanup

fa37afb

As per the comments by @FRidh (once no more questions/remarks remain, I'll squash the commits into a single one)

FRidh requested changes Sep 13, 2017

View reviewed changes

FRidh added the 2.status: work-in-progress label Sep 16, 2017

vanschelven closed this Dec 20, 2017

FRidh mentioned this pull request Oct 10, 2019

python kivi #70907

Closed

rissson mentioned this pull request Nov 23, 2020

python3Packages.kivy: init at 2.0.0 #104694

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python-kivy: init at 1.10.0 #29305

Python-kivy: init at 1.10.0 #29305

vanschelven commented Sep 13, 2017

FRidh Sep 13, 2017

FRidh Sep 13, 2017

vanschelven Sep 13, 2017

FRidh Sep 13, 2017

FRidh Sep 13, 2017

FRidh Sep 13, 2017

FRidh Sep 13, 2017

FRidh Sep 13, 2017

vanschelven Sep 13, 2017

FRidh Sep 13, 2017

vanschelven Sep 15, 2017 •

edited

FRidh Sep 13, 2017

vanschelven Sep 15, 2017

FRidh Sep 13, 2017

vanschelven Sep 15, 2017 •

edited

FRidh Sep 13, 2017

FRidh Sep 13, 2017

FRidh Sep 13, 2017

FRidh Sep 13, 2017

FRidh Sep 13, 2017

vanschelven commented Sep 27, 2017

vanschelven commented Dec 20, 2017

		@@ -0,0 +1,101 @@
		{ stdenv, buildPythonPackage, fetchPypi, maintainers, platforms, licenses, pkgs, self }:

		@@ -0,0 +1,25 @@
		# The sole reason for this package is that it is a (questionable) dependency of Kivy
		{ buildPythonPackage, fetchPypi, maintainers, platforms, licenses, pkgs, self }:

		@@ -11176,6 +11176,10 @@ in {

		keyring = callPackage ../development/python-modules/keyring { };

		kivy = callPackage ../development/python-modules/kivy { inherit stdenv buildPythonPackage fetchPypi maintainers platforms licenses pkgs self ; };

Python-kivy: init at 1.10.0 #29305

Python-kivy: init at 1.10.0 #29305

Conversation

vanschelven commented Sep 13, 2017

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

vanschelven Sep 15, 2017 • edited

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

vanschelven Sep 15, 2017 • edited

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

vanschelven commented Sep 27, 2017

vanschelven commented Dec 20, 2017

vanschelven Sep 15, 2017 •

edited

vanschelven Sep 15, 2017 •

edited