From debbugs-submit-bounces@debbugs.gnu.org Mon May 08 10:35:00 2017 Received: (at 26401) by debbugs.gnu.org; 8 May 2017 14:35:00 +0000 Received: from localhost ([127.0.0.1]:34479 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d7jka-0006Ls-Cl for submit@debbugs.gnu.org; Mon, 08 May 2017 10:35:00 -0400 Received: from o114.p8.mailjet.com ([87.253.233.114]:36563) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <285e9165.AEMAKF1MYlQAAAAAAAAAAAO8YckAAAACwQwAAAAAAAW9WABZEIIR@bnc3.mailjet.com>) id 1d7jkY-0006Lk-L8 for 26401@debbugs.gnu.org; Mon, 08 May 2017 10:34:59 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/simple; q=dns/txt; d=bnc3.mailjet.com; i=arunisaac=3Dsystemreboot.net@bnc3.mailjet.com; s=mailjet; h=message-id:mime-version:from:to:subject:date:list-unsubscribe:in-reply-to: references:x-csa-complaints:x-mj-mid:content-type:content-transfer-encoding; bh=PvANu91XD6l0AD/kwjdofqle7hE=; b=DbrNl8vP/fQ+OWjNdflJ1dbkiBuqIijyMVJtEgoHU+ts8gFkpOu1Rm6AX mnnMGkKD/4xvJ7LE+cPLe+gtzPLS44zEXrbC71umriiQDOS8BOcJkJqCjgM7 DREBSWW/pIqTi22GKTOiG62OxPOACH29prV5hwKKWfJvB7cSjdhRfs= Message-Id: <285e9165.AEMAKF1MYlQAAAAAAAAAAAO8YckAAAACwQwAAAAAAAW9WABZEIIR@mailjet.com> MIME-Version: 1.0 From: Arun Isaac To: 26401@debbugs.gnu.org Subject: Re: bug#26401: [PATCH] python-tryton (with no modules) Date: Mon, 08 May 2017 20:03:59 +0530 In-reply-to: References: X-CSA-Complaints: whitelist-complaints@eco.de X-MJ-Mid: AEMAKF1MYlQAAAAAAAAAAAO8YckAAAACwQwAAAAAAAW9WABZEIIRVoyAV42GThOorSvv5Ph80gAFgUc Content-Type: text/plain Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.8 (--) X-Debbugs-Envelope-To: 26401 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.8 (--) 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 serv= er > 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 > 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/5= 82 > + ;; 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-NameC= onst.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 python@2 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'. =