bug#26401: [PATCH] python-tryton (with no modules)

[Arun Isaac]

Thanks for the patch set!

I haven't properly tested the package yet. The following are just my
initial reactions and questions. This patch review will take a few
iterations. Do bear with me.

> Tryton has modules and without any module packaged, it will do nothing
>
> But at least you can launch it and test it, you can use it for packkaging
> the missing modules.
>
> Also a service would be useful. But in order to write a service, the server
> packkage has to be in already.

Agreed.

> This is supposedly the basis for GNUealth, a notable GNU project

GNU Health usually lags behind the latest Tryon, and currently runs on
Tryton 3.8. We will have to create a package for Tryton 3.8 as
well. This can just inherit from the latest tryton package, and modify
only the `version' and `source' fields. Could you do this?

> From e42a727312a454aeb19e07cfec6cbb03fe18e183 Mon Sep 17 00:00:00 2001
> From: humanitiesNerd <address@hidden>
> Date: Tue, 28 Mar 2017 12:25:06 +0200
> Subject: [PATCH 1/5] gnu: Add python-sql python2-sql.

It is enough to mention only python-sql here.

> * gnu/packages/python.scm (python-sql python2-sql): New variables.

Please put a comma between python-sql and python2-sql.

> +(define-public python-sql
> +  (package
> +    (name "python-sql")
> +    (version "0.8")

The latest version of python-sql is 0.9.

> +       (uri (pypi-uri
> +             "python-sql"
> +             version))

Could you put these on the same line?

> +(define-public python-genshi
> +  (package
> +    (name "python-genshi")
> +    (version "0.7")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append
> +             "https://ftp.edgewall.org/pub/genshi/Genshi-";
> +             version
> +             ".tar.gz"))

Please put version ".tar.gz" on the same line.

> +       (patches
> +        (search-patches
> +         ;; The first 4 patches are in the master branch upstream.
> +         ;; see this as a reference https://genshi.edgewall.org/ticket/582
> +         ;; The last 2 are NOT in any branch.
> +         ;; They were sent as attachments to a ticket opened at
> +         ;; https://genshi.edgewall.org/ticket/602#no1
> +         "python-genshi-stripping-of-unsafe-script-tags-Python-3.4.patch"
> +         
> "python-genshi-Disable-the-speedups-C-extension-on-CPython-3.3-sinc.patch"
> +         "python-genshi-isstring-helper.patch"
> +         
> "python-genshi-Add-support-for-Python-3.4-AST-support-for-NameConst.patch"
> +         "python-genshi-fixing-the-tests-on-python35.patch"
> +         "python-genshi-buildable-on-python27-too.patch"))

Why do we need these patches? Is the release tarball not sufficient?

> +    (propagated-inputs
> +     `(("lxml" ,python2-lxml)
> +       ("genshi" ,python2-genshi)))

Please put the full names of these inputs -- I mean "python-lxml"
instead of "lxml", "python-genshi" instead of "genshi", and so on.

> +(define-public python-trytond
> +  (package
> +    (name "python-trytond")

As far as I understand, trytond is an application, not a python
library. Only python libraries should have the "python-" prefix. So,
this package would just be called "trytond".

> +    (version "4.2.3")

The latest version of tryton is 4.4.

> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (pypi-uri
> +             "trytond"
> +             version
> +             ".tar.gz"))

We should use the tarballs available on the tryton website.
https://downloads.tryton.org/4.4/

> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-before 'check 'preparations
> +                     (lambda* _
> +                       ;; this is used in the tests
> +                       (setenv "DB_NAME" ":memory:"))))))

Though this is shorter, I think it would be clearer to replace the
`check' phase altogether.

> +    (propagated-inputs
> +     `(("polib" ,python-polib)
> +       ("dateutil" ,python-dateutil)
> +       ("werkzeug" ,python-werkzeug)
> +       ("wrapt" ,python-wrapt)
> +       ("python-sql" ,python-sql)
> +       ("genshi" ,python-genshi)
> +       ("relatorio" ,python-relatorio)
> +       ("lxml" ,python-lxml)
> +       ;; there's no pyton-mysql in Guix right now
> +       ;; so psycopg (postgresql) only for now
> +       ("psycopg" ,python-psycopg2)))

If trytond is only an application, these can just be `inputs', not
`propagated-inputs'. For applications, the python build system wraps the
executables with the correct PYTHONPATH environment variable.

> +  (license license:lgpl3)))

Tryton is GPL3.

> +(define-public python2-trytond
> +  (package-with-python2 python-trytond))

No need for python2-trytond if trytond is just an application.

> +;; this depends on pygtk that is available or address@hidden only
> +(define-public python2-tryton
> +  (package
> +    (name "python2-tryton")
> +    (version "4.2.4")

Latest version if 4.4

> +       (uri (pypi-uri
> +             "tryton"
> +             version
> +             ".tar.gz"))

We should use the tarballs available on the tryton website.
https://downloads.tryton.org/4.4/

> +    (propagated-inputs
> +     `(("chardet" ,python2-chardet)
> +       ("dateutil" ,python2-dateutil)
> +       ("pygtk" ,python2-pygtk)))

For an application, these can just be `inputs'.