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?

Yes, I think I could do it.

Bt if you don't mind I'd liie to delay this.

When and if we'll have the current Tryton thhen we'll see.

> From e42a727312a454aeb19e07cfec6cbb03fe18e183 Mon Sep 17 00:00:00 2001
> From: humanitiesNerd <catonano@gmail.com>
> 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.

done

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

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

done

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

The latest version of python-sql is 0.9.

updated

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

Could you put these on the same line?

done

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

done

> + (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?

No, the current tarball is not suficient.

Genshi builds with python 3.3 only. With python 3.4 and 3.5 it doesn't build.

This is mainly due to a change in thhe C API so a part of Genshi tat was written in C has to be re-written adgering to the new API

The authors claim to need more time to do this.

The Genshi issue tracker reports all this infomration, I linked the relevant issues in the comments

Admittedly I don't understand what these patches do. They're too entrenched in the Genshi code base

I shamelessly copied them from the Fedora package definition

See here
http://pkgs.fedoraproject.org/cgit/rpms/python-genshi.git/snapshot/python-genshi-f25.tar.gz

I understand that they made an effort to make their Genshi package compatible with pythhon 3.4 too and that is not necessary or Guix

But it's too complicated for me to excise the support for python 3.4

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

done

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

Oh, I didn't know this. Thanks

So,
this package would just be called "trytond".

done

> + (version "4.2.3")

The latest version of tryton is 4.4.

updated

> + (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/

done

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

mmm I'm not sure I can do this.

I don't know much about setuptools, eggs and the such

The check phase of the python build system is quite articulated, I don't feel like messing with it

Feel free to rearrange this yourself as you see fit.

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

Right, thanks.

Done

> + (license license:lgpl3)))

Tryton is GPL3.

fixed

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

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

erased

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

Latest version if 4.4

updated

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

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

done

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

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

done

Ok, so this is the first iteration.

I'm ready or the next one 😎