pythonPackages.voltron: init at 0.1.7 #21398

psychomario · 2016-12-25T00:54:17Z

Motivation for this change

Add the voltron package and required dependencies:

requests-unixsocket
pysigset
blessed
Flask-RESTful
aniso8601
scruffington
voltron

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 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.

Note that for testing purposes I had to recompile my gdb to use the full version of python27 due to the curses library not being there. This is likely due to my own stupidity.

Python27, Python3{3,4,5,6} all compile, fully tested with Python27.

Apologies if I've done this completely wrong, I'm very new to this packaging format.

mention-bot · 2016-12-25T00:54:28Z

@psychomario, thanks for your PR! By analyzing the history of the files in this pull request, we identified @FRidh to be a potential reviewer.

FRidh · 2016-12-25T07:37:39Z

Hi @psychomario. This looks good.

Note that for testing purposes I had to recompile my gdb to use the full version of python27 due to the curses library not being there. This is likely due to my own stupidity.

Nowadays all Python interpreters in Nixpkgs come with curses by default.

Python27, Python3{3,4,5,6} all compile, fully tested with Python27.

Good! Did you also check whether the tests are actually run? After building and "installing" a test runner is automatically run. The default runner doesn't always detect tests, and if that's the case the checkPhase should be fixed.

FRidh

Could you also add yourself as maintainer of these packages?

FRidh · 2016-12-25T07:39:35Z

pkgs/top-level/python-packages.nix

+    version = "0.3.7";
+    format = "wheel";
+    src = pkgs.fetchurl {
+      url = "https://pypi.python.org/packages/6c/73/18d180df19d55e8803731ad638513038906914063bd9bde8eea3cfa1ece5/${name}-py2.py3-none-any.whl";


please use url's like

url = "mirror://pypi/a/${pname}/${name}.tar.gz";

or, even better,

url = "mirror://pypi/${builtins.substring 0 1 pname}/${pname}/${name}.tar.gz";

The one you use now consists of a big hash which changes with every version requiring updating the url.

The two libraries I've not done this for only offer .whl files on pypi, and I couldn't find any examples of the hashless url, do you know the correct format for this?

you're right, then its fine!

While its possible to install wheels, the original source is still preferred. I've looked at the two packages, and they have repo's on GitHub and use setuptools.

FRidh · 2016-12-25T08:09:00Z

Then I have one more request (we don't have a guideline on this matter yet). Please put each expression in a default.nix in pkgs/development/python-modules/ and call it from python-packages.nix like is done with several other packages (e.g. setuptools). We should slowly move to separate files since python-packages.nix becomes huge.

psychomario · 2016-12-25T08:40:19Z

Hi @FRidh, thanks for the feedback. I'll make these changes when I can, but it will likely be tomorrow as today is Christmas day.
I've also noticed Travis is failing to build due to a bad license, unsure why my local build succeeded but I will fix this too.

FRidh · 2016-12-25T08:42:58Z

pkgs/top-level/python-packages.nix

+    meta = {
+      description = "Use requests to talk HTTP via a UNIX domain socket";
+      homepage    = "https://github.com/msabramo/requests-unixsocket";
+      license     = licenses.apache;


licenses.asl20

psychomario · 2016-12-25T12:19:44Z

Test status

I will fill this out as I verify each package.

	Python27	Python33	Python34	Python35	Python36
requests-unixsocket+/++
~~pysigset~~**
blessed+/++
Flask-RESTful
aniso8601
scruffington***
voltron++

* Tests not found
** No tests present
*** First run only
+

______ ERROR collecting dist/tmpbuild/blessed/blessed/tests/test_wrap.py _______
import file mismatch:
imported module 'blessed.tests.test_wrap' has this __file__ attribute:
  /tmp/nix-build-python3.6-blessed-1.14.1.drv-0/blessed-1.14.1/blessed/tests/test_wrap.py
which is not the same as the test file we want to collect:
  /tmp/nix-build-python3.6-blessed-1.14.1.drv-0/blessed-1.14.1/dist/tmpbuild/blessed/blessed/tests/test_wrap.py
HINT: remove __pycache__ / .pyc files and/or use a unique basename for your test file modules

++ fails
bold All testing and OK

psychomario · 2016-12-25T13:30:06Z

I've noticed some inconsistencies in the way - are handled in package names. For example, pytest-mock has the -, but pytestpep8 does not.

Which is the preferred method which I should apply to requests-unixsocket and Flask-RESTful?

FRidh · 2016-12-25T13:34:14Z

For Python packages the preferred name is the one used by upstream, which we consider PyPI, and if the package is not on PyPI then its the name given by the author.

psychomario · 2016-12-25T13:37:24Z

For those two examples, PyPI has the - in both, and the name/pname attribute is correct, but the definition start line varies. See below:

  pytest-mock = buildPythonPackage rec {
    name = "${pname}-${version}";
    pname = "pytest-mock";
    version = "1.2";

  pytestpep8 = buildPythonPackage rec {
    name = "pytest-pep8";

FRidh · 2016-12-25T13:38:31Z

Correct. The general guidelines for Nixpkgs are different. python-packages.nix used to follow those.

psychomario · 2016-12-25T21:03:21Z

Ok so I think I've implemented all of your feedback, and I've rebuilt with Python27 and Python3{3,4,5,6} to verify nothing has broken.

A few of the packages have broken tests so the testing has been disabled, I'm not sure if this is Ok or not. I've opened an issue against scruffington to fix the tests failing every time after the first run if the tests are run as a new nixbld user. On a related note, is there a reason why the package building environment does not have a fresh /tmp? I saw a test failure in both requests-unixsocket and blessed, seen in my comment above keyed as '+'; is this a known Nix thing with a fix?

FRidh

Some of my comments apply to all expressions.

FRidh · 2016-12-29T08:34:05Z

pkgs/development/python-modules/Flask-RESTful/default.nix

+    license     = licenses.bsd3;
+    platforms   = platforms.linux;
+  };
+  propagatedBuildInputs = [ flask pytz six aniso8601 itsdangerous click jinja2 werkzeug dateutil nose ];


does it have a runtime dependency on nose? That's unlikely.

FRidh · 2016-12-29T08:35:27Z

pkgs/development/python-modules/Flask-RESTful/default.nix

@@ -0,0 +1,23 @@
+{
+stdenv, pkgs, python, wrapPython, buildPythonPackage,


just pass lib instead of stdenv. Furthermore, never pass the whole package set pkgs, instead pass individual applications/libraries, like in this case fetchurl. wrapPython is not used so you don't need to pass it, and neither should you pass python.

FRidh · 2016-12-29T08:36:06Z

pkgs/development/python-modules/Flask-RESTful/default.nix

+  meta = with stdenv.lib; {
+    description = "Simple framework for creating REST APIs";
+    homepage    = "https://github.com/flask-restful/flask-restful/";
+    license     = licenses.bsd3;


lib.licenses.bsd3 (or stdenv.lib.licenses.bsd3 should you choose to keep stdenv in)

FRidh · 2016-12-29T08:36:14Z

pkgs/development/python-modules/Flask-RESTful/default.nix

+    description = "Simple framework for creating REST APIs";
+    homepage    = "https://github.com/flask-restful/flask-restful/";
+    license     = licenses.bsd3;
+    platforms   = platforms.linux;


lib.platforms.linux

FRidh · 2016-12-29T08:37:00Z

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

+    url = "mirror://pypi/b/${pname}/${name}.tar.gz";
+    sha256 = "0wy0na399swybp5bzys31pn57vg34bjsl0kv5zf496996gc8k8jq";
+  };
+  doCheck = false;


Don't just disable tests. Always include a comment explaining why they're disabled.

FRidh · 2016-12-29T08:38:56Z

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

+    license     = licenses.mit;
+    platforms   = platforms.linux;
+  };
+  propagatedBuildInputs = [ pyyaml six nose ];


nose as runtime dependency (read: install_requires)? Or only for tests?

FRidh · 2016-12-29T08:39:26Z

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

+    # $HOME is by default not a valid dir, so we have to set that too
+    # https://github.com/NixOS/nixpkgs/issues/12591
+    # taken from the voltron .travis.yml
+    checkPhase = ''


postPatch = ''

FRidh · 2017-02-10T16:24:38Z

ping

psychomario · 2017-02-10T16:26:33Z

Sorry, I forgot about this and I don't currently have the time to fix and support this. I'm sorry for wasting your time.

CMCDragonkai · 2018-10-23T04:09:05Z

This is almost there. I'd like to try and package this soon.

FRidh added 6.topic: python 8.has: package (new) labels Dec 25, 2016

FRidh changed the title ~~Pythonpackage.voltron: init at 0.1.7~~ pythonPackages.voltron: init at 0.1.7 Dec 25, 2016

FRidh requested changes Dec 25, 2016

View reviewed changes

FRidh reviewed Dec 25, 2016

View reviewed changes

Rory McNamara added 7 commits December 25, 2016 20:38

pythonPackages.requests-unixsocket: init at 0.1.5

77b90be

pythonPackages.pysigset: init at 0.2.1

cb9117e

pythonPackages.blessed: init at 1.14.1

f4a027b

pythonPackages.Flask-RESTful: init at 0.3.5

8269f59

pythonPackages.aniso8601: init at 1.2.0

da47350

pythonPackages.scruffington: init at 0.3.7

25589f3

pythonPackages.voltron: init at 0.1.7

f66bef7

FRidh requested changes Dec 29, 2016

View reviewed changes

psychomario closed this Feb 10, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pythonPackages.voltron: init at 0.1.7 #21398

pythonPackages.voltron: init at 0.1.7 #21398

psychomario commented Dec 25, 2016

mention-bot commented Dec 25, 2016

FRidh commented Dec 25, 2016

FRidh left a comment

FRidh Dec 25, 2016

psychomario Dec 25, 2016

FRidh Dec 25, 2016

FRidh Dec 25, 2016

FRidh commented Dec 25, 2016

psychomario commented Dec 25, 2016

FRidh Dec 25, 2016

psychomario commented Dec 25, 2016 •

edited

psychomario commented Dec 25, 2016

FRidh commented Dec 25, 2016

psychomario commented Dec 25, 2016

FRidh commented Dec 25, 2016

psychomario commented Dec 25, 2016 •

edited

FRidh left a comment

FRidh Dec 29, 2016

FRidh Dec 29, 2016

FRidh Dec 29, 2016

FRidh Dec 29, 2016

FRidh Dec 29, 2016

FRidh Dec 29, 2016

FRidh Dec 29, 2016

FRidh commented Feb 10, 2017

psychomario commented Feb 10, 2017

CMCDragonkai commented Oct 23, 2018

		@@ -0,0 +1,23 @@
		{
		stdenv, pkgs, python, wrapPython, buildPythonPackage,

pythonPackages.voltron: init at 0.1.7 #21398

pythonPackages.voltron: init at 0.1.7 #21398

Conversation

psychomario commented Dec 25, 2016

Motivation for this change

Things done

mention-bot commented Dec 25, 2016

FRidh commented Dec 25, 2016

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

FRidh commented Dec 25, 2016

psychomario commented Dec 25, 2016

Choose a reason for hiding this comment

psychomario commented Dec 25, 2016 • edited

Test status

psychomario commented Dec 25, 2016

FRidh commented Dec 25, 2016

psychomario commented Dec 25, 2016

FRidh commented Dec 25, 2016

psychomario commented Dec 25, 2016 • edited

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

FRidh commented Feb 10, 2017

psychomario commented Feb 10, 2017

CMCDragonkai commented Oct 23, 2018

psychomario commented Dec 25, 2016 •

edited

psychomario commented Dec 25, 2016 •

edited