papis: init at 0.5.2 + dependencies #34761

nico202 · 2018-02-09T12:56:59Z

Motivation for this change

Papis is a powerful and highly extensible command-line based document and bibliography manager.
https://github.com/alejandrogallo/papis

Things done

Added all papis missing dependencies (arxiv2bib, habanero, papis-python-rofi, pylibgen)
Updated python-magic from 0.4.10 to 0.4.13

(HELP WANTED: I don't know how to check if all packages depending on python-magic still works, how do I check it?
I checked beancount and it installs, relatorio and tests are ok, s3cmd installs (has no tests). How do I ask nix to do this automatically for all the dependencies?)

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.

dotlambda · 2018-02-09T13:03:26Z

pkgs/applications/office/papis/default.nix

+}:
+
+buildPythonPackage rec {
+  name = "papis-${version}";


Please specify pname and leave out name.
Same for the other packages.

dotlambda · 2018-02-09T13:05:43Z

pkgs/applications/office/papis/default.nix

+, pkgs
+}:
+
+buildPythonPackage rec {


buildPythonApplication

dotlambda · 2018-02-09T13:06:56Z

pkgs/applications/office/papis/default.nix

+, configparser, habanero, papis-python-rofi, pylibgen
+, prompt_toolkit, pyparser, python_magic, pyyaml
+, requests, unidecode, urwid, vobject, tkinter
+, pkgs


Please pass in vim directly.

dotlambda · 2018-02-09T13:08:29Z

pkgs/development/python-modules/papis-python-rofi/default.nix

+{ buildPythonPackage, lib, fetchFromGitHub }:
+
+buildPythonPackage rec {
+  name = "papis-python-rofi-${version}";


This should be called python-rofi.

dotlambda · 2018-02-09T13:08:44Z

pkgs/top-level/python-packages.nix

@@ -5810,6 +5816,10 @@ in {

  paperwork-backend = callPackage ../applications/office/paperwork/backend.nix { };

+  papis = callPackage ../applications/office/papis { };
+
+  papis-python-rofi = callPackage ../development/python-modules/papis-python-rofi { };


python-rofi

Ah, I see. papis-python-rofi seems to be a fork of python-rofi. Then it's all right like that,

dotlambda · 2018-02-09T13:09:11Z

pkgs/top-level/python-packages.nix

@@ -5810,6 +5816,10 @@ in {

  paperwork-backend = callPackage ../applications/office/paperwork/backend.nix { };

+  papis = callPackage ../applications/office/papis { };


Please move to all-packages.nix and use python3Packages.callPackage

dotlambda · 2018-02-09T13:10:34Z

pkgs/top-level/python-packages.nix


    src = pkgs.fetchurl {
      url = "mirror://pypi/p/python-magic/${name}.tar.gz";
-      sha256 = "1hx2sjd4fdswswj3yydn2azxb59rjmi9b7jzh94lf1wnxijjizbr";
+      sha256 = "128j9y30zih6cyjyjnxhghnvpjm8vw40a1q7pgmrp035yvkaqkk0";


Would you mind moving this to development/python-modules/, specifying pname, version and using fetchPypi?

dotlambda · 2018-02-09T13:18:54Z

For checking whether the dependents of python-magic still compile, use nix-shell -p nox --run "nox-review pr 34761"

dotlambda

The papis package should be put in pkgs/tools/... since it's command line only.

nico202 · 2018-02-09T14:34:38Z

use pname instead of name
move python-magic to its own file
use vim instead of pkgs.vim
move papis to all-packages, tools, use pythonApplication

nico202 · 2018-02-09T14:35:44Z

nox is failing, but even the following command fails on unstable

nix-build -A python27Packages.graphite_pager -A fava -A python36Packages.python_magic -A diffoscope -A python27Packages.thumbor -A python27Packages.alot -A python27Packages.python_magic -A alot -A beancount -A python36Packages.relatorio -A s3cmd

do we need to ask maintainers to fix the packages?

dotlambda · 2018-02-09T14:37:06Z

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

+  };
+
+  # Python 2.x is not supported.
+  disabled = !isPy3k && !isPyPy;


So, Python 2 isn't supported but PyPy is?

dotlambda · 2018-02-09T14:39:25Z

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

+  };
+
+  # strictly requires 1.6.5 but nix has 1.6.6 only
+  preConfigure = "sed -i 's/1.6.5/1.6.6/' requirements.txt";


prePatch
I think you should rather replace == by >= to make this patch future-proof.

yeah that was stupid by me, fixing

You could also create an upstream PR.

I'm not on bitbucket and I'm not willed to sign up, and I could not find his mail address. He is @grimmy on github

dotlambda · 2018-02-09T14:40:57Z

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

+  pname = "pyparser";
+  version = "1.0";
+
+  src = fetchFromBitbucket {


Why not fetchPypi? fetchPypi is generally preferred because this makes it easier to override the src. However, in case e.g. tests are not uploaded to PyPI, it is better to use other sources.
The same applies for all other packages.

for this and for papis tests are missing, for one other they are failing. Fixed others

dotlambda · 2018-02-09T14:43:41Z

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

+    sha256 = "1kp2iyx20lpc9dv4qg5fgwf83a1wx6f7hj1ldqyncg0kn9xcrhbg";
+  };
+
+  meta = {


You could put a with lib; here to get rid of the lib. prefix if you want.

dotlambda · 2018-02-09T14:44:51Z

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

+
+  meta = {
+    description = "Get a BibTeX entry from an arXiv id number, using the arxiv.org API";
+    homepage = http://nathangrigg.github.io/arxiv2bib/;


Wrong descripion and homepage.

dotlambda · 2018-02-09T14:46:51Z

If these packages do have maintainers, then yes it would nice to tell them about it.
But as long as all packages that were not broken beforehand compile fine after the update, you should not worry too much.

FRidh · 2018-02-09T15:16:56Z

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

+    sha256 = "0aplb4zdpgbpmaw9qj0vr7qip9q5w7sl1m1lp1nc9jmjfij9i0hf";
+  };
+
+  preConfigure = "sed -i 's/parse==/parse>=/' requirements.txt";


FRidh · 2018-02-09T15:17:06Z

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

+
+  propagatedBuildInputs = [ file ];
+
+  patchPhase = ''


FRidh · 2018-02-09T15:17:25Z

pkgs/tools/misc/papis/default.nix

+    sha256 = "0cw6ajdaknijka3j2bkkkn0bcxqifk825kq0a0rdbbmc6661pgxb";
+  };
+
+  preConfigure = "sed -i 's/configparser>=3.0.0/# configparser>=3.0.0/' setup.py";


FRidh · 2018-02-09T15:17:54Z

pkgs/tools/misc/papis/default.nix

+    vim
+  ];
+
+  preCheck = ''


Why is this needed? Leave an explanation.

dotlambda · 2018-02-09T15:31:22Z

@GrahamcOfBorg build python2.pkgs.python-magic python3.pkgs.python-magic
@GrahamcOfBorg build python2.pkgs.arxiv2bib python3.pkgs.arxiv2bib
@GrahamcOfBorg build python2.pkgs.habanero python3.pkgs.habanero
@GrahamcOfBorg build python2.pkgs.papis-python-rofi python3.pkgs.papis-python-rofi
@GrahamcOfBorg build python3.pkgs.pylibgen
@GrahamcOfBorg build python2.pkgs.pyparser python3.pkgs.pyparser
@GrahamcOfBorg build papis

GrahamcOfBorg · 2018-02-09T15:31:31Z

Failure on x86_64-linux (full log)

Partial log (click to expand)

error: attribute ‘python-magic’ in selection path ‘python2.pkgs.python-magic’ not found

GrahamcOfBorg · 2018-02-09T15:31:36Z

Failure on aarch64-linux (full log)

Partial log (click to expand)

error: attribute 'python-magic' in selection path 'python2.pkgs.python-magic' not found

GrahamcOfBorg · 2018-02-09T15:31:42Z

Success on x86_64-linux (full log)

Partial log (click to expand)

reading manifest file 'arxiv2bib.egg-info/SOURCES.txt'
writing manifest file 'arxiv2bib.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/xljrpmkadgzych8lrc23b28xqfwz5qjq-python2.7-arxiv2bib-1.0.8
/nix/store/sf4ix1gj00xcqs9bqpx6yahklv6f1mbs-python3.6-arxiv2bib-1.0.8

GrahamcOfBorg · 2018-02-09T15:31:57Z

Success on aarch64-linux (full log)

Partial log (click to expand)

reading manifest file 'arxiv2bib.egg-info/SOURCES.txt'
writing manifest file 'arxiv2bib.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/yqgy43rplkkl4qy10d119i22p52cv025-python2.7-arxiv2bib-1.0.8
/nix/store/cgvdq69mdrmppyhnxd4ykfl7h3iibjaq-python3.6-arxiv2bib-1.0.8

GrahamcOfBorg · 2018-02-09T15:31:59Z

Success on aarch64-linux (full log)

Partial log (click to expand)

reading manifest template 'MANIFEST.in'
writing manifest file 'papis_python_rofi.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/djbckq084gkc4w4fdjmiabhnnjwkgwz2-python2.7-papis-python-rofi-1.0.2
/nix/store/fa9v9m9z4fc6by8dcwl1maa0qgqk8crg-python3.6-papis-python-rofi-1.0.2

GrahamcOfBorg · 2018-02-09T15:32:01Z

Success on x86_64-linux (full log)

Partial log (click to expand)

reading manifest file 'habanero.egg-info/SOURCES.txt'
writing manifest file 'habanero.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/9vd466dillbzcyw8ggncbpw6zv0b39gy-python2.7-habanero-0.6.0
/nix/store/6hbx1pdybzl11x9dqyblvp9kg4302knk-python3.6-habanero-0.6.0

GrahamcOfBorg · 2018-02-09T15:32:03Z

Success on aarch64-linux (full log)

Partial log (click to expand)

reading manifest file 'habanero.egg-info/SOURCES.txt'
writing manifest file 'habanero.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.001s

OK
/nix/store/dp74cs064gvc3mkklfc2hdnm1zap1x62-python2.7-habanero-0.6.0
/nix/store/2g14lglbckmxp1my3vxj70p8h6jm8v8q-python3.6-habanero-0.6.0

GrahamcOfBorg · 2018-02-09T15:32:13Z

Success on x86_64-linux (full log)

Partial log (click to expand)

reading manifest template 'MANIFEST.in'
writing manifest file 'papis_python_rofi.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/p3k78inig9ll5fkn4f53xxa0pq5hvk90-python2.7-papis-python-rofi-1.0.2
/nix/store/5p0zl94450b80hvpa064pw5bg8kshay8-python3.6-papis-python-rofi-1.0.2

dotlambda · 2018-02-09T15:32:23Z

@GrahamcOfBorg build python2.pkgs.python_magic python3.pkgs.python_magic

nico202 · 2018-02-09T15:33:46Z

Should python_magic be renamed to python-magic? Any guideline on the naming convention? I don't like the underscore but I kept it as it was

dotlambda · 2018-02-09T15:34:53Z

Ran 0 tests in 0.000s

Please specify the appropriate checkPhase for arxiv2bib
For papis-python-rofi, you'll need to set doCheck = false and leave a comment that there are no tests

dotlambda · 2018-02-09T15:36:10Z

pylibgen also needs an appropriate checkPhase

nico202 · 2018-02-09T16:17:26Z

Fixed tests, let me know

dotlambda · 2018-02-09T16:19:10Z

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

+  };
+
+  # Required for tests only
+  buildInputs = [ mock ];


checkInputs

dotlambda · 2018-02-09T16:19:28Z

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

+}:
+
+buildPythonPackage rec {
+  name = "habanero-${version}";


dotlambda · 2018-02-09T16:20:13Z

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

+    sha256 = "128j9y30zih6cyjyjnxhghnvpjm8vw40a1q7pgmrp035yvkaqkk0";
+  };
+
+  propagatedBuildInputs = [ file ];


I don't think this is necessary. Is it, @FRidh?

No, it is not. The patch introduces it so it can be found during run-time. I doubt it needs to be explicitly added during build-time.

nico202 · 2018-02-09T16:49:19Z

I had to change dateparser too because it was needed in order to run habanero tests. However tests on python3 are failing (as per comment). Failing tests are only 3 and because of this error:

AssertionError: False is not true : Didn't found any of the expected messages (['year is out of range', "('year must be in 1..9999'"]) -- message was: ValueError('year -2986 is out of range',)

So it might be possible to fix it upstream, I'll bump the issue

dotlambda · 2018-02-09T16:55:11Z

I just created a PR for dateparser :D
We might want to merge #34768 first. Then you can rebase.

nico202 · 2018-02-09T16:57:28Z

:D no problem, I'll wait for dateparser merge.
Thanks

dotlambda · 2018-02-09T19:45:06Z

@nico202 You can remove your commit regarding dateparser and rebase on master.

nico202 · 2018-02-09T20:26:11Z

@dotlambda thanks for everything, I know that has been a long review I'm sorry, I hope this time it's ok

FRidh

Almost there ;)

FRidh · 2018-02-10T08:10:15Z

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

+  # Required for tests only
+  checkInputs = [ mock ];
+
+  checkPhase = "python -m unittest discover -s tests";


${python.interpreter} instead of python

ehm python is undefined

I add it to dependencies

FRidh · 2018-02-10T08:10:38Z

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

+  propagatedBuildInputs = [ requests ];
+
+  # It's not using unittest
+  checkPhase = "python tests/test_pylibgen.py -c 'test_api_endpoints()'";


${python.interpreter} instead of python

move python_magic

nico202 · 2018-02-10T08:28:24Z

Changed papis github url since yesterday it was moved to papis/papis

nico202 requested a review from FRidh as a code owner February 9, 2018 12:57

GrahamcOfBorg added the 6.topic: python label Feb 9, 2018

nico202 mentioned this pull request Feb 9, 2018

papis-python-rofi wrong name papis/papis#43

Closed

GrahamcOfBorg added 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Feb 9, 2018

dotlambda requested changes Feb 9, 2018

View reviewed changes

dotlambda reviewed Feb 9, 2018

View reviewed changes

nico202 force-pushed the papis branch 3 times, most recently from 6b0e120 to e4d8e76 Compare February 9, 2018 14:32

dotlambda reviewed Feb 9, 2018

View reviewed changes

nico202 force-pushed the papis branch from e4d8e76 to dbcb000 Compare February 9, 2018 15:15

FRidh requested changes Feb 9, 2018

View reviewed changes

nico202 force-pushed the papis branch 3 times, most recently from 5b5b272 to 3be6a08 Compare February 9, 2018 15:29

nico202 force-pushed the papis branch 2 times, most recently from f5b9b0f to 8d57878 Compare February 9, 2018 16:15

dotlambda reviewed Feb 9, 2018

View reviewed changes

nico202 force-pushed the papis branch 2 times, most recently from 7a27d43 to 3bd19ec Compare February 9, 2018 16:43

nico202 force-pushed the papis branch from 3bd19ec to 8a7bff7 Compare February 9, 2018 20:24

FRidh requested changes Feb 10, 2018

View reviewed changes

nico202 added 7 commits February 10, 2018 09:23

arxiv2bib: init at 1.0.8

4422a70

habanero: init at 0.6.0

e6a42f8

pyparser: init at 1.0

1e52167

papis-python-rofi: init at 1.0.2

78ab516

pylibgen: init at 1.3.1

c00d76c

python-magic: move to own file, 0.4.10 -> 0.4.13

3c3bc83

move python_magic

papis: init at 0.5.2

dd4144e

nico202 force-pushed the papis branch from 8a7bff7 to dd4144e Compare February 10, 2018 08:23

GrahamcOfBorg added the 8.has: package (new) label Feb 10, 2018

FRidh changed the title ~~[WIP] papis: init at 0.5.2 + dependencies~~ papis: init at 0.5.2 + dependencies Feb 10, 2018

FRidh merged commit d8d8a0a into NixOS:master Feb 10, 2018

nico202 deleted the papis branch February 10, 2018 09:17

nico202 mentioned this pull request Feb 14, 2018

Add papis: 0.5.0 #32760

Closed

8 tasks

		@@ -5810,6 +5816,10 @@ in {

		paperwork-backend = callPackage ../applications/office/paperwork/backend.nix { };

		papis = callPackage ../applications/office/papis { };

papis: init at 0.5.2 + dependencies #34761

papis: init at 0.5.2 + dependencies #34761

Conversation

nico202 commented Feb 9, 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

dotlambda commented Feb 9, 2018

dotlambda left a comment

Choose a reason for hiding this comment

nico202 commented Feb 9, 2018

nico202 commented Feb 9, 2018 • 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

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

dotlambda commented Feb 9, 2018

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

dotlambda commented Feb 9, 2018

GrahamcOfBorg commented Feb 9, 2018

GrahamcOfBorg commented Feb 9, 2018

GrahamcOfBorg commented Feb 9, 2018

GrahamcOfBorg commented Feb 9, 2018

GrahamcOfBorg commented Feb 9, 2018

GrahamcOfBorg commented Feb 9, 2018

GrahamcOfBorg commented Feb 9, 2018

GrahamcOfBorg commented Feb 9, 2018

dotlambda commented Feb 9, 2018

nico202 commented Feb 9, 2018

dotlambda commented Feb 9, 2018

dotlambda commented Feb 9, 2018

nico202 commented Feb 9, 2018

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

nico202 commented Feb 9, 2018 • edited

dotlambda commented Feb 9, 2018

nico202 commented Feb 9, 2018

dotlambda commented Feb 9, 2018

nico202 commented Feb 9, 2018 • 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

nico202 commented Feb 10, 2018

nico202 commented Feb 9, 2018 •

edited

nico202 commented Feb 9, 2018 •

edited

nico202 commented Feb 9, 2018 •

edited

nico202 commented Feb 9, 2018 •

edited